Skip to content

fix(ssh): pair reused servers with their own CLI version - #17550

Open
voltcrash wants to merge 4 commits into
pingdotgg:mainfrom
voltcrash:t3/fix-issue-16984
Open

voltcrash wants to merge 4 commits into
pingdotgg:mainfrom
voltcrash:t3/fix-issue-16984

Conversation

@voltcrash

@voltcrash voltcrash commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Problem

After a desktop update, SSH reconnect can reuse an older remote server while the desktop-pinned CLI writes its pairing grant into the shared database. The older server cannot decode newly added scopes, so token exchange returns HTTP 500 and the environment keeps reconnecting.

Fixes #16984.

Change

Before archive-based pairing, read the running server's version from its public descriptor through the existing SSH forward and use that release's CLI to mint the grant. This keeps the grant issuer's scopes and persistence format aligned with the consumer without introducing a server restart. Version discovery has a bounded timeout and rejects invalid release versions before minting a grant.

Write the pairing runner to pair-t3.sh so choosing another version cannot overwrite the desktop-pinned launch runner and affect later restart decisions. Development runners continue using their configured Node script.

Scope and approval

The maintainer triage comment establishes the version-skew failure and the need to preserve active remote work. This PR fixes only the SSH pairing issuer. Server lifecycle policy, wire contracts, providers, and UI behavior are unchanged.

Verification

  • vp test run packages/ssh/src/tunnel.test.ts: all 29 tests pass. New cases execute the generated pairing shell against installed CLI fixtures for older, matching, newer, and development runners; repeated pairing preserves the launch script. HTTP failures, malformed descriptors, invalid versions, and discovery timeouts mint no grant.
  • Ran the older-server regression against the original source: it fails by selecting the desktop CLI. The same case passes with the fix.
  • vp run --filter @t3tools/ssh typecheck, targeted vp lint, targeted vp fmt --check, and git diff --check: pass.
  • On macOS arm64, downloaded and checksum-verified the reported October 4 and October 7 archives. Using an isolated temporary database, the October 7 CLI's grant returned 500 access_token_issuance_failed from the October 4 server. The matching October 4 CLI's grant returned 200, and /api/auth/session confirmed an authenticated session on the same server PID. The isolated server was then stopped.

The Linux desktop/Tailscale flow was not exercised. An additional desktop IPC test run stalled during suite loading and was stopped; it is not counted as passing. No UI code changed, so screenshots are not applicable.

Model: GPT-6.1-sol. 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 9, 2026
@coderabbitai

coderabbitai Bot commented Oct 9, 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: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: add661b3-f7cd-411e-8800-77da07d03cac


📥 Commits

Reviewing files that changed from the base of the PR and between c52a763 and 7598801.



📒 Files selected for processing (2)
  • packages/ssh/src/tunnel.test.ts
  • packages/ssh/src/tunnel.ts


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




📝 Walkthrough
📝 Walkthrough

Walkthrough

SSH pairing now selects an archive runner using the running remote server’s version and writes its script to a separate file. Node-script runners bypass version discovery. Tests cover successful pairing, repeated pairing, and discovery failures.

Changes

SSH pairing runner selection

Layer / File(s) Summary
Resolve and use the pairing runner
packages/ssh/src/tunnel.ts, packages/ssh/src/tunnel.test.ts
Before issuing a pairing token, archive runners are resolved from the remote server’s environment descriptor. Discovery or version validation failures produce SshPairingError. Node-script runners bypass discovery. Pairing writes to pair-t3.sh; tests cover version matching, repeated pairing, and discovery failures.

Priority: ⬆️ High

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

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: juliusmarminge



Merge Risk: ⚪ Minimal · up to 75988

No actionable issue remains before merge. Pairing correctly stops when it cannot determine the running server’s version.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 75988

Version validation, existing SSH authentication, and separate pairing scripts limit the change’s exposure. No introduced security defect was established, but historical release behavior and concurrent server replacement remain incompletely verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — A descriptor-controlling process can influence which valid release executes with the existing SSH account’s privileges. Runner installation and grant creation remain in that account’s remote home and shared server state; no additional desktop execution or cloud identity authority was established.

Trust Boundaries and Controls

  • observed — The descriptor is a new executable-version selection input, but it must match the exact-version pattern and is shell-quoted by the existing builder. It cannot directly supply a release URL, filesystem separator, or shell command through serverVersion. Discovery failures do not fall back to minting with the desktop-selected CLI.

Resilience and Maintainability Implications

  • inferred — Discovery and grant issuance are separate operations, so an out-of-process server replacement can invalidate the discovered version. Likewise, issuance can complete remotely before transport or output validation fails, without rollback in this issuer. The latter failure mode already existed at the PR base; neither condition establishes a new authorization bypass.

Hardening Proposals

  • proposed — A per-invocation pairing script with scoped cleanup could further contain cross-process write races. This would strengthen a pre-existing shared-file pattern, not remediate a verified PR-introduced vulnerability.

Pre-merge checks | Passed 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check Passed Issue #16984 requires the reused SSH environment to reconnect without unsafe server replacement. packages/ssh/src/tunnel.ts reads /.well-known/t3/environment through the SSH forward, applies a bou…
Out of Scope Changes check Passed The changes stay within the SSH pairing fix for issue #16984. The production changes modify runner selection and server-version discovery in packages/ssh/src/tunnel.ts. The added tests in `packages/…
Description check Passed The description includes all required sections. It explains the version-skew problem, the implementation, scope approval, focused verification, limitations, and agent details.
Title check Passed The title clearly and concisely describes the main change: SSH pairing uses the reused server's CLI version.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR



  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@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 commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This changes production SSH pairing authentication by discovering the running server version and using that version's CLI to mint credentials, with new failure and timeout behavior. The scope is focused and tested, but authentication-sensitive runtime changes warrant human review.

Notes:

  • All code in this push has already been reviewed. Approvability was decided on eligibility alone.

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

Comment thread packages/ssh/src/tunnel.ts Outdated

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]: SSH environment stays reconnecting after desktop update reuses an older remote server

2 participants