Skip to content

fix(ssh): slow SSH hosts connect without stacking retries and pairing tokens - #18292

Closed
im-kvijay wants to merge 3 commits into
pingdotgg:mainfrom
im-kvijay:fix/ssh-connect-retries
Closed

im-kvijay wants to merge 3 commits into
pingdotgg:mainfrom
im-kvijay:fix/ssh-connect-retries

Conversation

@im-kvijay

Copy link
Copy Markdown
Contributor

Problem

Connecting the desktop app to an SSH environment on a slow host takes 25–60 s, even though SSH and the remote server itself respond quickly (#18229). The reporter's host spends about 5.5 s of CPU on every t3 start, and a connect starts t3 many times.

The connection supervisor gives each attempt 15 s, then retries. The renderer asks the desktop to set up the environment with ensureSshEnvironment(target, { issuePairingToken: true }) on every attempt. The desktop runs that setup under a per-target lock, and IPC cannot cancel it. A timed-out attempt therefore leaves its setup running. The next attempt queues a new setup behind it, so every retry runs t3 auth pairing create (about 7 s on that host) to mint a pairing credential that nobody uses. The reporter saw 9 credentials in one 64 s reconnect.

Change

Retries take over the running setup (apps/web/src/connection/platform.ts). The SSH gateway moves into makeDesktopSshEnvironmentGateway. When an attempt is interrupted while its setup is still running, the gateway keeps that setup. The next attempt for the same target awaits it instead of queueing another, and spends the credential it mints. Once a setup finishes, a later attempt starts its own. The desktop keeps the tunnel, so the repeat is cheap, and a credential that may have passed its 5-minute lifetime is never handed out.

Reconnects reuse the bearer. A bearer session lasts 30 days on the server. The gateway stores the bearer from the last pairing together with the environment that issued it. On reconnect it opens the tunnel without pairing and checks two things: the server answering is that environment, and /api/auth/session still accepts the bearer. If both hold, it presents the bearer; otherwise it pairs again. This also stops the remote from collecting a new session on every reconnect. Removing the environment forgets the bearer.

Pairing checks the machine first. prepare now reads the environment descriptor before spending the one-time credential, as provision already did. A route that reaches a different machine fails with the same environmentMismatchError the resolver already returned. That machine never gets a session, and the gateway never stores one from it.

One fewer t3 start per launch (packages/ssh/src/tunnel.ts). The launch script ran "$RUNNER_FILE" --version only so the runner would install the release archive before the port and readiness helpers used it. The runner now has an install-only mode (T3_RUNNER_INSTALL_ONLY=1) that installs when needed and exits without starting t3. On the reporter's host that saves about 5.5 s on every launch.

No contract, server, desktop main-process, or mobile changes. SSH environments are desktop-only, and the desktop's SshEnvironmentManager is unchanged.

Scope and approval

Fixes #18229, a triaged bug. The maintainer triage confirms the mechanism above. It puts these in scope: coalescing or cancelling orphaned ensureEnvironment calls, not minting a fresh pairing token on every retry when a prior grant is still usable, and trimming the launch path's t3 starts. Each change here serves that one problem, the slow SSH connect.

Lazy-loading the server bundle is out of scope, as the triage says. I left the launch script's server-reuse block alone because #17919 rewrites it, and its rewrite already removes the second readiness probe there (the issue's fourth t3 start). Neither this PR nor #17550 or #10588 touches the other's lines.

Verification

Establishing the problem. I traced the path in the code:

  • the 15 s per-attempt budget in supervisor.ts;
  • Effect.tryPromise around the IPC call, which nothing on the desktop side observes;
  • the per-target KeyedLock and pairing call in SshEnvironmentManager.ensureEnvironment;
  • the launch script's --version start.

That matches the reporter's measurements. I did not reproduce it on a slow host.

