Skip to content

audit: enforce declared size, pin PQ curve, gate relay-status, drop dead code - #36

Merged
polius merged 1 commit into
mainfrom
audit/followup-findings
Jun 9, 2026
Merged

polius merged 1 commit into
mainfrom
audit/followup-findings

Conversation

@polius

@polius polius commented Jun 9, 2026

Copy link
Copy Markdown
Owner

Follow-up to a senior production-readiness audit of the project. Each change is empirically verified — the full -race suite, golangci-lint, and deadcode -test are all green.

Findings addressed

# Sev Finding Fix
1 MEDIUM Receiver didn't enforce the sender's declared FileInfo.Size A PAKE-authenticated but misbehaving sender could advertise a small size yet stream unbounded chunks (the root-hash check only runs after the loop), exhausting the receiver's disk. The receive loop now rejects once bytesWritten + len > info.Size for non-streaming files; streaming items (stdin/--text) stay exempt by design.
3 LOW Three production methods with no production caller Removed connpath.Info.Glyph (its logic was duplicated inline in printPath), signaling.Client.Health (the Docker probe uses a raw http.Get), and quicconn.Listener.LocalAddr, plus their now-redundant tests.
4 LOW --connect default,<pw> silently dropped the password Now returns E024 — --connect default takes no password.
5 LOW --connect conflict list omitted --exclude Added exclude and mode (both transfer-only) to the conflict list.
6 LOW Post-quantum guarantee was unpinned and untested Pinned CurvePreferences = {X25519MLKEM768, X25519} explicitly in both TLS configs so a toolchain default change or GODEBUG=tlsmlkem=0 can't silently downgrade the advertised PQ exchange. New TestNegotiatesPostQuantum asserts two peers land on the hybrid on the wire.
7 LOW /v1/relay/status lacked per-session auth Bearer-gated with the session role token, matching /v1/relay/allocate. A third party who only sniffed the session_id could previously probe eviction state; missing-session and unauthorized callers now both return "unknown" (no existence oracle).

Finding #2 (the server unconditionally trusts X-Real-IP/X-Forwarded-For — safe behind the shipped Caddy config, a footgun for direct-exposure self-hosters) is intentionally not in this PR; it needs a deployment-policy decision (TrustProxyHeaders config vs. docs-only).

Verification

Notes

  • While wiring Bump actions/attest-build-provenance from 2 to 4 #3, swapping the removed Health() for Create() in two transport-error tests made internal/signaling hang under -race (a POST-with-body teardown left the test's hanging server handler alive, blocking httptest.Close). Switched those to the bodyless RelayStatus GET.
  • Bump actions/checkout from 4 to 6 #1 has no bespoke adversarial test: a protocol-faithful malicious sender would be ~100 lines of fragile wire-mirroring, the happy path (no false-trigger) is already covered by every roundtrip/e2e test, and the negative branch is trivially correct.

🤖 Generated with Claude Code

…ead code

Follow-up to the production-readiness audit. Each change is empirically
verified (race suite, golangci-lint, deadcode -test all green).

- transfer: enforce the sender's declared FileInfo.Size in the receive
  loop. A PAKE-authenticated but misbehaving sender could advertise a
  small size yet stream unbounded chunks (the root-hash check only runs
  after the loop), exhausting the receiver's disk. Streaming items are
  exempt by design.

- quicconn: pin CurvePreferences to {X25519MLKEM768, X25519} explicitly
  instead of relying on Go's default ordering, so a toolchain default
  change or GODEBUG=tlsmlkem=0 can't silently downgrade the advertised
  post-quantum exchange. New TestNegotiatesPostQuantum asserts two peers
  land on the hybrid on the wire.

- server: bearer-gate /v1/relay/status with the session role token,
  matching /v1/relay/allocate. A third party who only sniffed the
  session_id could previously probe eviction state; missing session and
  unauthorized caller now both return "unknown" (no existence oracle).

- cli: reject `--connect default,<pw>` (stray password was silently
  dropped) and add --exclude/--mode to the --connect conflict list.

- dead code: remove three production methods with no production caller
  (connpath.Info.Glyph — logic was duplicated inline in printPath;
  signaling.Client.Health — Docker probe uses a raw http.Get;
  quicconn.Listener.LocalAddr) and their now-redundant tests.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@polius
polius merged commit 0216b95 into main Jun 9, 2026
0 of 16 checks passed
@polius
polius deleted the audit/followup-findings branch June 11, 2026 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant