Sign up
View stargazersSign up

fix(server): omit Windows host-only adapters from LAN routes#25217

merged1
main
maria/takeover-pr-21056
opened6d agobyMmaria
2 files changed
View on GitHubTwo-way sync enabledPR last imported 1d ago

Problem

Windows VirtualBox and VMware host-only adapters can have localized or renamed connection names, bypassing the interface-name filter and being advertised as LAN routes.

Change

Exclude VirtualBox 0a:00:27:* and VMware VMnet host 00:50:56:c0:00:* MACs from automatic wildcard-bound Windows endpoint discovery. Keep explicit binds, guest NICs, LAN/Tailscale classification, and non-Windows discovery unchanged.

The VMware prefix is a host-adapter heuristic. A custom guest or bridge using a matching MAC is also omitted on Windows; explicit binding remains available. Desktop and CLI pairing-address selection are separate paths outside this fix.

Verification

  • Added one regression in the existing test file first: main failed on three leaked host routes; nine other tests passed. All 10 pass after the fix, covering guest MAC ranges, uppercase MACs, Linux/macOS, explicit binds, LAN, and Tailscale.

  • Blacksmith: vp run check:changed passed formatting, lint, knip, generated-operation checks, affected typechecks, and focused tests.

  • Production DirectEndpoints.make → resolve() runtime check with injected platform/interface fixtures: Windows wildcard routes drop from five to three; LAN, guest, and Tailscale remain. Linux's five routes and explicit host-only binding are unchanged.

  • Unverified: native Windows with real VirtualBox/VMware adapters and paired-client route persistence. This is a server discovery change with no UI rendering change.

  • Final-head GitHub CI passed: lint, build/typecheck, package tests, all eight server shards, and aggregate Check. Macroscope correctness passed. Two independent GPT-6.1-Sol reviews found no actionable issues on 501c6bf519c2470faccaf902da646f580b1f9acd.

  • Known review limits: CodeRabbit reported its organization review quota exhausted. Macroscope approvability requires human approval for this production discovery change involving credential-bearing HTTP routes; this PR remains unmerged and waiting for Maria's approval. No outstanding correctness findings.

Takeover

Replaces #21056, with thanks to @SunkenInTime for the original implementation.

Built with GPT-6.1-Sol in the Codex harness on T3 Code.

Checks

22 successful · 13 skipped · 1 neutral
All checks passed
501c6bf

Added 1 commit

  • fix(server): omit Windows host-only adapters from LAN routes501c6bf
CcoderabbitaiBot6d ago
Synced from GitHub

Warning

Review limit reached

Only developers with an assigned seat can use this organization's usage-based review budget, and seats here are assigned manually. Ask an admin to assign a seat, or change the review continuation mode in Billing.

Next included review available in 25 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

External image Run configuration
External image Commits

Reviewing files that changed from the base of the PR and between fe585bc5b67ac826ccf4fd05e7c7a5641d5342bd and 501c6bf519c2470faccaf902da646f580b1f9acd.

External image Files selected for processing: 2
  • apps/server/src/environment/DirectEndpoints.test.ts
  • apps/server/src/environment/DirectEndpoints.ts
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

MmacroscopeappBot6d ago
Synced from GitHub

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — This focused fix changes Windows production endpoint discovery and suppresses host-only LAN routes before they are advertised to clients. Because those routes can carry credentials over HTTP, the security-sensitive runtime behavior warrants human review.

You can add or adjust custom eligibility rules. Learn more.

Ggithub-actionsBot6d ago
Synced from GitHub

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ProviderMetricMain baselineThis PRImpactPR ceiling
CodexTotal thread wire5.0 KiB5.0 KiB+7 B (+0.1%)6.8 KiB✅
CodexThread snapshot wire3.8 KiB3.8 KiB0 B (0.0%)4.9 KiB✅
CodexLive turn WebSocket wire1.2 KiB1.2 KiB+7 B (+0.6%)2.0 KiB✅
CodexLive turn WebSocket decoded20.9 KiB20.9 KiB0 B (0.0%)29.3 KiB✅
CodexLive turn messages220 (0.0%)8✅
ClaudeTotal thread wire5.0 KiB5.0 KiB−24 B (−0.5%)6.8 KiB✅
ClaudeThread snapshot wire3.8 KiB3.8 KiB0 B (0.0%)4.9 KiB✅
ClaudeLive turn WebSocket wire1.2 KiB1.2 KiB−24 B (−2.0%)2.0 KiB✅
ClaudeLive turn WebSocket decoded21.2 KiB21.2 KiB−41 B (−0.2%)29.3 KiB✅
ClaudeLive turn messages21−1 (−50.0%)8✅

Baseline: fe585bc · PR result: 501c6bf · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

Mmaria4d ago

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

No changed runtime behavior could be verified on this Linux Boat VM. Result: unverifiable, not verified.

Read the diff, description, discussion and commit authors at 501c6bf519c2470faccaf902da646f580b1f9acd. The PR is open and its commit authors are maria-rcks and SunkenInTime; neither stop-condition author is present. Inspected the real host using uname -a, process.platform and os.networkInterfaces(): Linux, with no VirtualBox/VMware host-only adapters. Environment inspection transcript.

The production change filters Windows-only MAC prefixes during wildcard-bound direct endpoint discovery. The server returns those routes through its config RPC; the shared client connection supervisor consumes them. Linux cannot enter the changed platform === "win32" branch. Existing CI reports 29 successful checks, but that does not establish native runtime behavior.

Not verified

The original bug on github/main, suppression of renamed/localized Windows VirtualBox/VMware host-only routes on this head, client route persistence, and neighboring explicit-bind, guest-NIC, LAN and Tailscale behavior remain unverified. No main worktree or PR checkout, dependency installs, dev servers, browser sessions, provider turns, or new checks were run: none could reproduce this Windows-only defect with real adapters here. No screenshots or video were captured because no changed interaction was exercised. No code was changed.

Native verification needs a Windows host with real VirtualBox/VMware host-only adapters: wildcard-bind main and this exact head, connect a real client, inspect advertised/persisted direct routes, and confirm those host-only routes disappear while LAN, guest and Tailscale routes remain. Also confirm an explicit host-only bind remains advertised. macOS, packaged desktop and native iOS/Android were not exercised.

Mmaria4d ago

Note

Written by gpt-6.1-sol on behalf of Maria

Verified on Android emulator (API 35) against a real dev server

Not verified. No emulator interaction or real dev-server verification was completed in this pass.

Read the diff, discussion, commit authors and current status for head 501c6bf519c2470faccaf902da646f580b1f9acd. The PR is open, with no commits by juliusmarminge or t3dotgg. Existing CI reports 29 successful checks; these are not evidence of Android runtime verification.

The initial T3 device_list call returned: Device support is off. Ask the user to enable it in the Device panel before installing or starting device tools. The repository's test-t3-mobile skill requires stopping verification when T3 device tools are unavailable. No emulator, install, backend, Metro, provider turn or capture was started. No code was changed.

Not verified

Android pairing, real Codex gpt-6.1-sol turns, client route persistence, neighboring flows, before/after captures and recording remain unverified. iOS cannot run on this Linux VM and was not exercised.

The changed production branch requires a Windows server (platform === "win32") with real VirtualBox/VMware host-only adapters. An Android client against this Linux dev server cannot reproduce the original defect or prove suppression of those routes without mocks. Once device access is enabled, Linux-to-Android integration can be checked, but native Windows server verification is still needed for the actual fix.