From c0d625fdbcca6d14c06907aae8bea7d687975418 Mon Sep 17 00:00:00 2001 From: Tal Zaccai Date: Mon, 27 Jul 2026 16:28:09 -0700 Subject: [PATCH 1/2] Reject unsafe @func names in program validator and interpreter Add a shared isValidFunctionName predicate used by both createModuleTextFromProgram and evaluateJsonProgram so a program can never be validated by one path but dispatched by the other. - createModuleTextFromProgram now rejects @func values that aren't strict identifiers, closing a source-injection hole where comment tokens (/* ... */) in @func could comment out later steps so the type-checker-based validator never saw them. - evaluateJsonProgram now throws for @func values that aren't strict identifiers or that resolve to an Object.prototype member (constructor, __proto__, toString, valueOf, hasOwnProperty, etc.), instead of forwarding them to the host onCall dispatcher. @ref bounds checking is unrelated and already handled by #393. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- typescript/src/ts/program.ts | 19 +++++++- typescript/test/program.test.ts | 77 +++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 2 deletions(-) diff --git a/typescript/src/ts/program.ts b/typescript/src/ts/program.ts index 99231463..5e5338f4 100644 --- a/typescript/src/ts/program.ts +++ b/typescript/src/ts/program.ts @@ -68,6 +68,18 @@ export type ResultReference = { "@ref": number; }; +/** + * Determines whether a function name is safe to use as an `@func` value. The name must be a strict + * JS identifier and must not be a member of `Object.prototype` (e.g. `constructor`, `__proto__`, + * `toString`, `valueOf`, `hasOwnProperty`), which could otherwise be used to escape the intended + * API surface during code generation or dispatch. + */ +function isValidFunctionName(name: unknown): name is string { + return typeof name === "string" && + /^[a-zA-Z_$][0-9a-zA-Z_$]*$/.test(name) && + !Object.prototype.hasOwnProperty.call(Object.prototype, name); +} + /** * Transforms a JSON program object into an equivalent TypeScript module suitable for type checking. * The generated module takes the form: @@ -116,7 +128,7 @@ export function createModuleTextFromProgram(jsonObject: object): Result const func = obj["@func"]; const hasArgs = obj.hasOwnProperty("@args"); const args = hasArgs ? obj["@args"] : []; - if (typeof func === "string" && (Array.isArray(args)) && Object.keys(obj).length === (hasArgs ? 2 : 1)) { + if (isValidFunctionName(func) && (Array.isArray(args)) && Object.keys(obj).length === (hasArgs ? 2 : 1)) { return `api.${func}(${arrayToString(args)})`; } } @@ -168,7 +180,10 @@ export async function evaluateJsonProgram(program: Program, onCall: (func: strin else if (obj.hasOwnProperty("@func")) { const func = obj["@func"]; const args = obj.hasOwnProperty("@args") ? obj["@args"] : []; - if (typeof func === "string" && Array.isArray(args)) { + if (!isValidFunctionName(func)) { + throw new Error(`Invalid function name: ${JSON.stringify(func)}`); + } + if (Array.isArray(args)) { return await onCall(func, await evaluateArray(args)); } } diff --git a/typescript/test/program.test.ts b/typescript/test/program.test.ts index f6a53156..aa7a612d 100644 --- a/typescript/test/program.test.ts +++ b/typescript/test/program.test.ts @@ -104,3 +104,80 @@ describe("createModuleTextFromProgram result references", () => { assert.equal(result.success, false); }); }); + +// --------------------------------------------------------------------------- +// @func name validation (source injection / unchecked dispatch) +// --------------------------------------------------------------------------- + +describe("@func name validation", () => { + const maliciousProgram: Program = { + "@steps": [ + { "@func": "play('x'); /*", "@args": [] }, + { "@func": "deleteEverything", "@args": ["rm", "-rf", 0] }, + { "@func": "*/ api.play", "@args": ["x"] }, + ], + }; + + it("createModuleTextFromProgram rejects a comment-injection @func value", () => { + const result = createModuleTextFromProgram(maliciousProgram); + assert.equal(result.success, false); + }); + + it("createModuleTextFromProgram accepts a normal valid program", () => { + const result = createModuleTextFromProgram({ + "@steps": [ + { "@func": "first", "@args": [] }, + { "@func": "second", "@args": [{ "@ref": 0 }] }, + ], + }); + assert.equal(result.success, true); + assert.match((result as { data: string }).data, /api\.first\(\)/); + assert.match((result as { data: string }).data, /api\.second\(step1\)/); + }); + + it("evaluateJsonProgram throws instead of dispatching a comment-injection @func value", async () => { + const calls: Array<{ func: string; args: unknown[] }> = []; + await assert.rejects( + evaluateJsonProgram(maliciousProgram, async (func, args) => { + calls.push({ func, args }); + return undefined; + }), + /Invalid function name/ + ); + assert.equal(calls.length, 0, "no function should have been dispatched"); + }); + + for (const badName of ["constructor", "__proto__", "toString", "valueOf", "hasOwnProperty"]) { + it(`evaluateJsonProgram throws instead of dispatching prototype member "${badName}"`, async () => { + const program: Program = { "@steps": [{ "@func": badName, "@args": [] }] }; + let dispatched = false; + await assert.rejects( + evaluateJsonProgram(program, async (func, args) => { + dispatched = true; + return undefined; + }), + /Invalid function name/ + ); + assert.equal(dispatched, false); + }); + } + + it("evaluateJsonProgram dispatches a normal valid program correctly", async () => { + const calls: Array<{ func: string; args: unknown[] }> = []; + const result = await evaluateJsonProgram( + { + "@steps": [ + { "@func": "first", "@args": [] }, + { "@func": "second", "@args": [{ "@ref": 0 }] }, + ], + }, + async (func, args) => { + calls.push({ func, args }); + return `${func}-result`; + } + ); + assert.deepEqual(calls[0], { func: "first", args: [] }); + assert.deepEqual(calls[1], { func: "second", args: ["first-result"] }); + assert.equal(result, "second-result"); + }); +}); From 250b1dbe4d721edfec421c16d29c73f22178f33e Mon Sep 17 00:00:00 2001 From: Tal Zaccai Date: Mon, 27 Jul 2026 16:37:55 -0700 Subject: [PATCH 2/2] Address review: fix JSDoc, mirror shape checks in interpreter, reindent tests - Reword isValidFunctionName JSDoc to describe the actual ASCII identifier constraint instead of implying full JS identifier support. - evaluateJsonProgram's @func branch now mirrors the generator's shape checks (args must be an array when present, no unexpected extra keys) and throws instead of silently returning undefined when they don't hold. - Re-indent the @func name validation test block to 4 spaces. - Add tests covering non-array @args and extra keys for both createModuleTextFromProgram and evaluateJsonProgram. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- typescript/src/ts/program.ts | 17 ++-- typescript/test/program.test.ts | 142 ++++++++++++++++++++++---------- 2 files changed, 107 insertions(+), 52 deletions(-) diff --git a/typescript/src/ts/program.ts b/typescript/src/ts/program.ts index 5e5338f4..5fc21b5d 100644 --- a/typescript/src/ts/program.ts +++ b/typescript/src/ts/program.ts @@ -69,10 +69,11 @@ export type ResultReference = { }; /** - * Determines whether a function name is safe to use as an `@func` value. The name must be a strict - * JS identifier and must not be a member of `Object.prototype` (e.g. `constructor`, `__proto__`, - * `toString`, `valueOf`, `hasOwnProperty`), which could otherwise be used to escape the intended - * API surface during code generation or dispatch. + * Determines whether a function name is safe to use as an `@func` value. The name must be a plain + * ASCII identifier — a letter, underscore, or `$`, followed by letters, digits, underscores, or `$` + * — and must not be a member of `Object.prototype` (e.g. `constructor`, `__proto__`, `toString`, + * `valueOf`, `hasOwnProperty`), which could otherwise be used to escape the intended API surface + * during code generation or dispatch. */ function isValidFunctionName(name: unknown): name is string { return typeof name === "string" && @@ -179,13 +180,15 @@ export async function evaluateJsonProgram(program: Program, onCall: (func: strin } else if (obj.hasOwnProperty("@func")) { const func = obj["@func"]; - const args = obj.hasOwnProperty("@args") ? obj["@args"] : []; + const hasArgs = obj.hasOwnProperty("@args"); + const args = hasArgs ? obj["@args"] : []; if (!isValidFunctionName(func)) { throw new Error(`Invalid function name: ${JSON.stringify(func)}`); } - if (Array.isArray(args)) { - return await onCall(func, await evaluateArray(args)); + if (!Array.isArray(args) || Object.keys(obj).length !== (hasArgs ? 2 : 1)) { + throw new Error(`Invalid function call: ${JSON.stringify(obj)}`); } + return await onCall(func, await evaluateArray(args)); } else if (Array.isArray(obj)) { return evaluateArray(obj); diff --git a/typescript/test/program.test.ts b/typescript/test/program.test.ts index aa7a612d..b95515bb 100644 --- a/typescript/test/program.test.ts +++ b/typescript/test/program.test.ts @@ -1,6 +1,6 @@ import { describe, it } from "node:test"; import assert from "node:assert/strict"; -import { evaluateJsonProgram, createModuleTextFromProgram, Program } from "../dist/ts/index.js"; +import { evaluateJsonProgram, createModuleTextFromProgram, Program, FunctionCall } from "../dist/ts/index.js"; // --------------------------------------------------------------------------- // evaluateJsonProgram result-reference bounds checking @@ -111,73 +111,125 @@ describe("createModuleTextFromProgram result references", () => { describe("@func name validation", () => { const maliciousProgram: Program = { - "@steps": [ - { "@func": "play('x'); /*", "@args": [] }, - { "@func": "deleteEverything", "@args": ["rm", "-rf", 0] }, - { "@func": "*/ api.play", "@args": ["x"] }, - ], + "@steps": [ + { "@func": "play('x'); /*", "@args": [] }, + { "@func": "deleteEverything", "@args": ["rm", "-rf", 0] }, + { "@func": "*/ api.play", "@args": ["x"] }, + ], }; it("createModuleTextFromProgram rejects a comment-injection @func value", () => { - const result = createModuleTextFromProgram(maliciousProgram); - assert.equal(result.success, false); + const result = createModuleTextFromProgram(maliciousProgram); + assert.equal(result.success, false); }); it("createModuleTextFromProgram accepts a normal valid program", () => { - const result = createModuleTextFromProgram({ - "@steps": [ - { "@func": "first", "@args": [] }, - { "@func": "second", "@args": [{ "@ref": 0 }] }, - ], - }); - assert.equal(result.success, true); - assert.match((result as { data: string }).data, /api\.first\(\)/); - assert.match((result as { data: string }).data, /api\.second\(step1\)/); + const result = createModuleTextFromProgram({ + "@steps": [ + { "@func": "first", "@args": [] }, + { "@func": "second", "@args": [{ "@ref": 0 }] }, + ], + }); + assert.equal(result.success, true); + assert.match((result as { data: string }).data, /api\.first\(\)/); + assert.match((result as { data: string }).data, /api\.second\(step1\)/); }); it("evaluateJsonProgram throws instead of dispatching a comment-injection @func value", async () => { - const calls: Array<{ func: string; args: unknown[] }> = []; - await assert.rejects( - evaluateJsonProgram(maliciousProgram, async (func, args) => { - calls.push({ func, args }); - return undefined; - }), - /Invalid function name/ - ); - assert.equal(calls.length, 0, "no function should have been dispatched"); + const calls: Array<{ func: string; args: unknown[] }> = []; + await assert.rejects( + evaluateJsonProgram(maliciousProgram, async (func, args) => { + calls.push({ func, args }); + return undefined; + }), + /Invalid function name/ + ); + assert.equal(calls.length, 0, "no function should have been dispatched"); }); for (const badName of ["constructor", "__proto__", "toString", "valueOf", "hasOwnProperty"]) { - it(`evaluateJsonProgram throws instead of dispatching prototype member "${badName}"`, async () => { - const program: Program = { "@steps": [{ "@func": badName, "@args": [] }] }; + it(`evaluateJsonProgram throws instead of dispatching prototype member "${badName}"`, async () => { + const program: Program = { "@steps": [{ "@func": badName, "@args": [] }] }; + let dispatched = false; + await assert.rejects( + evaluateJsonProgram(program, async (func, args) => { + dispatched = true; + return undefined; + }), + /Invalid function name/ + ); + assert.equal(dispatched, false); + }); + } + + it("evaluateJsonProgram dispatches a normal valid program correctly", async () => { + const calls: Array<{ func: string; args: unknown[] }> = []; + const result = await evaluateJsonProgram( + { + "@steps": [ + { "@func": "first", "@args": [] }, + { "@func": "second", "@args": [{ "@ref": 0 }] }, + ], + }, + async (func, args) => { + calls.push({ func, args }); + return `${func}-result`; + } + ); + assert.deepEqual(calls[0], { func: "first", args: [] }); + assert.deepEqual(calls[1], { func: "second", args: ["first-result"] }); + assert.equal(result, "second-result"); + }); + + it("createModuleTextFromProgram rejects a call object with a non-array @args", () => { + const result = createModuleTextFromProgram({ + "@steps": [ + { "@func": "first", "@args": "not-an-array" as unknown as [] }, + ], + }); + assert.equal(result.success, false); + }); + + it("createModuleTextFromProgram rejects a call object with extra unexpected keys", () => { + const result = createModuleTextFromProgram({ + "@steps": [ + { "@func": "first", "@args": [], "extra": true } as unknown as FunctionCall, + ], + }); + assert.equal(result.success, false); + }); + + it("evaluateJsonProgram throws instead of dispatching a call object with a non-array @args", async () => { + const program = { + "@steps": [ + { "@func": "first", "@args": "not-an-array" }, + ], + } as unknown as Program; let dispatched = false; await assert.rejects( evaluateJsonProgram(program, async (func, args) => { dispatched = true; return undefined; }), - /Invalid function name/ + /Invalid function call/ ); assert.equal(dispatched, false); }); - } - it("evaluateJsonProgram dispatches a normal valid program correctly", async () => { - const calls: Array<{ func: string; args: unknown[] }> = []; - const result = await evaluateJsonProgram( - { + it("evaluateJsonProgram throws instead of dispatching a call object with extra unexpected keys", async () => { + const program = { "@steps": [ - { "@func": "first", "@args": [] }, - { "@func": "second", "@args": [{ "@ref": 0 }] }, + { "@func": "first", "@args": [], "extra": true }, ], - }, - async (func, args) => { - calls.push({ func, args }); - return `${func}-result`; - } - ); - assert.deepEqual(calls[0], { func: "first", args: [] }); - assert.deepEqual(calls[1], { func: "second", args: ["first-result"] }); - assert.equal(result, "second-result"); + } as unknown as Program; + let dispatched = false; + await assert.rejects( + evaluateJsonProgram(program, async (func, args) => { + dispatched = true; + return undefined; + }), + /Invalid function call/ + ); + assert.equal(dispatched, false); }); });