Skip to content

fix(desktop): exclude unrelated VPN addresses from Tailscale pairing - #15753

Open
arthurkatcher wants to merge 1 commit into
pingdotgg:mainfrom
arthurkatcher:fix/tailscale-endpoint-identity
Open

arthurkatcher wants to merge 1 commit into
pingdotgg:mainfrom
arthurkatcher:fix/tailscale-endpoint-identity

Conversation

@arthurkatcher

@arthurkatcher arthurkatcher commented Oct 4, 2026 •

Copy link
Copy Markdown

Problem

With Proton VPN and Tailscale on the same host, desktop discovery labels Proton's 100.85.0.1 interface as “Tailscale IP” and can put it in the pairing link. Phone pairing times out even though the real Tailscale address works. The provider currently treats membership in 100.64.0.0/10 as proof of Tailscale ownership.

Closes #15750.

Change

Use the parsed Tailscale status already read by the desktop service to identify assigned IPv4 addresses. Retain the full status in the existing 60-second cache and share it between IP and MagicDNS discovery.

Keep native interface discovery working when the CLI is unavailable or its cached response predates a connection: recognize standard Tailscale adapter names and interfaces with Tailscale's IPv6 prefix. Advertise only qualifying IPv4 addresses that exist on local interfaces. This excludes the unrelated Proton interface while preserving renamed IPv4-only adapters when status identifies them.

The service and provider changes address the same endpoint-identification failure. Tests cover the Proton collision, absent assignments, unavailable/malformed/stale status, macOS and Windows adapter fixtures, renamed adapters, absent local addresses, and reuse of one cached status read.

Scope and approval

This is submitted under the guide's exception for a small, focused fix of an obvious bug: an unrelated VPN address is incorrectly labeled as Tailscale and generates an unusable pairing URL. It changes two desktop backend files with two associated test files. It adds no configuration, contracts, or new workflow. The linked issue establishes reproduction; no maintainer approval is claimed.

Related work: merged #9882 fixes LAN/Tailscale separation; closed #11920 proposed broader discovery changes. Frontend refresh and persistent endpoint preference changes have independent scope and are left for separate work.

Verification

Current base: main at 4ae976dbae39b3e80243b864a8b61da00f642dc4; head 9b225017839cc5108e40f14ec22df5691412ebed. The rebase preserves upstream Effect import paths, test-helper names, and exposure-change serialization. Linux host: Ubuntu 24.04.4 LTS.

  • vp test run apps/desktop/src/backend/tailscaleEndpointProvider.test.ts apps/desktop/src/backend/DesktopServerExposure.test.ts packages/tailscale/src/tailscale.test.ts: 36 passed on the rebased head (including the new upstream exposure-serialization test).
  • At the original base 4ee6bfd50ef4a089440d5c3662db2298da9cc50e, restoring only the two production files makes the two core Proton/cached-identity regressions fail with the extra 100.85.0.1 URL. Restoring this patch makes both pass.
  • vp lint and vp fmt --check on the four changed files: passed.
  • vp run --filter @t3tools/desktop typecheck: passed, with one advisory in untouched DesktopClerk.test.ts.
  • git diff --check: passed.
  • Before the rebase, executed original and patched endpoint providers with this host's actual os.networkInterfaces() and tailscale status --json, using Node 24.13.1:
Before: ["http://100.85.0.1:3773/", "http://100.74.126.34:3773/"]
After:  ["http://100.74.126.34:3773/"]

The original pairing failure and successful manual address replacement were observed with an iPhone. The patched provider was verified against the live host inputs; a patched packaged Electron app was not run. macOS/Windows behavior is fixture-tested, not verified on live hosts.

Compatibility limit: a custom-named adapter with no Tailscale IPv6 address and no usable CLI status cannot be identified and is omitted. A successful status read supports that adapter. CLI timeout, cache duration, exposure gating, and HTTPS probing are preserved.

Model: GPT-6-Astra. Harness: Codex in T3 Code.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Oct 4, 2026
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Oct 4, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 9b22501

