Skip to content

fix(client-runtime): pairing a host that reports this client's own environment id fails before spending the code - #34835

Open
juliusmarminge wants to merge 2 commits into
mainfrom
factory/issue-27359
Open

juliusmarminge wants to merge 2 commits into
mainfrom
factory/issue-27359

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Nov 12, 2026 •

Copy link
Copy Markdown
Member

EnvironmentRegistry.assertKnownAddress returned early for platform-managed environment ids, and register returned false for them, so registerPairingConnection redeemed the one-time code at /oauth/token and then returned the id as if pairing had worked (UI: 'Backend added', nothing saved). assertKnownAddress now fails with ConnectionBlockedError(reason: configuration) when the paired host reports a platform-managed id. The message names the environment it collides with and explains that this happens when one machine's T3 data was copied to another. preparePairingRegistration already calls assertKnownAddress before bootstrapRemoteBearerSession, so the code is no longer spent. register(…, { knownAddressOnly: true }) now runs that check under the lease before its platform return-false path, so the same refusal holds if a platform registration lands during the exchange. Pairings that pass expectedEnvironmentId (adding a route to a known environment) and other register callers behave as before. No regex or parser added. The two repro tests from factory/repro-27359 (primary and bearer variants) fail on the base and pass with the fix.

Fixes #27359

Reproduction on main

Commit 6ba3de7 (origin/main). Started the dev server (host A, T3CODE_DEV_SEED=0 vp run dev, server port 15140, web 7100). Copied A's .t3/userdata/environment-id into a fresh base dir and started a second server from the same checkout (node apps/server/src/bin.ts serve --base-dir <dir> --port 15240, host B). Both /.well-known/t3/environment responses reported environmentId 7e6177a1-a390-4745-b9c9-4662b3703476. In A's web client: Settings → Environments → Add environment, pasted B's pairing URL (http://127.0.0.1:15240/pair#token=…), clicked Add environment. Observed: a toast 'Backend added — The environment is saved and will reconnect on app startup.' while the list still read 'No saved remote environments'. Exchanging the same pairing code against B's /oauth/token afterwards returned HTTP 401 invalid_credential, so the code was spent. The T3 Connect half (shared relay link, flapping tunnel) was not reproduced because T3 Connect is not available on the VM. A failing test on factory/repro-27359 (onboarding.test.ts, primary and bearer variants) shows registerPairing resolving to the environment ID instead of failing with ConnectionBlockedError, after calling /oauth/token.

Evidence

On both commits: start two fresh servers sharing a copied environment ID, open Settings → Environments → Add environment on A, paste B's pairing URL, submit, then try the identical code against B's /oauth/token. Chromium, 1280×800, light theme; pairing codes masked.

Before After
6ba3de70af 2126c8964b
Before: success toast despite no saved remote environments After: duplicate environment ID explained clearly
Reports success, saves nothing; subsequent code exchange returns HTTP 401. Shows the duplicate-ID error; subsequent code exchange returns HTTP 200, proving the code was preserved.
Before recording After recording

Recordings show the scripted interaction at half speed. Stills were checked for identical pixels 500 ms apart.

Scenario/browser driver · Token-exchange observations · Sanitized verification log

AI-assisted; verified with the tests and tools listed above.

Not shown:

  • T3 Connect shared relay-link and flapping-tunnel behavior: T3 Connect is unavailable on this VM.
  • Desktop shell and native mobile clients were not exercised; the demonstrated flow uses the web client.

Tests

vp test run src/connection/onboarding.test.ts src/connection/registry.test.ts (packages/client-runtime): 60 passed; the same onboarding file without the fix: 2 failed (the repro tests), 15 passed. vp run check:changed: format, lint, knip, typecheck (7 packages) and tests (2 files) all pass.

Not in this change

  • The T3 Connect half of the issue (two hosts sharing one relay link and a flapping tunnel) is not addressed and could not be reproduced on the VM.
  • Deriving the server environment ID from hardware so copied data does not duplicate it is a server-side design change and was not made.
  • registerPairingConnection still returns the id when register returns false for a pairing with expectedEnvironmentId set to a platform-managed environment; the UI does not appear to offer that path.
  • Open PR fix(client-runtime): pairing a host that reports this machine's own environment ID shows an error instead of silently doing nothing #27371 proposes a different client-side fix for the same bug; it may need closing or reconciling with this one.

AI-assisted; verified with the tests and tools listed above.

juliusmarminge and others added 2 commits November 12, 2026 07:01
…nt id fails without spending the code

Reproduces #27359. Fails on main: registerPairing redeems the code and
returns the id as if it succeeded.

AI-assisted; verified with the tests and tools listed above.
…vironment id fails before spending the code

Fixes #27359

assertKnownAddress skipped platform-managed environment ids, and register
returned false for them, so pairing redeemed the one-time code and reported
success while saving nothing. The pre-exchange check now refuses a
platform-managed id with a clear error, and register runs the same check
under the lease before its platform short-circuit.

AI-assisted; verified with the tests and tools listed above.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted Author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Nov 12, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 12, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 2126c89

Macroscope's review found this PR approvable — Pairing now reports an environment-ID collision before spending the one-time code, with a second check before registration. The change is limited to rejecting unsupported collisions and leaves existing saved environments unchanged.

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

@coderabbitai

coderabbitai Bot commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The registry now rejects known addresses that report a platform-managed environment ID. The pairing test verifies that the attempt returns a configuration error, requests only the environment descriptor, and leaves the registry unchanged.

