From 8f9af920efb39d22b5a915605403b67a1005bc0e Mon Sep 17 00:00:00 2001 From: gensokyo-dev <342580529+gensokyo-dev@users.noreply.github.com> Date: Thu, 29 Oct 2026 02:14:22 +0800 Subject: [PATCH] fix(pcap): redact HTTP credentials in failed capture diagnostics (#7871) Project bounded JSON diagnostics using shared URI roles and source spans. Keep complete parser input unchanged and compute safe diagnostics lazily. Preserve byte facts, error classification, path data and cleanup ownership. Reuse JSON escape-coordinate mapping with IDA diagnostics. Fixes #7812 Assisted-by: Reimu (gensokyo-skills:reimu-incident-triage) Assisted-by: Kogasa (gensokyo-skills:kogasa-surprise-testing) Assisted-by: Yukari (gensokyo-skills:yukari-boundary-analysis) --- docs/pcap-analysis.md | 11 + src/domain/jsonStringSourceSpans.ts | 31 +++ src/ida/IdaJsonDiagnostic.ts | 37 +-- src/pcap/PcapCommand.ts | 78 +++--- src/pcap/PcapDiagnosticOutput.test.ts | 117 ++++++++ src/pcap/PcapDiagnosticOutput.ts | 118 ++++++++ src/pcap/PcapHttpUri.ts | 23 ++ src/pcap/PcapOutput.ts | 8 +- src/pcap/PcapProvider.ts | 56 ++-- .../pcapCredentialErrorsCliMcp.test.ts | 145 ++++++++++ .../pcap/pcapCredentialErrors.test.ts | 254 ++++++++++++++++++ tests/fixtures/localToolsMcp.ts | 7 +- tests/fixtures/pcap/offlineHttpCapture.ts | 56 ++++ 13 files changed, 834 insertions(+), 107 deletions(-) create mode 100644 src/domain/jsonStringSourceSpans.ts create mode 100644 src/pcap/PcapDiagnosticOutput.test.ts create mode 100644 src/pcap/PcapDiagnosticOutput.ts create mode 100644 src/pcap/PcapHttpUri.ts create mode 100644 tests/acceptance/pcapCredentialErrorsCliMcp.test.ts create mode 100644 tests/boundary/pcap/pcapCredentialErrors.test.ts create mode 100644 tests/fixtures/pcap/offlineHttpCapture.ts diff --git a/docs/pcap-analysis.md b/docs/pcap-analysis.md index 567b1e62ca..9790804589 100644 --- a/docs/pcap-analysis.md +++ b/docs/pcap-analysis.md @@ -106,6 +106,17 @@ URI leaves coverage partial. The HTTP URI profile covers HTTP/1 and HTTP/2; HTTP/3 URI semantics are unexamined. All admitted URI fields share the same credential exclusion and source-value retention rules. Counts preserve distinct observed source URIs before omission or coalescing. + +Failed capture-summary TShark commands apply the same HTTP URI policy to +retained JSON diagnostics, including cancellation, output-limit and cleanup +failures. JSON spelling outside the excluded credential spans is preserved. +An unfinished or malformed JSON token and its following suffix are withheld, +with `captured_output.truncated: true`; complete preceding fields remain +available. Diagnostic byte counts and output allowances describe original +producer input, before credential exclusion. Caller-selected packet-field tools +continue to return their selected recorded values. +Origin-form request targets and HTTP/2 `:path` values retain path semantics, +including leading double slashes and userinfo-like text within the path. `coverage: "partial"` records unresolved DNS attribution or URI item loss; component credential exclusion is also counted explicitly. Diagnostics retain both tool stderr streams. Recorded fields do not establish decrypted payloads, diff --git a/src/domain/jsonStringSourceSpans.ts b/src/domain/jsonStringSourceSpans.ts new file mode 100644 index 0000000000..3b2c4f7836 --- /dev/null +++ b/src/domain/jsonStringSourceSpans.ts @@ -0,0 +1,31 @@ +/** Ordered, nonoverlapping UTF-16 spans in a decoded JSON string. */ +export interface JsonStringSpan { + readonly start: number; + readonly end: number; +} + +/** Map parsed string facts back to their original, validated JSON spelling. + * `encoded` excludes the quotes; callers own JSON decoding and span validity. + */ +export function* jsonStringSourceSpans( + encoded: string, + offset: number, + spans: Iterable, +): Generator { + let sourceCursor = 0; + let decodedCursor = 0; + const sourceOffset = (boundary: number): number => { + while (decodedCursor < boundary) { + sourceCursor += + encoded[sourceCursor] === "\\" + ? encoded[sourceCursor + 1] === "u" + ? 6 + : 2 + : 1; + decodedCursor++; + } + return offset + sourceCursor; + }; + for (const span of spans) + yield { start: sourceOffset(span.start), end: sourceOffset(span.end) }; +} diff --git a/src/ida/IdaJsonDiagnostic.ts b/src/ida/IdaJsonDiagnostic.ts index 4f0823b44f..dca431c407 100644 --- a/src/ida/IdaJsonDiagnostic.ts +++ b/src/ida/IdaJsonDiagnostic.ts @@ -1,3 +1,4 @@ +import { jsonStringSourceSpans } from "../domain/jsonStringSourceSpans.js"; import { constants } from "node:buffer"; import { createScanner, type ScanError, type SyntaxKind } from "jsonc-parser"; import { @@ -21,35 +22,6 @@ type Matches = (value: string) => Iterable; const JSON_CHARACTER_RUN = /(?:[^"\\\u0000-\u001f]|\\(?:["\\/bfnrt]|u[\dA-Fa-f]{4}))+/gu; -function* stringMatches( - encoded: string, - decoded: string, - offset: number, - matches: Matches, -): Generator { - // Match offsets and JSON escape widths are both UTF-16 units. No source bytes - // outside a matched value need to be normalized or serialized again. - let sourceCursor = 0; - let decodedCursor = 0; - const sourceOffset = (boundary: number): number => { - while (decodedCursor < boundary) { - sourceCursor += - encoded[sourceCursor] === "\\" - ? encoded[sourceCursor + 1] === "u" - ? 6 - : 2 - : 1; - decodedCursor++; - } - return offset + sourceCursor; - }; - for (const match of matches(decoded)) - yield { - start: sourceOffset(match.start), - end: sourceOffset(match.end), - }; -} - function* characterRunMatches( encoded: string, offset: number, @@ -69,7 +41,7 @@ function* characterRunMatches( const decoded: unknown = JSON.parse('"' + run[0] + '"'); if (typeof decoded !== "string") throw new TypeError("A JSON character run did not decode to a string."); - yield* stringMatches(run[0], decoded, offset + run.index, matches); + yield* jsonStringSourceSpans(run[0], offset + run.index, matches(decoded)); } } @@ -91,11 +63,10 @@ function* decodedMatches( text.length, ); if (scanner.getTokenError() === JSON_NO_ERROR) - yield* stringMatches( + yield* jsonStringSourceSpans( text.slice(start + 1, end - 1), - scanner.getTokenValue(), start + 1, - matches, + matches(scanner.getTokenValue()), ); else yield* characterRunMatches( diff --git a/src/pcap/PcapCommand.ts b/src/pcap/PcapCommand.ts index bcebcb2655..cf8e1327d6 100644 --- a/src/pcap/PcapCommand.ts +++ b/src/pcap/PcapCommand.ts @@ -8,7 +8,11 @@ import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js"; import { artifactFileCancellationCause } from "../artifacts/ArtifactFileOperation.js"; import { randomUUID } from "node:crypto"; import { fixedAllowanceOptions } from "../domain/fixedAllowanceRemediation.js"; -import { AnalysisError } from "../domain/analysisErrorBase.js"; +import { + AnalysisError, + type AnalysisCapturedOutput, +} from "../domain/analysisErrorBase.js"; +import { createPcapDiagnosticProjection } from "./PcapDiagnosticOutput.js"; import { AnalysisCancelledError, AnalysisProtocolError, @@ -22,7 +26,10 @@ import { prepareOwnedCommand, runOwnedCommand, } from "../process/OwnedCommand.js"; -import type { ProviderProcessSupervisor } from "../process/ProviderProcess.js"; +import type { + ProviderProcessSupervisor, + ProviderProcessSnapshot, +} from "../process/ProviderProcess.js"; import { isWindowsBatchFile, windowsBatchCommand, @@ -37,14 +44,12 @@ export type PcapCommandLauncher = NonNullable< export interface PcapCommandCleanup { readonly reason: string; readonly resources: readonly string[]; - readonly capturedOutput: { - readonly stdout: string; - readonly stderr: string; - readonly truncated: boolean; - }; + readonly capturedOutput: AnalysisCapturedOutput; } export interface PcapCommandOutput { + /** Lazy safe diagnostics; raw stdout remains owned by the successful parser. */ + readonly capturedOutput: AnalysisCapturedOutput; readonly stdout: string; readonly stderr: string; readonly invalidUtf8Streams: readonly ("stdout" | "stderr")[]; @@ -65,6 +70,7 @@ export const runPcapCommand = async (options: { readonly environment: Readonly>; readonly signal: AbortSignal | undefined; readonly inputPath?: string; + readonly redactHttpTransportCredentials?: boolean; readonly nativeBound?: { readonly limiter?: { command: ArtifactCommandPath; @@ -81,6 +87,23 @@ export const runPcapCommand = async (options: { ) => void; }): Promise => { const { operation, command, limits, signal } = options; + const projectDiagnostic = createPcapDiagnosticProjection( + options.redactHttpTransportCredentials === true, + ); + const captureDiagnostic = ( + snapshot: ProviderProcessSnapshot, + ): AnalysisCapturedOutput => + projectDiagnostic({ + stdout: snapshot.stdout.text, + stderr: snapshot.stderr.text, + stdout_bytes: snapshot.stdout.bytes, + stderr_bytes: snapshot.stderr.bytes, + exit_code: snapshot.exitCode ?? null, + signal: snapshot.signal ?? null, + truncated: + snapshot.diagnosticTruncated === true || + snapshot.outputIncomplete === true, + }); if (signal?.aborted) throw new AnalysisCancelledError(operation, { cause: signal.reason }); const hostEnvironment = { @@ -138,13 +161,7 @@ export const runPcapCommand = async (options: { ? ["owned-process"] : ownedCommandResources(owner.launch.ownership); const snapshot = owner.snapshot(); - const capturedOutput = { - stdout: snapshot.stdout.text, - stderr: snapshot.stderr.text, - truncated: - snapshot.diagnosticTruncated === true || - snapshot.outputIncomplete === true, - }; + const capturedOutput = captureDiagnostic(snapshot); options.retain(owner, resources, stopped.reason, capturedOutput); cleanup = { reason: stopped.reason, resources, capturedOutput }; }, @@ -152,19 +169,14 @@ export const runPcapCommand = async (options: { ); if (selectedLimiter !== undefined) await assertArtifactCommandPath(selectedLimiter, operation, { - capturedOutput: { - stdout: output.stdout.text, - stderr: output.stderr.text, - stdout_bytes: output.stdout.bytes, - stderr_bytes: output.stderr.bytes, - exit_code: output.exitCode ?? null, - signal: output.signal ?? null, - truncated: - output.diagnosticTruncated === true || - output.outputIncomplete === true, + get capturedOutput() { + return captureDiagnostic(output); }, }); return { + get capturedOutput() { + return captureDiagnostic(output); + }, stdout: output.stdout.text, stderr: output.stderr.text, invalidUtf8Streams: [ @@ -206,27 +218,13 @@ export const runPcapCommand = async (options: { cause.cleanupFailure ?? cause.message, cause.snapshot === null ? { stdout: "", stderr: "", truncated: true } - : { - stdout: cause.snapshot.stdout.text, - stderr: cause.snapshot.stderr.text, - truncated: - cause.snapshot.diagnosticTruncated === true || - cause.snapshot.outputIncomplete === true, - }, + : captureDiagnostic(cause.snapshot), ); const details = { cause, ...(cause instanceof OwnedCommandFailure && cause.snapshot !== null ? { - capturedOutput: { - stdout: cause.snapshot.stdout.text, - stderr: cause.snapshot.stderr.text, - truncated: - cause.snapshot.diagnosticTruncated === true || - cause.snapshot.outputIncomplete === true, - exit_code: cause.snapshot.exitCode ?? null, - signal: cause.snapshot.signal ?? null, - }, + capturedOutput: captureDiagnostic(cause.snapshot), } : {}), ...(cleanup !== undefined diff --git a/src/pcap/PcapDiagnosticOutput.test.ts b/src/pcap/PcapDiagnosticOutput.test.ts new file mode 100644 index 0000000000..c6df1b711d --- /dev/null +++ b/src/pcap/PcapDiagnosticOutput.test.ts @@ -0,0 +1,117 @@ +import { expect, it } from "vitest"; +import { createPcapDiagnosticProjection } from "./PcapDiagnosticOutput.js"; + +const project = (stdout: string, truncated = false) => + createPcapDiagnosticProjection(true)({ + stdout, + stderr: "selected path: /tmp/caller@data.pcap", + truncated, + }); + +it("removes only parsed HTTP userinfo spans without rewriting JSON or other URI bytes", () => { + const source = String.raw`{ "dns.qry.name": ["http://caller:keep@data"], "http.request.full_uri": ["http:\/\/u\u0073er:p\u0040ass@host/a/../b?q=%2F#tag"] }`; + expect(project(source)).toEqual({ + stdout: source.replace(String.raw`u\u0073er:p\u0040ass@`, ""), + stderr: "selected path: /tmp/caller@data.pcap", + truncated: false, + }); +}); + +it.each([ + "http.request.uri", + "http.request.full_uri", + "http2.request.full_uri", +])("uses HTTP URI context for %s", (field) => { + const source = JSON.stringify({ + [field]: ["https://user:password@host/path", "/users/caller@name?q=keep"], + }); + expect(project(source).stdout).toBe( + JSON.stringify({ + [field]: ["https://host/path", "/users/caller@name?q=keep"], + }), + ); +}); + +it("withholds incomplete HTTP values at every truncation position", () => { + const source = JSON.stringify({ + "http.request.full_uri": ["http://user:SYNTHETIC_PCAP_AUTH_83A4@host/path"], + }); + const start = source.indexOf("user:") + 1; + let checked = 0; + for (let cutoff = start; cutoff < source.length; cutoff++) { + const output = project(source.slice(0, cutoff), true); + expect(output.truncated).toBe(true); + expect(output.stdout).not.toContain("http://user:"); + expect(output.stdout).not.toContain("SYNTHETIC_PCAP_AUTH_83A4"); + checked++; + } + expect(checked).toBe(source.length - start); +}); + +it.each([ + String.raw`{"http.request.full_uri":["http://user:password@host\q"]}`, + String.raw`{"http.request.full_uri" "http://user:password@host"]}`, + `not-json http://user:password@host`, +])("withholds an unclassified malformed suffix", (source) => { + const output = project(source); + expect(output.truncated).toBe(true); + expect(output.stdout).not.toContain("password"); +}); + +it("marks ambiguous URL spellings as omitted and retains empty diagnostics", () => { + expect( + project( + JSON.stringify({ + "http.request.full_uri": ["http://u\tser:password@host/path"], + }), + ), + ).toMatchObject({ + stdout: '{"http.request.full_uri":[""]}', + truncated: true, + }); + expect(project("")).toMatchObject({ stdout: "", truncated: false }); +}); + +it("preserves non-JSON command modes and original truncation flags", () => { + const output = { + stdout: "http://caller:keep@path", + stderr: "ordinary warning", + truncated: true, + exit_code: 7, + }; + expect(createPcapDiagnosticProjection(false)(output)).toBe(output); + const projector = createPcapDiagnosticProjection(true); + const raw = JSON.stringify({ + "http.request.uri": ["http://user:password@host/path"], + }); + expect(projector({ ...output, stdout: raw })).toEqual({ + ...output, + stdout: '{"http.request.uri":["http://host/path"]}', + }); + expect(projector({ ...output, stdout: raw, exit_code: 9 }).exit_code).toBe(9); +}); + +it("keeps byte completeness separate from malformed complete producer output", () => { + expect(project("{")).toMatchObject({ stdout: "{", truncated: false }); + expect( + project('{"http.request.uri":["http://user:password@host"]'), + ).toMatchObject({ + stdout: '{"http.request.uri":["http://host"]', + truncated: false, + }); +}); + +it("preserves origin-form targets and HTTP/2 paths as recorded path data", () => { + const source = JSON.stringify({ + "http.request.uri": ["//caller:keep@path/a?value=%2F"], + "http2.headers.path": [ + "//caller:keep@path/a", + "https://caller:keep@path/a", + ], + }); + expect(project(source)).toEqual({ + stdout: source, + stderr: "selected path: /tmp/caller@data.pcap", + truncated: false, + }); +}); diff --git a/src/pcap/PcapDiagnosticOutput.ts b/src/pcap/PcapDiagnosticOutput.ts new file mode 100644 index 0000000000..e926bba5db --- /dev/null +++ b/src/pcap/PcapDiagnosticOutput.ts @@ -0,0 +1,118 @@ +import { visit } from "jsonc-parser"; +import { + PCAP_HTTP_URI_FIELDS, + parsePcapHttpUri, + type PcapHttpUriRole, +} from "./PcapHttpUri.js"; +import { jsonStringSourceSpans } from "../domain/jsonStringSourceSpans.js"; +import type { AnalysisCapturedOutput } from "../domain/analysisErrorBase.js"; + +// These are protocol facts, not a heuristic for names or arbitrary text. +const HTTP_URI_FIELDS = new Map( + Object.entries(PCAP_HTTP_URI_FIELDS), +); + +interface ProjectedJsonDiagnostic { + readonly text: string; + /** Unclassified data was withheld; format validity is a separate fact. */ + readonly partial: boolean; +} + +/** Preserve the source JSON spelling except for transport userinfo spans. + * An invalid/unfinished token and its suffix are withheld, never guessed. + * No object tree is assembled; input is already bounded by the command owner. + */ +const projectHttpJsonDiagnostic = (text: string): ProjectedJsonDiagnostic => { + if (text.length === 0) return { text, partial: false }; + const invalidJson = {}; + let cutoff = text.length; + let cursor = 0; + let partial = false; + let pending = ""; + const chunks: string[] = []; + const append = (part: string): void => { + pending += part; + if (pending.length >= 16 * 1024) { + chunks.push(pending); + pending = ""; + } + }; + const redact = ( + value: unknown, + offset: number, + length: number, + path: readonly (string | number)[], + ): void => { + if (typeof value !== "string") return; + let role: PcapHttpUriRole | undefined; + for (const part of path) + if (typeof part === "string") role = HTTP_URI_FIELDS.get(part) ?? role; + if (role === undefined) return; + const { credentials } = parsePcapHttpUri(value, role); + if (credentials === null) return; + const encoded = text.slice(offset + 1, offset + length - 1); + const spans = + credentials === "ambiguous" + ? [{ start: offset + 1, end: offset + length - 1 }] + : jsonStringSourceSpans(encoded, offset + 1, [credentials]); + for (const span of spans) { + append(text.slice(cursor, span.start)); + cursor = span.end; + } + if (credentials === "ambiguous") partial = true; + }; + try { + visit( + text, + { + onLiteralValue: ( + value: unknown, + offset, + length, + _line, + _column, + path, + ) => redact(value, offset, length, path()), + onObjectProperty: (value, offset, length, _line, _column, path) => + redact(value, offset, length, path()), + onError: (_error, offset) => { + partial ||= offset < text.length; + cutoff = offset; + throw invalidJson; + }, + }, + { + disallowComments: true, + allowTrailingComma: false, + allowEmptyContent: false, + }, + ); + } catch (cause) { + // Unexpected decoder/resource failures must not replace the command's + // primary failure or expose an unclassified raw diagnostic. + if (cause !== invalidJson || cutoff < cursor) + return { text: "", partial: true }; + } + append(text.slice(cursor, cutoff)); + if (chunks.length === 0) return { text: pending, partial }; + chunks.push(pending); + return { text: chunks.join(""), partial }; +}; + +/** One command's lazy, single-entry diagnostic cache; no cross-capture state. */ +export const createPcapDiagnosticProjection = (httpJson: boolean) => { + let cachedSource: string | undefined; + let cached: ProjectedJsonDiagnostic | undefined; + return (output: AnalysisCapturedOutput): AnalysisCapturedOutput => { + if (!httpJson) return output; + if (cached === undefined || cachedSource !== output.stdout) { + cached = projectHttpJsonDiagnostic(output.stdout); + cachedSource = output.stdout; + } + return { + ...output, + stdout: cached.text, + truncated: output.truncated || cached.partial, + }; + }; +}; diff --git a/src/pcap/PcapHttpUri.ts b/src/pcap/PcapHttpUri.ts new file mode 100644 index 0000000000..99b0f05bb6 --- /dev/null +++ b/src/pcap/PcapHttpUri.ts @@ -0,0 +1,23 @@ +import { + parseRecordedTransportUrl, + type RecordedTransportUrl, +} from "../domain/recordedTransportUrl.js"; + +/** Wireshark URI facts. HTTP/2 :path is a path, not an absolute URI. */ +export const PCAP_HTTP_URI_FIELDS = { + "http.request.full_uri": "full-uri", + "http.request.uri": "request-target", + "http2.request.full_uri": "full-uri", +} as const; + +export type PcapHttpUriRole = + (typeof PCAP_HTTP_URI_FIELDS)[keyof typeof PCAP_HTTP_URI_FIELDS]; + +/** Origin-form targets retain path semantics, including a leading double slash. */ +export const parsePcapHttpUri = ( + value: string, + role: PcapHttpUriRole, +): RecordedTransportUrl => + role === "request-target" && value.startsWith("/") + ? { origin: null, path: null, credentials: null } + : parseRecordedTransportUrl(value); diff --git a/src/pcap/PcapOutput.ts b/src/pcap/PcapOutput.ts index dad390aeab..fc5650062a 100644 --- a/src/pcap/PcapOutput.ts +++ b/src/pcap/PcapOutput.ts @@ -4,7 +4,7 @@ import { AnalysisProtocolError, AnalysisResourceConstraintError, } from "../domain/analysisErrorCore.js"; -import { parseRecordedTransportUrl } from "../domain/recordedTransportUrl.js"; +import { parsePcapHttpUri } from "./PcapHttpUri.js"; import { fixedAllowanceOptions } from "../domain/fixedAllowanceRemediation.js"; import { parseDnsMessageRaw } from "./PcapDnsWire.js"; import { compareUnicodeCodePoints } from "../domain/unicodeCodePointOrder.js"; @@ -528,11 +528,11 @@ export const parseTsharkJsonOutput = async ( const fullUris = field("http.request.full_uri"); const fullUriCandidates = fullUris.map((value) => ({ value, - parsed: parseRecordedTransportUrl(value), + parsed: parsePcapHttpUri(value, "full-uri"), })); const requestUriCandidates = field("http.request.uri").map((value) => ({ value, - parsed: parseRecordedTransportUrl(value), + parsed: parsePcapHttpUri(value, "request-target"), })); // Wireshark emits full_uri alongside each request URI it can resolve. // Raw absolute-form targets stand on their own; relative targets need a @@ -571,7 +571,7 @@ export const parseTsharkJsonOutput = async ( ...fullUriCandidates, ...http2Uris.map((value) => ({ value, - parsed: parseRecordedTransportUrl(value), + parsed: parsePcapHttpUri(value, "full-uri"), })), ...requestUriCandidates.filter(({ parsed }) => parsed.origin !== null), ]) { diff --git a/src/pcap/PcapProvider.ts b/src/pcap/PcapProvider.ts index f6a9ab3c1f..2b10f068c2 100644 --- a/src/pcap/PcapProvider.ts +++ b/src/pcap/PcapProvider.ts @@ -1,3 +1,4 @@ +import { PCAP_HTTP_URI_FIELDS } from "./PcapHttpUri.js"; import { type ExecutionOptions, type ProviderAvailability, @@ -17,7 +18,10 @@ import { AnalysisProtocolError, AnalysisResourceConstraintError, } from "../domain/analysisErrorCore.js"; -import { AnalysisError } from "../domain/analysisErrorBase.js"; +import { + AnalysisError, + type AnalysisCapturedOutput, +} from "../domain/analysisErrorBase.js"; import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup.js"; import { err, ok, type Result } from "../domain/result.js"; @@ -90,9 +94,7 @@ const FIELDS = [ "dns.a", "dns.aaaa", "tls.handshake.extensions_server_name", - "http.request.full_uri", - "http.request.uri", - "http2.request.full_uri", + ...Object.keys(PCAP_HTTP_URI_FIELDS), "http2.headers.path", ]; @@ -126,7 +128,7 @@ class PcapCommandCleanupError extends ProviderCleanupError {} const probeCleanupFailure = ( operation: PcapOperation, cleanup: PcapCommandCleanup, - output: { readonly stdout: string; readonly stderr: string }, + output: AnalysisCapturedOutput, ): ProviderCleanupError => new ProviderCleanupError( "pcap", @@ -135,9 +137,7 @@ const probeCleanupFailure = ( reason: cleanup.reason, previous_error: null, captured_output: { - stdout: output.stdout, - stderr: output.stderr, - truncated: cleanup.capturedOutput.truncated, + ...output, }, }, { operation }, @@ -147,7 +147,7 @@ const pcapCommandCleanupError = ( operation: PcapOperation, snapshotPath: string | undefined, cleanup: PcapCommandCleanup, - capturedOutput: { readonly stdout: string; readonly stderr: string }, + capturedOutput: AnalysisCapturedOutput, partialObservation?: ReturnType, ) => new PcapCommandCleanupError( @@ -162,9 +162,7 @@ const pcapCommandCleanupError = ( reason: cleanup.reason, previous_error: null, captured_output: { - stdout: capturedOutput.stdout, - stderr: capturedOutput.stderr, - truncated: cleanup.capturedOutput.truncated, + ...capturedOutput, }, }, { @@ -183,10 +181,8 @@ const assertValidPcapCommandText = ( 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, + get capturedOutput() { + return output.capturedOutput; }, diagnostics: { stage, @@ -314,6 +310,8 @@ export class PcapProvider { } output = await runPcapCommand({ operation, + redactHttpTransportCredentials: + stage === "tshark" && operation === "inspect_pcap_capture", command: selected?.invocationPath ?? command, arguments: args, limits, @@ -346,10 +344,8 @@ export class PcapProvider { commandReturned = true; if (selected !== undefined) { await assertArtifactCommandPath(selected, operation, { - capturedOutput: { - stdout: output.stdout, - stderr: output.stderr, - truncated: output.cleanup?.capturedOutput.truncated ?? false, + get capturedOutput() { + return output.capturedOutput; }, }); const after = await this.readToolIdentity( @@ -362,10 +358,8 @@ export class PcapProvider { selected.identity, wiresharkExecutableIdentity(after), { - capturedOutput: { - stdout: output.stdout, - stderr: output.stderr, - truncated: output.cleanup?.capturedOutput.truncated ?? false, + get capturedOutput() { + return output.capturedOutput; }, }, ); @@ -406,7 +400,11 @@ export class PcapProvider { throw cause; } if (inputPath === undefined && cleanup !== undefined) { - const failure = probeCleanupFailure(operation, cleanup, output); + const failure = probeCleanupFailure( + operation, + cleanup, + output.capturedOutput, + ); if (retainedCommand !== undefined) retainedCommand.failure = failure; throw failure; } @@ -889,7 +887,7 @@ export class PcapProvider { pendingCommand.resources, { reason: pendingCommand.reason, - captured_output: pendingCommand.capturedOutput, + captured_output: { ...pendingCommand.capturedOutput }, }, { operation: request.operation }, ); @@ -988,7 +986,7 @@ export class PcapProvider { request.operation, snapshot.path, command.cleanup, - command, + command.capturedOutput, ), request.operation, ); @@ -1018,7 +1016,7 @@ export class PcapProvider { request.operation, snapshot.path, info.cleanup, - info, + info.capturedOutput, pcapPartialEvidence( request, provider, @@ -1190,7 +1188,7 @@ export class PcapProvider { request.operation, snapshot.path, cleanupCommand.cleanup, - cleanupCommand, + cleanupCommand.capturedOutput, pcapPartialEvidence( request, provider, diff --git a/tests/acceptance/pcapCredentialErrorsCliMcp.test.ts b/tests/acceptance/pcapCredentialErrorsCliMcp.test.ts new file mode 100644 index 0000000000..61123d5477 --- /dev/null +++ b/tests/acceptance/pcapCredentialErrorsCliMcp.test.ts @@ -0,0 +1,145 @@ +import { existsSync } from "node:fs"; +import { readFile, writeFile } from "node:fs/promises"; +import { createHash } from "node:crypto"; +import { join } from "node:path"; +import { expect, it, onTestFinished } from "vitest"; +import { z } from "zod"; +import { analysisErrorProjectionSchema } from "../../src/contracts/errorSchemas.js"; +import { parseEvidence } from "../../src/domain/evidence.js"; +import { parseEvidenceBundle } from "../../src/domain/evidenceBundle.js"; +import { pcapResultSchemas } from "../../src/domain/pcap/pcapAnalysis.js"; +import { connectLocalToolsMcp } from "../fixtures/localToolsMcp.js"; +import { createTestTempDirectory } from "../fixtures/temporaryDirectory.js"; +import { parseMcpToolError } from "../fixtures/mcpToolError.js"; +import { offlineHttpCapture } from "../fixtures/pcap/offlineHttpCapture.js"; +import { cliTest } from "../support/cli/cliFixture.js"; + +const missingNative = + process.platform !== "linux" || + !existsSync("/usr/bin/tshark") || + !existsSync("/usr/bin/capinfos"); +const credential = "SYNTHETIC_PCAP_AUTH_83A4"; +const uri = `http://user:${credential}@example.test/a/../b?value=keep%2Fthis`; +const safeUri = "http://example.test/a/../b?value=keep%2Fthis"; +const environmentFor = (root: string) => ({ + REA_TSHARK_COMMAND: "/usr/bin/tshark", + REA_CAPINFOS_COMMAND: "/usr/bin/capinfos", + HOME: root, + USERPROFILE: root, + XDG_CONFIG_HOME: join(root, "config"), + XDG_CACHE_HOME: join(root, "cache"), + XDG_DATA_HOME: join(root, "data"), +}); +const verifyOriginalError = (document: unknown) => { + const error = analysisErrorProjectionSchema.parse(document); + expect(error.code).toBe("resource_constraint"); + const facts = z + .object({ + resource: z.literal("memory"), + reported_limits: z.object({ + max_output_bytes: z.literal(8 * 1024 * 1024), + }), + captured_output: z.object({ + stdout: z.string(), + stdout_bytes: z.literal(8 * 1024 * 1024), + truncated: z.literal(true), + }), + }) + .parse(error.details); + expect(facts.captured_output.stdout.includes(credential)).toBe(false); + expect(facts.captured_output.stdout.includes(safeUri)).toBe(true); + expect(facts.captured_output.stdout.includes('"ip.src"')).toBe(true); +}; + +cliTest.skipIf(missingNative)( + "keeps real pcap success and output-limit CLI Evidence credential-safe", + async ({ cli }) => { + const root = await createTestTempDirectory("rea-pcap-private-cli-"); + const single = join(root, "single.pcap"); + await writeFile(single, offlineHttpCapture(uri)); + const success = await cli.run({ + arguments: ["inspect-pcap-capture", single, "--json"], + environment: environmentFor(root), + timeoutMs: 60_000, + }); + expect(success.exitCode, success.stderr).toBe(0); + expect(success.stdout.includes(credential)).toBe(false); + const result = pcapResultSchemas.inspect_pcap_capture.parse( + parseEvidence(success.json).normalized_result, + ); + expect(result.http_request_uris).toEqual([safeUri]); + expect(result.http_request_uri_count).toBe(1); + expect(result.http_uri_redaction_count).toBe(1); + expect(result.target.path).toBe(single); + const many = join(root, "many.pcap"); + const bytes = offlineHttpCapture(uri, 40_000); + const before = createHash("sha256").update(bytes).digest("hex"); + await writeFile(many, bytes); + const failed = await cli.run({ + arguments: ["inspect-pcap-capture", many, "--json"], + environment: environmentFor(root), + timeoutMs: 60_000, + }); + expect(failed.exitCode, failed.stderr).toBe(1); + expect(failed.stdout.includes(credential)).toBe(false); + verifyOriginalError(failed.json); + expect( + createHash("sha256") + .update(await readFile(many)) + .digest("hex"), + ).toBe(before); + }, + 120_000, +); + +it.skipIf(missingNative)( + "keeps real pcap MCP errors and exported retained errors credential-safe", + async () => { + const root = await createTestTempDirectory("rea-pcap-private-mcp-"); + const path = join(root, "many.pcap"); + await writeFile(path, offlineHttpCapture(uri, 40_000)); + for (const direct of [true, false]) { + const environment = { + ...environmentFor(root), + ...(direct + ? { REA_MCP_MAX_RESPONSE_BYTES: String(20 * 1024 * 1024) } + : {}), + }; + const { client, call } = await connectLocalToolsMcp( + environment, + onTestFinished, + { maxBufferSize: 32 * 1024 * 1024 }, + ); + try { + const response = await call("inspect_pcap_capture", { path }); + expect(JSON.stringify(response).includes(credential)).toBe(false); + const error = parseMcpToolError(response).error; + if (direct) verifyOriginalError(error); + else { + expect(error.code).toBe("resource_constraint"); + const bundlePath = join(root, "retained-errors.json"); + const exported = await call("export_evidence_bundle", { + path: bundlePath, + }); + expect(exported.isError).not.toBe(true); + const text = await readFile(bundlePath, "utf8"); + expect(text.includes(credential)).toBe(false); + const bundle = parseEvidenceBundle(JSON.parse(text)); + const retained = bundle.records.find( + (record) => record.operation === "mcp_tool_error", + ); + if (retained === undefined) + throw new Error("Original oversized error was not retained"); + verifyOriginalError( + z + .object({ error: analysisErrorProjectionSchema }) + .parse(retained.normalized_result).error, + ); + } + } finally { + await client.close(); + } + } + }, + 120_000, +); diff --git a/tests/boundary/pcap/pcapCredentialErrors.test.ts b/tests/boundary/pcap/pcapCredentialErrors.test.ts new file mode 100644 index 0000000000..9f57a47ecb --- /dev/null +++ b/tests/boundary/pcap/pcapCredentialErrors.test.ts @@ -0,0 +1,254 @@ +import { z } from "zod"; +import { existsSync } from "node:fs"; +import { writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { expect, it, onTestFinished } from "vitest"; +import { + runPcapCommand, + type PcapCommandLauncher, +} from "../../../src/pcap/PcapCommand.js"; +import { + AnalysisCancelledError, + AnalysisProtocolError, + AnalysisResourceConstraintError, +} from "../../../src/domain/analysisErrorCore.js"; +import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js"; +import { + spawnOwnedProviderProcess, + type ProviderProcessSupervisor, +} from "../../../src/process/ProviderProcess.js"; +import { cleanupOwnedProcessGroup } from "../../../src/process/ProcessOwnership.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { offlineHttpCapture } from "../../fixtures/pcap/offlineHttpCapture.js"; + +const credential = "SYNTHETIC_PCAP_AUTH_83A4"; +const uri = `http://user:${credential}@example.test/a/../b?value=keep%2Fthis`; +const authoredJson = JSON.stringify([ + { + _source: { + layers: { + "http.request.full_uri": [uri], + "dns.qry.name": ["http://caller:keep@label"], + }, + }, + }, +]); +const ownedCommand = ( + code: string, + options: { signal?: AbortSignal; launcher?: PcapCommandLauncher } = {}, +) => + runPcapCommand({ + operation: "inspect_pcap_capture", + command: process.execPath, + arguments: ["-e", code], + redactHttpTransportCredentials: true, + inputPath: "/authored-capture.pcap", + limits: { timeoutMs: 10_000, maxOutputBytes: 4096 }, + environment: process.env, + signal: options.signal, + ...(options.launcher === undefined ? {} : { launcher: options.launcher }), + retain: (owner) => + onTestFinished(async () => { + expect((await owner.stop()).status).not.toBe("incomplete"); + }), + }); +const print = `process.stdout.write(${JSON.stringify(authoredJson)});`; + +it("keeps complete parser input while exposing safe late-failure diagnostics", async () => { + const result = await ownedCommand(print); + expect(result.stdout).toBe(authoredJson); + expect(result.capturedOutput.stdout).not.toContain(credential); + expect(result.capturedOutput.stdout).toContain("http://caller:keep@label"); + expect(result.capturedOutput.stdout).toContain( + "http://example.test/a/../b?value=keep%2Fthis", + ); + expect(result.capturedOutput).toMatchObject({ + exit_code: 0, + truncated: false, + }); +}, 20_000); + +it("preserves producer failure facts while excluding HTTP credentials", async () => { + const failure: unknown = await ownedCommand( + `${print}process.exitCode = 7;`, + ).catch((cause: unknown) => cause); + expect(failure).toBeInstanceOf(AnalysisProtocolError); + if (!(failure instanceof AnalysisProtocolError)) throw failure; + expect(failure.capturedOutput).toMatchObject({ + exit_code: 7, + truncated: false, + }); + expect(JSON.stringify(projectAnalysisError(failure))).not.toContain( + credential, + ); + expect(failure.capturedOutput?.stdout).toContain("http://caller:keep@label"); +}, 20_000); + +it("excludes HTTP credentials after output-observed cancellation and preserves its reason", async () => { + const controller = new AbortController(); + const reason = { request: "owned capture cancellation" }; + let observedOutput = false; + const failure: unknown = await ownedCommand( + `${print}setInterval(() => {}, 1000);`, + { + signal: controller.signal, + launcher: async (spawn) => { + const launched = await spawnOwnedProviderProcess(spawn); + launched.process.stdout?.once("data", () => { + observedOutput = true; + queueMicrotask(() => controller.abort(reason)); + }); + return launched; + }, + }, + ).catch((cause: unknown) => cause); + expect(observedOutput).toBe(true); + expect(failure).toBeInstanceOf(AnalysisCancelledError); + if (!(failure instanceof AnalysisCancelledError)) throw failure; + expect(failure.cause).toBe(reason); + expect(failure.capturedOutput?.stdout).toContain("http://example.test/"); + expect(JSON.stringify(projectAnalysisError(failure))).not.toContain( + credential, + ); +}, 20_000); + +it("retains safe cleanup observations and retries their original owner", async () => { + let mayClean = false; + let retained = false; + const output = await runPcapCommand({ + operation: "inspect_pcap_capture", + command: process.execPath, + arguments: ["-e", print], + redactHttpTransportCredentials: true, + inputPath: "/authored-capture.pcap", + limits: { timeoutMs: 10_000, maxOutputBytes: 4096 }, + environment: process.env, + signal: undefined, + launcher: async (spawn) => { + const launched = await spawnOwnedProviderProcess(spawn); + const cleanup = + launched.cleanup ?? + (() => cleanupOwnedProcessGroup(launched.ownership)); + return { + ...launched, + cleanup: async () => { + const result = await cleanup(); + if (!mayClean) throw new Error("injected cleanup uncertainty"); + return result; + }, + }; + }, + retain: (owner, resources, _reason, capturedOutput) => { + retained = true; + onTestFinished(async () => { + mayClean = true; + expect((await owner.stop()).status).not.toBe("incomplete"); + }); + expect(resources.length).toBeGreaterThan(0); + expect(capturedOutput.stdout).not.toContain(credential); + }, + }); + expect(retained).toBe(true); + expect(output.stdout).toBe(authoredJson); + expect(output.cleanup?.capturedOutput.stdout).not.toContain(credential); + expect(output.cleanup?.capturedOutput.stdout).toContain( + "http://caller:keep@label", + ); +}, 20_000); + +const nativeOptions = ( + path: string, +): Omit[0], "limits"> => ({ + operation: "inspect_pcap_capture", + command: "/usr/bin/tshark", + arguments: [ + "-n", + "-r", + path, + "-T", + "json", + "--no-duplicate-keys", + "-e", + "frame.number", + "-e", + "frame.protocols", + "-e", + "ip.src", + "-e", + "ip.dst", + "-e", + "http.request.full_uri", + "-e", + "http.request.uri", + ], + redactHttpTransportCredentials: true, + inputPath: path, + environment: process.env, + signal: undefined, + retain: (owner: ProviderProcessSupervisor) => + onTestFinished(async () => { + expect((await owner.stop()).status).not.toBe("incomplete"); + }), +}); + +it.skipIf(process.platform !== "linux" || !existsSync("/usr/bin/tshark"))( + "excludes recorded HTTP credentials when real TShark exceeds the output allowance", + async () => { + const root = await createTestTempDirectory("rea-pcap-private-native-"); + const path = join(root, "http.pcap"); + await writeFile(path, offlineHttpCapture(uri)); + const options = nativeOptions(path); + const full = await runPcapCommand({ + ...options, + limits: { timeoutMs: 10_000, maxOutputBytes: 65536 }, + }); + expect(full.stdout).toContain(uri); + const budget = + Buffer.from(full.stdout).lastIndexOf(credential) + + Buffer.byteLength(credential) + + 20; + expect(budget).toBeLessThan(Buffer.byteLength(full.stdout)); + const failure: unknown = await runPcapCommand({ + ...options, + limits: { timeoutMs: 10_000, maxOutputBytes: budget }, + }).catch((cause: unknown) => cause); + expect(failure).toBeInstanceOf(AnalysisResourceConstraintError); + if (!(failure instanceof AnalysisResourceConstraintError)) throw failure; + expect(failure).toMatchObject({ + resource: "memory", + retryable: false, + reportedLimits: { max_output_bytes: budget }, + }); + expect(failure.capturedOutput?.truncated).toBe(true); + expect(JSON.stringify(projectAnalysisError(failure))).not.toContain( + credential, + ); + }, + 20_000, +); + +it.skipIf(process.platform !== "linux" || !existsSync("/usr/bin/tshark"))( + "preserves origin-form request-target bytes in real TShark diagnostics", + async () => { + const root = await createTestTempDirectory("rea-pcap-path-native-"); + const path = join(root, "origin-form.pcap"); + const target = "//caller:keep@path/a?value=%2F"; + await writeFile(path, offlineHttpCapture(target)); + const result = await runPcapCommand({ + ...nativeOptions(path), + limits: { timeoutMs: 10_000, maxOutputBytes: 65536 }, + }); + const frames = z + .array( + z.object({ + _source: z.object({ + layers: z.record(z.string(), z.array(z.string())), + }), + }), + ) + .parse(JSON.parse(result.capturedOutput.stdout)); + expect(frames[0]?._source.layers["http.request.uri"]).toEqual([target]); + expect(result.capturedOutput.truncated).toBe(false); + }, + 20_000, +); diff --git a/tests/fixtures/localToolsMcp.ts b/tests/fixtures/localToolsMcp.ts index 590ef8e27e..9979939c9a 100644 --- a/tests/fixtures/localToolsMcp.ts +++ b/tests/fixtures/localToolsMcp.ts @@ -2,7 +2,10 @@ import { Client, type CallToolRequestOptions, } from "@modelcontextprotocol/client"; -import { StdioClientTransport } from "@modelcontextprotocol/client/stdio"; +import { + StdioClientTransport, + type StdioServerParameters, +} from "@modelcontextprotocol/client/stdio"; import { Ajv2020 } from "ajv/dist/2020.js"; import type { ValidateFunction } from "ajv"; import { fileURLToPath } from "node:url"; @@ -37,8 +40,10 @@ const compileValidator = (schema: unknown): ValidateFunction => { export async function connectLocalToolsMcp( environment: Readonly> = {}, registerCleanup: (cleanup: () => Promise) => void = onTestFinished, + transportOptions: Readonly> = {}, ) { const transport = new StdioClientTransport({ + ...transportOptions, command: process.execPath, args: [ fileURLToPath(new URL("../../scripts/rea.mjs", import.meta.url)), diff --git a/tests/fixtures/pcap/offlineHttpCapture.ts b/tests/fixtures/pcap/offlineHttpCapture.ts new file mode 100644 index 0000000000..cf3c8b421d --- /dev/null +++ b/tests/fixtures/pcap/offlineHttpCapture.ts @@ -0,0 +1,56 @@ +/** Owned offline Ethernet/IPv4/TCP fixture; no traffic or external input. */ +export const offlineHttpCapture = (uri: string, packets = 1): Buffer => { + if (!Number.isSafeInteger(packets) || packets < 1 || packets > 40_000) + throw new RangeError("Offline test captures support 1..40000 packets"); + const payload = Buffer.from( + `GET ${uri} HTTP/1.1\r\nHost: example.test\r\nConnection: close\r\n\r\n`, + ); + const checksum = (bytes: Buffer): number => { + let total = 0; + for (let index = 0; index < bytes.length; index += 2) + total += ((bytes[index] ?? 0) << 8) | (bytes[index + 1] ?? 0); + while (total >>> 16) total = (total & 0xffff) + (total >>> 16); + return ~total & 0xffff; + }; + const source = Buffer.from([192, 0, 2, 1]); + const destination = Buffer.from([198, 51, 100, 2]); + const tcp = Buffer.alloc(20); + tcp.writeUInt16BE(44000, 0); + tcp.writeUInt16BE(80, 2); + tcp.writeUInt32BE(1, 4); + tcp.writeUInt32BE(1, 8); + tcp.writeUInt16BE(0x5018, 12); + tcp.writeUInt16BE(65535, 14); + const pseudo = Buffer.concat([ + source, + destination, + Buffer.from([0, 6, 0, 0]), + ]); + pseudo.writeUInt16BE(tcp.length + payload.length, 10); + tcp.writeUInt16BE(checksum(Buffer.concat([pseudo, tcp, payload])), 16); + const ip = Buffer.alloc(20); + ip[0] = 0x45; + ip.writeUInt16BE(ip.length + tcp.length + payload.length, 2); + ip.writeUInt16BE(1, 4); + ip[8] = 64; + ip[9] = 6; + source.copy(ip, 12); + destination.copy(ip, 16); + ip.writeUInt16BE(checksum(ip), 10); + const ethernet = Buffer.from([2, 0, 0, 0, 0, 2, 2, 0, 0, 0, 0, 1, 8, 0]); + const frame = Buffer.concat([ethernet, ip, tcp, payload]); + const record = Buffer.alloc(16 + frame.length); + record.writeUInt32LE(1_700_000_000, 0); + record.writeUInt32LE(frame.length, 8); + record.writeUInt32LE(frame.length, 12); + frame.copy(record, 16); + const capture = Buffer.alloc(24 + packets * record.length); + capture.writeUInt32LE(0xa1b2c3d4, 0); + capture.writeUInt16LE(2, 4); + capture.writeUInt16LE(4, 6); + capture.writeUInt32LE(65535, 16); + capture.writeUInt32LE(1, 20); + for (let index = 0; index < packets; index++) + record.copy(capture, 24 + index * record.length); + return capture; +};