Macroscope's review found this PR approvable — This is a focused desktop bug fix that filters unrelated VPN addresses from existing Tailscale pairing discovery while preserving the existing cache, exposure gate, and MagicDNS behavior. The production change is localized and supported by targeted regression tests, with no new defaults, contracts, workflows, or infrastructure.

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

@coderabbitai

coderabbitai Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 41671833-7f5b-4356-9601-8db2f8c42ec9
📥 Commits

Reviewing files that changed from the base of the PR and between 4ee6bfd and 6e87ff8.

📒 Files selected for processing (4)
  • apps/desktop/src/backend/DesktopServerExposure.test.ts
  • apps/desktop/src/backend/DesktopServerExposure.ts
  • apps/desktop/src/backend/tailscaleEndpointProvider.test.ts
  • apps/desktop/src/backend/tailscaleEndpointProvider.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The endpoint provider now reads full Tailscale status to derive the MagicDNS name and filter advertised IPv4 addresses. Desktop endpoint resolution passes its cached status to the provider.

Changes

Tailscale endpoint ownership

Layer / File(s) Summary
Read and cache Tailscale status
apps/desktop/src/backend/tailscaleEndpointProvider.ts, apps/desktop/src/backend/DesktopServerExposure.ts, apps/desktop/src/backend/tailscaleEndpointProvider.test.ts
The provider reads or parses full Tailscale status and derives the MagicDNS name from it. Desktop endpoint resolution passes cached status to the provider. Tests check that one status read supplies both the address and MagicDNS name.
Filter advertised Tailscale IPv4 endpoints
apps/desktop/src/backend/tailscaleEndpointProvider.ts, apps/desktop/src/backend/tailscaleEndpointProvider.test.ts, apps/desktop/src/backend/DesktopServerExposure.test.ts
The resolver identifies Tailscale interfaces by adapter name or a non-internal Tailscale IPv6 address. It also accepts addresses listed in status. Tests cover address filtering, interface variants, stale or missing status, and cached endpoint reads.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 6e87f

No actionable issue remains in the supplied review evidence; the change is mergeable after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 6e87f

The change narrows pairing address selection without adding a listener, changing access settings, or expanding privileges. No introduced security concern was established. Remaining uncertainty concerns concurrent refresh and interruption recovery of the shared status cache.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The security-sensitive outcome in the inspected path is connection-target selection for a desktop backend’s credential-bearing pairing links. Local interface identity and CLI status influence that selection; they do not directly configure listener binding or grant additional authority.

Trust Boundaries and Controls

  • observed — Status assignments alone cannot advertise an absent local IPv4 address: the provider iterates current local interfaces and retains the non-internal and address-range checks. The pairing consumer excludes endpoints marked unavailable, and discovery retains the existing exposure opt-in gate.

Resilience and Maintainability Implications

  • inferred — A single parsed snapshot avoids separate assignment and DNS reads within one resolver invocation. Stale status can still influence renamed-adapter qualification until refresh, but current interface presence limits its effect. The inspected change neither mutates the cached assignments nor adds a persisted ownership transition; exact concurrency and cancellation recovery remain unproven.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed #15750 requires the pairing endpoints to exclude unrelated CGNAT addresses and advertise the host’s actual Tailscale address. tailscaleEndpointProvider.ts now admits an address only when its interfa…
Out of Scope Changes check ✅ Passed The production changes and associated tests in DesktopServerExposure and tailscaleEndpointProvider implement #15750. Sharing cached status supports address identification and MagicDNS discovery in…
Title check ✅ Passed The title clearly and concisely describes the main change: excluding unrelated VPN addresses from Tailscale pairing.
Description check ✅ Passed The description covers the problem, change, scope and approval rationale, and verification. It also states compatibility limits and checks that were not performed.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@arthurkatcher
arthurkatcher force-pushed the fix/tailscale-endpoint-identity branch from 6e87ff8 to 9b22501 Compare October 6, 2026 07:11
@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 11, 2026 — with ChatGPT Codex Connector
@macroscopeapp
macroscopeapp Bot dismissed their stale review October 11, 2026 06:33

Dismissing prior approval to re-evaluate 9b22501

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Proton VPN address is advertised as Tailscale IP and breaks phone pairing

2 participants