Changes: Platform-managed ID pairing

Layer / File(s) Summary
Registry validation and pairing behavior
packages/client-runtime/src/connection/registry.ts, packages/client-runtime/src/connection/onboarding.test.ts
assertKnownAddress rejects platform-managed IDs with a configuration error. register performs known-address validation before its platform-managed-ID check. The pairing test checks the error, confirms that only the environment descriptor was requested, and verifies that the registry remains unchanged.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Possibly related PRs

  • pingdotgg/t3code#27371: Implements an overlapping fix for pairing with a host that reports the client’s platform-managed environment ID.
  • pingdotgg/t3code#26178: Introduces the knownAddressOnly option and assertKnownAddress behavior changed here.
  • pingdotgg/t3code#27446: Changes the same pre-redemption pairing path and tests the opposite outcome for platform-managed IDs.

Suggested reviewers: t3dotgg, macroscopeapp, maria-rcks

Merge Risk: Minimal · up to 2126c

Pairing with a host that reports the client's own platform-managed environment ID now fails clearly before the one-time code is spent. No actionable merge-blocking risk remains. The relay and machine-identity parts of the linked issue are intentionally not addressed here.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title names the fix in the client-runtime pairing path: a host that reports this client's own environment ID now fails before the one-time code is spent. The raw summary and objectives confirm the…
Linked Issues check Passed Directly linked issue #27359 expects LAN pairing against a host that reports the client's own environment ID to fail with a clear error, not silently. At the reviewed head, registry.ts `assertKnownAdd…
Out of Scope Changes check Passed The whole-PR diff (merge base 6ba3de7 to head 2126c89) changes two files in packages/client-runtime/src/connection: registry.ts and onboarding.test.ts. registry.ts changes the platform-managed c…
Description check Passed The description is thorough and covers the required template sections. Problem: assertKnownAddress returned early for platform-managed environment IDs, so registerPairingConnection redeemed the one-ti…
Full details: Title check

Explanation

The title names the fix in the client-runtime pairing path: a host that reports this client's own environment ID now fails before the one-time code is spent. The raw summary and objectives confirm the change: assertKnownAddress now rejects platform-managed IDs with a configuration ConnectionBlockedError ahead of bootstrapRemoteBearerSession. The title is specific and uses a conventional commit prefix. It is somewhat long (112 characters), but it captures the primary change.

Full details: Linked Issues check

Explanation

Directly linked issue #27359 expects LAN pairing against a host that reports the client's own environment ID to fail with a clear error, not silently. At the reviewed head, registry.ts assertKnownAddress now returns ConnectionBlockedError (reason configuration) when the route's environment ID is in platformEnvironmentIds, with a detail naming the colliding environment and explaining copied T3 data. preparePairingRegistration in onboarding.ts calls this check before bootstrapRemoteBearerSession when expectedEnvironmentId is unset, so the one-time code is not redeemed. In register, the knownAddressOnly check now runs under the lease before the platform return false, which covers a platform registration that lands during the exchange. The onboarding test was updated to expect the configuration error and a registry that is unchanged. This satisfies the reported 'silent, code spent' behavior [#27359]. Not implemented: suggestion (a) machine-identity-based ID regeneration and suggestion (c) relay/CloudLink refusal for a second host with a live connector [#27359]. These are suggested handling in the issue, and the PR objectives state they are out of scope for this PR. Residual gap: when expectedEnvironmentId is set to a platform-managed ID, the early check is skipped and register returns false, so registerPairingConnection still returns the ID silently. The PR description discloses this path and states the UI does not appear to offer it. Test status was not re-run in this assessment; the reviewed code supports the stated behavior.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff (merge base 6ba3de7 to head 2126c89) changes two files in packages/client-runtime/src/connection: registry.ts and onboarding.test.ts. registry.ts changes the platform-managed check in assertKnownAddress, reorders the knownAddressOnly check in register, and updates the API doc comment. These directly implement the objective in #27359. The onboarding.test.ts change updates the platform-managed pairing test to expect the new configuration error and an unchanged registry, which supports the same objective. No unrelated files or behavior changes were found in the reviewed diff. The PR description lists the T3 Connect relay work and ID regeneration as not included, so it does not claim them.

Full details: Description check

Explanation

The description is thorough and covers the required template sections. Problem: assertKnownAddress returned early for platform-managed environment IDs, so registerPairingConnection redeemed the one-time code and reported success while saving nothing. Change: assertKnownAddress now fails with ConnectionBlockedError (reason: configuration) for a platform-managed ID, and register runs the known-address check under the lease before its platform return-false path. Scope: it links issue #27359 as the bug source, which satisfies the bug-fix-only rule. Verification: it names specific tests (onboarding.test.ts primary and bearer repro variants, 60 passing across two files, fails on base), lists the check:changed commands, gives a reproduction on main with observed results, and includes before/after evidence. It also states what was not checked (T3 Connect relay behavior, desktop and mobile clients). The template asks for an agent model and harness at the end when an agent was used; the description says 'AI-assisted' but does not name the model and harness, which is a minor gap. The title is not included in the provided description, so conventional-commit title compliance cannot be assessed here.

Finishing Touches

Generate unit tests (beta)

  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between dbfbe39 and 2126c89.

Files selected for processing: 2

  • packages/client-runtime/src/connection/onboarding.test.ts
  • packages/client-runtime/src/connection/registry.ts

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

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

size:S 10-29 changed lines (additions + deletions). vouch:trusted Author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pairing silently creates nothing when both hosts share a copied environment ID

1 participant