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..94deb715e3b --- /dev/null +++ b/packages/core/src/agents/runtime/workflow-meta-literal.test.ts @@ -0,0 +1,395 @@ +/** + * @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.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({ + 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.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' }`, + ) 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/); + }); + + 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. + // 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..e5a8a44e061 --- /dev/null +++ b/packages/core/src/agents/runtime/workflow-meta-literal.ts @@ -0,0 +1,399 @@ +/** + * @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)) { + 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)) { + 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' || 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'); + } + 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 '\u2028': + case '\u2029': + 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 2124881adfd..3a9c83c101c 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 47890cea9bc..0b91b2b8fcb 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 @@ -364,6 +299,7 @@ function isRegexContext(source: string, i: number): boolean { import * as vm from 'node:vm'; import { createDebugLogger } from '../../utils/debugLogger.js'; import { stripAnsiAndControl } from '../../utils/textUtils.js'; +import { parseWorkflowMetaLiteral } from './workflow-meta-literal.js'; import type { WorkflowDispatchScheduler } from './workflow-dispatch-scheduler.js'; // Shared with workflow-orchestrator (avoids a duplicate createDebugLogger