Focused tests.

  • platform.test.ts drives the gateway against a fake desktop bridge whose host issues a different bearer per machine and accepts only its own:
    • Two timed-out attempts and a third that takes over: one setup and one pairing exchange in total.
    • Once the abandoned setup finishes (successfully or not), the next attempt starts its own.
    • A reconnect after adding the environment presents the existing bearer without pairing.
    • A bearer the server no longer accepts triggers a fresh pairing.
    • A stored bearer is not presented when the route reaches a different machine.
    • A route that briefly reaches the wrong machine exchanges no credential there and keeps nothing from it. When it reaches the right machine again, it pairs fresh.
    • The three existing provision tests moved onto the gateway unchanged.
  • tunnel.test.ts runs the real runner script against a fake release archive. Install-only mode installs on a cold home, exits without output on a warm one, and a normal run still executes t3.
  • I removed each guard in turn and confirmed its test fails: adoption, dropping a finished setup, bearer reuse, the issuer check, the session check, the pre-exchange machine check, and the install-only exit.
vp test run apps/web/src/connection/platform.test.ts packages/ssh/src/tunnel.test.ts
  Test Files  2 passed (2)
       Tests  42 passed (42)

The web and packages/ssh typechecks are clean, and lint and format pass on the changed files.

Not checked.

  • A real slow SSH host or a desktop trace. The time saved is reasoned from the code and the issue's numbers, not measured.
  • A pass in the desktop app against a real SSH environment.

Reviewed with Codex (GPT-6.1 Sol, high) over 2 rounds. Round 1 found that prepare could pair with an unexpected machine and later present that machine's bearer to the expected one. That is fixed above: bearers carry their issuing environment, and pairing checks the machine before the exchange. Round 2 confirmed the fix and found no other blocking issues.

Created with Claude Opus 5.5 in Claude Code.

🤖 Generated with Claude Code

im-kvijay and others added 2 commits October 11, 2026 10:55
Each connection attempt gets 15 s, but the desktop's SSH setup cannot be
cancelled once sent. On a slow host a timed-out attempt left its setup
running under the per-target lock, every retry queued another setup
behind it, and each one minted a pairing credential nobody used.

The SSH gateway now hands a setup that a timed-out attempt left running
to the next attempt, which spends its credential. Reconnects present the
bearer from the last pairing when the environment that issued it is the
one answering and still honors it, and pair only otherwise. Pairing
checks the environment before spending the one-time credential, so a
route that reaches a different machine never creates or keeps a session
there.

Refs pingdotgg#18229

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The launch script ran `t3 --version` only so the runner would install
the release archive before the port and readiness helpers used it. Each
t3 start costs seconds of CPU on a small host. The runner now has an
install-only mode that installs when needed and exits without starting
t3.

Refs pingdotgg#18229

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Oct 11, 2026
@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 production SSH fix changes authentication and connection-session behavior by caching and reusing bearer tokens, coordinating abandoned setup calls, and altering remote launch sequencing. Although focused tests cover the new paths, the authentication-sensitive runtime changes warrant human review.

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

@coderabbitai

coderabbitai Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 53e0b8d1-06a9-4f3a-92f5-5cbd6f1db62b
📥 Commits

Reviewing files that changed from the base of the PR and between 11fd134 and cf6e078.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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: d5795bc0-5dd8-4477-b69c-ae5139483a75
📥 Commits

Reviewing files that changed from the base of the PR and between f23d008 and 11fd134.

📒 Files selected for processing (4)
  • apps/web/src/connection/platform.test.ts
  • apps/web/src/connection/platform.ts
  • 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; 9 remain after this review.

📝 Walkthrough

The desktop SSH connection now uses a gateway that can reuse authenticated bearers and retain setup work after cancellation. The SSH archive runner adds an install-only mode, which archive-mode launch uses before port selection and readiness checks.

Changes: Desktop SSH environment gateway

Layer / File(s) Summary
Gateway lifecycle and validation
apps/web/src/connection/platform.ts, apps/web/src/connection/platform.test.ts
The gateway provisions and prepares SSH environments, reuses a bearer when its environment matches and the desktop confirms authentication, and clears pending setup and bearer state on disconnect. Tests cover pairing, environment mismatches, authentication failures, and interrupted setup. Capability wiring now uses the gateway.

SSH archive runner

