From 3cfdc9cc8a8ff559f8b5b3ac8d0f18fc22e48454 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 25 Sep 2026 07:37:53 -0700 Subject: [PATCH 1/6] test(mcp): cover tool-level isError and structuredContent --- src/mcp/client-envelope.test.ts | 80 ++++++++++++++++++ src/mcp/plugin.test.ts | 145 +++++++++++++++++++++++++++++++- 2 files changed, 224 insertions(+), 1 deletion(-) create mode 100644 src/mcp/client-envelope.test.ts diff --git a/src/mcp/client-envelope.test.ts b/src/mcp/client-envelope.test.ts new file mode 100644 index 000000000..7485ac30d --- /dev/null +++ b/src/mcp/client-envelope.test.ts @@ -0,0 +1,80 @@ +import { describe, expect, test } from "bun:test"; +import { withMockedModule } from "../../tests/helpers/mock-module.js"; +import { connectMCPServer } from "./client.js"; + +let scriptedCallToolResult: unknown = { content: [] }; + +await withMockedModule( + import.meta.resolve("@modelcontextprotocol/sdk/client/index.js"), + (real: typeof import("@modelcontextprotocol/sdk/client/index.js")) => ({ + ...real, + Client: class { + async connect(): Promise { + return undefined; + } + async listTools(): Promise<{ tools: [] }> { + return { tools: [] }; + } + async callTool(): Promise { + return scriptedCallToolResult; + } + async close(): Promise { + return undefined; + } + }, + }), +); + +await withMockedModule( + import.meta.resolve("@modelcontextprotocol/sdk/client/stdio.js"), + (real: typeof import("@modelcontextprotocol/sdk/client/stdio.js")) => ({ + ...real, + StdioClientTransport: class {}, + }), +); + +describe("mcp client tool envelope", () => { + test("callResult preserves isError and structuredContent from the SDK", async () => { + scriptedCallToolResult = { + content: [{ type: "text", text: "tool failed: bad input" }], + isError: true, + structuredContent: { reason: "bad input" }, + }; + const connected = await connectMCPServer( + { name: "envelope", command: "true" }, + {}, + ); + if (!connected.ok) throw new Error("expected stdio connect to succeed"); + const envelope = await connected.client.callResult( + "do_thing", + {}, + new AbortController().signal, + ); + + expect(envelope.isError).toBe(true); + expect(envelope.blocks).toEqual([ + { type: "text", text: "tool failed: bad input" }, + ]); + expect(envelope.structuredContent).toEqual({ reason: "bad input" }); + await connected.client.close(); + }); + + test("legacy call still flattens text blocks", async () => { + scriptedCallToolResult = { + content: [{ type: "text", text: "hello" }], + }; + const connected = await connectMCPServer( + { name: "envelope", command: "true" }, + {}, + ); + if (!connected.ok) throw new Error("expected stdio connect to succeed"); + const text = await connected.client.call( + "do_thing", + {}, + new AbortController().signal, + ); + + expect(text).toBe("hello"); + await connected.client.close(); + }); +}); diff --git a/src/mcp/plugin.test.ts b/src/mcp/plugin.test.ts index 16497108f..22fb2292f 100644 --- a/src/mcp/plugin.test.ts +++ b/src/mcp/plugin.test.ts @@ -8,7 +8,49 @@ import { } from "../plugins/result-truncation-plugin.js"; import { toolOutputAbsolutePath } from "../plugins/tool-result-materialize.js"; import { CREDENTIAL_REDACTION } from "../plugins/tool-result-secret-scrub.js"; -import type { MCPClient } from "./client.js"; +import type { MCPClient, MCPContentBlock } from "./client.js"; + +interface ScriptedMcpEnvelope { + blocks: MCPContentBlock[]; + isError?: boolean; + structuredContent?: Record; +} + +function fakeEnvelopeClient(envelope: ScriptedMcpEnvelope): MCPClient { + const client: MCPClient = { + serverName: "acme", + tools: [ + { + name: "fetch_secret", + description: "returns a value", + inputSchema: { type: "object", properties: {} }, + }, + ], + call: async () => "", + callBlocks: async () => envelope.blocks, + close: async () => undefined, + }; + // callResult is the new envelope channel (GREEN); absent on RED code. + Object.assign(client, { + callResult: async () => envelope, + }); + return client; +} + +async function runEnvelopeTool( + envelope: ScriptedMcpEnvelope, + callId: string, + spillOptions?: Parameters[2], +) { + const gate = skipGate(); + const client = fakeEnvelopeClient(envelope); + const [tool] = mcpClientToAgentTools(client, gate, spillOptions); + if (tool?.kind !== "full") throw new Error("expected full tool"); + return tool.handler( + { id: callId, name: "mcp__acme__fetch_secret", arguments: {} }, + new AbortController().signal, + ); +} function fakeClient(reply: string): MCPClient { return { @@ -179,4 +221,105 @@ describe("mcpClientToAgentTools", () => { toolOutputAbsolutePath(contextDir, key, "text/plain"), ); }); + + test("tool-level failure surfaces as an error result with text preserved", async () => { + const result = await runEnvelopeTool( + { + blocks: [{ type: "text", text: "tool failed: bad input" }], + isError: true, + }, + "c-mcp-iserror-text", + ); + + expect(result.isError).toBe(true); + expect(result.content).toContain("tool failed: bad input"); + }); + + test("tool-level failure with empty content still yields a failure message", async () => { + const result = await runEnvelopeTool( + { blocks: [], isError: true }, + "c-mcp-iserror-empty", + ); + + expect(result.isError).toBe(true); + expect(typeof result.content).toBe("string"); + expect((result.content as string).length).toBeGreaterThan(0); + }); + + test("structured-only result surfaces a scrubbed JSON string, not empty text", async () => { + const result = await runEnvelopeTool( + { blocks: [], structuredContent: { answer: 42 } }, + "c-mcp-structured-only", + ); + + expect(result.isError).toBeUndefined(); + expect(typeof result.content).toBe("string"); + expect(result.content).toContain("42"); + expect(result.detail).toEqual({ answer: 42 }); + }); + + test("text plus structured content keeps the text and preserves structured detail", async () => { + const result = await runEnvelopeTool( + { + blocks: [{ type: "text", text: "hello from tool" }], + structuredContent: { answer: 42 }, + }, + "c-mcp-text-plus-structured", + ); + + expect(result.isError).toBeUndefined(); + expect(result.content).toContain("hello from tool"); + expect(result.detail).toEqual({ answer: 42 }); + }); + + test("credential-shaped values inside structured content are redacted", async () => { + const secret = "sk-live-abcdefghij1234567890"; + const result = await runEnvelopeTool( + { + blocks: [], + structuredContent: { token: secret }, + }, + "c-mcp-structured-secret", + ); + + expect(result.content).toContain(CREDENTIAL_REDACTION); + expect(result.content).not.toContain(secret); + expect(JSON.stringify(result.detail)).not.toContain(secret); + }); + + test("thrown transport failures still surface as error results", async () => { + const gate = skipGate(); + const client: MCPClient = { + serverName: "acme", + tools: [ + { + name: "fetch_secret", + description: "returns a value", + inputSchema: { type: "object", properties: {} }, + }, + ], + call: async () => { + throw new Error("transport exploded"); + }, + callBlocks: async () => { + throw new Error("transport exploded"); + }, + close: async () => undefined, + }; + Object.assign(client, { + callResult: async () => { + throw new Error("transport exploded"); + }, + }); + const [tool] = mcpClientToAgentTools(client, gate); + if (tool?.kind !== "full") throw new Error("expected full tool"); + + const result = await tool.handler( + { id: "c-mcp-throw", name: "mcp__acme__fetch_secret", arguments: {} }, + new AbortController().signal, + ); + + expect(result.isError).toBe(true); + expect(result.content).toContain("transport exploded"); + }); }); From 24d2c57924c6c643fc931899e1373e91aeaece36 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 25 Sep 2026 07:45:14 -0700 Subject: [PATCH 2/6] fix(mcp): surface tool-level isError and structuredContent --- src/mcp/client-envelope.test.ts | 18 ++----- src/mcp/client.ts | 46 ++++++++++++++++++ src/mcp/plugin.ts | 84 +++++++++++++++++++++++++++------ src/session/hooks.ts | 1 + 4 files changed, 122 insertions(+), 27 deletions(-) diff --git a/src/mcp/client-envelope.test.ts b/src/mcp/client-envelope.test.ts index 7485ac30d..e3bf33726 100644 --- a/src/mcp/client-envelope.test.ts +++ b/src/mcp/client-envelope.test.ts @@ -1,4 +1,5 @@ import { describe, expect, test } from "bun:test"; +import { defined } from "../../tests/helpers/defined.js"; import { withMockedModule } from "../../tests/helpers/mock-module.js"; import { connectMCPServer } from "./client.js"; @@ -25,14 +26,6 @@ await withMockedModule( }), ); -await withMockedModule( - import.meta.resolve("@modelcontextprotocol/sdk/client/stdio.js"), - (real: typeof import("@modelcontextprotocol/sdk/client/stdio.js")) => ({ - ...real, - StdioClientTransport: class {}, - }), -); - describe("mcp client tool envelope", () => { test("callResult preserves isError and structuredContent from the SDK", async () => { scriptedCallToolResult = { @@ -45,11 +38,10 @@ describe("mcp client tool envelope", () => { {}, ); if (!connected.ok) throw new Error("expected stdio connect to succeed"); - const envelope = await connected.client.callResult( - "do_thing", - {}, - new AbortController().signal, - ); + const envelope = await defined( + connected.client.callResult, + "mcp client callResult", + )("do_thing", {}, new AbortController().signal); expect(envelope.isError).toBe(true); expect(envelope.blocks).toEqual([ diff --git a/src/mcp/client.ts b/src/mcp/client.ts index a67dd626e..63d36241b 100644 --- a/src/mcp/client.ts +++ b/src/mcp/client.ts @@ -27,6 +27,23 @@ export interface MCPContentBlock { [key: string]: unknown; } +/** + * Scope lock (CL-8992): the pinned @modelcontextprotocol/sdk v1 CallToolResult + * is `{ content: blocks[] (default []), structuredContent?: Record, isError?: boolean }` — a tool-level failure still succeeds at the + * protocol layer. The envelope carries all three so the plugin can surface + * failures as errors and structured-only payloads as readable text. + * `structuredContent` reaches the model JSON-serialized into the content + * string under MCP_STRUCTURED_CONTENT_MARKER (see plugin.ts) with the raw + * (policy-scrubbed) record preserved under ToolResult `detail` and in the + * evidence archive — never raw. + */ +export interface MCPToolResultEnvelope { + blocks: MCPContentBlock[]; + isError: boolean; + structuredContent?: Record; +} + export interface MCPClient { serverName: string; tools: MCPTool[]; @@ -41,6 +58,12 @@ export interface MCPClient { args: Record, signal: AbortSignal, ): Promise; + /** Full tool-result envelope: blocks plus tool-level isError/structuredContent. */ + callResult?( + toolName: string, + args: Record, + signal: AbortSignal, + ): Promise; close(): Promise; } @@ -566,6 +589,29 @@ async function finishClient( return { serverName, tools, + async callResult(toolName, args, signal) { + const context = + authContext === undefined ? undefined : { ...authContext, signal }; + const result = await withHTTPAuthorizationRecovery(context, () => + client.callTool({ name: toolName, arguments: args }, undefined, { + signal, + }), + ); + const envelope: MCPToolResultEnvelope = { + blocks: validateMcpContentBlocks(result.content), + isError: result.isError === true, + }; + if ( + result.structuredContent !== null && + typeof result.structuredContent === "object" + ) { + envelope.structuredContent = result.structuredContent as Record< + string, + unknown + >; + } + return envelope; + }, async callBlocks(toolName, args, signal) { const context = authContext === undefined ? undefined : { ...authContext, signal }; diff --git a/src/mcp/plugin.ts b/src/mcp/plugin.ts index a2ddbb4aa..5ae8cc5d7 100644 --- a/src/mcp/plugin.ts +++ b/src/mcp/plugin.ts @@ -11,13 +11,24 @@ import { type SpillBlobWriter, } from "../plugins/result-truncation-plugin.js"; import type { CompactionArchive } from "../session/compaction-archive.js"; -import type { MCPClient, MCPContentBlock } from "./client.js"; +import type { + MCPClient, + MCPContentBlock, + MCPToolResultEnvelope, +} from "./client.js"; import { mcpToolName } from "./tool-name.js"; import { unwrapToolContent } from "./client.js"; export const MCP_RECONNECTING_TOOL_ERROR = "MCP server is reconnecting; retry the call once it reports connected."; +/** + * Stable marker prefixing JSON-serialized `structuredContent` when it is the + * only payload (or supplements an empty flatten). Lets the model — and log + * grep — distinguish server-structured data from free text. + */ +export const MCP_STRUCTURED_CONTENT_MARKER = "mcp structured result:"; + /** True while the server keeps its tools mounted but cannot execute. */ export function isDegradedMcpState(state: { state: string }): boolean { return state.state === "reconnecting"; @@ -90,22 +101,51 @@ export function mcpClientTools( signal: AbortSignal, ): Promise => { try { - const rawBlocks = - typeof client.callBlocks === "function" - ? await client.callBlocks(tool.name, call.arguments, signal) - : [ - { - type: "text", - text: await client.call(tool.name, call.arguments, signal), - } satisfies MCPContentBlock, - ]; - const authorizedBlocks = applyPolicyToBlocks(rawBlocks); + const envelope: MCPToolResultEnvelope = + typeof client.callResult === "function" + ? await client.callResult(tool.name, call.arguments, signal) + : typeof client.callBlocks === "function" + ? { + blocks: await client.callBlocks( + tool.name, + call.arguments, + signal, + ), + isError: false, + } + : { + blocks: [ + { + type: "text", + text: await client.call( + tool.name, + call.arguments, + signal, + ), + } satisfies MCPContentBlock, + ], + isError: false, + }; + const authorizedBlocks = applyPolicyToBlocks(envelope.blocks); + const scrubbedStructured = + envelope.structuredContent === undefined + ? undefined + : (scrubSecretShapedValue(envelope.structuredContent) as Record< + string, + unknown + >); const archive = getEvidenceArchive?.(); if (archive !== undefined) { try { await archive.recordAuthorizedPayload({ kind: "tool_result", - payload: { blocks: authorizedBlocks }, + payload: + scrubbedStructured === undefined + ? { blocks: authorizedBlocks } + : { + blocks: authorizedBlocks, + structuredContent: scrubbedStructured, + }, callId: call.id, provenance: "mcp:post-policy-pre-flatten", }); @@ -114,6 +154,15 @@ export function mcpClientTools( } } const flattened = unwrapToolContent(authorizedBlocks); + const isError = envelope.isError === true; + const baseContent = + flattened !== "" + ? flattened + : scrubbedStructured !== undefined + ? `${MCP_STRUCTURED_CONTENT_MARKER}\n${JSON.stringify(scrubbedStructured)}` + : isError + ? `MCP tool ${client.serverName}/${tool.name} reported an error with empty content.` + : flattened; const writeBlob = getBlobWriter?.(); const contextDir = getContextDir?.(); const spill = @@ -124,8 +173,15 @@ export function mcpClientTools( ...(contextDir !== undefined ? { contextDir } : {}), } : undefined; - const content = await sanitizeMcpResultContent(flattened, spill); - return { callId: call.id, content }; + const content = await sanitizeMcpResultContent(baseContent, spill); + return { + callId: call.id, + content, + ...(isError ? { isError: true as const } : {}), + ...(scrubbedStructured !== undefined + ? { detail: scrubbedStructured } + : {}), + }; } catch (err) { const message = err instanceof Error ? err.message : String(err); const scrubbed = scrubSecretShapedContent(message); diff --git a/src/session/hooks.ts b/src/session/hooks.ts index 713361e91..5d9967749 100644 --- a/src/session/hooks.ts +++ b/src/session/hooks.ts @@ -501,6 +501,7 @@ function truncateToolResultForHookPayload(result: ToolResult): ToolResult { callId: result.callId, content, ...(result.isError !== undefined ? { isError: result.isError } : {}), + ...(result.detail !== undefined ? { detail: result.detail } : {}), ...(result.pendingMarker !== undefined ? { pendingMarker: result.pendingMarker } : {}), From f3c995a17b985957b5d59ae8fc69bb870f3f6bdf Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 25 Sep 2026 08:00:53 -0700 Subject: [PATCH 3/6] test(mcp): use live-shaped token in structured redaction test The criterion-4 test fed the redaction marker itself, asserting both P and not-P. Feed a constructed live-shaped token and assert the marker is present with the raw token absent in content and detail. --- src/mcp/plugin.test.ts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/mcp/plugin.test.ts b/src/mcp/plugin.test.ts index 22fb2292f..ed909db6f 100644 --- a/src/mcp/plugin.test.ts +++ b/src/mcp/plugin.test.ts @@ -273,18 +273,23 @@ describe("mcpClientToAgentTools", () => { }); test("credential-shaped values inside structured content are redacted", async () => { - const secret = "sk-live-abcdefghij1234567890"; + const rawToken = ["sk-", "live-", "k".repeat(24)].join(""); const result = await runEnvelopeTool( { blocks: [], - structuredContent: { token: secret }, + structuredContent: { token: rawToken }, }, "c-mcp-structured-secret", ); + expect(result.detail).toEqual({ token: CREDENTIAL_REDACTION }); expect(result.content).toContain(CREDENTIAL_REDACTION); - expect(result.content).not.toContain(secret); - expect(JSON.stringify(result.detail)).not.toContain(secret); + expect(result.content).not.toContain(rawToken); + expect(result.content).not.toContain("sk-live-"); + const detailJson = JSON.stringify(result.detail); + expect(detailJson).toContain(CREDENTIAL_REDACTION); + expect(detailJson).not.toContain(rawToken); + expect(detailJson).not.toContain("sk-live-"); }); test("thrown transport failures still surface as error results", async () => { From 987966ff7dc0dce14db25b7321efdfe43b6a4834 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 25 Sep 2026 08:00:56 -0700 Subject: [PATCH 4/6] fix(session): omit oversized tool result detail from hook payloads Hook payloads capped content but passed detail through verbatim, letting a verbose server blow up payloads. Omit serialized detail over the payload budget while preserving small detail. --- src/session/hooks.test.ts | 34 +++++++++++++++++++++++++++++++++- src/session/hooks.ts | 15 ++++++++++++++- 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/src/session/hooks.test.ts b/src/session/hooks.test.ts index 61ac45e12..198927995 100644 --- a/src/session/hooks.test.ts +++ b/src/session/hooks.test.ts @@ -17,6 +17,7 @@ function event(type: string, data: unknown): ReactorEmittedEvent { function observeOneTurnWithToolResult( collector: ReturnType, toolResultContent: string, + toolResultDetail?: unknown, ): void { collector.observe( event("inference.done", { @@ -34,7 +35,11 @@ function observeOneTurnWithToolResult( ); collector.observe( event("tool.done", { - result: { callId: "call-1", content: toolResultContent }, + result: { + callId: "call-1", + content: toolResultContent, + ...(toolResultDetail !== undefined ? { detail: toolResultDetail } : {}), + }, }), ); } @@ -64,6 +69,33 @@ describe("createTurnContextCollector tool result truncation", () => { const [turn] = collector.getTurns(); expect(turn?.toolResults[0]?.content).toBe(smallOutput); }); + + test("preserves small structured detail in hook payloads", () => { + const collector = createTurnContextCollector(() => undefined); + const detail = { answer: 42 }; + + observeOneTurnWithToolResult(collector, "exit code 0", detail); + + const [turn] = collector.getTurns(); + expect(turn?.toolResults[0]?.detail).toEqual(detail); + }); + + test("omits oversized structured detail while keeping the content cap", () => { + const collector = createTurnContextCollector(() => undefined); + const hugeOutput = "x".repeat(HOOK_PAYLOAD_TOOL_RESULT_CHARS * 4); + const hugeDetail = { blob: "y".repeat(HOOK_PAYLOAD_TOOL_RESULT_CHARS * 4) }; + + observeOneTurnWithToolResult(collector, hugeOutput, hugeDetail); + + const [turn] = collector.getTurns(); + const result = turn?.toolResults[0]; + expect(result?.detail).toBeUndefined(); + const content = result?.content; + expect(typeof content).toBe("string"); + expect((content as string).length).toBeLessThanOrEqual( + HOOK_PAYLOAD_TOOL_RESULT_CHARS + 64, + ); + }); }); describe("lifecycle hook payload delivery", () => { diff --git a/src/session/hooks.ts b/src/session/hooks.ts index 5d9967749..5c475c92c 100644 --- a/src/session/hooks.ts +++ b/src/session/hooks.ts @@ -501,13 +501,26 @@ function truncateToolResultForHookPayload(result: ToolResult): ToolResult { callId: result.callId, content, ...(result.isError !== undefined ? { isError: result.isError } : {}), - ...(result.detail !== undefined ? { detail: result.detail } : {}), + ...(hookPayloadDetailWithinBudget(result.detail) + ? { detail: result.detail } + : {}), ...(result.pendingMarker !== undefined ? { pendingMarker: result.pendingMarker } : {}), }; } +function hookPayloadDetailWithinBudget(detail: unknown): boolean { + if (detail === undefined) return false; + let serialized: string; + try { + serialized = JSON.stringify(detail) ?? String(detail); + } catch { + return false; + } + return serialized.length <= HOOK_PAYLOAD_TOOL_RESULT_CHARS; +} + function addUsage(a: TokenUsage, b: TokenUsage): TokenUsage { return { input: a.input + b.input, From 6bb9b15e194d23105719103bf7bbab1f0e092cbf Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 25 Sep 2026 15:13:04 -0700 Subject: [PATCH 5/6] fix(mcp): bound and scrub structured result evidence --- src/mcp/client.ts | 6 +- src/mcp/plugin.test.ts | 183 +++++++++++++++++++ src/mcp/plugin.ts | 69 ++++--- src/plugins/tool-result-secret-scrub.test.ts | 40 ++++ src/plugins/tool-result-secret-scrub.ts | 19 +- src/session/hooks.test.ts | 19 ++ 6 files changed, 300 insertions(+), 36 deletions(-) diff --git a/src/mcp/client.ts b/src/mcp/client.ts index 63d36241b..e804d1e5f 100644 --- a/src/mcp/client.ts +++ b/src/mcp/client.ts @@ -34,9 +34,9 @@ export interface MCPContentBlock { * protocol layer. The envelope carries all three so the plugin can surface * failures as errors and structured-only payloads as readable text. * `structuredContent` reaches the model JSON-serialized into the content - * string under MCP_STRUCTURED_CONTENT_MARKER (see plugin.ts) with the raw - * (policy-scrubbed) record preserved under ToolResult `detail` and in the - * evidence archive — never raw. + * string under MCP_STRUCTURED_CONTENT_MARKER (see plugin.ts). Small + * policy-scrubbed records are preserved under ToolResult `detail`; the full + * scrubbed record is retained in the evidence archive — never raw. */ export interface MCPToolResultEnvelope { blocks: MCPContentBlock[]; diff --git a/src/mcp/plugin.test.ts b/src/mcp/plugin.test.ts index ed909db6f..c7184e2de 100644 --- a/src/mcp/plugin.test.ts +++ b/src/mcp/plugin.test.ts @@ -1,5 +1,9 @@ import { defined } from "../../tests/helpers/defined.js"; import { describe, test, expect } from "bun:test"; +import { mkdtempSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import type { ToolResult } from "@intx/types/runtime"; import { mcpClientToAgentTools } from "./plugin.js"; import { createPermissionGate } from "../permission/gate.js"; import { @@ -8,6 +12,7 @@ import { } from "../plugins/result-truncation-plugin.js"; import { toolOutputAbsolutePath } from "../plugins/tool-result-materialize.js"; import { CREDENTIAL_REDACTION } from "../plugins/tool-result-secret-scrub.js"; +import { createCompactionArchive } from "../session/compaction-archive.js"; import type { MCPClient, MCPContentBlock } from "./client.js"; interface ScriptedMcpEnvelope { @@ -78,6 +83,39 @@ function fakeBlobStore() { }; } +function memoryEvidenceArchive() { + const blobs = new Map(); + const archive = createCompactionArchive({ + sessionId: "mcp-plugin-test", + contextDir: mkdtempSync(join(tmpdir(), "mcp-plugin-archive-")), + writeBlob: async (key, bytes) => { + blobs.set(key, bytes); + }, + readBlob: async (key) => { + const bytes = blobs.get(key); + if (bytes === undefined) throw new Error(`missing archive blob ${key}`); + return bytes; + }, + }); + return { archive, blobs }; +} + +function serializePersistedToolResultTurn(result: ToolResult): string { + return JSON.stringify({ + role: "user", + content: [ + { + type: "tool_result", + callId: result.callId, + content: [{ type: "text", text: String(result.content) }], + ...(result.detail !== undefined ? { detail: result.detail } : {}), + ...(result.isError !== undefined ? { isError: result.isError } : {}), + }, + ], + timestamp: 0, + }); +} + function skipGate() { return createPermissionGate({ approvals: [], @@ -292,6 +330,151 @@ describe("mcpClientToAgentTools", () => { expect(detailJson).not.toContain("sk-live-"); }); + test("scrubs structured keys from detail, archive bytes, and model content", async () => { + const topLevelKey = ["sk-", "live-", "a".repeat(24)].join(""); + const nestedKey = ["sk-", "live-", "b".repeat(24)].join(""); + const { archive, blobs } = memoryEvidenceArchive(); + const result = await runEnvelopeTool( + { + blocks: [{ type: "resource", [topLevelKey]: "block-value" }], + isError: false, + structuredContent: { + [topLevelKey]: "top-level", + nested: { [nestedKey]: "nested" }, + }, + }, + "c-mcp-structured-key-secret", + { getEvidenceArchive: () => archive }, + ); + + const detail = JSON.stringify(result.detail); + const modelTurn = serializePersistedToolResultTurn(result); + const archiveBytes = [...blobs.values()].map((bytes) => + new TextDecoder().decode(bytes), + ); + const surfaces = [ + detail, + String(result.content), + modelTurn, + ...archiveBytes, + ]; + for (const surface of surfaces) { + expect(surface).not.toContain(topLevelKey); + expect(surface).not.toContain(nestedKey); + } + expect(detail).toContain(CREDENTIAL_REDACTION); + expect(String(result.content)).toContain(CREDENTIAL_REDACTION); + expect(modelTurn).toContain(CREDENTIAL_REDACTION); + expect(archiveBytes.join("\n")).toContain(CREDENTIAL_REDACTION); + }); + + test("keeps oversized structured content full only in the evidence archive", async () => { + const { archive } = memoryEvidenceArchive(); + const hugeValue = "x".repeat(MAX_RESULT_CHARS * 4); + const result = await runEnvelopeTool( + { + blocks: [], + isError: false, + structuredContent: { hugeValue }, + }, + "c-mcp-oversized-detail", + { getEvidenceArchive: () => archive }, + ); + + expect(result.detail).toBeUndefined(); + expect(JSON.stringify(result).length).toBeLessThanOrEqual( + MAX_RESULT_CHARS + 256, + ); + const serializedTurn = serializePersistedToolResultTurn(result); + expect(serializedTurn.length).toBeLessThanOrEqual(MAX_RESULT_CHARS + 512); + expect(serializedTurn).not.toContain(hugeValue); + + const [occurrence] = await archive.listOccurrences(); + if (occurrence === undefined) throw new Error("missing archive occurrence"); + const archived = JSON.parse( + await archive.readAuthorizedPayload(occurrence.occurrenceId), + ) as { structuredContent: { hugeValue: string } }; + expect(archived.structuredContent.hugeValue).toBe(hugeValue); + }); + + test("omits unserializable structured detail without failing text content", async () => { + const result = await runEnvelopeTool( + { + blocks: [{ type: "text", text: "usable text" }], + isError: false, + structuredContent: { unsupported: 1n }, + }, + "c-mcp-unserializable-detail", + ); + + expect(result.isError).toBeUndefined(); + expect(result.content).toBe("usable text"); + expect(result.detail).toBeUndefined(); + expect(JSON.stringify(result).length).toBeLessThan(MAX_RESULT_CHARS); + }); + + test("archives identical success and failure payloads with distinct isError", async () => { + const { archive } = memoryEvidenceArchive(); + const envelope = { + blocks: [{ type: "text", text: "same payload" }], + structuredContent: { answer: 42 }, + }; + + await runEnvelopeTool( + { ...envelope, isError: false }, + "c-mcp-archive-success", + { getEvidenceArchive: () => archive }, + ); + await runEnvelopeTool( + { ...envelope, isError: true }, + "c-mcp-archive-failure", + { getEvidenceArchive: () => archive }, + ); + + const occurrences = await archive.listOccurrences(); + expect(occurrences).toHaveLength(2); + const payloads = await Promise.all( + occurrences.map(async (occurrence) => + JSON.parse( + await archive.readAuthorizedPayload(occurrence.occurrenceId), + ), + ), + ); + expect(payloads.map((payload) => payload.isError)).toEqual([false, true]); + }); + + test("falls back to legacy call when block and envelope methods are absent", async () => { + let calls = 0; + const client: MCPClient = { + serverName: "legacy", + tools: [ + { + name: "echo", + description: "returns legacy text", + inputSchema: { type: "object", properties: {} }, + }, + ], + call: async () => { + calls++; + return "legacy response"; + }, + close: async () => undefined, + }; + const [tool] = mcpClientToAgentTools(client, skipGate()); + if (tool?.kind !== "full") throw new Error("expected full tool"); + + const result = await tool.handler( + { id: "c-mcp-legacy", name: "mcp__legacy__echo", arguments: {} }, + new AbortController().signal, + ); + + expect(calls).toBe(1); + expect(result).toEqual({ + callId: "c-mcp-legacy", + content: "legacy response", + }); + }); + test("thrown transport failures still surface as error results", async () => { const gate = skipGate(); const client: MCPClient = { diff --git a/src/mcp/plugin.ts b/src/mcp/plugin.ts index 5ae8cc5d7..556daedbe 100644 --- a/src/mcp/plugin.ts +++ b/src/mcp/plugin.ts @@ -7,6 +7,7 @@ import { scrubSecretShapedValue, } from "../plugins/tool-result-secret-scrub.js"; import { + MAX_RESULT_CHARS, truncateToolResultContent, type SpillBlobWriter, } from "../plugins/result-truncation-plugin.js"; @@ -43,21 +44,9 @@ export interface McpSpillOptions { } function applyPolicyToBlocks(blocks: MCPContentBlock[]): MCPContentBlock[] { - return blocks.map((block) => { - const next = { ...block }; - if (typeof next.text === "string") { - next.text = scrubSecretShapedContent(next.text); - } - for (const [key, value] of Object.entries(next)) { - if (key === "type" || key === "text") continue; - if (typeof value === "string") { - next[key] = scrubSecretShapedContent(value); - } else if (value !== null && typeof value === "object") { - next[key] = scrubSecretShapedValue(value); - } - } - return next; - }); + return blocks.map( + (block) => scrubSecretShapedValue(block) as MCPContentBlock, + ); } // MCP results never reach the posix runner, so the secret-scrub and truncation @@ -75,6 +64,20 @@ function sanitizeMcpResultContent( ); } +function serializeStructuredContent( + value: Record, +): { serialized: string; detail?: Record } | undefined { + try { + const serialized = JSON.stringify(value); + return { + serialized, + ...(serialized.length <= MAX_RESULT_CHARS ? { detail: value } : {}), + }; + } catch { + return undefined; + } +} + export function mcpClientTools( client: MCPClient, spillOptions: McpSpillOptions = {}, @@ -134,18 +137,23 @@ export function mcpClientTools( string, unknown >); + const isError = envelope.isError === true; + const serializedStructured = + scrubbedStructured === undefined + ? undefined + : serializeStructuredContent(scrubbedStructured); const archive = getEvidenceArchive?.(); if (archive !== undefined) { try { await archive.recordAuthorizedPayload({ kind: "tool_result", - payload: - scrubbedStructured === undefined - ? { blocks: authorizedBlocks } - : { - blocks: authorizedBlocks, - structuredContent: scrubbedStructured, - }, + payload: { + blocks: authorizedBlocks, + isError, + ...(scrubbedStructured !== undefined + ? { structuredContent: scrubbedStructured } + : {}), + }, callId: call.id, provenance: "mcp:post-policy-pre-flatten", }); @@ -154,15 +162,16 @@ export function mcpClientTools( } } const flattened = unwrapToolContent(authorizedBlocks); - const isError = envelope.isError === true; const baseContent = flattened !== "" ? flattened - : scrubbedStructured !== undefined - ? `${MCP_STRUCTURED_CONTENT_MARKER}\n${JSON.stringify(scrubbedStructured)}` - : isError - ? `MCP tool ${client.serverName}/${tool.name} reported an error with empty content.` - : flattened; + : serializedStructured !== undefined + ? `${MCP_STRUCTURED_CONTENT_MARKER}\n${serializedStructured.serialized}` + : scrubbedStructured !== undefined + ? `${MCP_STRUCTURED_CONTENT_MARKER}\n[structured content unavailable]` + : isError + ? `MCP tool ${client.serverName}/${tool.name} reported an error with empty content.` + : flattened; const writeBlob = getBlobWriter?.(); const contextDir = getContextDir?.(); const spill = @@ -178,8 +187,8 @@ export function mcpClientTools( callId: call.id, content, ...(isError ? { isError: true as const } : {}), - ...(scrubbedStructured !== undefined - ? { detail: scrubbedStructured } + ...(serializedStructured?.detail !== undefined + ? { detail: serializedStructured.detail } : {}), }; } catch (err) { diff --git a/src/plugins/tool-result-secret-scrub.test.ts b/src/plugins/tool-result-secret-scrub.test.ts index 78bea1c70..6cfdccb88 100644 --- a/src/plugins/tool-result-secret-scrub.test.ts +++ b/src/plugins/tool-result-secret-scrub.test.ts @@ -62,6 +62,46 @@ describe("scrubSecretShapedValue", () => { }); }); +describe("scrubSecretShapedValue key handling", () => { + test("scrubs credential-shaped keys recursively", () => { + const topLevelKey = ["sk-", "live-", "a".repeat(24)].join(""); + const nestedKey = `prefix-${["sk-", "live-", "b".repeat(24)].join("")}`; + + const out = scrubSecretShapedValue({ + [topLevelKey]: "top-level", + nested: { [nestedKey]: "nested" }, + }); + const serialized = JSON.stringify(out); + + expect(out).toEqual({ + [CREDENTIAL_REDACTION]: "top-level", + nested: { [`prefix-${CREDENTIAL_REDACTION}`]: "nested" }, + }); + expect(serialized).not.toContain(topLevelKey); + expect(serialized).not.toContain(nestedKey); + }); + + test("resolves scrubbed key collisions deterministically without raw keys", () => { + const firstKey = ["sk-", "live-", "a".repeat(24)].join(""); + const secondKey = ["sk-", "live-", "b".repeat(24)].join(""); + + const out = scrubSecretShapedValue({ + [firstKey]: "first", + [secondKey]: "second", + [CREDENTIAL_REDACTION]: "already-redacted", + }); + const serialized = JSON.stringify(out); + + expect(out).toEqual({ + [CREDENTIAL_REDACTION]: "first", + [`${CREDENTIAL_REDACTION} [2]`]: "second", + [`${CREDENTIAL_REDACTION} [3]`]: "already-redacted", + }); + expect(serialized).not.toContain(firstKey); + expect(serialized).not.toContain(secondKey); + }); +}); + describe("toolResultSecretScrubPlugin", () => { const next = (content: ToolResult["content"], isError = false) => diff --git a/src/plugins/tool-result-secret-scrub.ts b/src/plugins/tool-result-secret-scrub.ts index 3526f53a9..570e44783 100644 --- a/src/plugins/tool-result-secret-scrub.ts +++ b/src/plugins/tool-result-secret-scrub.ts @@ -78,8 +78,8 @@ export function scrubSecretShapedContent(text: string): string { /** * Structure-preserving scrub for validated JSON-shaped tool results. String - * leaves are scrubbed in place; objects/arrays keep their shape. Never - * stringifies a Record into the result content. + * keys and leaves are scrubbed in place; objects/arrays keep their shape. + * Never stringifies a Record into the result content. */ export function scrubSecretShapedValue(value: unknown): unknown { if (typeof value === "string") return scrubSecretShapedContent(value); @@ -88,9 +88,22 @@ export function scrubSecretShapedValue(value: unknown): unknown { if (value !== null && typeof value === "object") { const out: Record = {}; for (const [key, child] of Object.entries(value)) { - out[key] = scrubSecretShapedValue(child); + const scrubbedKey = uniqueScrubbedKey(out, scrubSecretShapedContent(key)); + out[scrubbedKey] = scrubSecretShapedValue(child); } return out; } return value; } + +function uniqueScrubbedKey( + target: Record, + scrubbedKey: string, +): string { + if (!Object.hasOwn(target, scrubbedKey)) return scrubbedKey; + let collisionIndex = 2; + while (Object.hasOwn(target, `${scrubbedKey} [${collisionIndex}]`)) { + collisionIndex++; + } + return `${scrubbedKey} [${collisionIndex}]`; +} diff --git a/src/session/hooks.test.ts b/src/session/hooks.test.ts index 198927995..5545333e0 100644 --- a/src/session/hooks.test.ts +++ b/src/session/hooks.test.ts @@ -3,6 +3,10 @@ import { mkdtemp, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import type { ReactorEmittedEvent } from "@intx/inference"; +import { + CREDENTIAL_REDACTION, + scrubSecretShapedValue, +} from "../plugins/tool-result-secret-scrub.js"; import { createLifecycleHookManager, createTurnContextCollector, @@ -80,6 +84,18 @@ describe("createTurnContextCollector tool result truncation", () => { expect(turn?.toolResults[0]?.detail).toEqual(detail); }); + test("retained hook payloads do not expose credential-shaped detail keys", () => { + const collector = createTurnContextCollector(() => undefined); + const rawKey = ["sk-", "live-", "h".repeat(24)].join(""); + const detail = scrubSecretShapedValue({ [rawKey]: "value" }); + + observeOneTurnWithToolResult(collector, "exit code 0", detail); + + const payload = JSON.stringify(collector.getTurns()[0]); + expect(payload).toContain(CREDENTIAL_REDACTION); + expect(payload).not.toContain(rawKey); + }); + test("omits oversized structured detail while keeping the content cap", () => { const collector = createTurnContextCollector(() => undefined); const hugeOutput = "x".repeat(HOOK_PAYLOAD_TOOL_RESULT_CHARS * 4); @@ -95,6 +111,9 @@ describe("createTurnContextCollector tool result truncation", () => { expect((content as string).length).toBeLessThanOrEqual( HOOK_PAYLOAD_TOOL_RESULT_CHARS + 64, ); + expect(JSON.stringify(turn).length).toBeLessThanOrEqual( + HOOK_PAYLOAD_TOOL_RESULT_CHARS + 512, + ); }); }); From 60714ebbbc0082d33975892769b373973241a4bf Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 25 Sep 2026 15:37:59 -0700 Subject: [PATCH 6/6] fix(mcp): harden structured result evidence --- src/mcp/plugin.test.ts | 145 +++++++++++++++++-- src/mcp/plugin.ts | 11 +- src/plugins/tool-result-secret-scrub.test.ts | 104 +++++++++++++ src/plugins/tool-result-secret-scrub.ts | 103 +++++++++++-- src/session/hooks.test.ts | 15 ++ 5 files changed, 355 insertions(+), 23 deletions(-) diff --git a/src/mcp/plugin.test.ts b/src/mcp/plugin.test.ts index c7184e2de..106d305d6 100644 --- a/src/mcp/plugin.test.ts +++ b/src/mcp/plugin.test.ts @@ -330,6 +330,61 @@ describe("mcpClientToAgentTools", () => { expect(detailJson).not.toContain("sk-live-"); }); + test("scrubs short credential-keyed values from every retained surface", async () => { + const { archive, blobs } = memoryEvidenceArchive(); + const result = await runEnvelopeTool( + { + blocks: [{ type: "resource", authorization: "block-short" }], + isError: false, + structuredContent: { + apiKey: "top-short", + nested: { auth: "nested-short", access_token: "access-short" }, + }, + }, + "c-mcp-short-credential-values", + { getEvidenceArchive: () => archive }, + ); + + const surfaces = [ + JSON.stringify(result.detail), + String(result.content), + serializePersistedToolResultTurn(result), + ...[...blobs.values()].map((bytes) => new TextDecoder().decode(bytes)), + ]; + for (const surface of surfaces) { + expect(surface).toContain(CREDENTIAL_REDACTION); + expect(surface).not.toContain("top-short"); + expect(surface).not.toContain("nested-short"); + expect(surface).not.toContain("access-short"); + expect(surface).not.toContain("block-short"); + } + }); + + test("preserves special structured keys without prototype mutation", async () => { + const { archive } = memoryEvidenceArchive(); + const structuredContent = JSON.parse( + '{"__proto__":"top","constructor":"ctor","nested":{"__proto__":"nested"}}', + ) as Record; + const result = await runEnvelopeTool( + { blocks: [], structuredContent }, + "c-mcp-special-keys", + { getEvidenceArchive: () => archive }, + ); + + const detail = result.detail as Record; + const nested = detail.nested as Record; + expect(Object.getPrototypeOf(detail)).toBeNull(); + expect(Object.getPrototypeOf(nested)).toBeNull(); + expect(JSON.stringify(detail)).toBe(JSON.stringify(structuredContent)); + expect(({} as Record).top).toBeUndefined(); + + const [occurrence] = await archive.listOccurrences(); + if (occurrence === undefined) throw new Error("missing archive occurrence"); + expect( + await archive.readAuthorizedPayload(occurrence.occurrenceId), + ).toContain('"__proto__":"top"'); + }); + test("scrubs structured keys from detail, archive bytes, and model content", async () => { const topLevelKey = ["sk-", "live-", "a".repeat(24)].join(""); const nestedKey = ["sk-", "live-", "b".repeat(24)].join(""); @@ -370,21 +425,28 @@ describe("mcpClientToAgentTools", () => { test("keeps oversized structured content full only in the evidence archive", async () => { const { archive } = memoryEvidenceArchive(); + const store = fakeBlobStore(); const hugeValue = "x".repeat(MAX_RESULT_CHARS * 4); + const callId = "c-mcp-oversized-detail"; const result = await runEnvelopeTool( { blocks: [], isError: false, structuredContent: { hugeValue }, }, - "c-mcp-oversized-detail", - { getEvidenceArchive: () => archive }, + callId, + { + getEvidenceArchive: () => archive, + getBlobWriter: () => store.writeBlob, + }, ); expect(result.detail).toBeUndefined(); expect(JSON.stringify(result).length).toBeLessThanOrEqual( MAX_RESULT_CHARS + 256, ); + expect(store.blobs.has(spillBlobKey(callId))).toBe(false); + expect(result.content).not.toContain("tool-output:///"); const serializedTurn = serializePersistedToolResultTurn(result); expect(serializedTurn.length).toBeLessThanOrEqual(MAX_RESULT_CHARS + 512); expect(serializedTurn).not.toContain(hugeValue); @@ -397,20 +459,83 @@ describe("mcpClientToAgentTools", () => { expect(archived.structuredContent.hugeValue).toBe(hugeValue); }); - test("omits unserializable structured detail without failing text content", async () => { + test("spills oversized structured content when the evidence archive fails", async () => { + const { archive } = memoryEvidenceArchive(); + const failingArchive = { + ...archive, + recordAuthorizedPayload: async () => { + throw new Error("archive unavailable"); + }, + }; + const store = fakeBlobStore(); + const hugeValue = "y".repeat(MAX_RESULT_CHARS * 4); + const callId = "c-mcp-oversized-archive-failure"; + const result = await runEnvelopeTool( + { blocks: [], structuredContent: { hugeValue } }, + callId, { - blocks: [{ type: "text", text: "usable text" }], - isError: false, - structuredContent: { unsupported: 1n }, + getEvidenceArchive: () => failingArchive, + getBlobWriter: () => store.writeBlob, }, - "c-mcp-unserializable-detail", ); - expect(result.isError).toBeUndefined(); - expect(result.content).toBe("usable text"); + const spill = store.blobs.get(spillBlobKey(callId)); + expect(spill).toBeDefined(); + expect(new TextDecoder().decode(defined(spill).bytes)).toContain(hugeValue); + expect(result.content).toContain(`tool-output:///${spillBlobKey(callId)}`); expect(result.detail).toBeUndefined(); - expect(JSON.stringify(result).length).toBeLessThan(MAX_RESULT_CHARS); + }); + + test("rejects non-JSON structured values without invoking hooks or leaking", async () => { + const leakMarker = "short-private-marker"; + let accessorReads = 0; + let toJSONCalls = 0; + const accessor = Object.defineProperty({}, "value", { + enumerable: true, + get: () => { + accessorReads++; + return leakMarker; + }, + }); + const customJSON = { + toJSON: () => { + toJSONCalls++; + return { leaked: leakMarker }; + }, + }; + const cycle: Record = {}; + cycle.self = cycle; + const invalidValues: unknown[] = [ + accessor, + customJSON, + { value: () => leakMarker }, + { value: Symbol(leakMarker) }, + { value: 1n }, + cycle, + ]; + + for (const [index, structuredContent] of invalidValues.entries()) { + const { archive, blobs } = memoryEvidenceArchive(); + const result = await runEnvelopeTool( + { + blocks: [{ type: "text", text: "usable text" }], + structuredContent: structuredContent as Record, + }, + `c-mcp-invalid-structured-${index}`, + { getEvidenceArchive: () => archive }, + ); + + expect(result.isError).toBe(true); + expect(result.detail).toBeUndefined(); + expect(result.content).toBe("Tool result is not JSON-safe"); + expect(JSON.stringify(result)).not.toContain(leakMarker); + for (const bytes of blobs.values()) { + expect(new TextDecoder().decode(bytes)).not.toContain(leakMarker); + } + } + expect(accessorReads).toBe(0); + expect(toJSONCalls).toBe(0); }); test("archives identical success and failure payloads with distinct isError", async () => { diff --git a/src/mcp/plugin.ts b/src/mcp/plugin.ts index 556daedbe..9cb205fae 100644 --- a/src/mcp/plugin.ts +++ b/src/mcp/plugin.ts @@ -143,6 +143,7 @@ export function mcpClientTools( ? undefined : serializeStructuredContent(scrubbedStructured); const archive = getEvidenceArchive?.(); + let archivedFullEnvelope = false; if (archive !== undefined) { try { await archive.recordAuthorizedPayload({ @@ -157,6 +158,7 @@ export function mcpClientTools( callId: call.id, provenance: "mcp:post-policy-pre-flatten", }); + archivedFullEnvelope = true; } catch { // Archive write must not fail a successful tool result. } @@ -182,7 +184,14 @@ export function mcpClientTools( ...(contextDir !== undefined ? { contextDir } : {}), } : undefined; - const content = await sanitizeMcpResultContent(baseContent, spill); + const structuredOnlyArchived = + archivedFullEnvelope && + flattened === "" && + scrubbedStructured !== undefined; + const content = await sanitizeMcpResultContent( + baseContent, + structuredOnlyArchived ? undefined : spill, + ); return { callId: call.id, content, diff --git a/src/plugins/tool-result-secret-scrub.test.ts b/src/plugins/tool-result-secret-scrub.test.ts index 6cfdccb88..2a2635a43 100644 --- a/src/plugins/tool-result-secret-scrub.test.ts +++ b/src/plugins/tool-result-secret-scrub.test.ts @@ -62,6 +62,110 @@ describe("scrubSecretShapedValue", () => { }); }); +describe("scrubSecretShapedValue normalization", () => { + test("redacts short values selected by credential-named keys", () => { + const out = scrubSecretShapedValue({ + apiKey: "a", + nested: { + api_key: "b", + accessToken: "c", + token: "d", + password: "e", + secret: "f", + credential: "g", + authorization: "h", + auth: "i", + }, + }); + + expect(out).toEqual({ + apiKey: CREDENTIAL_REDACTION, + nested: { + api_key: CREDENTIAL_REDACTION, + accessToken: CREDENTIAL_REDACTION, + token: CREDENTIAL_REDACTION, + password: CREDENTIAL_REDACTION, + secret: CREDENTIAL_REDACTION, + credential: CREDENTIAL_REDACTION, + authorization: CREDENTIAL_REDACTION, + auth: CREDENTIAL_REDACTION, + }, + }); + }); + + test("preserves special own keys without changing object prototypes", () => { + const input = JSON.parse( + '{"__proto__":"top","constructor":"ctor","nested":{"__proto__":"nested"}}', + ) as Record; + + const out = scrubSecretShapedValue(input) as Record; + const nested = out.nested as Record; + + expect(Object.getPrototypeOf(out)).toBeNull(); + expect(Object.getPrototypeOf(nested)).toBeNull(); + expect(Object.hasOwn(out, "__proto__")).toBe(true); + expect(Object.hasOwn(out, "constructor")).toBe(true); + expect(Object.hasOwn(nested, "__proto__")).toBe(true); + expect(JSON.stringify(out)).toBe(JSON.stringify(input)); + expect(({} as Record).top).toBeUndefined(); + expect(({} as Record).nested).toBeUndefined(); + }); + + test("rejects accessors without invoking them", () => { + let reads = 0; + const input = Object.defineProperty({}, "secret", { + enumerable: true, + get: () => { + reads++; + return "short-secret"; + }, + }); + + expect(() => scrubSecretShapedValue(input)).toThrow( + "Tool result is not JSON-safe", + ); + expect(reads).toBe(0); + }); + + test("rejects custom serialization without invoking it", () => { + let calls = 0; + const input = { + safe: "value", + toJSON: () => { + calls++; + return { leaked: "short-secret" }; + }, + }; + + expect(() => scrubSecretShapedValue(input)).toThrow( + "Tool result is not JSON-safe", + ); + expect(calls).toBe(0); + }); + + test.each([ + ["function", () => undefined], + ["symbol", Symbol("unsupported")], + ["bigint", 1n], + ["undefined", undefined], + ])("rejects %s values", (_name, value) => { + expect(() => scrubSecretShapedValue({ value })).toThrow( + "Tool result is not JSON-safe", + ); + }); + + test("rejects cycles and custom object behavior", () => { + const cyclic: Record = {}; + cyclic.self = cyclic; + + expect(() => scrubSecretShapedValue(cyclic)).toThrow( + "Tool result is not JSON-safe", + ); + expect(() => scrubSecretShapedValue({ value: new Date(0) })).toThrow( + "Tool result is not JSON-safe", + ); + }); +}); describe("scrubSecretShapedValue key handling", () => { test("scrubs credential-shaped keys recursively", () => { const topLevelKey = ["sk-", "live-", "a".repeat(24)].join(""); diff --git a/src/plugins/tool-result-secret-scrub.ts b/src/plugins/tool-result-secret-scrub.ts index 570e44783..6a22b1f87 100644 --- a/src/plugins/tool-result-secret-scrub.ts +++ b/src/plugins/tool-result-secret-scrub.ts @@ -76,24 +76,103 @@ export function scrubSecretShapedContent(text: string): string { return result; } +const JSON_SAFE_ERROR = "Tool result is not JSON-safe"; +const CREDENTIAL_FIELD = + /^(?:api[_ -]?key|access[_ -]?token|token|password|secret|credential|authorization|auth)$/i; + /** - * Structure-preserving scrub for validated JSON-shaped tool results. String - * keys and leaves are scrubbed in place; objects/arrays keep their shape. - * Never stringifies a Record into the result content. + * Returns a detached JSON-safe value without invoking input accessors or custom + * serialization. Records use a null prototype so every JSON key remains data. */ export function scrubSecretShapedValue(value: unknown): unknown { + try { + return normalizeJSONValue(value, new Set()); + } catch { + throw new TypeError(JSON_SAFE_ERROR); + } +} + +function normalizeJSONValue(value: unknown, ancestors: Set): unknown { + if (value === null) return null; if (typeof value === "string") return scrubSecretShapedContent(value); - if (Array.isArray(value)) - return value.map((item) => scrubSecretShapedValue(item)); - if (value !== null && typeof value === "object") { - const out: Record = {}; - for (const [key, child] of Object.entries(value)) { - const scrubbedKey = uniqueScrubbedKey(out, scrubSecretShapedContent(key)); - out[scrubbedKey] = scrubSecretShapedValue(child); + if (typeof value === "boolean") return value; + if (typeof value === "number") { + if (!Number.isFinite(value)) throw new TypeError(JSON_SAFE_ERROR); + return value; + } + if (typeof value !== "object") throw new TypeError(JSON_SAFE_ERROR); + if (ancestors.has(value)) throw new TypeError(JSON_SAFE_ERROR); + + ancestors.add(value); + try { + if (Array.isArray(value)) return normalizeJSONArray(value, ancestors); + return normalizeJSONObject(value, ancestors); + } finally { + ancestors.delete(value); + } +} + +function normalizeJSONArray( + value: unknown[], + ancestors: Set, +): unknown[] { + if (Object.getPrototypeOf(value) !== Array.prototype) { + throw new TypeError(JSON_SAFE_ERROR); + } + for (const key of Reflect.ownKeys(value)) { + if (typeof key !== "string") throw new TypeError(JSON_SAFE_ERROR); + if (key === "length") continue; + const index = Number(key); + if (!Number.isInteger(index) || index < 0 || String(index) !== key) { + throw new TypeError(JSON_SAFE_ERROR); + } + const descriptor = Object.getOwnPropertyDescriptor(value, key); + if (descriptor === undefined || !("value" in descriptor)) { + throw new TypeError(JSON_SAFE_ERROR); + } + } + + return Array.from({ length: value.length }, (_, index) => { + const descriptor = Object.getOwnPropertyDescriptor(value, String(index)); + if (descriptor === undefined) return null; + if (!("value" in descriptor)) throw new TypeError(JSON_SAFE_ERROR); + return normalizeJSONValue(descriptor.value, ancestors); + }); +} + +function normalizeJSONObject( + value: object, + ancestors: Set, +): Record { + const prototype = Object.getPrototypeOf(value); + if (prototype !== Object.prototype && prototype !== null) { + throw new TypeError(JSON_SAFE_ERROR); + } + + const out: Record = Object.create(null) as Record< + string, + unknown + >; + for (const key of Reflect.ownKeys(value)) { + if (typeof key !== "string") throw new TypeError(JSON_SAFE_ERROR); + const descriptor = Object.getOwnPropertyDescriptor(value, key); + if ( + descriptor === undefined || + !("value" in descriptor) || + descriptor.enumerable !== true + ) { + throw new TypeError(JSON_SAFE_ERROR); } - return out; + const normalized = normalizeJSONValue(descriptor.value, ancestors); + const scrubbedKey = uniqueScrubbedKey(out, scrubSecretShapedContent(key)); + Object.defineProperty(out, scrubbedKey, { + value: CREDENTIAL_FIELD.test(key) ? CREDENTIAL_REDACTION : normalized, + enumerable: true, + configurable: true, + writable: true, + }); } - return value; + return out; } function uniqueScrubbedKey( diff --git a/src/session/hooks.test.ts b/src/session/hooks.test.ts index 5545333e0..c60f437d9 100644 --- a/src/session/hooks.test.ts +++ b/src/session/hooks.test.ts @@ -96,6 +96,21 @@ describe("createTurnContextCollector tool result truncation", () => { expect(payload).not.toContain(rawKey); }); + test("retained hook payloads redact short credential-keyed values", () => { + const collector = createTurnContextCollector(() => undefined); + const detail = scrubSecretShapedValue({ + apiKey: "top-short", + nested: { auth: "nested-short" }, + }); + + observeOneTurnWithToolResult(collector, "exit code 0", detail); + + const payload = JSON.stringify(collector.getTurns()[0]); + expect(payload).toContain(CREDENTIAL_REDACTION); + expect(payload).not.toContain("top-short"); + expect(payload).not.toContain("nested-short"); + }); + test("omits oversized structured detail while keeping the content cap", () => { const collector = createTurnContextCollector(() => undefined); const hugeOutput = "x".repeat(HOOK_PAYLOAD_TOOL_RESULT_CHARS * 4);