mirror of
https://github.com/n8n-io/n8n.git
synced 2026-06-19 07:36:52 +00:00
fix(core): Throw a clear error for $evaluateExpression in the Code node under secure mode (#31721)
This commit is contained in:
+21
@@ -212,6 +212,27 @@ describe('JS TaskRunner execution on internal mode', () => {
|
||||
result: { hello: 'world' },
|
||||
});
|
||||
});
|
||||
|
||||
// CAT-3208 / GH #24307: secure-mode task runners disable code generation
|
||||
// (--disallow-code-generation-from-strings) and freeze Object.prototype, so
|
||||
// expressions can't be evaluated inside the Code node. $evaluateExpression
|
||||
// must surface a clear, actionable error instead of crashing with
|
||||
// "Cannot assign to read only property '__lookupGetter__'" or silently
|
||||
// returning null.
|
||||
it('should throw a clear error for $evaluateExpression in the Code node', async () => {
|
||||
// Act
|
||||
const result = await runTaskWithCode("return { val: $evaluateExpression('{{ 1 + 1 }}') }");
|
||||
|
||||
// Assert
|
||||
expect(result).toEqual({
|
||||
ok: false,
|
||||
error: expect.objectContaining({
|
||||
message: expect.stringContaining(
|
||||
'in the Code node while task runners run in secure mode',
|
||||
),
|
||||
}),
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('Internal and external libs', () => {
|
||||
|
||||
@@ -373,10 +373,23 @@ export class Expression {
|
||||
data.Reflect = {};
|
||||
data.Proxy = {};
|
||||
|
||||
data.__lookupGetter__ = undefined;
|
||||
data.__lookupSetter__ = undefined;
|
||||
data.__defineGetter__ = undefined;
|
||||
data.__defineSetter__ = undefined;
|
||||
// These four names are inherited from `Object.prototype`. In the secure-mode
|
||||
// task-runner sandbox `Object.prototype` is frozen, so plain assignment walks
|
||||
// the prototype chain to the now read-only inherited property and throws in
|
||||
// strict mode. Define them as own properties to overwrite them safely.
|
||||
for (const key of [
|
||||
'__lookupGetter__',
|
||||
'__lookupSetter__',
|
||||
'__defineGetter__',
|
||||
'__defineSetter__',
|
||||
]) {
|
||||
Object.defineProperty(data, key, {
|
||||
value: undefined,
|
||||
writable: true,
|
||||
enumerable: true,
|
||||
configurable: true,
|
||||
});
|
||||
}
|
||||
|
||||
// Deprecated
|
||||
data.escape = {};
|
||||
|
||||
@@ -52,6 +52,26 @@ const PAIRED_ITEM_METHOD = {
|
||||
|
||||
type PairedItemMethod = (typeof PAIRED_ITEM_METHOD)[keyof typeof PAIRED_ITEM_METHOD];
|
||||
|
||||
/**
|
||||
* Whether the runtime can compile expressions. The expression engine compiles
|
||||
* expressions via `new Function`, which throws when the process is started with
|
||||
* `--disallow-code-generation-from-strings` — as the secure-mode task runner is.
|
||||
* This is a process-wide invariant, so we probe once and cache the result.
|
||||
*/
|
||||
let codeGenerationAllowed: boolean | undefined;
|
||||
const isCodeGenerationAllowed = (): boolean => {
|
||||
if (codeGenerationAllowed === undefined) {
|
||||
try {
|
||||
// eslint-disable-next-line @typescript-eslint/no-implied-eval, no-new-func
|
||||
new Function('return 1');
|
||||
codeGenerationAllowed = true;
|
||||
} catch {
|
||||
codeGenerationAllowed = false;
|
||||
}
|
||||
}
|
||||
return codeGenerationAllowed;
|
||||
};
|
||||
|
||||
export class WorkflowDataProxy {
|
||||
private runExecutionData: IRunExecutionData | null;
|
||||
|
||||
@@ -1496,6 +1516,15 @@ export class WorkflowDataProxy {
|
||||
that.envProviderState ?? createEnvProviderState(),
|
||||
),
|
||||
$evaluateExpression: (expression: string, itemIndex?: number) => {
|
||||
if (!isCodeGenerationAllowed()) {
|
||||
throw new ExpressionError(
|
||||
"$evaluateExpression can't be used in the Code node while task runners run in secure mode",
|
||||
{
|
||||
description:
|
||||
'Secure-mode task runners disable evaluating strings as code, which expressions rely on. Evaluate the expression in a node field instead, for example an Edit Fields (Set) node before the Code node.',
|
||||
},
|
||||
);
|
||||
}
|
||||
itemIndex = itemIndex || that.itemIndex;
|
||||
return that.workflow.expression.getParameterValue(
|
||||
`=${expression}`,
|
||||
|
||||
Reference in New Issue
Block a user