feat(mirror): replay captured traffic live to the local process via --to#5794
Conversation
Phase-1 local delivery (#4521): a LiveReplay sink tees off the capture stream, parses the pcap live (LINUX_SLL2), reassembles inbound TCP payload streams with a minimal per-flow sequencer (in-order delivery, bounded out-of-order buffering, retransmissions dropped, overlaps trimmed), and writes each flow to one local connection at the --to address while the capture runs. Read-only stays read-only: local responses are drained and discarded. Local-side failures never stop the capture; they surface when the session ends. Pure in-process Go (gopacket/pcapgo, already embedded).
📝 WalkthroughWalkthroughThis PR adds live one-way replay of mirrored TCP traffic to a local process for 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
✅MegaLinter analysis: Success✅ Linters with no issuesactionlint, bash-exec, git_diff, hadolint, jscpd, jsonlint, lychee, markdown-table-formatter, markdownlint, prettier, prettier, shellcheck, shfmt, stylelint, syft, trivy-sbom, trufflehog, v8r, v8r, yamllint Notices📣 MegaLinter 9.5.0 is out! Discover the new features and security recommendations in the release announcement. (Skip this info by defining See detailed reports in MegaLinter artifacts
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/cli/cmd/workload/mirror.go (1)
271-313: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for
--output -combined with--to.
setupLiveReplayteesoutviaio.MultiWriter(out, replay), but the new tests only exercise the default file-output path. The stdout-piping path (--output -, used for tshark piping per the docs example) combined with--tois a distinct writer chain (stdout writer + replay pipe) that isn't covered by any test in this PR.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/cli/cmd/workload/mirror.go` around lines 271 - 313, Add test coverage for the stdout piping path in captureToOutput when --output - is combined with --to. The missing case is the writer chain created by setupLiveReplay in mirror.go, where out is wrapped with io.MultiWriter(out, replay) for tshark-style piping. Extend the existing mirror command tests to exercise captureToOutput/openMirrorOutput/setupLiveReplay with opts.output set to "-" and a non-empty opts.to, and assert both the stdout writer path and replay/live output behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/svc/mirror/replay.go`:
- Around line 302-304: The FIN/RST handling in replay processing closes the flow
too early when an out-of-order FIN arrives. Update the segment handling logic in
replay.go so that the Replay/flow state only marks completion for FIN after the
FIN sequence is contiguous with f.next, buffering the FIN close point until the
gap is filled; keep RST handling separate if it should still abort immediately.
- Around line 361-365: The local replay write in LiveReplay.Write can block the
parser goroutine indefinitely, stalling capture when the local peer stops
reading. Update the f.conn.Write path to enforce a bounded timeout (for example
by setting a write deadline before writing and clearing it afterward) or route
delivery through a bounded per-flow queue with timeout handling. If the timeout
is hit, record the failure, close f.conn, and reset it so replay cannot block
capture.
- Around line 324-347: The overlap handling in replayFlow.flushPending() is too
strict because it only drains f.pending[f.next], so overlapping buffered
segments whose start is before f.next can leave a new tail undelivered; update
flushPending and the pending insertion path to advance through any buffered
segment that extends past the current sequence, not just exact matches. In
replay.go, use the replayFlow.deliver/flushPending flow to detect when a pending
payload overlaps the already-replayed region and emit the unseen suffix, and
when storing into f.pending avoid overwriting an existing longer buffered
segment with a shorter duplicate for the same or overlapping sequence range.
- Around line 181-188: The replay loop in ReadPacketData currently treats
io.ErrUnexpectedEOF the same as io.EOF, which hides truncated or malformed pcap
input as a normal shutdown. Update the error handling in replay.go so only
io.EOF is treated as clean end-of-stream, and let io.ErrUnexpectedEOF (and other
packet read errors) fall through to the existing error path that closes the
reader with a wrapped error from pcapReader.ReadPacketData.
---
Outside diff comments:
In `@pkg/cli/cmd/workload/mirror.go`:
- Around line 271-313: Add test coverage for the stdout piping path in
captureToOutput when --output - is combined with --to. The missing case is the
writer chain created by setupLiveReplay in mirror.go, where out is wrapped with
io.MultiWriter(out, replay) for tshark-style piping. Extend the existing mirror
command tests to exercise captureToOutput/openMirrorOutput/setupLiveReplay with
opts.output set to "-" and a non-empty opts.to, and assert both the stdout
writer path and replay/live output behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5927a884-39f5-49c2-9b98-1baaf11eb679
⛔ Files ignored due to path filters (2)
pkg/cli/cmd/workload/__snapshots__/workload_test.snapis excluded by!**/*.snappkg/toolgen/__snapshots__/toolsurface_snapshot_test.snapis excluded by!**/*.snap
📒 Files selected for processing (8)
docs/src/content/docs/cli-flags/workload/workload-mirror.mdxpkg/cli/cmd/workload/export_test.gopkg/cli/cmd/workload/mirror.gopkg/cli/cmd/workload/mirror_test.gopkg/svc/chat/docs_generated.gopkg/svc/mirror/doc.gopkg/svc/mirror/replay.gopkg/svc/mirror/replay_test.go
📜 Review details
⏰ Context from checks skipped due to timeout. (11)
- GitHub Check: 🧪 Test
- GitHub Check: 🧹 Lint - mega-linter
- GitHub Check: 📊 Code Coverage
- GitHub Check: 🔍 Dead Code Analysis
- GitHub Check: 🛡️ Vulnerability Scan
- GitHub Check: 🏗️ Build
- GitHub Check: 🧹 Lint - golangci-lint
- GitHub Check: 🏗️ Build KSail Binary
- GitHub Check: 🏠 Home Isolation Guard
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (go)
🧰 Additional context used
📓 Path-based instructions (1)
docs/src/content/docs/cli-flags/**/*.mdx
📄 CodeRabbit inference engine (AGENTS.md)
Keep the generated CLI reference under
docs/src/content/docs/cli-flags/in sync with the command tree; do not edit generated CLI flag docs by hand.
Files:
docs/src/content/docs/cli-flags/workload/workload-mirror.mdx
🪛 ast-grep (0.44.0)
pkg/svc/mirror/replay.go
[warning] 311-311: Narrowing a non-constant integer to a smaller fixed-width type (int8/int16/int32, uint8/uint16/uint32) can silently overflow or wrap, yielding negative or truncated values that are dangerous in size, length, or index logic. Validate the source value is within the target type's range before converting (e.g. bounds-check, or use a checked helper), and avoid narrowing untrusted or len()/parsed values.
Context: int32(sequence - f.next)
Note: [CWE-190] Integer Overflow or Wraparound.
(integer-overflow-narrowing-conversion-go)
[warning] 354-354: Narrowing a non-constant integer to a smaller fixed-width type (int8/int16/int32, uint8/uint16/uint32) can silently overflow or wrap, yielding negative or truncated values that are dangerous in size, length, or index logic. Validate the source value is within the target type's range before converting (e.g. bounds-check, or use a checked helper), and avoid narrowing untrusted or len()/parsed values.
Context: uint32(len(payload))
Note: [CWE-190] Integer Overflow or Wraparound.
(integer-overflow-narrowing-conversion-go)
🔇 Additional comments (10)
pkg/svc/mirror/replay_test.go (1)
128-134: 🎯 Functional CorrectnessNo change needed
sync.WaitGroup.Gois supported by the declaredgo 1.26.4toolchain.> Likely an incorrect or invalid review comment.pkg/cli/cmd/workload/mirror.go (4)
8-8: LGTM!Also applies to: 27-29, 58-64, 75-78
108-122: LGTM!
155-156: LGTM!Also applies to: 171-177
315-360: LGTM!pkg/cli/cmd/workload/export_test.go (1)
430-441: LGTM!pkg/svc/mirror/doc.go (1)
45-49: LGTM!pkg/cli/cmd/workload/mirror_test.go (2)
6-10: LGTM!Also applies to: 38-40, 334-376, 378-388
390-461: LGTM! Error propagation and payload assertions correctly reflect thesetupLiveReplay/finishCapturewiring inmirror.go.docs/src/content/docs/cli-flags/workload/workload-mirror.mdx (1)
20-26: LGTM! Matchesmirror.go'smirrorCmdLong/mirrorCmdExample/flag description verbatim, consistent with the generated-docs guideline.Also applies to: 43-45, 54-54
github-project-automation
Bot
moved this from 🫴 Ready
to 🏃🏻♂️ In Progress
in 🌊 Project Board
Code Coverage OverviewLanguages: Go Go / code-coverage/goThe overall coverage in the branch remains at 65%, unchanged from the branch. Show a code coverage summary of the most impacted files.
Updated |
- surface a truncated pcap stream (mid-packet EOF) via Close instead of treating it as a clean shutdown - hold an out-of-order FIN until delivery reaches its sequence point so pending payload is not dropped; RST stays an immediate abort - flush pending segments that overlap the delivered prefix (deliver the still-new tail) and keep the longer buffered copy over a shorter duplicate - bound local writes with a per-write deadline so a stalled local reader cannot block the parser and, through the pipe, the capture Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CodeRabbit review-body resolution record (
Same commit also fixes the four inline findings (truncated-pcap surfacing, FIN held until contiguous, overlapping-pending flush + longer-duplicate retention, bounded local writes) — each with a dedicated test. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/cli/cmd/workload/mirror_test.go`:
- Around line 443-461: Guard the channel receive in mirror_test so it fails fast
instead of hanging on an open replay connection. Update the test setup around
the received channel and the LiveReplay/NewLiveReplay path to wait on either the
received data or a timeout, and make the assertion report a clear failure if no
payload arrives in time. Keep the existing net.Pipe/io.ReadAll goroutine, but
bound the final receive using a timeout-aware select in the test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 37f69516-e079-4d4d-960a-bb9f491414f3
📒 Files selected for processing (3)
pkg/cli/cmd/workload/mirror_test.gopkg/svc/mirror/replay.gopkg/svc/mirror/replay_test.go
📜 Review details
⏰ Context from checks skipped due to timeout. (11)
- GitHub Check: 🏠 Home Isolation Guard
- GitHub Check: 🏗️ Build KSail Binary
- GitHub Check: 🧪 Test
- GitHub Check: 📊 Code Coverage
- GitHub Check: 🧹 Lint - golangci-lint
- GitHub Check: 🏗️ Build
- GitHub Check: 🛡️ Vulnerability Scan
- GitHub Check: 🧹 Lint - mega-linter
- GitHub Check: 🔍 Dead Code Analysis
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (go)
🧰 Additional context used
🪛 ast-grep (0.44.0)
pkg/svc/mirror/replay.go
[warning] 352-352: Narrowing a non-constant integer to a smaller fixed-width type (int8/int16/int32, uint8/uint16/uint32) can silently overflow or wrap, yielding negative or truncated values that are dangerous in size, length, or index logic. Validate the source value is within the target type's range before converting (e.g. bounds-check, or use a checked helper), and avoid narrowing untrusted or len()/parsed values.
Context: uint32(len(segment.Payload))
Note: [CWE-190] Integer Overflow or Wraparound.
(integer-overflow-narrowing-conversion-go)
[warning] 356-356: Narrowing a non-constant integer to a smaller fixed-width type (int8/int16/int32, uint8/uint16/uint32) can silently overflow or wrap, yielding negative or truncated values that are dangerous in size, length, or index logic. Validate the source value is within the target type's range before converting (e.g. bounds-check, or use a checked helper), and avoid narrowing untrusted or len()/parsed values.
Context: int32(f.finPoint-f.next)
Note: [CWE-190] Integer Overflow or Wraparound.
(integer-overflow-narrowing-conversion-go)
[warning] 410-410: Narrowing a non-constant integer to a smaller fixed-width type (int8/int16/int32, uint8/uint16/uint32) can silently overflow or wrap, yielding negative or truncated values that are dangerous in size, length, or index logic. Validate the source value is within the target type's range before converting (e.g. bounds-check, or use a checked helper), and avoid narrowing untrusted or len()/parsed values.
Context: int32(start - f.next)
Note: [CWE-190] Integer Overflow or Wraparound.
(integer-overflow-narrowing-conversion-go)
[warning] 437-437: Narrowing a non-constant integer to a smaller fixed-width type (int8/int16/int32, uint8/uint16/uint32) can silently overflow or wrap, yielding negative or truncated values that are dangerous in size, length, or index logic. Validate the source value is within the target type's range before converting (e.g. bounds-check, or use a checked helper), and avoid narrowing untrusted or len()/parsed values.
Context: uint32(len(payload))
Note: [CWE-190] Integer Overflow or Wraparound.
(integer-overflow-narrowing-conversion-go)
🔇 Additional comments (10)
pkg/cli/cmd/workload/mirror_test.go (2)
445-460: LGTM!
462-477: LGTM!Also applies to: 480-482
pkg/svc/mirror/replay.go (7)
27-42: LGTM!Also applies to: 51-52, 66-76, 97-100, 128-143
210-232: 🎯 Functional Correctness | ⚡ Quick winConsider recording the header-parse failure to
firstErrfor consistency.The new truncation path correctly does both
recordErrandCloseWithError, soClose()reliably surfacesErrReplayTruncatedCapture. The earlier header-parse branch inrun()only callsCloseWithErrorand never records tofirstErr, so a malformed pcap header is surfaced only if the writer happens to observe the pipe error on a laterWrite;Close()alone can returnnil. Recording it makesClose()authoritative for both failure modes.♻️ Proposed fix (applies to the header-parse branch)
pcapReader, err := pcapgo.NewReader(reader) if err != nil { wrapped := fmt.Errorf("read pcap header: %w", err) r.recordErr(wrapped) _ = reader.CloseWithError(wrapped) return }
279-307: LGTM!
312-360: LGTM!
364-398: LGTM!
400-430: LGTM!
437-452: LGTM!pkg/svc/mirror/replay_test.go (1)
311-354: LGTM!Also applies to: 356-393, 395-429
Fails fast with a clear message instead of hanging to the Go test timeout if a regression leaves the replay connection open. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
github-project-automation
Bot
moved this from 🏃🏻♂️ In Progress
to ✅ Done
in 🌊 Project Board
1 participant
Why
ksail workload mirrorcaptured traffic to a pcap file but never delivered it anywhere — the mirror bridge's headline promise, your locally-running service receives the mirrored requests, was still unfulfilled. The pcap was forensics, not a dev loop.What
Adds
--to localhost:<port>: while the capture runs, the mirrored inbound requests are replayed live to the local process, one local connection per cluster flow. Replay stays one-way (local responses are discarded — nothing flows back into the cluster), file output and the summary keep working unchanged, and a local-side failure never interrupts the capture. Docs regenerated in the same PR.Fixes #5791. Part of #4521 (Phase 1 — mirror-only).