From 05b8bcb7b17b7a0a87d167a827bb7b4bce3b2652 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carl-Gerhard=20Lindesv=C3=A4rd?= Date: Fri, 4 Sep 2026 07:57:17 +0000 Subject: [PATCH 1/2] Close gaps in the JS template allowlist The validator only inspected call targets and constructors when they were written as plain, non-computed identifiers. Everything else walked straight past it: payload['someMethod']() was never matched against the allowlist, new (expr)() with a non-identifier callee fell through with no error, and import() was not considered at all. A template using any of those forms was accepted as valid even though the validator had not actually checked what it resolved to. Reject the forms the allowlist cannot reason about instead of skipping them, and refuse writes that pass through __proto__, constructor or prototype so a template cannot modify objects outside its own result. execute() also compiled whatever string it was handed. Templates are checked when saved, but the saved value is what reaches new Function(), so validate it again before compiling rather than trusting the stored copy. Co-Authored-By: Claude Opus 5 --- packages/js-runtime/src/execute.ts | 10 ++ packages/js-runtime/src/validate.test.ts | 96 +++++++++++++++++++ packages/js-runtime/src/validate.ts | 113 ++++++++++++++++++++--- 3 files changed, 207 insertions(+), 12 deletions(-) diff --git a/packages/js-runtime/src/execute.ts b/packages/js-runtime/src/execute.ts index a17918f15..48fada97e 100644 --- a/packages/js-runtime/src/execute.ts +++ b/packages/js-runtime/src/execute.ts @@ -1,3 +1,5 @@ +import { validate } from './validate'; + /** * Executes a JavaScript function template * @param code - JavaScript function code (arrow function or function expression) @@ -8,6 +10,14 @@ export function execute( code: string, payload: Record, ): unknown { + // Templates are checked when they are saved, but the stored string is what + // ends up in new Function() here. Check it again at run time rather than + // trusting whatever passed validation at save time. + const validation = validate(code); + if (!validation.valid) { + throw new Error(`Invalid JavaScript template: ${validation.error}`); + } + try { // Create the function code that will be executed // 'use strict' ensures 'this' is undefined (not global object) diff --git a/packages/js-runtime/src/validate.test.ts b/packages/js-runtime/src/validate.test.ts index f7278af55..af884eeb5 100644 --- a/packages/js-runtime/src/validate.test.ts +++ b/packages/js-runtime/src/validate.test.ts @@ -139,6 +139,93 @@ describe('validate', () => { }); }); + describe('Computed and indirect access', () => { + it('should block a computed call target', () => { + const result = validate( + `(payload) => payload['constructor']['constructor']('return 1')()`, + ); + expect(result.valid).toBe(false); + expect(result.error).toContain('Computed property access'); + }); + + it('should block a computed call target reached by optional chaining', () => { + const result = validate( + `(payload) => payload?.['constructor']['constructor']('return 1')()`, + ); + expect(result.valid).toBe(false); + expect(result.error).toContain('Computed property access'); + }); + + it('should block a computed key written with escape sequences', () => { + const result = validate( + `(payload) => payload['\\u0063onstructor']['constructor']('return 1')()`, + ); + expect(result.valid).toBe(false); + expect(result.error).toContain('Computed property access'); + }); + + it('should block a computed call target on a nested value', () => { + const result = validate( + `(payload) => payload.properties['toString']()`, + ); + expect(result.valid).toBe(false); + expect(result.error).toContain('Computed property access'); + }); + + it("should block 'new' on a non-identifier callee", () => { + const result = validate( + `(payload) => new (payload['constructor']['constructor'])('return 1')`, + ); + expect(result.valid).toBe(false); + expect(result.error).toContain("target of 'new'"); + }); + + it("should block 'new' on a member expression", () => { + const result = validate('(payload) => new payload.Thing()'); + expect(result.valid).toBe(false); + expect(result.error).toContain("target of 'new'"); + }); + + it('should block dynamic import()', () => { + const result = validate(`(payload) => import('node:fs')`); + expect(result.valid).toBe(false); + expect(result.error).toContain('Dynamic import()'); + }); + }); + + describe('Prototype writes', () => { + it('should block writing through __proto__', () => { + const result = validate( + '(payload) => { payload.__proto__.x = 1; return payload; }', + ); + expect(result.valid).toBe(false); + expect(result.error).toContain('__proto__'); + }); + + it('should block writing through a computed __proto__ key', () => { + const result = validate( + `(payload) => { payload['__proto__'].x = 1; return payload; }`, + ); + expect(result.valid).toBe(false); + expect(result.error).toContain('__proto__'); + }); + + it('should block writing to a prototype', () => { + const result = validate( + '(payload) => { payload.constructor.prototype.x = 1; return payload; }', + ); + expect(result.valid).toBe(false); + expect(result.error).toContain('not allowed'); + }); + + it('should still allow writing an ordinary property', () => { + const result = validate( + '(payload) => { const out = {}; out.event = payload.name; return out; }', + ); + expect(result.valid).toBe(true); + }); + }); + describe('Invalid syntax', () => { it('should reject non-function code', () => { const result = validate('const x = 1;'); @@ -328,5 +415,14 @@ describe('execute', () => { execute(code, basePayload); }).toThrow('Error executing JavaScript template'); }); + + it('should refuse to run a stored template that fails validation', () => { + // A template that was persisted at some point but does not pass + // validation must not reach new Function(). + const code = `(payload) => payload['constructor']['constructor']('return 1')()`; + expect(() => { + execute(code, basePayload); + }).toThrow('Invalid JavaScript template'); + }); }); }); diff --git a/packages/js-runtime/src/validate.ts b/packages/js-runtime/src/validate.ts index c52a523a9..f59687be0 100644 --- a/packages/js-runtime/src/validate.ts +++ b/packages/js-runtime/src/validate.ts @@ -11,6 +11,60 @@ import { walkNode, } from './ast-walker'; +/** + * Property names that must never be written through. Assigning to any of + * these reaches objects shared with the rest of the worker process. + */ +const FORBIDDEN_WRITE_PROPERTIES = new Set([ + '__proto__', + 'constructor', + 'prototype', +]); + +/** + * The static name of a member expression's property, or undefined when the key + * is only known at run time (obj[someVariable]). + */ +function staticPropertyName( + member: Record +): string | undefined { + const prop = member.property as Record | undefined; + if (!prop) { + return undefined; + } + if (member.computed) { + // Only a literal key can be resolved. Babel has already decoded any + // \u / \x escapes into StringLiteral.value by this point. + return prop.type === 'StringLiteral' ? (prop.value as string) : undefined; + } + return prop.type === 'Identifier' ? (prop.name as string) : undefined; +} + +/** + * Walk an assignment target back down its member chain and return the first + * forbidden property name it passes through, if any. payload.__proto__.x is a + * write to 'x' but goes through '__proto__', so the whole chain matters. + */ +function forbiddenPropertyInChain( + target: Record +): string | undefined { + let current: Record | undefined = target; + + while ( + current && + (current.type === 'MemberExpression' || + current.type === 'OptionalMemberExpression') + ) { + const name = staticPropertyName(current); + if (name && FORBIDDEN_WRITE_PROPERTIES.has(name)) { + return name; + } + current = current.object as Record | undefined; + } + + return undefined; +} + /** * Validates that a JavaScript function is safe to execute * by checking the AST for allowed operations only (allowlist approach) @@ -107,6 +161,13 @@ export function validate(code: string): { return; } + // Block dynamic import(). @babel/parser emits 'Import' as the callee of + // the surrounding CallExpression; 'ImportExpression' is the ESTree shape. + if (node.type === 'Import' || node.type === 'ImportExpression') { + validationError = 'Dynamic import() is not allowed'; + return; + } + // Block function declarations inside the function body // (FunctionDeclaration creates a named function, not allowed) if (node.type === 'FunctionDeclaration') { @@ -212,12 +273,17 @@ export function validate(code: string): { const prop = callee.property as Record; const computed = callee.computed as boolean; + // A computed key (obj[expr]()) cannot be matched against the + // allowlist, because the property name is only known at run time. + // Refuse it rather than letting it past the checks below unseen. + if (computed) { + validationError = + 'Computed property access on a call target is not allowed. Use a literal method name, e.g. value.toUpperCase().'; + return; + } + // Static method call on global object: Math.random(), JSON.parse() - if ( - obj.type === 'Identifier' && - prop.type === 'Identifier' && - !computed - ) { + if (obj.type === 'Identifier' && prop.type === 'Identifier') { const objName = obj.name as string; const methodName = prop.name as string; @@ -232,7 +298,7 @@ export function validate(code: string): { // Instance method call: arr.map(), str.toLowerCase(), arr?.map() // We allow these if the method name is in ALLOWED_INSTANCE_METHODS - if (prop.type === 'Identifier' && !computed) { + if (prop.type === 'Identifier') { const methodName = prop.name as string; // If calling on something other than an allowed global, @@ -253,12 +319,35 @@ export function validate(code: string): { // Check 'new' expressions - only allow new Date() if (node.type === 'NewExpression') { const callee = node.callee as Record; - if (callee.type === 'Identifier') { - const name = callee.name as string; - if (name !== 'Date') { - validationError = `'new ${name}()' is not allowed. Only 'new Date()' is permitted.`; - return; - } + + // Anything other than a bare identifier (a member expression, a + // parenthesised expression, another call) names a constructor we + // cannot resolve, so it can never be the Date we allow. + if (callee.type !== 'Identifier') { + validationError = + "The target of 'new' must be a plain identifier. Only 'new Date()' is permitted."; + return; + } + + const name = callee.name as string; + if (name !== 'Date') { + validationError = `'new ${name}()' is not allowed. Only 'new Date()' is permitted.`; + return; + } + } + + // Block writes that reach the prototype chain: payload.__proto__.x = 1, + // payload['constructor'].prototype.y = 2. Reading these is already + // handled by the call and 'new' checks above. + if ( + node.type === 'AssignmentExpression' || + node.type === 'UpdateExpression' + ) { + const target = (node.left ?? node.argument) as Record; + const reached = forbiddenPropertyInChain(target); + if (reached) { + validationError = `Assigning through '${reached}' is not allowed.`; + return; } } }); From e73c91fabb7aeb10bb35e470713be4bd8cc32cd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carl-Gerhard=20Lindesv=C3=A4rd?= Date: Fri, 4 Sep 2026 08:42:52 +0000 Subject: [PATCH 2/2] fix(js-runtime): resolve no-substitution template-literal keys in write-chain check staticPropertyName only recognized StringLiteral computed keys, so a prototype write spelled with a template literal (payload[`__proto__`].x = 1) resolved to undefined and walked past forbiddenPropertyInChain unseen. A no-substitution template literal is exactly as static as a string literal, so resolve it the same way. --- packages/js-runtime/src/validate.test.ts | 8 ++++++++ packages/js-runtime/src/validate.ts | 20 +++++++++++++++++--- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/packages/js-runtime/src/validate.test.ts b/packages/js-runtime/src/validate.test.ts index af884eeb5..2a4bb8cc7 100644 --- a/packages/js-runtime/src/validate.test.ts +++ b/packages/js-runtime/src/validate.test.ts @@ -218,6 +218,14 @@ describe('validate', () => { expect(result.error).toContain('not allowed'); }); + it('should block writing through a no-substitution template literal key', () => { + const result = validate( + '(payload) => { payload[`__proto__`].x = 1; return payload; }', + ); + expect(result.valid).toBe(false); + expect(result.error).toContain('__proto__'); + }); + it('should still allow writing an ordinary property', () => { const result = validate( '(payload) => { const out = {}; out.event = payload.name; return out; }', diff --git a/packages/js-runtime/src/validate.ts b/packages/js-runtime/src/validate.ts index f59687be0..5e1e96208 100644 --- a/packages/js-runtime/src/validate.ts +++ b/packages/js-runtime/src/validate.ts @@ -33,9 +33,23 @@ function staticPropertyName( return undefined; } if (member.computed) { - // Only a literal key can be resolved. Babel has already decoded any - // \u / \x escapes into StringLiteral.value by this point. - return prop.type === 'StringLiteral' ? (prop.value as string) : undefined; + // A literal key can be resolved. Babel has already decoded any \u / \x + // escapes into StringLiteral.value by this point. A template literal + // with no substitutions (`__proto__`) is just as static as a string + // literal and must resolve the same way, or it walks past this check + // unseen. + if (prop.type === 'StringLiteral') { + return prop.value as string; + } + if (prop.type === 'TemplateLiteral') { + const expressions = prop.expressions as unknown[]; + const quasis = prop.quasis as Record[]; + if (expressions.length === 0 && quasis.length === 1) { + const value = quasis[0]!.value as Record; + return value.cooked as string; + } + } + return undefined; } return prop.type === 'Identifier' ? (prop.name as string) : undefined; }