From 4ca3032e7fe323bda8ad1a55af102ab99a5d39c1 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Mon, 26 Oct 2026 18:05:53 +0800 Subject: [PATCH] fix(json): preserve configuration and capture data at admission --- src/application/ClientRegistration.test.ts | 38 ++++++ src/application/ClientRegistration.ts | 3 +- .../history/HistoricalCaptureReply.test.ts | 86 +++++++++++++ src/browser/history/HistoricalCaptureReply.ts | 11 +- .../EnvironmentManifestAccumulator.ts | 13 +- .../environmentManifestView.test.ts | 12 +- .../analysisView/environmentManifestView.ts | 3 +- src/domain/jsonRecord.test.ts | 116 ++++++++++++++++++ src/domain/jsonRecord.ts | 30 +++++ src/ida/IdaConfiguration.test.ts | 54 +++++++- src/ida/IdaConfiguration.ts | 5 +- tests/boundary/mcp/analysisViewMcp.test.ts | 27 +++- 12 files changed, 373 insertions(+), 25 deletions(-) create mode 100644 src/application/ClientRegistration.test.ts create mode 100644 src/browser/history/HistoricalCaptureReply.test.ts create mode 100644 src/domain/jsonRecord.test.ts create mode 100644 src/domain/jsonRecord.ts diff --git a/src/application/ClientRegistration.test.ts b/src/application/ClientRegistration.test.ts new file mode 100644 index 0000000000..f7304020e4 --- /dev/null +++ b/src/application/ClientRegistration.test.ts @@ -0,0 +1,38 @@ +import { expect, it } from "vitest"; + +import { parseClientRegistration } from "./ClientRegistration.js"; +import type { SetupClient } from "./SupportedClients.js"; + +const client: SetupClient = { + name: "opencode", + format: "opencode", + configPath: "/fixture/opencode.json", +}; + +it("preserves every OpenCode environment key while normalizing its launcher", () => { + const input = JSON.parse( + '{"type":"local","command":["rea","mcp"],"environment":{"__proto__":"operator","constructor":"selected","normal":"value"}}', + ); + const result = parseClientRegistration(input, client); + expect(result.ok).toBe(true); + if (!result.ok) throw result.error; + expect(result.value.command).toBe("rea"); + expect(result.value.args).toEqual(["mcp"]); + expect(result.value.environment).toEqual(input.environment); + const environment = result.value.environment; + if (typeof environment !== "object" || environment === null) + throw new Error("Missing parsed environment"); + expect(Object.hasOwn(environment, "__proto__")).toBe(true); +}); + +it.each([42, null, {}])( + "rejects malformed OpenCode environment values at __proto__: %j", + (value) => { + const environment = Object.fromEntries([["__proto__", value]]); + const result = parseClientRegistration( + { type: "local", command: ["rea", "mcp"], environment }, + client, + ); + expect(result.ok).toBe(false); + }, +); diff --git a/src/application/ClientRegistration.ts b/src/application/ClientRegistration.ts index caa1e1a28f..9ae1b0b12a 100644 --- a/src/application/ClientRegistration.ts +++ b/src/application/ClientRegistration.ts @@ -1,4 +1,5 @@ import { z } from "zod"; +import { jsonRecordSchema } from "../domain/jsonRecord.js"; import { ConfigurationError } from "../domain/configurationErrors.js"; import { err, ok, type Result } from "../domain/result.js"; @@ -141,7 +142,7 @@ export const parseClientRegistration = ( if (client.format === "opencode") { const entry = opencodeLauncherSchema .extend({ - environment: z.record(z.string(), z.string()).optional(), + environment: jsonRecordSchema(z.string()).optional(), }) .transform(normalizeOpencodeLauncher) .safeParse(value); diff --git a/src/browser/history/HistoricalCaptureReply.test.ts b/src/browser/history/HistoricalCaptureReply.test.ts new file mode 100644 index 0000000000..684c50a693 --- /dev/null +++ b/src/browser/history/HistoricalCaptureReply.test.ts @@ -0,0 +1,86 @@ +import { writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { expect, it, onTestFinished } from "vitest"; +import { ArtifactResourceScope } from "../../artifacts/ArtifactResourceScope.js"; +import { createTestTempDirectory } from "../../../tests/fixtures/temporaryDirectory.js"; +import { historicalHar } from "../../../tests/fixtures/historicalHar.js"; +import { decodeHarCapture } from "./HarCapture.js"; +import { readHistoricalCaptureReply } from "./HistoricalCaptureReply.js"; + +const replyFile = async (bytes: Uint8Array) => { + const root = await createTestTempDirectory("rea-capture-reply-utf8-"); + const path = join(root, "reply.json"); + await writeFile(path, bytes); + const resources = new ArtifactResourceScope(); + onTestFinished(() => resources.close()); + return { path, resources }; +}; + +const replyText = (kind: "error" | "output", text: string) => { + if (kind === "error") + return JSON.stringify({ + ok: false, + reason: "format", + message: text, + pointer: "", + }); + const capture = historicalHar(); + capture.log.entries[0]!.response.content.text = text; + return JSON.stringify({ + ok: true, + value: decodeHarCapture(JSON.stringify(capture), []), + }); +}; + +it.each(["error", "output"] as const)( + "rejects malformed bytes inside an otherwise schema-valid %s reply", + async (kind) => { + const text = replyText(kind, "WIRE_MARKER"); + const offset = text.indexOf("WIRE_MARKER"); + expect(offset).toBeGreaterThan(0); + const bytes = Buffer.concat([ + Buffer.from(text.slice(0, offset)), + Buffer.from([0xff]), + Buffer.from(text.slice(offset + "WIRE_MARKER".length)), + ]); + const file = await replyFile(bytes); + await expect( + readHistoricalCaptureReply(file.path, "/capture.har", file.resources), + ).rejects.toMatchObject({ + _tag: "AnalysisOutputError", + reason: expect.stringContaining("Reply is not valid UTF-8."), + cause: { code: "ERR_ENCODING_INVALID_ENCODED_DATA" }, + }); + }, +); + +it.each(["error", "output"] as const)( + "preserves valid replacement-character text and exact byte counts in %s replies", + async (kind) => { + const text = replyText(kind, "valid � 雪"); + const bytes = Buffer.from(text); + const file = await replyFile(bytes); + const result = await readHistoricalCaptureReply( + file.path, + "/capture.har", + file.resources, + ); + expect(result.bytes).toBe(bytes.length); + expect(result.finalization).toBeUndefined(); + expect(result.value).toEqual(JSON.parse(text)); + }, +); + +it.each(["{broken", "\uFEFF{}"])( + "retains malformed JSON handling for %j", + async (text) => { + const file = await replyFile(Buffer.from(text)); + await expect( + readHistoricalCaptureReply(file.path, "/capture.har", file.resources), + ).rejects.toMatchObject({ + _tag: "AnalysisOutputError", + reason: expect.stringContaining("Reply is malformed JSON."), + cause: expect.any(SyntaxError), + }); + }, +); diff --git a/src/browser/history/HistoricalCaptureReply.ts b/src/browser/history/HistoricalCaptureReply.ts index c2607a95dd..f038eb2cfd 100644 --- a/src/browser/history/HistoricalCaptureReply.ts +++ b/src/browser/history/HistoricalCaptureReply.ts @@ -1,4 +1,5 @@ import { z } from "zod"; +import { safeParseUtf8Json } from "../../domain/safeJson.js"; import { ArtifactReaderFailure } from "../../artifacts/ArtifactReader.js"; import type { ArtifactResourceScope } from "../../artifacts/ArtifactResourceScope.js"; import { @@ -57,8 +58,10 @@ export const readHistoricalCaptureReply = async ( finalization = cause; file = completed; } + const decoded = safeParseUtf8Json(file.bytes); + if (!decoded.ok) throw decoded.cause; return { - value: replySchema.parse(JSON.parse(file.bytes.toString("utf8"))), + value: replySchema.parse(decoded.value), bytes: file.bytes.length, finalization, }; @@ -87,6 +90,12 @@ const replyFailureReason = (cause: unknown): string => { if (cause instanceof z.ZodError) return `Reply violates its schema at ${cause.issues[0]?.path.map(String).join(".") ?? "root"}.`; if (cause instanceof SyntaxError) return "Reply is malformed JSON."; + if ( + cause instanceof Error && + "code" in cause && + cause.code === "ERR_ENCODING_INVALID_ENCODED_DATA" + ) + return "Reply is not valid UTF-8."; if (cause instanceof Error && "code" in cause) return `Reply file could not be read (${String(cause.code)}).`; return "Reply could not be validated."; diff --git a/src/domain/analysisView/EnvironmentManifestAccumulator.ts b/src/domain/analysisView/EnvironmentManifestAccumulator.ts index affb846412..9d0becd771 100644 --- a/src/domain/analysisView/EnvironmentManifestAccumulator.ts +++ b/src/domain/analysisView/EnvironmentManifestAccumulator.ts @@ -231,7 +231,6 @@ export class EnvironmentManifestAccumulator extends WebNetworkProjectionAccumula #scenarioTransactions = 0; #scenarioWithoutBody = 0; #scenarioRedactedBodies = 0; - #unrepresentableBodyKey = false; constructor( maximumBytes = WEB_NETWORK_CAPTURE_LIMITS.outputBytes, @@ -335,11 +334,8 @@ export class EnvironmentManifestAccumulator extends WebNetworkProjectionAccumula const root = parseJsonObject(body.text); if (root === null) return ok(null); const entries: [string, JsonTypeName][] = []; - for (const [name, value] of Object.entries(root)) { - // JSON parsers and schema clones drop a __proto__ own property; omit it explicitly. - if (name === "__proto__") this.#unrepresentableBodyKey = true; - else entries.push([name, jsonValueType(value)]); - } + for (const [name, value] of Object.entries(root)) + entries.push([name, jsonValueType(value)]); return ok({ type: "object", properties: Object.fromEntries(entries) }); } @@ -547,11 +543,6 @@ export class EnvironmentManifestAccumulator extends WebNetworkProjectionAccumula "Body shapes list top-level JSON key types from one recorded request each; they do not establish required fields or a complete schema.", ] : []), - ...(this.#unrepresentableBodyKey - ? [ - "A top-level __proto__ key in a recorded JSON body is omitted from body_shape; it cannot be an own property of the published object.", - ] - : []), ...this.#segmentLimitLimitations(toolGroups), ...(this.#scenarioWithoutBody > 0 ? [ diff --git a/src/domain/analysisView/environmentManifestView.test.ts b/src/domain/analysisView/environmentManifestView.test.ts index e6b4ae661b..6a4c5915c3 100644 --- a/src/domain/analysisView/environmentManifestView.test.ts +++ b/src/domain/analysisView/environmentManifestView.test.ts @@ -888,7 +888,7 @@ it("refuses a path whose templated representation exceeds projection capacity", }); }); -it("omits a __proto__ body key explicitly instead of publishing a prototype", () => { +it("preserves every own body key while publishing only its observed type", () => { const projected = manifest( [], syntheticCapture([ @@ -903,12 +903,14 @@ it("omits a __proto__ body key explicitly instead of publishing a prototype", () const shape = projected.tools[0]?.body_shape; expect(shape).toEqual({ type: "object", - properties: { constructor: "string", name: "number" }, + properties: { + ["__proto__"]: "object", + constructor: "string", + name: "number", + }, }); expect(Object.getPrototypeOf(shape?.properties)).toBe(Object.prototype); - expect(projected.limitations).toContain( - "A top-level __proto__ key in a recorded JSON body is omitted from body_shape; it cannot be an own property of the published object.", - ); + expect(Object.hasOwn(shape!.properties, "__proto__")).toBe(true); expect(JSON.stringify(projected)).not.toContain("admin"); }); diff --git a/src/domain/analysisView/environmentManifestView.ts b/src/domain/analysisView/environmentManifestView.ts index 37519362e6..9dcd88f8f4 100644 --- a/src/domain/analysisView/environmentManifestView.ts +++ b/src/domain/analysisView/environmentManifestView.ts @@ -1,4 +1,5 @@ import { z } from "zod"; +import { jsonRecordSchema } from "../jsonRecord.js"; import type { WebNetworkCaptureRecord } from "../webNetworkCapture.js"; import { AnalysisUnsupportedTargetError } from "../analysisErrorCore.js"; import type { AnalysisError } from "../analysisErrorBase.js"; @@ -72,7 +73,7 @@ const toolSchema = z.strictObject({ body_shape: z .strictObject({ type: z.literal("object"), - properties: z.record(z.string(), jsonTypeNameSchema), + properties: jsonRecordSchema(jsonTypeNameSchema), }) .nullable(), statuses: z.array(z.number()), diff --git a/src/domain/jsonRecord.test.ts b/src/domain/jsonRecord.test.ts new file mode 100644 index 0000000000..baa8c2b1f1 --- /dev/null +++ b/src/domain/jsonRecord.test.ts @@ -0,0 +1,116 @@ +import { expect, it } from "vitest"; +import { z } from "zod"; +import { jsonRecordSchema } from "./jsonRecord.js"; +import type { JsonValue } from "./jsonValue.js"; + +it("owns every observed key while retaining normalized values", () => { + const source: unknown = JSON.parse( + '{"__proto__":"observed","constructor":"source","ordinary":"value"}', + ); + const schema = jsonRecordSchema( + z.string().transform((value) => value.toUpperCase()), + ); + const parsed = schema.parse(source); + expect(Object.hasOwn(parsed, "__proto__")).toBe(true); + expect(JSON.stringify(parsed)).toBe( + '{"__proto__":"OBSERVED","constructor":"SOURCE","ordinary":"VALUE"}', + ); + expect(JSON.stringify(source)).toBe( + '{"__proto__":"observed","constructor":"source","ordinary":"value"}', + ); + expect(Object.getPrototypeOf(parsed)).toBe(Object.prototype); +}); + +it("prefixes value failures with every original key and nested value path", () => { + const schema = jsonRecordSchema(z.strictObject({ name: z.string() })); + const source: unknown = JSON.parse( + '{"__proto__":{"name":7},"ordinary":{"name":false}}', + ); + const parsed = schema.safeParse(source); + expect(parsed.success).toBe(false); + if (parsed.success) throw new Error("Expected value validation failure"); + expect(parsed.error.issues.map(({ path }) => path)).toEqual([ + ["__proto__", "name"], + ["ordinary", "name"], + ]); +}); + +it("retains defaults produced within JSON entries without mutating their source", () => { + const source: unknown = JSON.parse( + '{"__proto__":{},"ordinary":{"name":"given"}}', + ); + const schema = jsonRecordSchema( + z.strictObject({ name: z.string().default("default") }), + ); + const parsed = schema.parse(source); + expect(JSON.stringify(parsed)).toBe( + '{"__proto__":{"name":"default"},"ordinary":{"name":"given"}}', + ); + expect(JSON.stringify(source)).toBe( + '{"__proto__":{},"ordinary":{"name":"given"}}', + ); +}); + +it("keeps the canonical JSON input boundary before per-entry defaults", () => { + const schema = jsonRecordSchema(z.string().default("default")); + expect(schema.safeParse({ ordinary: undefined }).success).toBe(false); + expect(schema.safeParse({ ordinary: Infinity }).success).toBe(false); + expect(schema.safeParse([]).success).toBe(false); + expect(schema.parse({})).toEqual({}); +}); + +it.each(["draft-04", "draft-07", "draft-2020-12", "openapi-3.0"] as const)( + "exports the same typed-record schema in every IO mode for %s", + (target) => { + const values: z.ZodType[] = [ + z.string(), + z.enum(["unknown", "object", "array"]), + z.strictObject({ name: z.string().default("default") }), + ]; + for (const value of values) + for (const io of ["input", "output"] as const) { + const actual = z.strictObject({ entries: jsonRecordSchema(value) }); + const expected = z.strictObject({ + entries: z.record(z.string(), value), + }); + expect(z.toJSONSchema(actual, { target, io })).toEqual( + z.toJSONSchema(expected, { target, io }), + ); + } + }, +); + +it("preserves input schema projection for a normalizing transform", () => { + const value = z.string().transform((entry) => entry.toUpperCase()); + expect(z.toJSONSchema(jsonRecordSchema(value), { io: "input" })).toEqual( + z.toJSONSchema(z.record(z.string(), value), { io: "input" }), + ); + expect(() => + z.toJSONSchema(jsonRecordSchema(value), { io: "output" }), + ).toThrow(); +}); + +it.each(["draft-04", "draft-07", "draft-2020-12", "openapi-3.0"] as const)( + "preserves shared schema references through Standard Schema export for %s", + (target) => { + const value = z.strictObject({ name: z.string().default("default") }); + const map = jsonRecordSchema(value); + const record = z.record(z.string(), value); + const actual = z.strictObject({ left: map, right: map, entry: value }); + const expected = z.strictObject({ + left: record, + right: record, + entry: value, + }); + for (const io of ["input", "output"] as const) + for (const reused of ["inline", "ref"] as const) { + expect(z.toJSONSchema(actual, { target, io, reused })).toEqual( + z.toJSONSchema(expected, { target, io, reused }), + ); + const options = { target, libraryOptions: { reused } }; + expect(actual["~standard"].jsonSchema[io](options)).toEqual( + expected["~standard"].jsonSchema[io](options), + ); + } + }, +); diff --git a/src/domain/jsonRecord.ts b/src/domain/jsonRecord.ts new file mode 100644 index 0000000000..ef9f71692f --- /dev/null +++ b/src/domain/jsonRecord.ts @@ -0,0 +1,30 @@ +import { z } from "zod"; +import { jsonObjectSchema, type JsonValue } from "./jsonValue.js"; + +/** Own every JSON map key, then validate and retain each value's normalized facts. */ +export const jsonRecordSchema = ( + valueSchema: z.ZodType, +) => { + const projection = z.record(z.string(), valueSchema); + const schema = jsonObjectSchema.transform( + (value, context): Record => { + const entries: [string, Value][] = []; + let invalid = false; + for (const [key, entry] of Object.entries(value)) { + const parsed = valueSchema.safeParse(entry); + if (parsed.success) entries.push([key, parsed.data]); + else { + invalid = true; + for (const issue of parsed.error.issues) + context.addIssue({ ...issue, path: [key, ...issue.path] }); + } + } + return invalid ? z.NEVER : Object.fromEntries(entries); + }, + ); + // Delegate to Zod's own record projection with the caller's dialect, IO and + // reference context; a context-free JSON override would lose those choices. + schema._zod.processJSONSchema = (context, json, params) => + projection._zod.processJSONSchema!(context, json, params); + return schema; +}; diff --git a/src/ida/IdaConfiguration.test.ts b/src/ida/IdaConfiguration.test.ts index 000d3e3597..46d6af4c61 100644 --- a/src/ida/IdaConfiguration.test.ts +++ b/src/ida/IdaConfiguration.test.ts @@ -1,7 +1,6 @@ import { rm, writeFile } from "node:fs/promises"; import { join } from "node:path"; -import { afterEach, describe, expect, it } from "vitest"; -import { vi } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { createIdaTarget } from "../../tests/fixtures/idaMcp.js"; import { readIdaConfiguration } from "./IdaConfiguration.js"; import { decodeIdaToolResult } from "./IdaMcpConnection.js"; @@ -249,3 +248,54 @@ describe("IDA MCP result decoding", () => { expect(() => decodeIdaToolResult({ content: "invalid" })).toThrow(); }); }); + +it.each(["env", "headers"] as const)( + "preserves all explicit IDA %s keys", + async (field) => { + const { root } = await createIdaTarget(); + roots.push(root); + const path = join(root, "mcp.json"); + const mapping = JSON.parse( + '{"__proto__":"operator","constructor":"selected","normal":"value"}', + ); + const transport = + field === "env" + ? { command: "python" } + : { url: "http://127.0.0.1:8745/mcp" }; + await writeFile(path, JSON.stringify({ ...transport, [field]: mapping })); + const result = await read(path); + expect(result.ok).toBe(true); + if (!result.ok) throw result.error; + const parsed = + field === "env" && "env" in result.value + ? result.value.env + : "headers" in result.value + ? result.value.headers + : undefined; + expect(parsed).toEqual(mapping); + expect(Object.hasOwn(parsed ?? {}, "__proto__")).toBe(true); + }, +); + +it.each(["env", "headers"] as const)( + "rejects malformed explicit IDA %s values at __proto__", + async (field) => { + const { root } = await createIdaTarget(); + roots.push(root); + const path = join(root, "mcp.json"); + const transport = + field === "env" + ? { command: "python" } + : { url: "http://127.0.0.1:8745/mcp" }; + for (const value of [42, null, {}]) { + await writeFile( + path, + JSON.stringify({ + ...transport, + [field]: Object.fromEntries([["__proto__", value]]), + }), + ); + expect((await read(path)).ok).toBe(false); + } + }, +); diff --git a/src/ida/IdaConfiguration.ts b/src/ida/IdaConfiguration.ts index a85956c9bb..b8127fb610 100644 --- a/src/ida/IdaConfiguration.ts +++ b/src/ida/IdaConfiguration.ts @@ -1,5 +1,6 @@ import { isAbsolute } from "node:path"; import { z } from "zod"; +import { jsonRecordSchema } from "../domain/jsonRecord.js"; import { AnalysisCleanupLease } from "../artifacts/AnalysisCleanupLease.js"; import type { RetainedResourceScope } from "../application/AnalysisProvider.js"; import { ConfigurationError } from "../domain/configurationErrors.js"; @@ -48,7 +49,7 @@ const registrationSchema = z type: z.literal("stdio").optional(), command: z.string().min(1), args: z.array(z.string()).default([]), - env: z.record(z.string(), z.string()).default({}), + env: jsonRecordSchema(z.string()).default({}), }) .strict(), z @@ -64,7 +65,7 @@ const registrationSchema = z url.password === "" ); }, "IDA MCP must use a local HTTP endpoint without URL credentials"), - headers: z.record(z.string(), z.string()).default({}), + headers: jsonRecordSchema(z.string()).default({}), }) .strict(), ]) diff --git a/tests/boundary/mcp/analysisViewMcp.test.ts b/tests/boundary/mcp/analysisViewMcp.test.ts index 238ba67955..aeea895809 100644 --- a/tests/boundary/mcp/analysisViewMcp.test.ts +++ b/tests/boundary/mcp/analysisViewMcp.test.ts @@ -1517,7 +1517,14 @@ it("projects an environment manifest identically from retained, portable and fil "inspect_analysis_view", schemaValidation.assertions, ); - const capture = environmentManifestCaptureFixture(); + const har = environmentManifestHar(); + const request = har.log.entries.find( + (entry) => entry.request.postData, + )!.request; + request.postData!.text = + '{"__proto__":{"source":"recorded body value"},"constructor":"caller","name":1}'; + request.bodySize = Buffer.byteLength(request.postData!.text); + const capture = environmentManifestCaptureFixture(har.log.entries); const parent = analysisViewCaptureEvidence(capture); expect(session.recordEvidence(parent).ok).toBe(true); const evidencePath = join( @@ -1558,6 +1565,16 @@ it("projects an environment manifest identically from retained, portable and fil name: "grafana-example", templated_groups: 13, tools: expect.arrayContaining([ + expect.objectContaining({ + body_shape: { + type: "object", + properties: { + ["__proto__"]: "object", + constructor: "string", + name: "number", + }, + }, + }), expect.objectContaining({ name: "get_plugins_settings_by_plugin_id", path_template: "/api/plugins/{plugin_id}/settings", @@ -1569,7 +1586,13 @@ it("projects an environment manifest identically from retained, portable and fil ]), }); expect(session.evidenceById(evidence.evidence_id)).toEqual(evidence); - for (const value of ["probe folder", "Main Org", "JFIF", "console.log"]) + for (const value of [ + "probe folder", + "Main Org", + "JFIF", + "console.log", + "recorded body value", + ]) expect(JSON.stringify(response)).not.toContain(value); outputs.push(evidence.normalized_result); }