Skip to content

Commit 60714eb

Browse files
committed
fix(mcp): harden structured result evidence
1 parent 6bb9b15 commit 60714eb

5 files changed

Lines changed: 355 additions & 23 deletions

File tree

‎src/mcp/plugin.test.ts‎

Lines changed: 135 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,61 @@ describe("mcpClientToAgentTools", () => {
330330
expect(detailJson).not.toContain("sk-live-");
331331
});
332332

333+
test("scrubs short credential-keyed values from every retained surface", async () => {
334+
const { archive, blobs } = memoryEvidenceArchive();
335+
const result = await runEnvelopeTool(
336+
{
337+
blocks: [{ type: "resource", authorization: "block-short" }],
338+
isError: false,
339+
structuredContent: {
340+
apiKey: "top-short",
341+
nested: { auth: "nested-short", access_token: "access-short" },
342+
},
343+
},
344+
"c-mcp-short-credential-values",
345+
{ getEvidenceArchive: () => archive },
346+
);
347+
348+
const surfaces = [
349+
JSON.stringify(result.detail),
350+
String(result.content),
351+
serializePersistedToolResultTurn(result),
352+
...[...blobs.values()].map((bytes) => new TextDecoder().decode(bytes)),
353+
];
354+
for (const surface of surfaces) {
355+
expect(surface).toContain(CREDENTIAL_REDACTION);
356+
expect(surface).not.toContain("top-short");
357+
expect(surface).not.toContain("nested-short");
358+
expect(surface).not.toContain("access-short");
359+
expect(surface).not.toContain("block-short");
360+
}
361+
});
362+
363+
test("preserves special structured keys without prototype mutation", async () => {
364+
const { archive } = memoryEvidenceArchive();
365+
const structuredContent = JSON.parse(
366+
'{"__proto__":"top","constructor":"ctor","nested":{"__proto__":"nested"}}',
367+
) as Record<string, unknown>;
368+
const result = await runEnvelopeTool(
369+
{ blocks: [], structuredContent },
370+
"c-mcp-special-keys",
371+
{ getEvidenceArchive: () => archive },
372+
);
373+
374+
const detail = result.detail as Record<string, unknown>;
375+
const nested = detail.nested as Record<string, unknown>;
376+
expect(Object.getPrototypeOf(detail)).toBeNull();
377+
expect(Object.getPrototypeOf(nested)).toBeNull();
378+
expect(JSON.stringify(detail)).toBe(JSON.stringify(structuredContent));
379+
expect(({} as Record<string, unknown>).top).toBeUndefined();
380+
381+
const [occurrence] = await archive.listOccurrences();
382+
if (occurrence === undefined) throw new Error("missing archive occurrence");
383+
expect(
384+
await archive.readAuthorizedPayload(occurrence.occurrenceId),
385+
).toContain('"__proto__":"top"');
386+
});
387+
333388
test("scrubs structured keys from detail, archive bytes, and model content", async () => {
334389
const topLevelKey = ["sk-", "live-", "a".repeat(24)].join("");
335390
const nestedKey = ["sk-", "live-", "b".repeat(24)].join("");
@@ -370,21 +425,28 @@ describe("mcpClientToAgentTools", () => {
370425

371426
test("keeps oversized structured content full only in the evidence archive", async () => {
372427
const { archive } = memoryEvidenceArchive();
428+
const store = fakeBlobStore();
373429
const hugeValue = "x".repeat(MAX_RESULT_CHARS * 4);
430+
const callId = "c-mcp-oversized-detail";
374431
const result = await runEnvelopeTool(
375432
{
376433
blocks: [],
377434
isError: false,
378435
structuredContent: { hugeValue },
379436
},
380-
"c-mcp-oversized-detail",
381-
{ getEvidenceArchive: () => archive },
437+
callId,
438+
{
439+
getEvidenceArchive: () => archive,
440+
getBlobWriter: () => store.writeBlob,
441+
},
382442
);
383443

384444
expect(result.detail).toBeUndefined();
385445
expect(JSON.stringify(result).length).toBeLessThanOrEqual(
386446
MAX_RESULT_CHARS + 256,
387447
);
448+
expect(store.blobs.has(spillBlobKey(callId))).toBe(false);
449+
expect(result.content).not.toContain("tool-output:///");
388450
const serializedTurn = serializePersistedToolResultTurn(result);
389451
expect(serializedTurn.length).toBeLessThanOrEqual(MAX_RESULT_CHARS + 512);
390452
expect(serializedTurn).not.toContain(hugeValue);
@@ -397,20 +459,83 @@ describe("mcpClientToAgentTools", () => {
397459
expect(archived.structuredContent.hugeValue).toBe(hugeValue);
398460
});
399461

400-
test("omits unserializable structured detail without failing text content", async () => {
462+
test("spills oversized structured content when the evidence archive fails", async () => {
463+
const { archive } = memoryEvidenceArchive();
464+
const failingArchive = {
465+
...archive,
466+
recordAuthorizedPayload: async () => {
467+
throw new Error("archive unavailable");
468+
},
469+
};
470+
const store = fakeBlobStore();
471+
const hugeValue = "y".repeat(MAX_RESULT_CHARS * 4);
472+
const callId = "c-mcp-oversized-archive-failure";
473+
401474
const result = await runEnvelopeTool(
475+
{ blocks: [], structuredContent: { hugeValue } },
476+
callId,
402477
{
403-
blocks: [{ type: "text", text: "usable text" }],
404-
isError: false,
405-
structuredContent: { unsupported: 1n },
478+
getEvidenceArchive: () => failingArchive,
479+
getBlobWriter: () => store.writeBlob,
406480
},
407-
"c-mcp-unserializable-detail",
408481
);
409482

410-
expect(result.isError).toBeUndefined();
411-
expect(result.content).toBe("usable text");
483+
const spill = store.blobs.get(spillBlobKey(callId));
484+
expect(spill).toBeDefined();
485+
expect(new TextDecoder().decode(defined(spill).bytes)).toContain(hugeValue);
486+
expect(result.content).toContain(`tool-output:///${spillBlobKey(callId)}`);
412487
expect(result.detail).toBeUndefined();
413-
expect(JSON.stringify(result).length).toBeLessThan(MAX_RESULT_CHARS);
488+
});
489+
490+
test("rejects non-JSON structured values without invoking hooks or leaking", async () => {
491+
const leakMarker = "short-private-marker";
492+
let accessorReads = 0;
493+
let toJSONCalls = 0;
494+
const accessor = Object.defineProperty({}, "value", {
495+
enumerable: true,
496+
get: () => {
497+
accessorReads++;
498+
return leakMarker;
499+
},
500+
});
501+
const customJSON = {
502+
toJSON: () => {
503+
toJSONCalls++;
504+
return { leaked: leakMarker };
505+
},
506+
};
507+
const cycle: Record<string, unknown> = {};
508+
cycle.self = cycle;
509+
const invalidValues: unknown[] = [
510+
accessor,
511+
customJSON,
512+
{ value: () => leakMarker },
513+
{ value: Symbol(leakMarker) },
514+
{ value: 1n },
515+
cycle,
516+
];
517+
518+
for (const [index, structuredContent] of invalidValues.entries()) {
519+
const { archive, blobs } = memoryEvidenceArchive();
520+
const result = await runEnvelopeTool(
521+
{
522+
blocks: [{ type: "text", text: "usable text" }],
523+
structuredContent: structuredContent as Record<string, unknown>,
524+
},
525+
`c-mcp-invalid-structured-${index}`,
526+
{ getEvidenceArchive: () => archive },
527+
);
528+
529+
expect(result.isError).toBe(true);
530+
expect(result.detail).toBeUndefined();
531+
expect(result.content).toBe("Tool result is not JSON-safe");
532+
expect(JSON.stringify(result)).not.toContain(leakMarker);
533+
for (const bytes of blobs.values()) {
534+
expect(new TextDecoder().decode(bytes)).not.toContain(leakMarker);
535+
}
536+
}
537+
expect(accessorReads).toBe(0);
538+
expect(toJSONCalls).toBe(0);
414539
});
415540

416541
test("archives identical success and failure payloads with distinct isError", async () => {

‎src/mcp/plugin.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ export function mcpClientTools(
143143
? undefined
144144
: serializeStructuredContent(scrubbedStructured);
145145
const archive = getEvidenceArchive?.();
146+
let archivedFullEnvelope = false;
146147
if (archive !== undefined) {
147148
try {
148149
await archive.recordAuthorizedPayload({
@@ -157,6 +158,7 @@ export function mcpClientTools(
157158
callId: call.id,
158159
provenance: "mcp:post-policy-pre-flatten",
159160
});
161+
archivedFullEnvelope = true;
160162
} catch {
161163
// Archive write must not fail a successful tool result.
162164
}
@@ -182,7 +184,14 @@ export function mcpClientTools(
182184
...(contextDir !== undefined ? { contextDir } : {}),
183185
}
184186
: undefined;
185-
const content = await sanitizeMcpResultContent(baseContent, spill);
187+
const structuredOnlyArchived =
188+
archivedFullEnvelope &&
189+
flattened === "" &&
190+
scrubbedStructured !== undefined;
191+
const content = await sanitizeMcpResultContent(
192+
baseContent,
193+
structuredOnlyArchived ? undefined : spill,
194+
);
186195
return {
187196
callId: call.id,
188197
content,

‎src/plugins/tool-result-secret-scrub.test.ts‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,110 @@ describe("scrubSecretShapedValue", () => {
6262
});
6363
});
6464

65+
describe("scrubSecretShapedValue normalization", () => {
66+
test("redacts short values selected by credential-named keys", () => {
67+
const out = scrubSecretShapedValue({
68+
apiKey: "a",
69+
nested: {
70+
api_key: "b",
71+
accessToken: "c",
72+
token: "d",
73+
password: "e",
74+
secret: "f",
75+
credential: "g",
76+
authorization: "h",
77+
auth: "i",
78+
},
79+
});
80+
81+
expect(out).toEqual({
82+
apiKey: CREDENTIAL_REDACTION,
83+
nested: {
84+
api_key: CREDENTIAL_REDACTION,
85+
accessToken: CREDENTIAL_REDACTION,
86+
token: CREDENTIAL_REDACTION,
87+
password: CREDENTIAL_REDACTION,
88+
secret: CREDENTIAL_REDACTION,
89+
credential: CREDENTIAL_REDACTION,
90+
authorization: CREDENTIAL_REDACTION,
91+
auth: CREDENTIAL_REDACTION,
92+
},
93+
});
94+
});
95+
96+
test("preserves special own keys without changing object prototypes", () => {
97+
const input = JSON.parse(
98+
'{"__proto__":"top","constructor":"ctor","nested":{"__proto__":"nested"}}',
99+
) as Record<string, unknown>;
100+
101+
const out = scrubSecretShapedValue(input) as Record<string, unknown>;
102+
const nested = out.nested as Record<string, unknown>;
103+
104+
expect(Object.getPrototypeOf(out)).toBeNull();
105+
expect(Object.getPrototypeOf(nested)).toBeNull();
106+
expect(Object.hasOwn(out, "__proto__")).toBe(true);
107+
expect(Object.hasOwn(out, "constructor")).toBe(true);
108+
expect(Object.hasOwn(nested, "__proto__")).toBe(true);
109+
expect(JSON.stringify(out)).toBe(JSON.stringify(input));
110+
expect(({} as Record<string, unknown>).top).toBeUndefined();
111+
expect(({} as Record<string, unknown>).nested).toBeUndefined();
112+
});
113+
114+
test("rejects accessors without invoking them", () => {
115+
let reads = 0;
116+
const input = Object.defineProperty({}, "secret", {
117+
enumerable: true,
118+
get: () => {
119+
reads++;
120+
return "short-secret";
121+
},
122+
});
123+
124+
expect(() => scrubSecretShapedValue(input)).toThrow(
125+
"Tool result is not JSON-safe",
126+
);
127+
expect(reads).toBe(0);
128+
});
129+
130+
test("rejects custom serialization without invoking it", () => {
131+
let calls = 0;
132+
const input = {
133+
safe: "value",
134+
toJSON: () => {
135+
calls++;
136+
return { leaked: "short-secret" };
137+
},
138+
};
139+
140+
expect(() => scrubSecretShapedValue(input)).toThrow(
141+
"Tool result is not JSON-safe",
142+
);
143+
expect(calls).toBe(0);
144+
});
145+
146+
test.each([
147+
["function", () => undefined],
148+
["symbol", Symbol("unsupported")],
149+
["bigint", 1n],
150+
["undefined", undefined],
151+
])("rejects %s values", (_name, value) => {
152+
expect(() => scrubSecretShapedValue({ value })).toThrow(
153+
"Tool result is not JSON-safe",
154+
);
155+
});
156+
157+
test("rejects cycles and custom object behavior", () => {
158+
const cyclic: Record<string, unknown> = {};
159+
cyclic.self = cyclic;
160+
161+
expect(() => scrubSecretShapedValue(cyclic)).toThrow(
162+
"Tool result is not JSON-safe",
163+
);
164+
expect(() => scrubSecretShapedValue({ value: new Date(0) })).toThrow(
165+
"Tool result is not JSON-safe",
166+
);
167+
});
168+
});
65169
describe("scrubSecretShapedValue key handling", () => {
66170
test("scrubs credential-shaped keys recursively", () => {
67171
const topLevelKey = ["sk-", "live-", "a".repeat(24)].join("");

0 commit comments

Comments
 (0)