Skip to content

feat(relay): refuse to boot if listener ports exceed expected set (#81)#86

Merged
ilmoniemi merged 3 commits into
mainfrom
feature/81
May 13, 2026
Merged

feat(relay): refuse to boot if listener ports exceed expected set (#81)#86
ilmoniemi merged 3 commits into
mainfrom
feature/81

Conversation

@ilmoniemi
Copy link
Copy Markdown
Contributor

What

Adds a boot-time set-comparison so the relay refuses to start when it is about to bind any TCP port outside the expected set derived from parsed flags. Catches the surplus-listener leak shapes called out in the spec: stray net/http/pprof on :6060, env-flipped debug ports, accidentally-enabled metrics exporters.

Pairs the runtime guard with a build-graph test that fails if net/http/pprof is in the binary's transitive imports — second defence on a different layer (compile-time vs runtime) against the highest-frequency leak shape, per the spec's "belt-and-suspenders means different fabric" guidance.

Issue

Closes #81. Split from #42. Sibling of #77 / #78 / #79.

Files

  • internal/relay/listeners.go (new) — ErrUnexpectedListener sentinel (branchable via errors.Is), ListenerPort parser (net.SplitHostPort + ParseUint(_, 10, 16); rejects :0 ephemeral-port placeholder), CheckListenerPorts (asymmetric: surplus → wrapped sentinel naming offending ports in ascending order + expected set; missing → nil).
  • internal/relay/listeners_test.go (new) — AC matrix (cases a–d in both autocert and insecure shapes), sorted-surplus determinism, multiple-surplus reporting (all listed, not just the first), errors.Is branchability, full ListenerPort value matrix including :0/:99999/:notaport/empty/no-colon/IPv6-bracket forms.
  • cmd/pyrycode-relay/main.go — builds expected and actual port sets in both branches, invokes CheckListenerPorts before the first ListenAndServe, emits os.Exit(2) (boot-time refusal, distinct from os.Exit(1) runtime listener failures) with a structured log line carrying err, unexpected_ports, expected_ports. Parse failures on --insecure-listen get their own log line + os.Exit(2). Silent on success per the spec's SHOULD-FIX.
  • cmd/pyrycode-relay/deps_test.go (new) — shells out to go list -deps -json and asserts no transitive import has ImportPath == "net/http/pprof". Skips with a clear message if go is not in $PATH.

Testing

go test -race ./... and go vet ./... pass.

  • TestListenerPort_Matrix — 12-row value matrix; the :0 row locks in the explicit ephemeral-port reject.
  • TestCheckListenerPorts_ACMatrix — verbatim AC cases (a)–(d) in both autocert ({443, 80}) and insecure ({8080}) shapes.
  • TestCheckListenerPorts_SurplusListedSorted — ascending order in the error message (locks in deterministic logging across map iteration randomisation).
  • TestCheckListenerPorts_MultipleSurplus — "report all surplus, not just the first" contract.
  • TestErrUnexpectedListener_IsBranchableerrors.Is contract.
  • TestBinaryDoesNotImportPprofgo list -deps -json build-graph assertion.

Test plan:

  • go test -race ./... passes
  • go vet ./... clean
  • go build -o pyry ./cmd/pyrycode-relay succeeds
  • Operator-facing manual check (out of scope here): a misconfigured deploy with a stray :6060 listener fails the health check with unexpected_ports=[6060] in the boot log.

Architecture compliance

Follows the spec at docs/specs/architecture/81-refuse-extra-listeners.md exactly:

  • Three new files + one edited file; no refactor of unrelated code.
  • Sentinel + parser + check shapes mirror the precedent in tls.go (ErrCacheDirInsecure) and production.go (ErrInsecureListenInProduction).
  • Exit code 2 matches the harmonised contract across relay: refuse to boot with --insecure-listen in production mode #77/relay: refuse to boot as effective uid 0 in production mode #78/relay: refuse to boot when Linux effective capabilities exceed allowlist #79 (the AC literally said 1; spec overrode to keep boot-misconfig distinct from runtime listener failures).
  • Asymmetric check: surplus is error, missing is not. Missing ports surface as EADDRINUSE-style runtime errors from ListenAndServe, which has its own exit path.
  • Port-only matching (no interface comparison); out-of-scope items deferred as documented.
  • Build-graph test uses go list -deps -json (not build-tag-gated compile check) — catches transitive imports too, fails on the offending CI run unconditionally.
  • SHOULD-FIX from the spec's security review honoured: success path emits no expected/actual port log line.

🤖 Generated with Claude Code

ilmoniemi and others added 2 commits May 13, 2026 09:52
Adds a boot-time set-comparison: ports the process is about to bind
(http.Server.Addr values) vs ports flag-parsing permits. Any actual port
absent from the expected set fails the deploy's health check before
traffic flows — catching stray pprof / metrics / debug listeners.

- internal/relay/listeners.go: ErrUnexpectedListener sentinel,
  ListenerPort parser (rejects :0), CheckListenerPorts (asymmetric:
  surplus is an error, missing is not).
- internal/relay/listeners_test.go: AC matrix (cases a–d), sorted-
  surplus determinism, multiple-surplus reporting, errors.Is branch.
- cmd/pyrycode-relay/main.go: build expected/actual sets from parsed
  flags + http.Server.Addr values; check before each ListenAndServe;
  exit 2 (boot misconfig) with structured log line carrying err,
  unexpected_ports, expected_ports.
- cmd/pyrycode-relay/deps_test.go: build-graph assertion that
  net/http/pprof is not in the binary's transitive imports — second
  defence (compile-time) against the highest-frequency leak shape.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor Author

@ilmoniemi ilmoniemi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review: #81

Decision: PASS

Findings

  • [NIT] internal/relay/listeners.go:23-32 — the explicit if addr == "" pre-check is redundant: net.SplitHostPort("") already returns an error. The custom message ("listener address is empty") is slightly cleaner than the wrapped stdlib one, so this is purely cosmetic — keep or drop as you prefer.
  • [NIT] cmd/pyrycode-relay/main.go:123-138 — in insecure mode, expected and actual are both derived from the same port variable, so CheckListenerPorts is structurally tautological in this branch (can never fail). This matches the spec verbatim, and the value is forward-looking — if a future change adds a second http.Server and updates actual but forgets expected (or vice versa), the check catches the drift. Worth a one-line comment explaining "the check is here so future drift between expected/actual is caught" if you want to make the intent obvious to a future reader; not required.

Verification

  • Spec is security-sensitive and the architect's ## Security review section is present with verdict PASS — required precondition satisfied.
  • Security-goggles walk: no tokens/secrets in diff; no file ops; one subprocess (go list -deps -json) with hardcoded args — no injection surface; no crypto changes; no new network listeners (the whole point is to prevent one); error messages contain only port numbers and operator flag values (acceptable per the spec's security review). The SHOULD-FIX from the security review (silence-on-success) is honoured — port sets are only logged in the failure branch.
  • All AC items covered: sentinel branchable via errors.Is; asymmetric check (missing → nil, surplus → wrapped sentinel naming offending ports); wiring in both insecure and autocert branches before any Listen* call; structured log fields err / unexpected_ports / expected_ports; build-graph TestBinaryDoesNotImportPprof; unit-test matrix covers cases (a)–(d) plus the sorted-determinism and multiple-surplus lock-ins.
  • go test -race ./... passes; go vet ./... clean.
  • Exit code 2 (spec override of the AC's literal 1) is consistent with sibling boot-checks #77/#78/#79/#80 — distinguishes boot-time configuration refusal from runtime listener failures.

Summary

Spec-faithful, well-tested, no leaks introduced. The asymmetric port-set check + the net/http/pprof build-graph assertion together encode the "belt-and-suspenders means different fabric" guidance. Ship it.

- features/listener-port-allowlist.md — contract, API, wiring, threat-model alignment, build-graph defence
- codebase/81.md — implementation summary, AC verification map, patterns, lessons learned
- INDEX.md — new feature entry pointer

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@ilmoniemi ilmoniemi merged commit c752b86 into main May 13, 2026
4 checks passed
@ilmoniemi ilmoniemi deleted the feature/81 branch May 13, 2026 07:03
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.

relay: refuse to boot if listener ports exceed expected set

1 participant