fix: pre-dispatch rejection of unloggable args, mutation-proof event copies, proto-safe bindings (Codex round 1)
Three findings from the PR-4 convergence round: (A) A root-undefined binding argument passed normalization untouched, so the sub-call DISPATCHED and only then failed the tool/code-dispatch append (Session.append rejects undefined event data) — a sub-call executed with no log record, violating the nothing-executes-unlogged contract. And the tool received the SAME object later handed to the append, so a tool mutating its args desynced the logged record from what was dispatched (or re-poisoned the append). jsonNormalizeArgs now rejects undefined up front with a model-correctable message and returns TWO independent parses of the canonical JSON text: the tool gets one, the event logs the sibling — identical by construction, mutation-proof. (B) The bridge built its bindings record with plain-object assignment, so a registered tool named __proto__ hit the prototype setter and silently vanished (the runtime host resolves binding names as own properties). The record is now null-prototype with defineProperty, mirroring the worker-side namespace build. (B) The header-pin sanity assertions ran only inside NON-pinning scenarios, so a class consisting solely of its pinning scenario (the two Code Mode classes) would accept a re-recorded pin carrying several headers or a header-delta. A fixtures meta-test now asserts every pinning fixture directly.
This commit is contained in:
@@ -408,6 +408,20 @@ describe('snapshot fixtures', () => {
|
|||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('every pinning fixture carries exactly one request/header and no deltas', async () => {
|
||||||
|
// The live uniformity guard runs only in NON-pinning scenarios, so a
|
||||||
|
// class made of just its pinning scenario (the Code Mode classes) would
|
||||||
|
// otherwise accept a re-recorded pin with several headers or a mid-run
|
||||||
|
// header-delta — shapes the pin design cannot represent. Assert the
|
||||||
|
// committed pins directly.
|
||||||
|
for (const scenario of pinningByClass.values()) {
|
||||||
|
const fixture = await readFile(join(SNAPSHOTS_DIR, scenario.name, 'session.jsonl'), 'utf8')
|
||||||
|
const headers = normalizedHeaders(fixture, fixtureContext(fixture))
|
||||||
|
expect(headers.length, `${scenario.name}: a pinning fixture must carry exactly one request/header`).toBe(1)
|
||||||
|
expect(headerDeltaCount(fixture), `${scenario.name}: a pinning fixture must carry no request/header-delta`).toBe(0)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
it('committed fixtures carry request-header content ONLY in the pinning scenario', async () => {
|
it('committed fixtures carry request-header content ONLY in the pinning scenario', async () => {
|
||||||
// The whole point of the pin: a system-prompt or tool-schema change must
|
// The whole point of the pin: a system-prompt or tool-schema change must
|
||||||
// churn exactly one committed line. A non-pinning fixture that carries the
|
// churn exactly one committed line. A non-pinning fixture that carries the
|
||||||
|
|||||||
@@ -87,17 +87,21 @@ function summarize(text: string): string {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* JSON-normalize one binding call's argument: a `JSON.parse(JSON.stringify(…))`
|
* JSON-normalize one binding call's argument into TWO independent parses of
|
||||||
* round-trip, so the value dispatched to the tool and the value logged on the
|
* the same canonical text: `dispatched` goes to the tool, `logged` to the
|
||||||
* `tool/code-dispatch` event are the same JSON value by construction (the
|
* `tool/code-dispatch` event — identical by construction (the runtime's
|
||||||
* runtime's structured-clone boundary is wider than JSON; the session log
|
* structured-clone boundary is wider than JSON; the session log accepts only
|
||||||
* accepts only JSON). A value that does not survive (`BigInt`, a circular
|
* JSON), and separate objects, so a tool mutating its args can neither
|
||||||
* structure, a bare function) rejects that one call with a model-correctable
|
* desync the log from what was dispatched nor re-poison the append. A value
|
||||||
* error. `undefined` passes through — the tool's own schema validation
|
* that does not survive the round-trip (`undefined` — the log rejects it as
|
||||||
* rejects it with its usual "must be an object" feedback.
|
* event data — `BigInt`, a circular structure, a bare function) rejects that
|
||||||
|
* one call BEFORE dispatch with a model-correctable error: nothing ever
|
||||||
|
* executes unlogged.
|
||||||
*/
|
*/
|
||||||
function jsonNormalizeArgs(value: unknown): unknown {
|
function jsonNormalizeArgs(value: unknown): { dispatched: unknown; logged: unknown } {
|
||||||
if (value === undefined) return undefined
|
if (value === undefined) {
|
||||||
|
throw new Error('tool arguments must be JSON-serializable (call the tool with an arguments object, e.g. `{}`)')
|
||||||
|
}
|
||||||
let text: string | undefined
|
let text: string | undefined
|
||||||
try {
|
try {
|
||||||
text = JSON.stringify(value)
|
text = JSON.stringify(value)
|
||||||
@@ -108,7 +112,7 @@ function jsonNormalizeArgs(value: unknown): unknown {
|
|||||||
// root really yields `undefined` at runtime — the guard is live.
|
// root really yields `undefined` at runtime — the guard is live.
|
||||||
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
|
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
|
||||||
if (text === undefined) throw new Error('tool arguments must be JSON-serializable (got a value JSON cannot represent)')
|
if (text === undefined) throw new Error('tool arguments must be JSON-serializable (got a value JSON cannot represent)')
|
||||||
return JSON.parse(text) as unknown
|
return { dispatched: JSON.parse(text) as unknown, logged: JSON.parse(text) as unknown }
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Render the program's completion value for the model-facing result text (`''` when the program returned nothing). */
|
/** Render the program's completion value for the model-facing result text (`''` when the program returned nothing). */
|
||||||
@@ -198,7 +202,7 @@ export function createRunCodeTool(registry: ToolRegistry, requireRuntime: () =>
|
|||||||
const result = await registry.execute({
|
const result = await registry.execute({
|
||||||
callId: subCallId,
|
callId: subCallId,
|
||||||
name,
|
name,
|
||||||
arguments: normalized,
|
arguments: normalized.dispatched,
|
||||||
...exec.agent ? { agent: exec.agent } : {},
|
...exec.agent ? { agent: exec.agent } : {},
|
||||||
signal: runController.signal,
|
signal: runController.signal,
|
||||||
})
|
})
|
||||||
@@ -212,7 +216,10 @@ export function createRunCodeTool(registry: ToolRegistry, requireRuntime: () =>
|
|||||||
parentCallId: exec.callId,
|
parentCallId: exec.callId,
|
||||||
subCallId,
|
subCallId,
|
||||||
name,
|
name,
|
||||||
arguments: normalized,
|
// The SIBLING parse of the dispatched value: byte-identical JSON,
|
||||||
|
// but a separate object — a tool mutating its args cannot desync
|
||||||
|
// this record from what it actually received.
|
||||||
|
arguments: normalized.logged,
|
||||||
isError: result.isError,
|
isError: result.isError,
|
||||||
resultSummary: summarize(text),
|
resultSummary: summarize(text),
|
||||||
})
|
})
|
||||||
@@ -231,10 +238,15 @@ export function createRunCodeTool(registry: ToolRegistry, requireRuntime: () =>
|
|||||||
return outcome.text
|
return outcome.text
|
||||||
}
|
}
|
||||||
|
|
||||||
const functions: Record<string, CodeBindingFunction> = {}
|
// Null-prototype + defineProperty, mirroring the worker-side namespace
|
||||||
|
// build: a registered tool named `__proto__` must become an ordinary
|
||||||
|
// own key (a plain-object assignment would hit the prototype setter,
|
||||||
|
// silently dropping the binding), and the runtime host resolves
|
||||||
|
// binding names as own properties only.
|
||||||
|
const functions: Record<string, CodeBindingFunction> = Object.create(null) as Record<string, CodeBindingFunction>
|
||||||
for (const schema of registry.schemas()) {
|
for (const schema of registry.schemas()) {
|
||||||
if (schema.name === RUN_CODE_NAME) continue
|
if (schema.name === RUN_CODE_NAME) continue
|
||||||
functions[schema.name] = binding(schema.name)
|
Object.defineProperty(functions, schema.name, { enumerable: true, value: binding(schema.name) })
|
||||||
}
|
}
|
||||||
|
|
||||||
try {
|
try {
|
||||||
|
|||||||
@@ -431,17 +431,18 @@ describe('the run_code dispatch bridge', () => {
|
|||||||
expect(dispatch.resultSummary.endsWith('…')).toBe(true)
|
expect(dispatch.resultSummary.endsWith('…')).toBe(true)
|
||||||
})
|
})
|
||||||
|
|
||||||
it('rejects undefined, JSON-throwing, and JSON-unrepresentable binding arguments', async () => {
|
it('rejects undefined, JSON-throwing, and JSON-unrepresentable binding arguments BEFORE dispatch', async () => {
|
||||||
const { ctx, runtime } = await setup({ mode: 'code' })
|
const { ctx, runtime } = await setup({ mode: 'code' })
|
||||||
registerEcho(ctx)
|
const calls = registerEcho(ctx)
|
||||||
|
const { agent, events } = fakeAgent()
|
||||||
runtime.behavior = async (request) => {
|
runtime.behavior = async (request) => {
|
||||||
const echo = request.bindings[0]!.functions.echo!
|
const echo = request.bindings[0]!.functions.echo!
|
||||||
const catchMessage = (promise: Promise<unknown>) => promise.then(() => 'resolved', (error: unknown) => error instanceof Error ? error.message : String(error))
|
const catchMessage = (promise: Promise<unknown>) => promise.then(() => 'resolved', (error: unknown) => error instanceof Error ? error.message : String(error))
|
||||||
return {
|
return {
|
||||||
logs: [],
|
logs: [],
|
||||||
value: [
|
value: [
|
||||||
// undefined passes normalization untouched; the tool's own schema
|
// Root undefined must reject up front: the event log rejects it as
|
||||||
// validation rejects it with its usual feedback.
|
// data, and nothing may execute unlogged.
|
||||||
await catchMessage(echo(undefined)),
|
await catchMessage(echo(undefined)),
|
||||||
// A toJSON that throws a NON-Error propagates out of JSON.stringify.
|
// A toJSON that throws a NON-Error propagates out of JSON.stringify.
|
||||||
await catchMessage(echo({ toJSON() { throw 'raw-throw' } })),
|
await catchMessage(echo({ toJSON() { throw 'raw-throw' } })),
|
||||||
@@ -450,11 +451,55 @@ describe('the run_code dispatch bridge', () => {
|
|||||||
].join(' | '),
|
].join(' | '),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
const result = await runCode(ctx, 'program')
|
const result = await runCode(ctx, 'program', { agent })
|
||||||
const text = (result.content[0] as { text: string }).text
|
const text = (result.content[0] as { text: string }).text
|
||||||
expect(text).toContain('must be an object')
|
expect(text).toContain('call the tool with an arguments object')
|
||||||
expect(text).toContain('JSON-serializable: raw-throw')
|
expect(text).toContain('JSON-serializable: raw-throw')
|
||||||
expect(text).toContain('a value JSON cannot represent')
|
expect(text).toContain('a value JSON cannot represent')
|
||||||
|
// None of the three dispatched, none logged.
|
||||||
|
expect(calls).toEqual([])
|
||||||
|
expect(events.filter(event => event.type === 'tool/code-dispatch')).toEqual([])
|
||||||
|
})
|
||||||
|
|
||||||
|
it('logs the value the tool RECEIVED even when the tool mutates its arguments', async () => {
|
||||||
|
const { ctx, runtime } = await setup({ mode: 'code' })
|
||||||
|
const { agent, events } = fakeAgent()
|
||||||
|
ctx.tools.register(defineTool({
|
||||||
|
name: 'mutator',
|
||||||
|
description: 'Mutates its own args object.',
|
||||||
|
parameters: { list: { type: 'array', required: true } },
|
||||||
|
execute(args) {
|
||||||
|
args.list.push('injected-by-tool')
|
||||||
|
return Promise.resolve([{ type: 'text' as const, text: 'mutated' }])
|
||||||
|
},
|
||||||
|
}))
|
||||||
|
runtime.behavior = async (request) => {
|
||||||
|
await request.bindings[0]!.functions.mutator!({ list: ['original'] })
|
||||||
|
return { logs: [] }
|
||||||
|
}
|
||||||
|
const result = await runCode(ctx, 'program', { agent })
|
||||||
|
expect(result.isError).toBe(false)
|
||||||
|
const dispatch = events.find(event => event.type === 'tool/code-dispatch')?.data as SessionEventMap['tool/code-dispatch']
|
||||||
|
expect(dispatch.arguments).toEqual({ list: ['original'] })
|
||||||
|
})
|
||||||
|
|
||||||
|
it('exposes a tool named __proto__ as an ordinary own binding', async () => {
|
||||||
|
const { ctx, runtime } = await setup({ mode: 'code' })
|
||||||
|
ctx.tools.register(defineTool({
|
||||||
|
name: '__proto__',
|
||||||
|
description: 'A prototype-colliding tool name.',
|
||||||
|
parameters: {},
|
||||||
|
execute() { return Promise.resolve([{ type: 'text' as const, text: 'proto-tool-ok' }]) },
|
||||||
|
}))
|
||||||
|
runtime.behavior = async (request) => {
|
||||||
|
const functions = request.bindings[0]!.functions
|
||||||
|
expect(Object.getPrototypeOf(functions)).toBeNull()
|
||||||
|
const value = await functions['__proto__']!({})
|
||||||
|
return { logs: [], value }
|
||||||
|
}
|
||||||
|
const result = await runCode(ctx, 'program')
|
||||||
|
expect(result.isError).toBe(false)
|
||||||
|
expect(result.content[0]).toEqual({ type: 'text', text: 'proto-tool-ok' })
|
||||||
})
|
})
|
||||||
|
|
||||||
it('renders a non-string completion value inspect-style', async () => {
|
it('renders a non-string completion value inspect-style', async () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user