fix(server): device-host SSH tunnels exit when the server dies

[kcazin]

Detached device-host SSH tunnels can survive an abrupt server exit and keep their ports open. This runs a remote stdin reader with piped stdin, so server death sends EOF; SSH exits after existing forwarded connections close. Graceful teardown and the existing ControlMaster opt-out remain in place. Fixes #17438.

The change keeps @kcazin's implementation from #17448. Main was merged into the contributor branch. The standalone local cat test was removed because it largely tested the shell and relied on a removed shared export. The existing host test now checks the SSH command and stdin contract: 2 tests → 1, failing against main and passing with the fix. Targeted lint and server typecheck passed; the current head's CI is green.

Live verification on Linux with OpenSSH 9.6 and local sshd exercised a forward and killed its Node parent: the old -N tunnel survived; the new tunnel exited within 10 ms and released its listener. Captured output. An independent live check held a forwarded connection open: exit waits for that connection to close. This change does not promise immediate termination while an external client holds a forward open. The existing ControlMaster=no, ControlPath=none, and ControlPersist=no prevent handing the forwards to a persistent master.

Not verified: a complete service-launcher self-update; macOS or Windows server hosts; Electron desktop, iOS or Android clients; non-POSIX remote login shells. No visual UI changes.

Initial takeover: Claude Fable 5.1 through Claude Code. Resumed by GPT-6.1-Sol through the Codex harness in T3 Code; adversarial reviews by Claude Opus 5.5 and GPT-6-Sol.

[macroscopeapp]

Approvability

Verdict: Approved at 33459db

Macroscope's review found this PR approvable — This is a narrowly scoped SSH tunnel lifecycle bug fix: tunnels now terminate when the server dies, while forwarding and graceful cleanup behavior remain unchanged. The accompanying test verifies the new command and stdin contract, with no schema, deployment, default, or static-analysis changes.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

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: a42b8dab-6613-448f-be56-59a71fad72ae

📥 Commits

Reviewing files that changed from the base of the PR and between 43f8a8d and 66b12ec.

📒 Files selected for processing (2)

• apps/server/src/device/SshDeviceHost.test.ts
• apps/server/src/device/SshDeviceHost.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

The SSH forwarding process now runs a remote command that reads from piped stdin. The test suite checks that the command exits after receiving EOF.

Changes

SSH Tunnel Lifetime

Layer / File(s)	Summary
Pipe-driven tunnel exit apps/server/src/device/SshDeviceHost.ts, apps/server/src/device/SshDeviceHost.test.ts	The forwarding process runs a remote command that reads stdin, with piped stdin replacing ignored stdin. The hub and optional daemon forwarding arguments remain unchanged. A live test checks that the command exits successfully after receiving 1 MiB and then stopping the host. The mock identifies tunnel commands by -L.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant SshDeviceHost
  participant SSH
  participant RemoteShell
  participant Cat
  SshDeviceHost->>SSH: Start forwarding process with piped stdin
  SSH->>RemoteShell: Run remote shell command
  RemoteShell->>Cat: Execute cat reading stdin
  SshDeviceHost->>SSH: Close stdin when the server exits
  SSH->>Cat: Forward EOF
  Cat-->>SSH: Exit after EOF

Loading

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 66b12

The tunnel exited in the reported abrupt-shutdown check, and no concrete merge-blocking issue remains.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 66b12

The change aims to reduce orphaned SSH tunnels without broadening network access or remote-command authority. Normal cleanup remains unchanged. Confidence is limited because the added test exercises a local shell rather than real SSH forwarding during abrupt server termination.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The directly affected capability is each configured device-host SSH session and its local hub or daemon listeners. The inspected change does not enlarge destination selection, listener bindings, or credential authority; it attempts to shorten capability lifetime after server death.

Trust Boundaries and Controls

• observed — The forwarding process retains batch-mode authentication, configured identity selection, ExitOnForwardFailure, and server-alive settings. The added EOF reader executes under the same configured remote SSH account already used for bootstrap.

Resilience and Maintainability Implications

• observed — The added non-Windows test uses a real local shell to verify stdin consumption and successful exit after EOF. It substitutes that shell for SSH, so it does not establish listener release after SIGKILL, behavior with surviving forwarded connections, or interruption and recovery races.

Hardening Proposals

• proposed — Validate the capability-lifetime guarantee with real SSH and abrupt parent termination, including an independently held forwarded connection. Check listener release and subsequent server recovery rather than treating remote-command exit alone as complete tunnel cleanup.

Pre-merge checks: 4

Check name	Status	Explanation
Linked Issues check		For directly linked issue #17438, SshDeviceHost.ts changes the device-host tunnel from -N to -T, adds the remote sh -c 'exec cat >/dev/null' command, and sets stdin: "pipe". This couples SSH…
Out of Scope Changes check		The changed files are limited to the device-host tunnel implementation and its focused test. The implementation preserves the existing forwards, SSH keepalive options, forward-failure option, graceful…
Title check		The title clearly and concisely describes the main fix: device-host SSH tunnels now exit when the server dies.
Description check		The description covers the problem, implementation, scope, linked issue, verification results, limitations, and agent usage. It is mostly complete and directly related to the change.

Full details: Linked Issues check

Explanation

For directly linked issue #17438, SshDeviceHost.ts changes the device-host tunnel from -N to -T, adds the remote sh -c 'exec cat >/dev/null' command, and sets stdin: "pipe". This couples SSH lifetime to the server stdin pipe. The focused live test in SshDeviceHost.test.ts verifies the pipe, writes input, closes stdin, and verifies exit code 0. The issue lists this lifetime-coupling approach as a valid fix. The separate ensureReady interruption path is not a required completion condition for this approach.

Full details: Out of Scope Changes check

Explanation

The changed files are limited to the device-host tunnel implementation and its focused test. The implementation preserves the existing forwards, SSH keepalive options, forward-failure option, graceful stop path, and detached process behavior. The test directly verifies the new stdin lifetime behavior. No unrelated change is demonstrated.

✨ 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]

Taking over this one. I'll merge main into the branch, trim the tests to the minimum, rebuild the implementation to repo conventions where needed, run adversarial reviews, and push here. Thanks @kcazin for the diagnosis and the live evidence.

Dismissing prior approval to re-evaluate 33459db