From 307946d8ff8c6dc1d9a9e238f09d11ff85dc9f7c Mon Sep 17 00:00:00 2001 From: qqqys Date: Mon, 17 Aug 2026 14:19:44 +0800 Subject: [PATCH 1/2] fix(core): parse the workflow meta literal instead of evaluating it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `extractAndStripMeta` evaluated the model-authored `export const meta = {...}` block in a vm and then walked the result. Both halves are unbounded, and each hangs the host on its own: { name: (function () { while (true) {} })() } // spins during evaluation { get phases() { while (true) {} } } // spins during the walk The loops are synchronous, so the event loop is blocked outright — no timer fires and nothing in-process can cancel it. Bounding them turned out to be a moving target: a vm timeout does not reach a getter invoked on the host, and moving the walk into the vm still leaves promise reactions, proxy traps and runaway allocation. Each fix invited the next. Meta is a declaration, not a computation. Every contract field is a string, and upstream states the rule outright: the meta object must be a pure literal, with no variables, calls, spreads or interpolation. Given that contract, evaluating it was the wrong mechanism. Parse it instead. A parser has no execution semantics, so none of those failures are bounded — they are unrepresentable. There is nothing to time out, sandbox or isolate. The vm context, the timeout and the thenable walker all go away. The grammar is JSON's value grammar plus the spellings a model actually writes: unquoted keys, single-quoted and substitution-free template strings, trailing commas, and comments. Anything meaning "evaluate something" is rejected by name so the diagnostic tells the author which rule they hit. This narrows the contract. A meta block that computed a value used to work if the computed field was outside the contract surface, because validateMeta dropped it silently; now the whole literal is refused. Verified against every meta literal in the existing suite: of the ten the parser refuses and the vm accepted, nine are the attacks above, and the tenth is a regex literal in a non-contract field. Zero cases where both accept and disagree on the value. No workflow scripts ship in the repo, so nothing in tree changes behaviour. Parsing is also ~70x faster than the vm path it replaces, which matters because this call is on the path to the confirmation dialog and saved-workflow enumeration. Co-Authored-By: Claude Opus 5 (1M context) --- .../runtime/workflow-meta-literal.test.ts | 344 ++++++++++++++++ .../agents/runtime/workflow-meta-literal.ts | 381 ++++++++++++++++++ .../agents/runtime/workflow-sandbox.test.ts | 116 +++--- .../src/agents/runtime/workflow-sandbox.ts | 106 +---- 4 files changed, 808 insertions(+), 139 deletions(-) create mode 100644 packages/core/src/agents/runtime/workflow-meta-literal.test.ts create mode 100644 packages/core/src/agents/runtime/workflow-meta-literal.ts diff --git a/packages/core/src/agents/runtime/workflow-meta-literal.test.ts b/packages/core/src/agents/runtime/workflow-meta-literal.test.ts new file mode 100644 index 00000000000..e4d93eb431b --- /dev/null +++ b/packages/core/src/agents/runtime/workflow-meta-literal.test.ts @@ -0,0 +1,344 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect } from 'vitest'; +import { + parseWorkflowMetaLiteral, + WorkflowMetaSyntaxError, +} from './workflow-meta-literal.js'; + +/** Strip null prototypes so `toEqual` compares against plain object literals. */ +function plain(value: unknown): unknown { + if (Array.isArray(value)) return value.map(plain); + if (value && typeof value === 'object') { + const out: Record = {}; + for (const [k, v] of Object.entries(value)) out[k] = plain(v); + return out; + } + return value; +} + +describe('parseWorkflowMetaLiteral', () => { + describe('the contract shape', () => { + it('parses the minimal required fields', () => { + expect( + plain(parseWorkflowMetaLiteral(`{ name: 'w', description: 'd' }`)), + ).toEqual({ name: 'w', description: 'd' }); + }); + + it('parses the full contract including phases', () => { + const src = `{ + name: 'deep-research', + description: 'Research a question across sources', + whenToUse: 'when the question spans sources', + phases: [ + { title: 'Scout', detail: 'find the sources' }, + { title: 'Read', detail: 'read each one', model: 'fast' }, + { title: 'Synthesize' }, + ], + }`; + expect(plain(parseWorkflowMetaLiteral(src))).toEqual({ + name: 'deep-research', + description: 'Research a question across sources', + whenToUse: 'when the question spans sources', + phases: [ + { title: 'Scout', detail: 'find the sources' }, + { title: 'Read', detail: 'read each one', model: 'fast' }, + { title: 'Synthesize' }, + ], + }); + }); + }); + + describe('the JS spellings a model actually writes', () => { + it.each([ + ['double-quoted keys and values', `{ "name": "w", "description": "d" }`], + ['single quotes', `{ name: 'w', description: 'd' }`], + ['substitution-free template strings', '{ name: `w`, description: `d` }'], + ['trailing comma in an object', `{ name: 'w', description: 'd', }`], + ['line comments', `{ // lead\n name: 'w', description: 'd' }`], + ['block comments', `{ /* a */ name: 'w', /* b */ description: 'd' }`], + ])('accepts %s', (_label, src) => { + expect(plain(parseWorkflowMetaLiteral(src))).toEqual({ + name: 'w', + description: 'd', + }); + }); + + it('accepts a trailing comma in an array', () => { + const src = `{ name: 'w', description: 'd', phases: [{ title: 'A' },] }`; + expect(plain(parseWorkflowMetaLiteral(src))).toEqual({ + name: 'w', + description: 'd', + phases: [{ title: 'A' }], + }); + }); + + it('accepts a multi-line template string', () => { + const src = '{ name: `a\nb`, description: `d` }'; + expect(plain(parseWorkflowMetaLiteral(src))).toEqual({ + name: 'a\nb', + description: 'd', + }); + }); + + it('accepts numbers, booleans and null in non-contract fields', () => { + const src = `{ name: 'w', description: 'd', n: -1.5e3, b: true, f: false, z: null }`; + expect(plain(parseWorkflowMetaLiteral(src))).toEqual({ + name: 'w', + description: 'd', + n: -1500, + b: true, + f: false, + z: null, + }); + }); + + it('treats get/set/async as ordinary keys when no property name follows', () => { + const src = `{ name: 'w', description: 'd', get: 'a', set: 'b', async: 'c' }`; + expect(plain(parseWorkflowMetaLiteral(src))).toEqual({ + name: 'w', + description: 'd', + get: 'a', + set: 'b', + async: 'c', + }); + }); + }); + + // The hand-rolled escape handling is the fiddliest part of the parser, so + // each form is pinned against the equivalent JS string literal rather than + // against a hand-written expectation. + describe('string escapes', () => { + it.each([ + ['\\n', '\n'], + ['\\t', '\t'], + ['\\r', '\r'], + ['\\b', '\b'], + ['\\f', '\f'], + ['\\v', '\v'], + ['\\0', '\0'], + ['\\\\', '\\'], + ['\\/', '/'], + ['\\x41', 'A'], + ['\\u0041', 'A'], + ['\\u{1F600}', '\u{1F600}'], + ['\\u{41}', 'A'], + ['\\q', 'q'], + ])('decodes %s', (escape, expected) => { + const { name } = parseWorkflowMetaLiteral( + `{ name: "${escape}", description: "d" }`, + ) as { name: string }; + expect(name).toBe(expected); + }); + + it('decodes a quote escaped with its own quote character', () => { + expect( + parseWorkflowMetaLiteral( + `{ name: 'it\\'s', description: "a\\"b" }`, + ) as { + name: string; + description: string; + }, + ).toMatchObject({ name: "it's", description: 'a"b' }); + }); + + it('treats a backslash-newline as a line continuation', () => { + const { name } = parseWorkflowMetaLiteral( + '{ name: "a\\\nb", description: "d" }', + ) as { name: string }; + expect(name).toBe('ab'); + }); + + it('preserves non-ASCII text verbatim', () => { + const { name } = parseWorkflowMetaLiteral( + `{ name: '工作流 🪜', description: 'd' }`, + ) as { name: string }; + expect(name).toBe('工作流 🪜'); + }); + + it.each([ + ['an octal escape', String.raw`{ name: "\01", description: "d" }`], + ['a short \\x escape', String.raw`{ name: "\xZZ", description: "d" }`], + ['a short \\u escape', String.raw`{ name: "\u00", description: "d" }`], + [ + 'an out-of-range \\u{} escape', + String.raw`{ name: "\u{FFFFFF}", description: "d" }`, + ], + ['an unterminated escape', `{ name: "a\\`], + ])('rejects %s', (_label, src) => { + expect(() => parseWorkflowMetaLiteral(src)).toThrow( + WorkflowMetaSyntaxError, + ); + }); + + it('rejects a newline inside a non-template string', () => { + expect(() => + parseWorkflowMetaLiteral('{ name: "a\nb", description: "d" }'), + ).toThrow(/unterminated string/); + }); + }); + + // Each of these drove at least one review finding while meta was evaluated. + // They are not "bounded" now — they are unrepresentable. + describe('everything that would mean "evaluate something"', () => { + it.each([ + [ + 'an identifier', + `{ name: someVar, description: 'd' }`, + /unsupported value/, + ], + ['a call', `{ name: compute(), description: 'd' }`, /unsupported value/], + [ + 'an IIFE', + `{ name: (function(){ while(true){} })(), description: 'd' }`, + /unsupported value/, + ], + [ + 'a getter', + `{ name: 'x', description: 'd', get phases() { return []; } }`, + /getters are not allowed/, + ], + [ + 'a setter', + `{ name: 'x', description: 'd', set phases(v) {} }`, + /setters are not allowed/, + ], + [ + 'a method', + `{ name: 'x', description: 'd', toString() { return 'x'; } }`, + /methods are not allowed/, + ], + [ + 'an async member', + `{ name: 'x', description: 'd', async load() {} }`, + /async members are not allowed/, + ], + [ + 'a spread', + `{ ...other, name: 'x', description: 'd' }`, + /spread is not allowed/, + ], + [ + 'a computed key', + `{ ['na' + 'me']: 'x', description: 'd' }`, + /computed keys are not allowed/, + ], + [ + 'a template substitution', + '{ name: `v${version}`, description: `d` }', + /template substitutions are not allowed/, + ], + [ + 'a regex literal', + `{ name: 'x', description: 'd', pattern: /a[b]c/g }`, + /regular expressions are not allowed/, + ], + [ + 'a dynamic import', + `{ name: 'x', description: import('node:fs') }`, + /unsupported value/, + ], + [ + 'a new expression', + `{ name: new String('x'), description: 'd' }`, + /unsupported value/, + ], + [ + 'an operator', + `{ name: 'a' + 'b', description: 'd' }`, + /expected "," or "}"/, + ], + [ + 'a bigint', + `{ name: 'x', description: 'd', n: 1n }`, + /unsupported numeric literal/, + ], + [ + 'a hex literal', + `{ name: 'x', description: 'd', n: 0x10 }`, + /unsupported numeric literal/, + ], + [ + 'an array hole', + `{ name: 'x', description: 'd', phases: [, {}] }`, + /missing array element/, + ], + ])('rejects %s', (_label, src, pattern) => { + expect(() => parseWorkflowMetaLiteral(src)).toThrow(pattern); + }); + + it('names the rule in every rejection so the author knows what to write', () => { + try { + parseWorkflowMetaLiteral(`{ name: someVar, description: 'd' }`); + throw new Error('expected a rejection'); + } catch (e) { + expect((e as Error).message).toMatch( + /meta must be a plain object literal/, + ); + expect((e as Error).message).toMatch(/no variables, function calls/); + } + }); + + it('reports the offending position', () => { + try { + parseWorkflowMetaLiteral(`{ name: 'x', description: someVar }`); + throw new Error('expected a rejection'); + } catch (e) { + expect(e).toBeInstanceOf(WorkflowMetaSyntaxError); + expect((e as WorkflowMetaSyntaxError).index).toBe(26); + } + }); + }); + + describe('structural limits', () => { + it('rejects nesting past the depth cap without overflowing the stack', () => { + const src = `{ name: 'x', description: 'd', a: ${'['.repeat(200)}${']'.repeat(200)} }`; + expect(() => parseWorkflowMetaLiteral(src)).toThrow(/nested too deeply/); + }); + + it('accepts nesting up to the depth cap', () => { + const src = `{ a: ${'['.repeat(30)}${']'.repeat(30)} }`; + expect(() => parseWorkflowMetaLiteral(src)).not.toThrow(); + }); + + it.each([ + ['an unterminated object', `{ name: 'x'`], + ['an unterminated array', `{ name: 'x', phases: [`], + ['an unterminated comment', `{ /* name: 'x' }`], + ['trailing content', `{ name: 'x', description: 'd' } trailing`], + ['a missing colon', `{ name 'x' }`], + ['an empty source', ``], + ])('rejects %s', (_label, src) => { + expect(() => parseWorkflowMetaLiteral(src)).toThrow( + WorkflowMetaSyntaxError, + ); + }); + }); + + // A `__proto__` key in JSON.parse is an own property; in an object literal it + // would set the prototype. The parser builds null-prototype objects so the + // key is inert either way. + describe('prototype safety', () => { + it('treats __proto__ as an ordinary own key and does not pollute', () => { + const value = parseWorkflowMetaLiteral( + `{ name: 'x', description: 'd', "__proto__": { polluted: 1 } }`, + ) as Record; + expect(Object.getPrototypeOf(value)).toBeNull(); + expect(Object.prototype.hasOwnProperty.call(value, '__proto__')).toBe( + true, + ); + expect(({} as Record)['polluted']).toBeUndefined(); + }); + + it('treats constructor as an ordinary key', () => { + const value = parseWorkflowMetaLiteral( + `{ name: 'x', description: 'd', constructor: 'safe' }`, + ) as Record; + expect(value['constructor']).toBe('safe'); + }); + }); +}); diff --git a/packages/core/src/agents/runtime/workflow-meta-literal.ts b/packages/core/src/agents/runtime/workflow-meta-literal.ts new file mode 100644 index 00000000000..c899b5d0f4c --- /dev/null +++ b/packages/core/src/agents/runtime/workflow-meta-literal.ts @@ -0,0 +1,381 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Static parser for a workflow script's `export const meta = {...}` block. + * + * The meta contract is `{ name, description, whenToUse?, phases?: [{ title, detail?, model? }] }` + * — every field is a string. Meta is a declaration, not a computation, so this + * parses the literal instead of executing it. + * + * That distinction is the point. The literal is model-authored source; running it + * means running whatever the model wrote, and the ways that can fail are open-ended: + * a loop in a field value, a getter that only spins when the value is read, a proxy + * trap, a promise reaction that never settles, an allocation large enough to exhaust + * memory. Bounding each of those is a moving target. A parser has no execution + * semantics at all, so none of them are representable — there is nothing to bound, + * time out, or isolate. + * + * The grammar is JSON's value grammar plus the JS spellings a model actually + * writes: unquoted keys, single-quoted and substitution-free template strings, + * trailing commas, and comments. Anything that would mean "evaluate something" — + * an identifier, a call, a spread, a computed key, an accessor, a method, a + * template substitution, a regex — is rejected by name, so the diagnostic tells the + * author which rule they hit. + */ + +/** Thrown when the meta block is not a plain literal. */ +export class WorkflowMetaSyntaxError extends Error { + readonly index: number; + constructor(message: string, index: number) { + super(message); + this.name = 'WorkflowMetaSyntaxError'; + this.index = index; + } +} + +export type MetaLiteralValue = + | string + | number + | boolean + | null + | MetaLiteralValue[] + | { [key: string]: MetaLiteralValue }; + +/** + * Parse a `{...}` meta literal into a plain host value. + * + * Objects are built with a null prototype so a `__proto__` key in the source is an + * ordinary own property and cannot reach `Object.prototype`. Callers copy the + * contract fields out into their own object, so the null prototype never escapes. + * + * @throws {WorkflowMetaSyntaxError} if the source is not a pure literal. + */ +export function parseWorkflowMetaLiteral(source: string): MetaLiteralValue { + const parser = new MetaLiteralParser(source); + parser.skipTrivia(); + const value = parser.parseValue(0); + parser.skipTrivia(); + if (parser.index < source.length) { + throw parser.fail('unexpected trailing content'); + } + return value; +} + +// Deep enough for any real contract object (`phases[]` of flat objects is depth 3) +// and shallow enough that the recursive descent cannot overflow the stack. +const MAX_DEPTH = 32; + +const WHITESPACE = new Set([' ', '\t', '\n', '\r', '\f', '\v', ' ', '']); +const ID_START = /[A-Za-z_$]/; +const ID_PART = /[A-Za-z0-9_$]/; + +class MetaLiteralParser { + index = 0; + constructor(private readonly src: string) {} + + fail(message: string): WorkflowMetaSyntaxError { + // A window around the offence reads far better than a bare offset. + const from = Math.max(0, this.index - 24); + const snippet = this.src + .slice(from, this.index + 24) + .replace(/\s+/g, ' ') + .trim(); + return new WorkflowMetaSyntaxError( + `${message} at position ${this.index} (near "${snippet}"). ` + + `meta must be a plain object literal — strings, numbers, booleans, null, ` + + `arrays and objects only, with no variables, function calls, spreads, ` + + `accessors or template substitutions.`, + this.index, + ); + } + + skipTrivia(): void { + for (;;) { + while ( + this.index < this.src.length && + WHITESPACE.has(this.src[this.index]!) + ) { + this.index++; + } + if (this.src.startsWith('//', this.index)) { + const newline = this.src.indexOf('\n', this.index); + this.index = newline === -1 ? this.src.length : newline + 1; + continue; + } + if (this.src.startsWith('/*', this.index)) { + const end = this.src.indexOf('*/', this.index + 2); + if (end === -1) throw this.fail('unterminated comment'); + this.index = end + 2; + continue; + } + return; + } + } + + parseValue(depth: number): MetaLiteralValue { + if (depth > MAX_DEPTH) throw this.fail('meta literal is nested too deeply'); + this.skipTrivia(); + const c = this.src[this.index]; + if (c === undefined) throw this.fail('unexpected end of meta literal'); + if (c === '{') return this.parseObject(depth); + if (c === '[') return this.parseArray(depth); + if (c === '"' || c === "'" || c === '`') return this.parseString(); + if (c === '-' || (c >= '0' && c <= '9')) return this.parseNumber(); + if (this.src.startsWith('true', this.index)) return this.word('true', true); + if (this.src.startsWith('false', this.index)) { + return this.word('false', false); + } + if (this.src.startsWith('null', this.index)) return this.word('null', null); + if (c === '/') + throw this.fail('regular expressions are not allowed in meta'); + throw this.fail('unsupported value'); + } + + private word(literal: string, value: T): T { + // `truey` / `nullish` are identifiers, not the keyword. + const after = this.src[this.index + literal.length]; + if (after !== undefined && ID_PART.test(after)) { + throw this.fail('unsupported value'); + } + this.index += literal.length; + return value; + } + + private parseObject(depth: number): MetaLiteralValue { + this.index++; // '{' + const out = Object.create(null) as { [key: string]: MetaLiteralValue }; + for (;;) { + this.skipTrivia(); + const c = this.src[this.index]; + if (c === '}') { + this.index++; + return out; + } + if (c === undefined) throw this.fail('unterminated object'); + if (this.src.startsWith('...', this.index)) { + throw this.fail('spread is not allowed in meta'); + } + if (c === '[') throw this.fail('computed keys are not allowed in meta'); + const key = this.parseKey(); + this.skipTrivia(); + if (this.src[this.index] === '(') { + throw this.fail('methods are not allowed in meta'); + } + if (this.src[this.index] !== ':') + throw this.fail('expected ":" after key'); + this.index++; + out[key] = this.parseValue(depth + 1); + this.skipTrivia(); + if (this.src[this.index] === ',') { + this.index++; + continue; + } + if (this.src[this.index] === '}') { + this.index++; + return out; + } + throw this.fail('expected "," or "}"'); + } + } + + private parseKey(): string { + const c = this.src[this.index]; + if (c === '"' || c === "'" || c === '`') return this.parseString(); + if (c !== undefined && ID_START.test(c)) { + const start = this.index; + while ( + this.index < this.src.length && + ID_PART.test(this.src[this.index]!) + ) { + this.index++; + } + const word = this.src.slice(start, this.index); + // `get title() {...}` is executable code wearing a property's clothes; so is + // `async name() {...}`. Only treat the word as a modifier when a property + // name follows it — `{ get: 'x' }` is a legitimate field called "get". + if (word === 'get' || word === 'set' || word === 'async') { + const save = this.index; + this.skipTrivia(); + const next = this.src[this.index]; + const startsPropertyName = + next !== undefined && + (ID_START.test(next) || next === '"' || next === "'" || next === '['); + this.index = save; + if (startsPropertyName) { + throw this.fail( + word === 'async' + ? 'async members are not allowed in meta' + : `${word}ters are not allowed in meta`, + ); + } + } + return word; + } + throw this.fail('expected a property name'); + } + + private parseArray(depth: number): MetaLiteralValue { + this.index++; // '[' + const out: MetaLiteralValue[] = []; + for (;;) { + this.skipTrivia(); + const c = this.src[this.index]; + if (c === ']') { + this.index++; + return out; + } + if (c === undefined) throw this.fail('unterminated array'); + if (this.src.startsWith('...', this.index)) { + throw this.fail('spread is not allowed in meta'); + } + // `[1, , 2]` — an elision is a hole, which has no literal meaning here. + if (c === ',') throw this.fail('missing array element'); + out.push(this.parseValue(depth + 1)); + this.skipTrivia(); + if (this.src[this.index] === ',') { + this.index++; + continue; + } + if (this.src[this.index] === ']') { + this.index++; + return out; + } + throw this.fail('expected "," or "]"'); + } + } + + private parseString(): string { + const quote = this.src[this.index++]!; + let out = ''; + for (;;) { + const c = this.src[this.index]; + if (c === undefined) throw this.fail('unterminated string'); + if (c === quote) { + this.index++; + return out; + } + // Only a template literal may span lines. + if (c === '\n' && quote !== '`') throw this.fail('unterminated string'); + if (quote === '`' && c === '$' && this.src[this.index + 1] === '{') { + throw this.fail('template substitutions are not allowed in meta'); + } + if (c === '\\') { + out += this.parseEscape(); + continue; + } + out += c; + this.index++; + } + } + + private parseEscape(): string { + this.index++; // '\' + const c = this.src[this.index++]; + switch (c) { + case 'n': + return '\n'; + case 't': + return '\t'; + case 'r': + return '\r'; + case 'b': + return '\b'; + case 'f': + return '\f'; + case 'v': + return '\v'; + case '0': + // `\0` is NUL; `\01` is a legacy octal escape, which is a syntax error in + // strict mode and would silently mean something else here. + if (/[0-9]/.test(this.src[this.index] ?? '')) { + throw this.fail('octal escapes are not allowed in meta'); + } + return '\0'; + case 'x': { + const hex = this.src.slice(this.index, this.index + 2); + if (!/^[0-9a-fA-F]{2}$/.test(hex)) { + throw this.fail('invalid \\x escape'); + } + this.index += 2; + return String.fromCharCode(parseInt(hex, 16)); + } + case 'u': { + if (this.src[this.index] === '{') { + const end = this.src.indexOf('}', this.index); + const hex = end === -1 ? '' : this.src.slice(this.index + 1, end); + if (!/^[0-9a-fA-F]{1,6}$/.test(hex)) { + throw this.fail('invalid \\u{...} escape'); + } + const code = parseInt(hex, 16); + if (code > 0x10ffff) throw this.fail('invalid \\u{...} escape'); + this.index = end + 1; + return String.fromCodePoint(code); + } + const hex = this.src.slice(this.index, this.index + 4); + if (!/^[0-9a-fA-F]{4}$/.test(hex)) { + throw this.fail('invalid \\u escape'); + } + this.index += 4; + return String.fromCharCode(parseInt(hex, 16)); + } + case '\n': + return ''; // line continuation + case '\r': + // CRLF line continuation. + if (this.src[this.index] === '\n') this.index++; + return ''; + case undefined: + throw this.fail('unterminated escape sequence'); + default: + // `\\`, `\'`, `\"`, `` \` ``, `\/` and any other identity escape. + return c; + } + } + + private parseNumber(): number { + const start = this.index; + if (this.src[this.index] === '-') this.index++; + while ( + this.index < this.src.length && + /[0-9]/.test(this.src[this.index]!) + ) { + this.index++; + } + if (this.src[this.index] === '.') { + this.index++; + while ( + this.index < this.src.length && + /[0-9]/.test(this.src[this.index]!) + ) { + this.index++; + } + } + if (this.src[this.index] === 'e' || this.src[this.index] === 'E') { + this.index++; + if (this.src[this.index] === '+' || this.src[this.index] === '-') { + this.index++; + } + while ( + this.index < this.src.length && + /[0-9]/.test(this.src[this.index]!) + ) { + this.index++; + } + } + // Rejects `1n`, `0x10`, `1abc` — anything the scan above did not consume. + const after = this.src[this.index]; + if (after !== undefined && ID_PART.test(after)) { + throw this.fail('unsupported numeric literal'); + } + const text = this.src.slice(start, this.index); + const value = Number(text); + if (text.length === 0 || !Number.isFinite(value)) { + throw this.fail('unsupported numeric literal'); + } + return value; + } +} diff --git a/packages/core/src/agents/runtime/workflow-sandbox.test.ts b/packages/core/src/agents/runtime/workflow-sandbox.test.ts index 73037a81fa4..bde21321ef4 100644 --- a/packages/core/src/agents/runtime/workflow-sandbox.test.ts +++ b/packages/core/src/agents/runtime/workflow-sandbox.test.ts @@ -221,43 +221,36 @@ describe('extractAndStripMeta', () => { }); // Security regression: the meta-eval vm context has no globals at all - // (Object.create(null) prototype), so the model cannot reach host - // primitives during meta evaluation — even ones that the script-side - // sandbox normally provides (args, agent, phase, log, parallel, - // pipeline). Referencing any of them throws ReferenceError. Two - // shapes pinned: a truly unknown identifier (R7 dedup — was a - // duplicate of the bridge-global case below) and explicit `args` - // bridge-global access. + // meta is parsed, not executed, so an identifier is not "resolved to + // undefined" or "looked up and not found" — it is simply not a value the + // grammar admits. Two shapes pinned: a truly unknown identifier and + // explicit `args` bridge-global access. it('rejects meta that references an unknown identifier', () => { const src = `export const meta = { name: totallyUnknown, description: 'd' }\nreturn 1`; expect(() => extractAndStripMeta(src)).toThrow( - /failed to evaluate meta object literal/, + /invalid meta object literal/, ); }); - // Security regression: the meta-eval context's globalThis is null- - // prototyped, so the model has no bridge to host primitives like - // `process`, `require`, or the workflow-sandbox bridge globals - // (`args` / `agent` / `phase` / `log` / etc.). The vm realm still - // exposes its OWN intrinsics (`Object`, `Math`, `Date`, …) which is - // fine — meta extraction is one-shot at tool-invocation time, not - // replayed on resume, so it can be non-deterministic without breaking - // the resume contract that the script body honors. + // Security regression: there is no evaluation, so there is no scope to + // reach out of — neither the workflow-sandbox bridge globals + // (`args` / `agent` / `phase` / `log` / …) nor host primitives like + // `process` and `require` are reachable, because no identifier is. it('meta source cannot reference a workflow-sandbox bridge global (args)', () => { const src = `export const meta = { name: args.x, description: 'd' }\nreturn 1`; expect(() => extractAndStripMeta(src)).toThrow( - /failed to evaluate meta object literal/, + /invalid meta object literal/, ); }); it('meta source cannot reach the host process / require / fs', () => { const src1 = `export const meta = { name: process.version, description: 'd' }\nreturn 1`; expect(() => extractAndStripMeta(src1)).toThrow( - /failed to evaluate meta object literal/, + /invalid meta object literal/, ); const src2 = `export const meta = { name: 'x', description: require('fs').readFileSync('/etc/passwd', 'utf8') }\nreturn 1`; expect(() => extractAndStripMeta(src2)).toThrow( - /failed to evaluate meta object literal/, + /invalid meta object literal/, ); }); @@ -297,23 +290,37 @@ describe('extractAndStripMeta', () => { }); // P4a Round 3 (wenshao): a Promise (e.g. `import('node:fs')`) used as a - // value in the meta literal previously crashed the host process. The - // synchronous `runInContext` returns normally with a dangling rejection - // scheduled for the next tick; validateMeta passes (the field isn't on - // the contract surface so it's silently dropped); the workflow even - // returns its result; THEN the unhandled rejection terminates the - // process under Node's default `--unhandled-rejections=throw`. The fix - // is to walk the eval result, neutralise any thenables with a `.catch` - // so they no longer trigger the unhandled-rejection handler, and throw - // an explicit error so the bad meta is rejected up front. - it('throws when meta value is a Promise (dynamic import) — no unhandled rejection crash', () => { - const src = `export const meta = { name: 'x', description: 'd', extra: import('node:fs') }\nreturn 1`; - expect(() => extractAndStripMeta(src)).toThrow( - /meta values must not be Promises/, - ); + // value in the meta literal used to crash the host process — evaluation + // returned normally with a dangling rejection scheduled for the next tick, + // validateMeta dropped the non-contract field, the workflow returned its + // result, and only THEN did the unhandled rejection terminate the process + // under Node's default `--unhandled-rejections=throw`. That hazard was + // handled by walking the evaluated value and neutralising thenables. + // + // Nothing is evaluated now, so no Promise is ever constructed and there is + // no rejection to neutralise. The literal is rejected on syntax instead. + // These tests assert both halves: the call throws, AND the process records + // no unhandled rejection — the second is the property users actually cared + // about, and it is now guaranteed structurally rather than by a walker. + it('rejects a Promise-valued meta field (dynamic import) with no unhandled rejection', async () => { + const unhandled: unknown[] = []; + const onUnhandled = (reason: unknown) => unhandled.push(reason); + process.on('unhandledRejection', onUnhandled); + try { + const src = `export const meta = { name: 'x', description: 'd', extra: import('node:fs') }\nreturn 1`; + expect(() => extractAndStripMeta(src)).toThrow( + /invalid meta object literal/, + ); + // Let any rejection that a previous implementation would have scheduled + // reach the handler before asserting none arrived. + await new Promise((resolve) => setImmediate(resolve)); + expect(unhandled).toEqual([]); + } finally { + process.off('unhandledRejection', onUnhandled); + } }); - it('throws when meta value is a Promise nested inside a phases entry', () => { + it('rejects a Promise-valued meta field nested inside a phases entry', () => { const src = `export const meta = { name: 'x', description: 'd', @@ -321,7 +328,7 @@ describe('extractAndStripMeta', () => { } return 1`; expect(() => extractAndStripMeta(src)).toThrow( - /meta values must not be Promises/, + /invalid meta object literal/, ); }); @@ -343,34 +350,34 @@ describe('extractAndStripMeta', () => { expect(sandbox.getPhases()).toEqual(['X', 'Y', 'X']); }); - // P4 Round 4 (wenshao): the R3 thenable walker recursed without a - // seen-guard. A meta literal that builds a cyclic object via spread - // (no getters, no Promises, no exotic constructs — just self-reference) - // overflows the call stack. The walker's RangeError propagates OUT of - // extractAndStripMeta because the try/catch only wraps the vm-eval, so - // the run failure surfaces as `Maximum call stack size exceeded` rather - // than the meta-validation error this guard exists to produce. A - // WeakSet bounds the recursion against cycles AND against future - // shapes where the same node is reached through multiple keys. - it('rejects a cyclic meta value built via spread without stack-overflowing', () => { + // P4 Round 4 (wenshao): a meta literal that built a cyclic object via + // spread used to overflow the walker's stack. Both fixtures below used to + // SUCCEED — the cyclic field was not a contract field, so validateMeta + // dropped it and the run continued. + // + // They are now refused, and that is a deliberate narrowing of the contract + // rather than a regression: a spread means "evaluate this expression and + // merge the result", which is exactly what meta no longer does. A literal + // cannot be cyclic, so the whole class of cycle-walking concerns is gone + // with it. The message names the rule so the author knows what to write + // instead. + it('refuses a spread in meta rather than evaluating it', () => { const src = `export const meta = { name: 'x', description: 'y', ...(function () { const a = {}; a.self = a; return a; })(), } return 1`; - // The cyclic field should be silently ignored by validateMeta (it's - // not a contract field), so the run succeeds with just the required - // fields surviving — but only if the walker terminates first. - const { meta } = extractAndStripMeta(src); - expect(meta).toEqual({ name: 'x', description: 'y' }); + expect(() => extractAndStripMeta(src)).toThrow( + /spread is not allowed in meta/, + ); }); - it('rejects a cyclic meta value reached through nested arrays/objects', () => { + it('refuses a spread that would have built a cycle through nested arrays', () => { const src = `export const meta = { name: 'x', description: 'y', - // Cycle reached through phases[0].back → ref back to outer container. + // Cycle reached through items[0].ref → ref back to outer container. ...(function () { const outer = { items: [] }; outer.items.push({ ref: outer }); @@ -378,8 +385,9 @@ describe('extractAndStripMeta', () => { })(), } return 1`; - const { meta } = extractAndStripMeta(src); - expect(meta).toEqual({ name: 'x', description: 'y' }); + expect(() => extractAndStripMeta(src)).toThrow( + /spread is not allowed in meta/, + ); }); }); diff --git a/packages/core/src/agents/runtime/workflow-sandbox.ts b/packages/core/src/agents/runtime/workflow-sandbox.ts index 2eba8def988..e26c6f95386 100644 --- a/packages/core/src/agents/runtime/workflow-sandbox.ts +++ b/packages/core/src/agents/runtime/workflow-sandbox.ts @@ -155,25 +155,23 @@ export interface WorkflowMeta { * Implementation: * 1. `findMetaBlockBounds` (shared with `stripExportMeta`) locates the * object-literal source range via the brace-walker. - * 2. The literal source is evaluated as `(${metaSource})` inside a fresh - * vm context whose globalThis is a null-prototyped object — no - * bridge to the host realm, no access to host primitives like - * `process` / `require` / the workflow-sandbox bridge globals - * (`args` / `agent` / `phase` / `log` / etc.). The vm realm DOES - * provide its own intrinsics (`Object`, `Array`, `Math`, `Date`, - * `JSON`, …) which is fine: meta extraction is a one-shot at tool- - * invocation time, not replayed during resume, so non-determinism in - * the meta literal (a `Date.now()` call in `meta.name`) does not - * break the resume contract that the script body honors. - * 3. The vm result is walked field-by-field and copied into a new - * host-realm plain object. No JSON round-trip is needed because every - * contract field is a primitive — strings and arrays of plain - * objects with string fields — so prototype identity on the - * intermediate values is irrelevant. + * 2. `parseWorkflowMetaLiteral` parses that range. Meta is a declaration — + * every contract field is a string — so it is parsed, never executed. + * 3. `validateMeta` copies the contract fields into a fresh host object. + * + * Parsing rather than evaluating is what keeps this safe. The literal is + * model-authored source, so executing it means executing whatever the model + * wrote, and the ways that can go wrong are open-ended: a loop in a field + * value, a getter that only spins when the value is read, a proxy trap, a + * promise reaction that never settles, an allocation large enough to exhaust + * memory, a dynamic `import()` whose rejection lands on a later tick and takes + * the host process with it. Bounding each of those in turn is a moving target. + * A parser has no execution semantics, so none of them can be expressed — + * there is nothing to time out, sandbox, or isolate. * * Returns `{ stripped, meta: null }` when no meta declaration is present * (callers treat this as "no meta"). Throws when meta is present but - * malformed: vm eval failure, missing required field, or wrong field type. + * malformed: not a pure literal, missing required field, or wrong field type. * Error messages for the missing-required-field cases match upstream * 2.1.168 verbatim so script authors see one consistent error text. */ @@ -188,86 +186,23 @@ export function extractAndStripMeta(source: string): { const stripped = source.slice(0, bounds.exportIdx) + source.slice(bounds.afterMeta); - // Null-prototyped globalThis: no host bridge (no `process` / `require` - // / `args` / workflow-sandbox bridge globals). The vm realm still - // provides its own intrinsics, but that's intentional — see the - // docstring above. - const metaContext = vm.createContext(Object.create(null)); let raw: unknown; try { - raw = new vm.Script(`(${metaSource})`).runInContext(metaContext); + raw = parseWorkflowMetaLiteral(metaSource); } catch (e) { const msg = e instanceof Error ? e.message : String(e); - throw new Error( - `extractAndStripMeta: failed to evaluate meta object literal: ${msg}`, - ); + throw new Error(`extractAndStripMeta: invalid meta object literal: ${msg}`); } - // P4a R3 (wenshao): a Promise (e.g. `import('node:fs')`) used as a - // value in the meta literal would otherwise leave a dangling rejection - // behind — `runInContext` returns synchronously with the Promise scheduled - // to reject on the next tick, validateMeta drops the non-contract field - // silently, and the run completes successfully. Then Node's default - // `--unhandled-rejections=throw` terminates the host process, decoupled - // from the run that triggered it. Walk `raw`, neutralise any thenables - // with `.catch(() => {})` so the rejection is marked handled, and reject - // the meta literal up front. - rejectThenablesInMeta(raw); - const meta = validateMeta(raw); return { stripped, meta }; } /** - * Recursively scan a vm-eval'd value, marking any thenable as handled - * (so its rejection cannot terminate the host on the next tick) and - * throwing an explicit "meta values must not be Promises" so the - * malformed meta is reported clearly. - * - * Recurses through plain objects and arrays — `phases[]` entries may - * embed an `import()` below the top level. - */ -function rejectThenablesInMeta( - value: unknown, - seen: WeakSet = new WeakSet(), -): void { - if (value === null || typeof value !== 'object') return; - // P4 Round 4 (wenshao): a cyclic meta literal built via spread of a - // self-referential object would otherwise overflow the call stack on - // this walk — the walker exists to reject Promises before they leave - // a dangling rejection, but the walk itself must terminate on any - // shape vm-eval can return. Track visited nodes in a WeakSet so cycles - // and shared subgraphs both early-return without re-walking. - if (seen.has(value as object)) return; - seen.add(value as object); - const maybeThen = (value as { then?: unknown }).then; - if (typeof maybeThen === 'function') { - // Mark handled so Node's unhandled-rejection trap does not later kill - // the process. `.catch` on a non-Promise thenable would synchronously - // throw if the implementation is non-standard, so swallow defensively. - try { - (value as Promise).catch(() => {}); - } catch { - /* non-standard thenable — already rejecting below */ - } - throw new Error( - 'extractAndStripMeta: meta values must not be Promises ' + - '(no async / dynamic import allowed in meta literal)', - ); - } - if (Array.isArray(value)) { - for (const v of value) rejectThenablesInMeta(v, seen); - return; - } - for (const v of Object.values(value as Record)) { - rejectThenablesInMeta(v, seen); - } -} - -/** - * Validate the vm-eval'd meta value and copy it into a fresh host-realm - * plain object. Throws on shape violation with the upstream-aligned error - * message text for the required-field cases. + * Validate the parsed meta value and copy it into a fresh plain object — the + * parser returns null-prototype objects, and this is where the contract fields + * cross over into ordinary ones. Throws on shape violation with the + * upstream-aligned error message text for the required-field cases. * * Field rules: * - `name` required, non-empty string @@ -363,6 +298,7 @@ function isRegexContext(source: string, i: number): boolean { import * as vm from 'node:vm'; import { createDebugLogger } from '../../utils/debugLogger.js'; +import { parseWorkflowMetaLiteral } from './workflow-meta-literal.js'; import type { WorkflowDispatchScheduler } from './workflow-dispatch-scheduler.js'; // Shared with workflow-orchestrator (avoids a duplicate createDebugLogger From c82e98af9e6e740966e85636d298b4674200ee79 Mon Sep 17 00:00:00 2001 From: qqqys <266654365+qqqys@users.noreply.github.com> Date: Mon, 17 Aug 2026 17:38:45 +0800 Subject: [PATCH 2/2] fix(core): handle workflow meta line terminators --- .../runtime/workflow-meta-literal.test.ts | 51 +++++++++++++++++++ .../agents/runtime/workflow-meta-literal.ts | 24 +++++++-- 2 files changed, 72 insertions(+), 3 deletions(-) diff --git a/packages/core/src/agents/runtime/workflow-meta-literal.test.ts b/packages/core/src/agents/runtime/workflow-meta-literal.test.ts index e4d93eb431b..94deb715e3b 100644 --- a/packages/core/src/agents/runtime/workflow-meta-literal.test.ts +++ b/packages/core/src/agents/runtime/workflow-meta-literal.test.ts @@ -85,6 +85,24 @@ describe('parseWorkflowMetaLiteral', () => { }); }); + it.each([ + ['LF', '\n'], + ['CRLF', '\r\n'], + ['CR', '\r'], + ['line separator', '\u2028'], + ['paragraph separator', '\u2029'], + ])('ends a line comment at %s', (_label, terminator) => { + const src = + `{ name: 'w', description: 'd' // note` + + terminator + + `, whenToUse: 'u' }`; + expect(plain(parseWorkflowMetaLiteral(src))).toEqual({ + name: 'w', + description: 'd', + whenToUse: 'u', + }); + }); + it('accepts numbers, booleans and null in non-contract fields', () => { const src = `{ name: 'w', description: 'd', n: -1.5e3, b: true, f: false, z: null }`; expect(plain(parseWorkflowMetaLiteral(src))).toEqual({ @@ -153,6 +171,31 @@ describe('parseWorkflowMetaLiteral', () => { expect(name).toBe('ab'); }); + it.each([ + ['LF', '\n'], + ['CRLF', '\r\n'], + ['CR', '\r'], + ['line separator', '\u2028'], + ['paragraph separator', '\u2029'], + ])('treats a backslash-%s as a line continuation', (_label, terminator) => { + for (const quote of ['"', "'", '`']) { + const src = + `{ name: ${quote}a\\` + terminator + `b${quote}, description: 'd' }`; + const { name } = parseWorkflowMetaLiteral(src) as { name: string }; + expect(name).toBe('ab'); + } + }); + + it.each([ + ['CR', '\r'], + ['CRLF', '\r\n'], + ])('cooks raw %s to LF in a template string', (_label, terminator) => { + const { name } = parseWorkflowMetaLiteral( + '{ name: `a' + terminator + "b`, description: 'd' }", + ) as { name: string }; + expect(name).toBe('a\nb'); + }); + it('preserves non-ASCII text verbatim', () => { const { name } = parseWorkflowMetaLiteral( `{ name: '工作流 🪜', description: 'd' }`, @@ -180,6 +223,14 @@ describe('parseWorkflowMetaLiteral', () => { parseWorkflowMetaLiteral('{ name: "a\nb", description: "d" }'), ).toThrow(/unterminated string/); }); + + it.each(['"', "'"])('rejects a raw CR inside a %s string', (quote) => { + expect(() => + parseWorkflowMetaLiteral( + `{ name: ${quote}a\rb${quote}, description: 'd' }`, + ), + ).toThrow(/unterminated string/); + }); }); // Each of these drove at least one review finding while meta was evaluated. diff --git a/packages/core/src/agents/runtime/workflow-meta-literal.ts b/packages/core/src/agents/runtime/workflow-meta-literal.ts index c899b5d0f4c..e5a8a44e061 100644 --- a/packages/core/src/agents/runtime/workflow-meta-literal.ts +++ b/packages/core/src/agents/runtime/workflow-meta-literal.ts @@ -102,8 +102,15 @@ class MetaLiteralParser { this.index++; } if (this.src.startsWith('//', this.index)) { - const newline = this.src.indexOf('\n', this.index); - this.index = newline === -1 ? this.src.length : newline + 1; + let end = this.index + 2; + while ( + end < this.src.length && + !'\n\r\u2028\u2029'.includes(this.src[end]!) + ) { + end++; + } + if (this.src[end] === '\r' && this.src[end + 1] === '\n') end++; + this.index = end < this.src.length ? end + 1 : this.src.length; continue; } if (this.src.startsWith('/*', this.index)) { @@ -259,7 +266,15 @@ class MetaLiteralParser { return out; } // Only a template literal may span lines. - if (c === '\n' && quote !== '`') throw this.fail('unterminated string'); + if (c === '\n' || c === '\r') { + if (quote !== '`') throw this.fail('unterminated string'); + if (c === '\r') { + this.index++; + if (this.src[this.index] === '\n') this.index++; + out += '\n'; + continue; + } + } if (quote === '`' && c === '$' && this.src[this.index + 1] === '{') { throw this.fail('template substitutions are not allowed in meta'); } @@ -328,6 +343,9 @@ class MetaLiteralParser { // CRLF line continuation. if (this.src[this.index] === '\n') this.index++; return ''; + case '\u2028': + case '\u2029': + return ''; case undefined: throw this.fail('unterminated escape sequence'); default: