Fix v1 endpoint port pre-probe race (bind 0.0.0.0:0 + readback)#1513
Merged
Conversation
ApprovabilityVerdict: Approved Straightforward bug fix for a port binding race condition. The change replaces a get-then-bind pattern with bind-to-zero-and-readback, which is a well-known correct approach for avoiding TOCTOU races. You can customize Macroscope's approvability policy. Learn more. |
Endpoint pre-allocated its interception-server port at construction via get_free_port(), which binds a throwaway socket to 127.0.0.1:0, reads the number, and closes it. InterceptionServer then bound that cached port on 0.0.0.0 at the first rollout. That left two defects on the shared v1 path: - the port was unreserved between Endpoint construction and the first rollout (a TOCTOU race — another port consumer can take it), and - it was validated on 127.0.0.1 but bound on 0.0.0.0, so a port free on loopback could already be taken on another interface and the real bind could collide. Fix: stop pre-probing. Hand InterceptionServer port 0 so it binds 0.0.0.0:0 and adopts the OS-assigned port via the getsockname() readback that already existed in InterceptionServer.start() (previously dead code on the v1 path). Endpoint now reads server.port directly; it is 0 only in the construct->start() window, which the rollout path never observes (register_rollout calls start() before building any URL). Probe and bind become the same held operation on the same interface. Explicit Endpoint(port=...) construction is unchanged. This matches the bind-and-readback pattern already used by cli_agent_env and rlm_env. Verified: ruff clean; test_v1_endpoint_protocols.py and test_interception_utils.py (33 tests) pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1ef2d92 to
35f71bf
Compare
mikasenghaas
approved these changes
Jun 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Endpoint pre-allocated its interception-server port at construction via get_free_port(), which binds a throwaway socket to 127.0.0.1:0, reads the number, and closes it. InterceptionServer then bound that cached port on 0.0.0.0 at the first rollout. That left two defects on the shared v1 path:
Fix: stop pre-probing. Hand InterceptionServer port 0 so it binds 0.0.0.0:0 and adopts the OS-assigned port via the getsockname() readback that already existed in InterceptionServer.start() (previously dead code on the v1 path). Endpoint.port is now a property over server.port, so it always reflects the live socket and is 0 only in the construct->start() window, which the rollout path never observes. Probe and bind become the same held operation on the same interface.
Explicit Endpoint(port=...) construction is unchanged. This matches the bind-and-readback pattern already used by cli_agent_env and rlm_env.
Type of Change
Testing
uv run pytestlocally.Checklist
Note
Low Risk
Narrow change to how the interception server picks and exposes its listen port; explicit port construction is unchanged and rollout paths already start the server before URLs are built.
Overview
Fixes a port allocation race on the v1
Endpointpath by stopping the upfrontget_free_port()probe and lettingInterceptionServerbind0.0.0.0:0(or an explicit port) and adopt the OS-assigned port via the existinggetsockname()readback instart().Local URLs and tunnels now use
self.server.portafter the server is listening, so the advertised port matches the live socket instead of a number that was only reserved briefly on loopback.Reviewed by Cursor Bugbot for commit 994e4bb. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Fix pre-probe port race in v1
Endpointby binding to0.0.0.0:0and reading back the assigned portPreviously,
Endpoint.__init__calledget_free_port()to select a port before binding, creating a race window where another process could claim that port. Now,InterceptionServeris started with port0so the OS assigns an ephemeral port, andurl_baseandget_tunnel_urlread the actual bound port fromself.server.port. Theself.portattribute andget_free_portimport are removed from endpoint_utils.py.Macroscope summarized 994e4bb.