fix(core): Reject unsafe property tokens in in-isolate $jmespath (backport to 1.x) (#31643)
Some checks are pending
CI: Master (Build, Test, Lint) / Build for Github Cache (push) Waiting to run
CI: Master (Build, Test, Lint) / Unit tests (22.x) (push) Waiting to run
CI: Master (Build, Test, Lint) / Unit tests (24.13.1) (push) Waiting to run
CI: Master (Build, Test, Lint) / Unit tests (25.x) (push) Waiting to run
CI: Master (Build, Test, Lint) / Lint (push) Waiting to run
CI: Master (Build, Test, Lint) / Performance (push) Waiting to run
CI: Master (Build, Test, Lint) / Notify Slack on failure (push) Blocked by required conditions

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Mike Repeć 2026-06-03 13:01:06 +02:00 committed by GitHub
parent fbb90050c1
commit 08de252a0f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 56 additions and 33 deletions

View File

@ -2,6 +2,37 @@ import * as jmespath from 'jmespath';
import { ExpressionError } from './safe-globals';
const unsafeJmespathProperties = [
'__proto__',
'prototype',
'constructor',
'getPrototypeOf',
'setPrototypeOf',
'getOwnPropertyDescriptor',
'getOwnPropertyDescriptors',
'defineProperty',
'defineProperties',
'mainModule',
'binding',
'_linkedBinding',
'_load',
'prepareStackTrace',
'__lookupGetter__',
'__lookupSetter__',
'__defineGetter__',
'__defineSetter__',
'caller',
'arguments',
'getBuiltinModule',
'dlopen',
'execve',
'loadEnvFile',
];
const unsafeJmespathPropertyPattern = new RegExp(
`\\b(?:${unsafeJmespathProperties.map((p) => p.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')).join('|')})\\b`,
);
/**
* In-isolate `$jmespath` / `$jmesPath` implementation.
*
@ -15,6 +46,7 @@ import { ExpressionError } from './safe-globals';
*
* Behavioural parity with the host wrapper:
* - Throws `ExpressionError` (same name) when args are wrong.
* - Rejects queries that contain unsafe property tokens.
* - Spreads non-array, non-null objects to drop proxies at the top level.
*
* Note on lazy proxies: when `data` is a lazy proxy (e.g. `$json`), each
@ -28,6 +60,15 @@ export function jmesPath(data: unknown, query: string): unknown {
throw new ExpressionError('expected two arguments (Object, string) for this function');
}
// jmespath decodes escape sequences inside quoted identifiers, so the
// token check must run against an unescaped query. Reject any backslash
// up front to keep the property-name match meaningful.
if (query.includes('\\') || unsafeJmespathPropertyPattern.test(query)) {
throw new ExpressionError(
'Cannot access this property in a jmespath query due to security concerns',
);
}
if (data !== null && !Array.isArray(data) && typeof data === 'object') {
return jmespath.search({ ...(data as Record<string, unknown>) }, query);
}

View File

@ -470,46 +470,28 @@ describe('Expression', () => {
);
});
// The $jmespath restricted-identifier defense differs by engine:
// legacy → the host wrapper (workflow-data-proxy.ts) throws
// "due to security concerns" via an explicit token denylist.
// vm → jmespath runs in-isolate with no denylist, but a restricted
// identifier can never leak a usable constructor/prototype to
// the host: `constructor` resolves to the Object constructor,
// which fails to serialize across the isolate boundary (throws),
// while `getPrototypeOf`/`prototype`/`__proto__` are key-misses
// or non-cloneable internals → the result is never a callable.
// (Structural isolation rather than a denylist — see
// packages/@n8n/expression-runtime/src/runtime/jmespath.ts.)
const expectJmespathBlocked = (expr: string) => {
if (process.env.N8N_EXPRESSION_ENGINE === 'vm') {
let result: unknown;
let threw = false;
try {
result = evaluate(expr);
} catch {
threw = true;
}
expect(threw || typeof result !== 'function').toBe(true);
} else {
expect(() => evaluate(expr)).toThrow(/due to security concerns/);
}
};
it('should reject jmespath queries that reference restricted identifiers', () => {
expectJmespathBlocked('={{ $jmespath({a:1}, "constructor") }}');
expectJmespathBlocked('={{ $jmespath({a:1}, "__proto__") }}');
expectJmespathBlocked('={{ $jmespath({a:1}, "prototype") }}');
expect(() => evaluate('={{ $jmespath({a:1}, "constructor") }}')).toThrow(
/due to security concerns/,
);
expect(() => evaluate('={{ $jmespath({a:1}, "__proto__") }}')).toThrow(
/due to security concerns/,
);
expect(() => evaluate('={{ $jmespath({a:1}, "prototype") }}')).toThrow(
/due to security concerns/,
);
});
it('should reject jmespath queries that reference restricted identifiers (alias)', () => {
expectJmespathBlocked('={{ $jmesPath({a:1}, "getPrototypeOf") }}');
expect(() => evaluate('={{ $jmesPath({a:1}, "getPrototypeOf") }}')).toThrow(
/due to security concerns/,
);
});
it('should reject computed-string jmespath queries built from restricted identifiers', () => {
expectJmespathBlocked(
'={{ $jmespath({a:1}, String.fromCharCode(99,111,110,115,116,114,117,99,116,111,114)) }}',
);
const payload =
'={{ $jmespath({a:1}, String.fromCharCode(99,111,110,115,116,114,117,99,116,111,114)) }}';
expect(() => evaluate(payload)).toThrow(/due to security concerns/);
});
it('should still allow jmespath queries that contain restricted names as substrings', () => {