Layer / File(s) Summary
Install-only launch and validation
packages/ssh/src/tunnel.ts, packages/ssh/src/tunnel.test.ts
The runner exits after installing the pinned runtime when T3_RUNNER_INSTALL_ONLY=1. Archive-mode launch sets this mode. Integration tests cover cold and warm install-only runs and a subsequent normal run.

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 11fd1

The change reduces repeated SSH setup work and avoids redundant pairing-token exchanges. No concrete merge-blocking issue was identified in the supplied evidence.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 11fd1

Bearer reuse introduces a credential-binding risk when a saved SSH alias moves to a different accepted host. A replacement service can claim the original environment ID and receive its cached bearer. SSH authentication limits the attack prerequisites, while the install-only change preserves existing installation safeguards.

Retained concerns

  • Medium · security · inferred: A cached bearer is selected using the saved SSH target, while setup re-resolves its alias and may return a different host. The gateway does not compare that resolved target with the credential's original target before trusting the replacement service's public environment ID and sending the bearer to its session endpoint. If the alias moves to another SSH-accepted host with an attacker-controlled service that knows the original environment ID, that service can capture the original environment's credential before session validation completes. Previously, preparation minted a credential on the current target rather than forwarding the original environment's bearer. Exploitation requires control of the replacement service and an accepted SSH route change; SSH authentication and loopback forwarding remain significant controls.
Security review details

Security Blast Radius

  • inferred — The credential-binding concern affects an independently redirected saved SSH target and its original environment session. A captured bearer carries that session's granted authority while valid; the inspected flow does not establish automatic exposure of other environments or a broader administrative credential.

Security Findings and Attack Paths

  • inferred — A replacement service on an SSH-accepted destination can echo the original public environment ID, causing the new reuse path to send the original bearer in its Authorization header. The service need not successfully validate the credential to capture it. This conditional disclosure path is introduced by cached-bearer forwarding; the prior preparation path exchanged a newly minted current-target credential.

Trust Boundaries and Controls

  • observed — Desktop HTTP bridge requests enforce a loopback hostname, and remote traffic uses SSH forwarding without a new host-key-checking override. The gateway blocks honest environment-ID mismatches before bearer transmission. These controls constrain ordinary network redirection but do not bind the reused bearer to the previously paired SSH peer.

Resilience and Maintainability Implications

  • observed — Install-only success occurs after lock cleanup. Existing installation logic retains exclusive locking, abandoned-owner recovery, checksum comparison, executable validation, staging cleanup, and publication before marking the runtime ready. The new exit does not bypass those safeguards.

Hardening Proposals

  • proposed — Bind cached-bearer reuse to the resolved target and authenticated peer identity established during pairing. At minimum, a changed resolved target should invalidate the cached bearer before any authenticated HTTP request and require fresh pairing; peer continuity should not depend solely on a public environment ID.
Pre-merge checks | Passed 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main SSH fix: preventing stacked retries and repeated pairing-token creation on slow hosts.
Description check Passed The description fully covers the problem, implementation, scope and approval, focused verification, observed results, and limitations. It links the triaged issue and identifies untested real-host scen…
Linked Issues check Passed Issue [#18229] requires reducing repeated SSH launch and pairing costs. packages/ssh/src/tunnel.ts adds T3_RUNNER_INSTALL_ONLY=1, so archive-mode setup installs the runner without starting t3. `…
Out of Scope Changes check Passed The changes stay within issue [#18229]. The gateway implementation and tests support retry coalescing, bearer reuse, and environment validation. The runner change and its integration test support the …
✨ 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.

@maria-rcks

Copy link
Copy Markdown
Collaborator

Note

Written by gpt-6.1-sol on behalf of Maria

Thanks for documenting the slow-host connection failure. Under Read this first, our review capacity is limited, and right now we are only taking focused bug fixes from outside contributors. This 562-line patch exceeds the size we can take over from an unvouched contributor. Please preserve the reproduction in issue #18229 and propose a smaller fix for reconsideration.

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

Labels

size:L 100-499 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 connect takes 25–60 s: every t3 start costs ~5.5 s and each attempt mints a new pairing token

2 participants