From a8fc3307cd2f21f4202def78bd2783216c5f7f6a Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sat, 24 Oct 2026 04:57:57 +0800 Subject: [PATCH] fix(protobuf): own descriptor file identity before registry admission Coalesce equal parsed file definitions and reject conflicting same-name definitions before registry import resolution. Preserve field presence, known and opaque facts through one budgeted SDK parse while retaining original descriptor bytes and digest. Keep unique files in their original order, reuse the existing admission traversal, and cover public CLI/MCP import, ordering, presence and source-provenance controls. Document REA ambiguity policy separately from protoc precedence. --- docs/web-network-captures.md | 9 + .../history/protobuf/ProtobufSchema.ts | 27 +- .../tools/protobufDescriptorFiles.test.ts | 298 ++++++++++++++++++ 3 files changed, 330 insertions(+), 4 deletions(-) create mode 100644 tests/acceptance/tools/protobufDescriptorFiles.test.ts diff --git a/docs/web-network-captures.md b/docs/web-network-captures.md index 6a1621c864..dc1d119914 100644 --- a/docs/web-network-captures.md +++ b/docs/web-network-captures.md @@ -174,6 +174,15 @@ service and extension names must also identify one declaration across the descriptor set, including nested and synthetic map-entry messages. Different packages or enclosing messages may reuse a local type name. Schema-admission failures retain the partial capture Evidence and its original artifact identity. +REA also requires an unambiguous descriptor file identity. Repeated exact file +names are coalesced only when protobuf-es equality confirms matching field +presence, declared values and retained opaque unknown/custom-option fields. +Opaque fields must match in their retained order and bytes; REA does not +interpret them. Conflicting definitions are refused rather than selecting a +file by its position. This is REA's supplied-schema admission policy, separate +from protobuf wire validity and the [first-occurrence precedence in protoc v32.1](https://github.com/protocolbuffers/protobuf/blob/v32.1/src/google/protobuf/compiler/command_line_interface.cc#L1402). +The descriptor-set byte count and SHA-256 continue to identify the complete +original input, including repeated file facts. Map semantics, extensions and required-field validation are unsupported. A supplied schema does not establish the sender's actual schema. An invalid nested interpretation records `schema_status: "invalid-message"` diff --git a/src/browser/history/protobuf/ProtobufSchema.ts b/src/browser/history/protobuf/ProtobufSchema.ts index 0c4ec98733..234e6f65f8 100644 --- a/src/browser/history/protobuf/ProtobufSchema.ts +++ b/src/browser/history/protobuf/ProtobufSchema.ts @@ -1,5 +1,7 @@ import { createFileRegistry, + createRegistry, + equals, fromBinary, ScalarType, type DescField, @@ -8,6 +10,8 @@ import { } from "@bufbuild/protobuf"; import { FileDescriptorSetSchema, + FileDescriptorProtoSchema, + type FileDescriptorProto, type FileDescriptorSet, } from "@bufbuild/protobuf/wkt"; import { BinaryReader } from "@bufbuild/protobuf/wire"; @@ -39,11 +43,15 @@ const scalarWire = (scalar: ScalarType): ProtobufField["wire_type"] => { const floatingValue = (value: number): string => Object.is(value, -0) ? "-0" : String(value); +// Descriptor custom options and future fields remain opaque identity facts. +const descriptorEquality = { registry: createRegistry(), unknown: true }; + /** Admit descriptor identities and bound qualified names before registry construction. */ const admitRegistryDescriptor = async ( descriptor: FileDescriptorSet, budget: ProtobufBudget, -): Promise => { +): Promise => { + const files = new Map(); const registryNames = new Set(); const chargeQualifiedName = (name: string, prefix: string) => budget.charge((prefix.length + 1 + name.length) * 8 + 128); @@ -59,7 +67,17 @@ const admitRegistryDescriptor = async ( return qualified; }; for (const file of descriptor.file) { + await budget.checkpoint(); budget.charge(file.name.length * 8 + 128); + const previous = files.get(file.name); + if (previous !== undefined) { + if ( + !equals(FileDescriptorProtoSchema, previous, file, descriptorEquality) + ) + throw new Error("Conflicting file identity in FileDescriptorSet"); + continue; + } + files.set(file.name, file); const stack = file.messageType.map((message) => ({ message, prefix: file.package, @@ -95,6 +113,7 @@ const admitRegistryDescriptor = async ( stack.push({ message: nested, prefix: qualified }); } } + return { ...descriptor, file: [...files.values()] }; }; const scalarValue = (reader: BinaryReader, scalar: ScalarType): string => { @@ -166,11 +185,11 @@ export class ProtobufSchema implements WireSchema { ) throw new Error("Malformed FileDescriptorSet wire format"); const descriptor = fromBinary(FileDescriptorSetSchema, bytes, { - readUnknownFields: false, + readUnknownFields: true, recursionLimit: WEB_NETWORK_CAPTURE_LIMITS.depth, }); - await admitRegistryDescriptor(descriptor, budget); - const message = createFileRegistry(descriptor).getMessage( + const admitted = await admitRegistryDescriptor(descriptor, budget); + const message = createFileRegistry(admitted).getMessage( typeName.replace(/^\./, ""), ); if (message === undefined) diff --git a/tests/acceptance/tools/protobufDescriptorFiles.test.ts b/tests/acceptance/tools/protobufDescriptorFiles.test.ts new file mode 100644 index 0000000000..adf1669cb5 --- /dev/null +++ b/tests/acceptance/tools/protobufDescriptorFiles.test.ts @@ -0,0 +1,298 @@ +import { create, toBinary } from "@bufbuild/protobuf"; +import { + DescriptorProtoSchema, + FieldDescriptorProtoSchema, + FileDescriptorProtoSchema, + FileDescriptorSetSchema, + FileOptionsSchema, +} from "@bufbuild/protobuf/wkt"; +import { BinaryWriter, WireType } from "@bufbuild/protobuf/wire"; +import { createHash } from "node:crypto"; +import { readFile, writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { expect } from "vitest"; +import { z } from "zod"; + +import { analysisViewResultSchema } from "../../../src/domain/analysisView/analysisView.js"; +import { parseEvidence } from "../../../src/domain/evidence.js"; +import { webNetworkCaptureSchema } from "../../../src/domain/webNetworkCapture.js"; +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; +import { protobufHar } from "../../fixtures/protobufCapture.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { cliTest } from "../../support/cli/cliFixture.js"; + +const sha256 = (bytes: Uint8Array) => + createHash("sha256").update(bytes).digest("hex"); +const body = Buffer.from("0a020801", "hex"); // Message { payload { value=1 } } +const descriptorDefinition = () => + create(FileDescriptorSetSchema, { + file: [ + { + name: "shared.proto", + package: "capture", + syntax: "proto3", + messageType: [ + { + name: "Payload", + field: [{ name: "value", number: 1, type: 5, label: 1 }], + }, + ], + }, + { + name: "entry.proto", + package: "entry", + syntax: "proto3", + dependency: ["shared.proto"], + messageType: [ + { + name: "Message", + field: [ + { + name: "payload", + number: 1, + type: 11, + typeName: ".capture.Payload", + label: 1, + }, + ], + }, + ], + }, + ], + }); + +const conflictingFiles = (change: string, reverse: boolean) => { + const definition = descriptorDefinition(); + const shared = definition.file[0]!; + const duplicate = structuredClone(shared); + let captured = body; + if (change === "declarations") duplicate.messageType[0]!.name = "Other"; + else if (change === "package") duplicate.package = "other"; + else if (change === "imports") { + definition.file.unshift( + create(FileDescriptorProtoSchema, { name: "a.proto", syntax: "proto3" }), + create(FileDescriptorProtoSchema, { name: "b.proto", syntax: "proto3" }), + ); + shared.messageType = []; + duplicate.messageType = []; + shared.dependency = ["a.proto"]; + duplicate.dependency = ["b.proto"]; + definition.file.at(-1)!.messageType[0]!.field[0] = create( + FieldDescriptorProtoSchema, + { name: "value", number: 1, type: 5, label: 1 }, + ); + captured = Buffer.from([8, 1]); + } else if (change === "presence") { + shared.options = create(FileOptionsSchema); + duplicate.options = create(FileOptionsSchema, { javaMultipleFiles: false }); + } else if (change === "opaque-file") { + duplicate.$unknown = [ + { no: 50000, wireType: WireType.Varint, data: Uint8Array.of(1) }, + ]; + } else { + shared.options = create(FileOptionsSchema); + duplicate.options = create(FileOptionsSchema); + shared.options.$unknown = [ + { no: 50000, wireType: WireType.Varint, data: Uint8Array.of(1) }, + ]; + duplicate.options.$unknown = [ + { no: 50000, wireType: WireType.Varint, data: Uint8Array.of(2) }, + ]; + } + const sharedIndex = definition.file.indexOf(shared); + definition.file.splice( + sharedIndex, + 1, + ...(reverse ? [duplicate, shared] : [shared, duplicate]), + ); + return { bytes: toBinary(FileDescriptorSetSchema, definition), captured }; +}; + +const retainedDetailsSchema = z.object({ + partial_observation: z.strictObject({ + kind: z.literal("retained-evidence"), + evidence_id: z.string(), + }), +}); + +cliTest( + "refuses conflicting descriptor file identities before registry import lookup through CLI/MCP", + async ({ cli }) => { + const root = await createTestTempDirectory("rea-protobuf-file-conflicts-"); + const call = await cli.sharedMcpCall!(); + for (const change of [ + "declarations", + "package", + "imports", + "presence", + "opaque-file", + "opaque-option", + ]) + for (const reverse of [false, true]) { + const { bytes, captured } = conflictingFiles(change, reverse); + const descriptor = join(root, `${change}-${reverse}.pb`); + const capture = join(root, `${change}-${reverse}.har`); + const text = protobufHar(captured, "application/x-protobuf"); + await writeFile(descriptor, bytes); + await writeFile(capture, text); + const expected = { + code: "invalid_request", + details: { + issues: [ + { path: ["protobuf", "schema"], reason: "invalid_format" }, + ], + }, + }; + if (change === "declarations") { + const output = await cli.sharedCliRun!({ + arguments: [ + "inspect-web-network-capture", + capture, + "har", + "--record", + "0", + "--protobuf-schema", + descriptor, + "--protobuf-message", + "entry.Message", + "--json", + ], + }); + expect(output.exitCode).toBe(1); + expect(output.json).toMatchObject(expected); + } + const response = await call("inspect_web_network_capture", { + capture_path: capture, + format: "har", + record_ordinals: [0], + protobuf: { + schema: { + descriptor_set_path: descriptor, + message_type: "entry.Message", + }, + }, + }); + const error = parseMcpToolError(response).error; + expect(error, `${change}-${reverse}`).toMatchObject(expected); + if (change === "declarations") { + const source = retainedDetailsSchema.parse( + error.details, + ).partial_observation; + const retained = await call("inspect_analysis_view", { + source, + view: { kind: "summary" }, + }); + expect(retained.isError, JSON.stringify(retained)).not.toBe(true); + expect( + analysisViewResultSchema.parse( + parseEvidence(retained.structuredContent).normalized_result, + ), + ).toMatchObject({ + summary: { + artifact: { path: capture, sha256: sha256(Buffer.from(text)) }, + retained_records: 1, + protobuf_coverage: { status: "partial", inspected_records: 0 }, + }, + }); + } + expect(await readFile(descriptor)).toEqual(Buffer.from(bytes)); + expect(await readFile(capture, "utf8")).toBe(text); + } + }, +); + +/** Reorder known FileDescriptorProto fields without changing their facts. */ +const reorderedSharedFile = () => { + const shared = descriptorDefinition().file[0]!; + return new BinaryWriter() + .tag(12, WireType.LengthDelimited) + .string(shared.syntax) + .tag(4, WireType.LengthDelimited) + .bytes(toBinary(DescriptorProtoSchema, shared.messageType[0]!)) + .tag(2, WireType.LengthDelimited) + .string(shared.package) + .tag(1, WireType.LengthDelimited) + .string(shared.name) + .finish(); +}; + +cliTest( + "coalesces equal descriptor file facts while preserving original descriptor identity through CLI/MCP", + async ({ cli }) => { + const root = await createTestTempDirectory("rea-protobuf-file-repeat-"); + const capture = join(root, "capture.har"); + await writeFile(capture, protobufHar(body, "application/x-protobuf")); + const call = await cli.sharedMcpCall!(); + for (const mode of ["identical", "reordered", "opaque"]) { + const definition = descriptorDefinition(); + const shared = definition.file[0]!; + if (mode === "opaque") { + shared.options = create(FileOptionsSchema); + shared.options.$unknown = [ + { no: 50000, wireType: WireType.Varint, data: Uint8Array.of(7) }, + ]; + } + const duplicate = + mode === "reordered" + ? reorderedSharedFile() + : toBinary(FileDescriptorProtoSchema, shared); + const writer = new BinaryWriter(); + writer + .tag(1, WireType.LengthDelimited) + .bytes(toBinary(FileDescriptorProtoSchema, shared)); + writer.tag(1, WireType.LengthDelimited).bytes(duplicate); + writer + .tag(1, WireType.LengthDelimited) + .bytes(toBinary(FileDescriptorProtoSchema, definition.file[1]!)); + // Repeated imported facts after their consumer must coalesce as well. + writer.tag(1, WireType.LengthDelimited).bytes(duplicate); + const bytes = writer.finish(); + const descriptor = join(root, `${mode}.pb`); + await writeFile(descriptor, bytes); + const output = await cli.sharedCliRun!({ + arguments: [ + "inspect-web-network-capture", + capture, + "har", + "--record", + "0", + "--protobuf-schema", + descriptor, + "--protobuf-message", + "entry.Message", + "--json", + ], + }); + expect(output.exitCode, output.stdout + output.stderr).toBe(0); + const evidence = parseEvidence(output.json); + const report = webNetworkCaptureSchema.parse(evidence.normalized_result); + expect(report.protobuf_coverage?.status).toBe("complete"); + const decoded = report.records[0]?.protobuf?.[0]; + expect(decoded?.schema).toMatchObject({ + descriptor_set_path: descriptor, + descriptor_set_sha256: sha256(bytes), + descriptor_set_bytes: bytes.length, + }); + expect(decoded?.fields).toMatchObject([ + { number: 1, schema_name: "payload", schema_type: "capture.Payload" }, + { number: 1, schema_name: "value", schema_value: "1" }, + ]); + const response = await call("inspect_web_network_capture", { + capture_path: capture, + format: "har", + record_ordinals: [0], + protobuf: { + schema: { + descriptor_set_path: descriptor, + message_type: "entry.Message", + }, + }, + }); + expect(response.isError, JSON.stringify(response)).not.toBe(true); + expect(parseEvidence(response.structuredContent).evidence_id).toBe( + evidence.evidence_id, + ); + expect(await readFile(descriptor)).toEqual(Buffer.from(bytes)); + } + }, +);