Skip to content

Production port-selection TOCTOU: FindOrUsePort bind-close-rebind race across 7 call sites #6141

Description

@jhrozek

Summary

Same bug class as #6034/#6045, but in production code paths rather than test helpers. pkg/networking.FindAvailable()/FindOrUsePort() bind a probe listener, read the port, and close it before returning — the caller binds the real thing later, so anything else on the machine can steal the number in the gap.

Found and deliberately scoped out while fixing the test-side instances of the same root cause in test/e2e/oidc_mock.go, test/e2e/llm_gateway_mock.go, test/e2e/api_helpers.go, and three e2e test files (that PR closes the flake described in a handover doc tracking PR #6088's CI failure). This issue tracks the remaining production call sites, which weren't touched there to keep that PR to one logical change.

Call sites

In-process binders — each ultimately calls net.Listen/ListenAndServe itself, later, in the same process:

  • pkg/auth/oauth/flow.go:117
  • pkg/auth/discovery/discovery.go:586 (the dynamic-client-registration branch only — the pre-registered/CIMD branch at :599 must keep its current strict IsAvailable check, since that redirect URI is already fixed externally and can't be silently renumbered)
  • pkg/runner/config.go:583 (the proxy host port; the target port selected at :597 is not a real host bind — it becomes a container-internal env var — and is unaffected by this issue)
  • cmd/thv/app/proxy.go:203

Docker-handoff binders — the real bind happens inside a container Docker starts, after the port is already committed to a PortBindings map, so there is no listener to hold:

  • pkg/container/docker/client.go:1713 (squid ingress)
  • pkg/container/docker/client.go:1805 (main container host port)
  • pkg/container/docker/envoy.go:897 (envoy ingress — also bakes the port into a rendered bootstrap config file written to disk before the container is created, so a fix here needs to regenerate that file too, not just re-pick a port)
  • pkg/vmcp/cli/embedding_manager.go:255 (TEI container, via the portFinder field)

Suggested fix

Two different shapes, matching #6034/#6044's precedent of closing the window rather than narrowing it where the API allows:

  • In-process binders: pass port 0 into the eventual bind step and read the actual bound port back afterward (e.g. from the listener's own address, or a status/Address()-style accessor), instead of pre-selecting via FindOrUsePort and handing the number across to a later call. This is the same "hold the listener from construction" pattern just applied to the e2e mock servers in the sibling PR.
  • Docker-handoff binders: no listener to hold, so this can only be narrowed, not closed. Add a bounded retry-on-bind-conflict loop at the actual docker create/start call (catch the port-bind failure, re-pick a port, retry container creation) — mirroring the existing precedent already in this codebase at test/e2e/vmcp_dual_era_helpers_test.go:52-66 (startEraBackendOnPort), which does exactly this for a lost Docker port-bind race today.

Risk

Real-world single-user CLI contention is much lower than sharded CI, so this reads as a hardening/correctness fix rather than an active incident — no live sighting reported yet, unlike #6034's CI-flake trigger. Still worth closing given the mechanism is identical to two issues already fixed/in-flight in this codebase.

Generated with Claude Code

Metadata

Metadata

Assignees

No one assigned

    Labels

    needs-triageIssue needs initial triage by a maintainer

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions