From 51e593877642ab42fb5bf25812f4a4110ee7ac26 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sat, 24 Oct 2026 00:11:41 +0800 Subject: [PATCH] fix(pcap): retain UTF-8 decoding failures across tool output Validate supervised text once before version or capture interpretation. Exercise POSIX client probes through compiled CLI and stdio MCP; capture-summary native bounds remain host-specific. Fixes #6775 --- docs/pcap-analysis.md | 9 +- src/pcap/PcapCommand.ts | 5 + src/pcap/PcapProvider.ts | 29 +++ tests/acceptance/pcapOutputUtf8CliMcp.test.ts | 122 ++++++++++++ tests/boundary/pcap/pcapProvider.test.ts | 176 +++++++++--------- tests/fixtures/pcap/pcapTestTools.ts | 118 ++++++++++++ 6 files changed, 365 insertions(+), 94 deletions(-) create mode 100644 tests/acceptance/pcapOutputUtf8CliMcp.test.ts create mode 100644 tests/fixtures/pcap/pcapTestTools.ts diff --git a/docs/pcap-analysis.md b/docs/pcap-analysis.md index da0e607e8b..8618b7bd75 100644 --- a/docs/pcap-analysis.md +++ b/docs/pcap-analysis.md @@ -112,9 +112,12 @@ wall deadlines are 300 s for dissection, 60 s for metadata and 20 s for probes. Native ownership preparation has its own bounded lifecycle. Malformed/oversized output returns no partial success. Unvalidated partial JSON -is not returned as facts. Cancellation reaches probes, acquisition, commands and -projection. Cleanup failure returns a typed error, retains completed Evidence -when available, and retains owned resources for retry on close. +is not returned as facts. Invalid UTF-8 in either `capinfos` or TShark stdout or +stderr is an unreadable-output error; replacement-decoded text is not parsed as +producer evidence. A literal U+FFFD character in valid UTF-8 remains text. +Cancellation reaches probes, acquisition, commands and projection. Cleanup +failure returns a typed error, retains completed Evidence when available, and +retains owned resources for retry on close. ## Verification scope diff --git a/src/pcap/PcapCommand.ts b/src/pcap/PcapCommand.ts index 8e16648e5b..c7e3206ac0 100644 --- a/src/pcap/PcapCommand.ts +++ b/src/pcap/PcapCommand.ts @@ -39,6 +39,7 @@ export interface PcapCommandCleanup { export interface PcapCommandOutput { readonly stdout: string; readonly stderr: string; + readonly invalidUtf8Streams: readonly ("stdout" | "stderr")[]; readonly cleanup?: PcapCommandCleanup; } @@ -139,6 +140,10 @@ export const runPcapCommand = async (options: { return { stdout: output.stdout.text, stderr: output.stderr.text, + invalidUtf8Streams: [ + ...(output.stdout.invalidUtf8 === true ? (["stdout"] as const) : []), + ...(output.stderr.invalidUtf8 === true ? (["stderr"] as const) : []), + ], ...(cleanup === undefined ? {} : { cleanup }), }; } catch (cause) { diff --git a/src/pcap/PcapProvider.ts b/src/pcap/PcapProvider.ts index 04baa195ea..e1b67a476c 100644 --- a/src/pcap/PcapProvider.ts +++ b/src/pcap/PcapProvider.ts @@ -14,6 +14,7 @@ import { import { AnalysisCancelledError, AnalysisCapabilityUnavailableError, + AnalysisOutputError, AnalysisProtocolError, } from "../domain/analysisErrorCore.js"; import { AnalysisError } from "../domain/analysisErrorBase.js"; @@ -154,6 +155,29 @@ const pcapCommandCleanupError = ( }, ); +const assertValidPcapCommandText = ( + operation: PcapOperation, + stage: "version" | "capinfos" | "tshark", + output: Awaited>, +): void => { + if (output.invalidUtf8Streams.length === 0) return; + throw new AnalysisOutputError( + operation, + `${stage} output is not valid UTF-8; retained text contains replacement characters.`, + { + capturedOutput: { + stdout: output.stdout, + stderr: output.stderr, + truncated: output.cleanup?.capturedOutput.truncated ?? false, + }, + diagnostics: { + stage, + invalid_utf8_streams: [...output.invalidUtf8Streams], + }, + }, + ); +}; + export interface PcapProviderOptions { readonly environment?: Readonly>; readonly launcher?: PcapCommandLauncher; @@ -213,6 +237,7 @@ export class PcapProvider { private async run( operation: PcapOperation, + stage: "version" | "capinfos" | "tshark", command: string, args: readonly string[], limits: PcapCommandLimits, @@ -290,6 +315,7 @@ export class PcapProvider { }, ); } + assertValidPcapCommandText(operation, stage, output); } catch (cause) { if ( retainedCommand !== undefined && @@ -382,6 +408,7 @@ export class PcapProvider { ( await this.run( operation, + "version", command, ["--version"], { timeoutMs: 20_000, maxOutputBytes: 64 * 1024 }, @@ -888,6 +915,7 @@ export class PcapProvider { ) { const info = await this.run( "inspect_pcap_capture", + "capinfos", selection.capinfos.command, ["-M", "-S", "--", snapshot.path], { timeoutMs: 60_000, maxOutputBytes: 64 * 1024 }, @@ -930,6 +958,7 @@ export class PcapProvider { ); const dissected = await this.run( "inspect_pcap_capture", + "tshark", selection.tshark.command, [ "-n", diff --git a/tests/acceptance/pcapOutputUtf8CliMcp.test.ts b/tests/acceptance/pcapOutputUtf8CliMcp.test.ts new file mode 100644 index 0000000000..65b3e91500 --- /dev/null +++ b/tests/acceptance/pcapOutputUtf8CliMcp.test.ts @@ -0,0 +1,122 @@ +import { expect } from "vitest"; + +import { toolContract } from "../../src/contracts/toolContracts.js"; +import { parseEvidence } from "../../src/domain/evidence.js"; +import { pcapResultSchemas } from "../../src/domain/pcap/pcapAnalysis.js"; +import { connectLocalToolsMcp } from "../fixtures/localToolsMcp.js"; +import { createTestTempDirectory } from "../fixtures/temporaryDirectory.js"; +import { + writeStubTools, + type PcapStubToolsOptions, +} from "../fixtures/pcap/pcapTestTools.js"; +import { cliTest } from "../support/cli/cliFixture.js"; + +const invalidVersionCases = [ + { + tool: "tshark", + stream: "stdout", + options: { invalidTsharkVersionStream: "stdout" }, + }, + { + tool: "tshark", + stream: "stderr", + options: { invalidTsharkVersionStream: "stderr" }, + }, + { + tool: "capinfos", + stream: "stdout", + options: { invalidCapinfosVersionStream: "stdout" }, + }, + { + tool: "capinfos", + stream: "stderr", + options: { invalidCapinfosVersionStream: "stderr" }, + }, +] as const; + +const environmentFor = ( + root: string, + tools: { readonly tshark: string; readonly capinfos: string }, +) => ({ + HOME: root, + USERPROFILE: root, + XDG_CONFIG_HOME: root, + XDG_CACHE_HOME: root, + REA_LOG_LEVEL: "silent", + REA_TSHARK_COMMAND: tools.tshark, + REA_CAPINFOS_COMMAND: tools.capinfos, +}); + +cliTest.skipIf(process.platform === "win32")( + "returns the same unreadable-output diagnostic for invalid version streams through CLI and stdio MCP", + async ({ cli }) => { + for (const scenario of invalidVersionCases) { + const root = await createTestTempDirectory("rea-pcap-version-utf8-"); + const tools = await writeStubTools( + root, + scenario.options as PcapStubToolsOptions, + ); + const environment = environmentFor(root, tools); + const cliResponse = await cli.run({ + arguments: ["inspect-pcap-client", "--json"], + environment, + }); + expect(cliResponse.exitCode, cliResponse.stdout).toBe(1); + expect(cliResponse.json).toMatchObject({ + code: "unreadable_output", + details: { + diagnostics: { + stage: "version", + invalid_utf8_streams: [scenario.stream], + }, + }, + }); + + const { client, call } = await connectLocalToolsMcp(environment); + try { + const reply = await call("inspect_pcap_client", {}); + expect(reply.isError).toBe(true); + const content = reply.content.find((item) => item.type === "text"); + if (content?.type !== "text") throw new Error("Missing MCP error text"); + expect(JSON.parse(content.text)).toEqual({ error: cliResponse.json }); + } finally { + await client.close(); + } + } + }, +); + +cliTest.skipIf(process.platform === "win32")( + "preserves literal replacement characters in version output through CLI and stdio MCP", + async ({ cli }) => { + const root = await createTestTempDirectory("rea-pcap-version-literal-"); + const tools = await writeStubTools(root, { literalTsharkVersion: true }); + const environment = environmentFor(root, tools); + const outputSchema = toolContract("inspect_pcap_client").outputSchema; + const cliResponse = await cli.run({ + arguments: ["inspect-pcap-client", "--json"], + environment, + }); + expect(cliResponse.exitCode, cliResponse.stdout).toBe(0); + const cliEvidence = parseEvidence(outputSchema.parse(cliResponse.json)); + const cliClient = pcapResultSchemas.inspect_pcap_client.parse( + cliEvidence.normalized_result, + ).client; + + const { client, call } = await connectLocalToolsMcp(environment); + try { + const reply = await call("inspect_pcap_client", {}); + expect(reply.isError).not.toBe(true); + const mcpEvidence = parseEvidence( + outputSchema.parse(reply.structuredContent), + ); + const mcpClient = pcapResultSchemas.inspect_pcap_client.parse( + mcpEvidence.normalized_result, + ).client; + expect(cliClient).toEqual(mcpClient); + expect(cliClient.tshark_version).toBe("TShark (Wireshark) 4.6.6�-stub"); + } finally { + await client.close(); + } + }, +); diff --git a/tests/boundary/pcap/pcapProvider.test.ts b/tests/boundary/pcap/pcapProvider.test.ts index 582579e0d4..7802046793 100644 --- a/tests/boundary/pcap/pcapProvider.test.ts +++ b/tests/boundary/pcap/pcapProvider.test.ts @@ -1,6 +1,6 @@ import { createHash } from "node:crypto"; import { watch } from "node:fs"; -import { chmod, readFile, writeFile } from "node:fs/promises"; +import { readFile, writeFile } from "node:fs/promises"; import { dirname, join } from "node:path"; import { expect, onTestFinished, test, vi } from "vitest"; @@ -20,100 +20,18 @@ import { PrivateRuntimeRoot } from "../../../src/process/PrivateRuntimeRoot.js"; import { cleanupOwnedProcessGroup } from "../../../src/process/ProcessOwnership.js"; import { spawnOwnedProviderProcess } from "../../../src/process/ProviderProcess.js"; import type { PcapCommandLauncher } from "../../../src/pcap/PcapCommand.js"; +import { + CAPINFOS_OUTPUT, + TSHARK_JSON, + writeStubTools, + type PcapStubToolsOptions, +} from "../../fixtures/pcap/pcapTestTools.js"; const it = test.skipIf(process.platform !== "linux"); - -const CAPINFOS_OUTPUT = [ - "File name: /work/capture.pcap", - "File type: Wireshark/tcpdump/... - pcap", - "File encapsulation: Ethernet", - "Packet size limit: file hdr: 262144 bytes", - "Number of packets: 3", - "Capture duration: 0.123 seconds", - "Earliest packet time: 1791728020.608505", - "Latest packet time: 1791728020.981978", -].join("\n"); - -const TSHARK_JSON = JSON.stringify([ - { - _source: { - layers: { - "frame.number": ["1"], - "frame.protocols": ["eth:ethertype:ip:udp:dns"], - "ip.src": ["192.168.1.100"], - "ip.dst": ["8.8.8.8"], - "dns.qry.name": ["api.example.com"], - "dns.flags.response": ["1"], - "dns.a": ["93.184.216.34"], - }, - }, - }, - { - _source: { - layers: { - "frame.number": ["2"], - "frame.protocols": ["eth:ethertype:ip:tcp:tls"], - "ip.src": ["192.168.1.100"], - "ip.dst": ["93.184.216.34"], - "tls.handshake.extensions_server_name": ["api.example.com"], - }, - }, - }, - { - _source: { - layers: { - "frame.number": ["3"], - "frame.protocols": ["eth:ethertype:ip:tcp:http"], - "ip.src": ["192.168.1.100"], - "ip.dst": ["93.184.216.34"], - "http.request.full_uri": ["http://api.example.com/v1/data"], - }, - }, - }, -]); - -const writeStubTools = async ( - root: string, - options: { - readonly failCapinfos?: boolean; - readonly httpAbsoluteUriOnly?: boolean; - } = {}, -): Promise<{ tshark: string; capinfos: string }> => { - const tshark = join(root, "tshark"); - const capinfos = join(root, "capinfos"); - await writeFile( - tshark, - `#!/bin/sh -if [ "$1" = "--version" ]; then printf 'TShark (Wireshark) 4.6.6-stub\\n'; exit 0; fi -[ "$1" = "-n" ] && [ "$2" = "-r" ] && [ "$4" = "-T" ] && [ "$5" = "json" ] || exit 2 -case " $* " in *" -e http.request.uri "*) ;; *) exit 2 ;; esac -printf '%s' '${(options.httpAbsoluteUriOnly ? TSHARK_JSON.replace('"http.request.full_uri":["http://api.example.com/v1/data"]', '"http.request.uri":["http://proxy.example.test/path"]') : TSHARK_JSON).replace(/'/gu, "'\\''")}' -exit 0 -`, - { mode: 0o755 }, - ); - await writeFile( - capinfos, - `#!/bin/sh -if [ "$1" = "--version" ]; then printf 'Capinfos (Wireshark) 4.6.6-stub\\n'; exit 0; fi -[ "$1" = "-M" ] && [ "$2" = "-S" ] && [ "$3" = "--" ] && [ "$#" = "4" ] || exit 2 -if [ "${String(options.failCapinfos === true)}" = "true" ]; then - printf 'capinfos: The file is not a capture file' >&2; exit 1 -fi -printf '%s\\n' '${CAPINFOS_OUTPUT.replace(/'/gu, "'\\''")}' -exit 0 -`, - { mode: 0o755 }, - ); - await chmod(tshark, 0o755); - await chmod(capinfos, 0o755); - return { tshark, capinfos }; -}; +const posixIt = test.skipIf(process.platform === "win32"); const providerWith = async ( - options: { - failCapinfos?: boolean; - httpAbsoluteUriOnly?: boolean; + options: PcapStubToolsOptions & { launcher?: PcapCommandLauncher; beforeClose?: () => void; } = {}, @@ -247,6 +165,82 @@ it("retains an absolute HTTP request target without a Host-derived full URI", as expect(result.coverage).toBe("complete"); }); +it.each(["stdout", "stderr"] as const)( + "refuses replacement-decoded TShark %s and names its invalid stream", + async (stream) => { + const { provider, root } = await providerWith({ + invalidTsharkStream: stream, + }); + const path = await captureFixture(root); + const result = await provider.execute({ + operation: "inspect_pcap_capture", + input: { path }, + }); + expect(result.ok).toBe(false); + if (result.ok) throw new Error("Expected malformed producer encoding"); + expect(projectAnalysisError(result.error)).toMatchObject({ + code: "unreadable_output", + details: { + diagnostics: { stage: "tshark", invalid_utf8_streams: [stream] }, + }, + }); + }, +); + +it("keeps a literal replacement character in valid TShark UTF-8", async () => { + const { provider, root } = await providerWith({ literalReplacement: true }); + const path = await captureFixture(root); + const result = await evidenceResult(provider, { + operation: "inspect_pcap_capture", + input: { path }, + }); + expect(result.http_request_uris).toEqual(["http://api.example.com/�"]); + expect(result.coverage).toBe("complete"); +}); + +posixIt.each(["stdout", "stderr"] as const)( + "refuses replacement-decoded TShark version %s before banner parsing", + async (stream) => { + const { provider } = await providerWith({ + invalidTsharkVersionStream: stream, + }); + const result = await provider.execute({ + operation: "inspect_pcap_client", + input: {}, + }); + expect(result.ok).toBe(false); + if (result.ok) throw new Error("Expected malformed version output"); + expect(projectAnalysisError(result.error)).toMatchObject({ + code: "unreadable_output", + details: { + diagnostics: { stage: "version", invalid_utf8_streams: [stream] }, + }, + }); + }, +); + +it.each(["stdout", "stderr"] as const)( + "refuses replacement-decoded capinfos %s before metadata parsing", + async (stream) => { + const { provider, root } = await providerWith({ + invalidCapinfosStream: stream, + }); + const path = await captureFixture(root); + const result = await provider.execute({ + operation: "inspect_pcap_capture", + input: { path }, + }); + expect(result.ok).toBe(false); + if (result.ok) throw new Error("Expected malformed producer encoding"); + expect(projectAnalysisError(result.error)).toMatchObject({ + code: "unreadable_output", + details: { + diagnostics: { stage: "capinfos", invalid_utf8_streams: [stream] }, + }, + }); + }, +); + it.each([ { stage: "capinfos", denySnapshotClose: false }, { stage: "tshark", denySnapshotClose: false }, diff --git a/tests/fixtures/pcap/pcapTestTools.ts b/tests/fixtures/pcap/pcapTestTools.ts new file mode 100644 index 0000000000..025164cb82 --- /dev/null +++ b/tests/fixtures/pcap/pcapTestTools.ts @@ -0,0 +1,118 @@ +import { chmod, writeFile } from "node:fs/promises"; +import { join } from "node:path"; + +export const CAPINFOS_OUTPUT = [ + "File name: /work/capture.pcap", + "File type: Wireshark/tcpdump/... - pcap", + "File encapsulation: Ethernet", + "Packet size limit: file hdr: 262144 bytes", + "Number of packets: 3", + "Capture duration: 0.123 seconds", + "Earliest packet time: 1791728020.608505", + "Latest packet time: 1791728020.981978", +].join("\n"); + +export const TSHARK_JSON = JSON.stringify([ + { + _source: { + layers: { + "frame.number": ["1"], + "frame.protocols": ["eth:ethertype:ip:udp:dns"], + "ip.src": ["192.168.1.100"], + "ip.dst": ["8.8.8.8"], + "dns.qry.name": ["api.example.com"], + "dns.flags.response": ["1"], + "dns.a": ["93.184.216.34"], + }, + }, + }, + { + _source: { + layers: { + "frame.number": ["2"], + "frame.protocols": ["eth:ethertype:ip:tcp:tls"], + "ip.src": ["192.168.1.100"], + "ip.dst": ["93.184.216.34"], + "tls.handshake.extensions_server_name": ["api.example.com"], + }, + }, + }, + { + _source: { + layers: { + "frame.number": ["3"], + "frame.protocols": ["eth:ethertype:ip:tcp:http"], + "ip.src": ["192.168.1.100"], + "ip.dst": ["93.184.216.34"], + "http.request.full_uri": ["http://api.example.com/v1/data"], + }, + }, + }, +]); + +export interface PcapStubToolsOptions { + readonly failCapinfos?: boolean; + readonly httpAbsoluteUriOnly?: boolean; + readonly invalidTsharkStream?: "stdout" | "stderr"; + readonly invalidTsharkVersionStream?: "stdout" | "stderr"; + readonly invalidCapinfosStream?: "stdout" | "stderr"; + readonly invalidCapinfosVersionStream?: "stdout" | "stderr"; + readonly literalReplacement?: boolean; + readonly literalTsharkVersion?: boolean; +} + +/** Write caller-selected POSIX tool stubs for pcap provider and adapter tests. */ +export const writeStubTools = async ( + root: string, + options: PcapStubToolsOptions = {}, +): Promise<{ tshark: string; capinfos: string }> => { + const tshark = join(root, "tshark"); + const capinfos = join(root, "capinfos"); + const tsharkOutput = options.literalReplacement + ? TSHARK_JSON.replace( + "http://api.example.com/v1/data", + "http://api.example.com/�", + ) + : options.httpAbsoluteUriOnly + ? TSHARK_JSON.replace( + '"http.request.full_uri":["http://api.example.com/v1/data"]', + '"http.request.uri":["http://proxy.example.test/path"]', + ) + : TSHARK_JSON; + await writeFile( + tshark, + `#!/bin/sh +if [ "$1" = "--version" ]; then + printf 'TShark (Wireshark) 4.6.6${options.literalTsharkVersion ? "�" : ""}-stub\\n' + ${options.invalidTsharkVersionStream === "stdout" ? `printf '\\377'` : options.invalidTsharkVersionStream === "stderr" ? `printf '\\377' >&2` : ""} + exit 0 +fi +[ "$1" = "-n" ] && [ "$2" = "-r" ] && [ "$4" = "-T" ] && [ "$5" = "json" ] || exit 2 +case " $* " in *" -e http.request.uri "*) ;; *) exit 2 ;; esac +${options.invalidTsharkStream === "stdout" ? `printf '%s' '[{"_source":{"layers":{"frame.number":["1"],"frame.protocols":["eth:ip:tcp:http"],"http.request.full_uri":["http://example.test/'; printf '\\377'; printf '%s' '"]}}}]'` : `printf '%s' '${tsharkOutput.replace(/'/gu, "'\\''")}'`} +${options.invalidTsharkStream === "stderr" ? `printf '\\377' >&2` : ""} +exit 0 +`, + { mode: 0o755 }, + ); + await writeFile( + capinfos, + `#!/bin/sh +if [ "$1" = "--version" ]; then + printf 'Capinfos (Wireshark) 4.6.6-stub\\n' + ${options.invalidCapinfosVersionStream === "stdout" ? `printf '\\377'` : options.invalidCapinfosVersionStream === "stderr" ? `printf '\\377' >&2` : ""} + exit 0 +fi +[ "$1" = "-M" ] && [ "$2" = "-S" ] && [ "$3" = "--" ] && [ "$#" = "4" ] || exit 2 +if [ "${String(options.failCapinfos === true)}" = "true" ]; then + printf 'capinfos: The file is not a capture file' >&2; exit 1 +fi +${options.invalidCapinfosStream === "stdout" ? `printf '\\377'` : options.invalidCapinfosStream === "stderr" ? `printf '\\377' >&2` : `printf '%s\\n' '${CAPINFOS_OUTPUT.replace(/'/gu, "'\\''")}'`} +exit 0 +`, + { mode: 0o755 }, + ); + await chmod(tshark, 0o755); + await chmod(capinfos, 0o755); + return { tshark, capinfos }; +};