fix(receipt): finalize and guard all egress paths

[Desperado]

Summary

• finalize signed receipts on command failures and REPL signal exits
• record opt-in telemetry and VNC WebSocket handshakes through the egress chokepoint
• avoid signing partial request-body hashes and strengthen the AST-based egress guard
• isolate provider-env tests from real keychain values and prevent env-map logging

Verification

• go test ./...
• go vet ./...
• pre-commit run --all-files (not configured in this repository)

[sigilix]

Sigilix Overview

Effort: 4/5 (large)

Quality gates

• ✅ PR title follows convention
• ✅ PR description is complete
• ℹ️ PR is linked to an issue — No Closes #N / Closes SIG-N keyword found in PR body or commit messages.

Summary — latest push

Finalizes and guards all egress paths by preventing partial request-body hashes from being signed as complete, redacting transport error details from receipts, and recording VNC WebSocket handshakes through the egress chokepoint. The change introduces a mutex-protected hashingBody that tracks stream completion, adds a DialWebSocket helper with a bounded handshake timeout, and uses an explicit category context key for protocol handshakes. A specialist review flagged security and logic risks around the redaction approach and concurrency handling that must be addressed before merging.

Important files

File	Score	Notes	Next step
internal/httpx/httpx.go	5/5	Core egress logic changes: adds mutex-protected hashingBody to track complete reads, redacts transport errors, introduces DialWebSocket with a 15s timeout, and adds category context for VNC handshakes.	Verify that the transportErrorNote redaction does not suppress actionable, non-sensitive error types (e.g. context cancellation) that should be distinguished from a generic transport failure, and confirm the hashingBody mutex protects against concurrent reads if RoundTrip is ever invoked concurrently on a pipelined request.
main.go	5/5	Replaces all bare os.Exit calls with exitWithReceipt and reorders defers so telemetry flushes are recorded by the active receipt before finalization.	Confirm that exitWithReceipt safely handles re-entrant or concurrent calls, given it now governs all process exits and finalization logic.
internal/httpx/guard_test.go	5/5	Replaces the string-matching egress guard with a full AST-based scanner that catches import aliases, new() constructors, and direct net.Dial calls, removing the VNC carve-out.	Ensure the VNC package now uses httpx.DialWebSocket and that removing its sanctioned carve-out does not break existing VNC stream functionality.
internal/httpx/httpx_test.go	4/5	Extensive new test coverage for incomplete body hashing, transport error redaction, WebSocket handshake recording, and note appending logic.	Add a concurrent read test for hashingBody.snapshot() to guarantee the mutex behaves correctly under race conditions, and add a test ensuring DialWebSocket respects context cancellation during the handshake.
exit_test.go	4/5	Tests exitWithReceipt finalization order and concurrent finalizer reconfiguration to ensure no deadlocks or missed finalizations on exit.	Add a test verifying that a nil finalizeBeforeExit does not panic and that processExit is still called correctly.

Sequence diagram

sequenceDiagram
    participant C as Caller
    participant D as DialWebSocket
    participant R as receiptTransport
        participant H as hashingBody
    participant S as Remote Server
    C->>D: DialWebSocket(ctx, url, opts)
    D->>D: WithCategory(ctx, vnc-control)
    D->>R: RoundTrip(req)
    R->>H: Wrap req.Body
    R->>S: Send Request
    S-->>R: Response / Error
    R->>H: snapshot() [mutex locked]
    H-->>R: bytes, digest, complete
    R->>R: Record Entry in Receipt
    R-->>D: Response / Error
    D-->>C: Conn / Error

Loading

Confidence: 2/5

The specialist review flagged P1/P2 security and logic risks around error redaction and concurrency handling that remain unaddressed in the current diff.

• Verify that discarding the transport error string in transportErrorNote (line ~225) does not suppress necessary, non-sensitive diagnostic information that a customer or support engineer would need to differentiate connection timeouts from context cancellations.
• Confirm the hashingBody mutex (line ~148) is strictly necessary and correctly scoped; if RoundTrip is never called concurrently on the same body, the mutex adds overhead without benefit, but if it can be, the current lock granularity needs validation under race conditions.
• Check the 15-second webSocketHandshakeTimeout (line ~68) is appropriate for all VNC targets and that it correctly bounds only the handshake without leaking into the upgraded stream's lifetime as the comment suggests.
• Ensure the categoryKey context value (line ~66) cannot be spoofed or overridden by downstream callers to misclassify egress traffic in the signed receipt.
• Validate that the empty SHA-256 hash assigned when hb is nil (line ~205) is the correct sentinel value for the receipt schema and does not create ambiguity with a legitimate zero-byte payload.

Suggested labels: security

---

_{Posted · fed30ee · 5 findings — View review
Proof: 5 model-only
runner-verified = CI receipt · reproduced = sandbox observed diff · grounded = deterministic detector/worker-token · model-only = model judgment only
Dismiss @sigilix dismiss <reason> (not-a-bug | bad-anchor | already-covered | too-minor | wrong-context) · Re-run /sigilix review · Review #4
Sigilix · 3 of 50 reviews used in past 5h}

[qualitymaxapp]

✅ QualityMax Pipeline

Gate	Result
🔍 AI Review	✅ Clean
🧪 Repo Tests	✅ 470/470 passed (go)
🤖 AI Tests	✅ 47/51 passed

_{Powered by QualityMax — AI-Powered Test Automation}