From bbbd208743d30edc0ec260667c51a184ddad6007 Mon Sep 17 00:00:00 2001 From: gensokyo-dev <342580529+gensokyo-dev@users.noreply.github.com> Date: Wed, 21 Oct 2026 12:00:16 +0800 Subject: [PATCH] fix(pcap): preserve frames with empty protocol stacks (#5762) Preserve valid empty TShark protocol stacks at the shared parser, retain frame and other fact inventories, and cover malformed producer output and mixed-frame ordering. Validated with focused source/boundary tests, full static checks, docs checks, real Linux CLI/MCP and a mixed-interface public HTTP pcapng. Latest-head Codex found no issues; Greptile is 5/5 and its test-coverage thread is addressed. Remaining queued CI was bypassed under the human owner's explicit authorization; this does not claim that the full CI suite passed. Fixes #5734. --- docs/pcap-analysis.md | 6 ++- src/pcap/PcapOutput.test.ts | 87 +++++++++++++++++++++++++++++++++++++ src/pcap/PcapOutput.ts | 16 +++---- 3 files changed, 100 insertions(+), 9 deletions(-) diff --git a/docs/pcap-analysis.md b/docs/pcap-analysis.md index 3e13ec2251..58c5dcd037 100644 --- a/docs/pcap-analysis.md +++ b/docs/pcap-analysis.md @@ -67,7 +67,11 @@ Addresses count once per packet, including loopback and nested IP occurrences. DNS addresses are associated only with one question in the same response packet; additional records may be included, so this does not prove final resolution. Ambiguous addresses remain in a separate inventory. Protocol distribution counts -exact `frame.protocols` stack strings. TLS SNI is recorded handshake metadata, +exact nonempty `frame.protocols` stack strings. Frames for which TShark reports +an empty stack, including zero-length packets, still contribute to the packet +count without adding a protocol row. An all-empty stack inventory can have +complete examined coverage; it does not establish the absence of network traffic. +TLS SNI is recorded handshake metadata, not process identity or proof of the destination's authenticity. All examined inventories are retained within the resource budget; no fixed diff --git a/src/pcap/PcapOutput.test.ts b/src/pcap/PcapOutput.test.ts index 12569151f0..251abb39e2 100644 --- a/src/pcap/PcapOutput.test.ts +++ b/src/pcap/PcapOutput.test.ts @@ -16,6 +16,93 @@ const capture = (layers: ReadonlyArray>) => })), ); +it.each([ + [[], []], + [["", ""], []], + [["user_dlt:data", ""], [{ protocol: "user_dlt:data", packet_count: 1 }]], + [["", "user_dlt:data"], [{ protocol: "user_dlt:data", packet_count: 1 }]], + [ + ["user_dlt:data", "", "user_dlt:data"], + [{ protocol: "user_dlt:data", packet_count: 2 }], + ], +] as const)( + "counts every frame while aggregating only reported protocol stacks: %j", + async (stacks, expected) => { + const result = await parseTsharkJsonOutput( + capture(stacks.map((stack) => ({ "frame.protocols": [stack] }))), + ); + expect(result.packetCount).toBe(stacks.length); + expect(result.protocolDistribution).toEqual(expected); + expect(result.protocolCount).toBe(expected.length); + expect(result.partial).toBe(false); + }, +); + +it.each([ + { "frame.number": ["1"] }, + { "frame.number": ["1"], "frame.protocols": [] }, + { "frame.number": ["1"], "frame.protocols": ["", "ip"] }, + { "frame.number": ["1"], "frame.protocols": ["ip", "ip"] }, + { "frame.number": ["1"], "frame.protocols": [null] }, + { "frame.number": ["1"], "frame.protocols": "" }, + { "frame.protocols": [""] }, + { "frame.number": ["2"], "frame.protocols": [""] }, +])("rejects malformed or ambiguous frame inventories: %j", async (layers) => { + await expect( + parseTsharkJsonOutput(JSON.stringify([{ _source: { layers } }])), + ).rejects.toMatchObject({ _tag: "AnalysisProtocolError" }); +}); + +it.each(["before", "after"] as const)( + "retains endpoint and DNS facts with an empty stack %s the response", + async (position) => { + const empty = { "frame.protocols": [""] }; + const response = { + "frame.protocols": ["eth:ethertype:ip:udp:dns"], + "ip.src": ["192.0.2.53"], + "ip.dst": ["192.0.2.1"], + "dns.qry.name": ["api.example"], + "dns.flags.response": ["True"], + "dns.a": ["203.0.113.5"], + }; + const result = await parseTsharkJsonOutput( + capture(position === "before" ? [empty, response] : [response, empty]), + ); + expect(result).toMatchObject({ + packetCount: 2, + protocolCount: 1, + protocolDistribution: [ + { protocol: "eth:ethertype:ip:udp:dns", packet_count: 1 }, + ], + endpointCount: 2, + endpoints: [ + { address: "192.0.2.1", port: null, packet_count: 1 }, + { address: "192.0.2.53", port: null, packet_count: 1 }, + ], + dnsQueryCount: 1, + dnsQueries: [ + { + query: "api.example", + response_addresses: ["203.0.113.5"], + response_address_count: 1, + }, + ], + partial: false, + }); + }, +); + +it("rejects duplicate frame identities even when protocol stacks are empty", async () => { + await expect( + parseTsharkJsonOutput( + capture([ + { "frame.protocols": [""] }, + { "frame.number": ["1"], "frame.protocols": [""] }, + ]), + ), + ).rejects.toMatchObject({ _tag: "AnalysisProtocolError" }); +}); + it("binds interleaved DNS answers to the question in their response packet", async () => { const result = await parseTsharkJsonOutput( capture([ diff --git a/src/pcap/PcapOutput.ts b/src/pcap/PcapOutput.ts index 7226e46370..1f6da7ee6e 100644 --- a/src/pcap/PcapOutput.ts +++ b/src/pcap/PcapOutput.ts @@ -269,13 +269,14 @@ export const parseTsharkJsonOutput = async ( return protocolFailure( "frame inventory is missing, duplicated or out of order", ); - if ( - field("frame.protocols").length !== 1 || - field("frame.protocols")[0] === "" - ) + const protocolStack = field("frame.protocols"); + if (protocolStack.length !== 1) return protocolFailure( "packet protocol inventory is missing or ambiguous", ); + // TShark reports [""] for a frame with no dissected protocol, including + // zero-length packets. Keep the frame without inventing a protocol row. + const protocol = protocolStack[0]!; // Count one occurrence per packet even for loopback or nested IP layers. for (const address of new Set([ ...field("ip.src"), @@ -314,10 +315,9 @@ export const parseTsharkJsonOutput = async ( } for (const value of field("tls.handshake.extensions_server_name")) if (value !== "") add(sni, value); - for (const value of new Set(field("frame.protocols"))) { - if (value === "") continue; - if (!protocols.has(value)) charge(value); - protocols.set(value, (protocols.get(value) ?? 0) + 1); + if (protocol !== "") { + if (!protocols.has(protocol)) charge(protocol); + protocols.set(protocol, (protocols.get(protocol) ?? 0) + 1); } for (const value of field("http.request.full_uri")) { if (value === "" || observedUris.has(value)) continue;