[build-tools] Add worker-side SSH session helper

[gwdp]

Why

The worker needs a piece that starts an SSH session on the build VM and keeps the worker alive while someone is connected.

How

New helper resolves the upterm client vendored in the worker package, dials the relay, reports the connection details back so the CLI can attach, and holds on an idle timer. Redialing is capped at a few attempts per drop so a flapping relay can't retry forever. Connection secrets are scrubbed from logs.

Test Plan

Unit tests cover the connection parsing, secret redaction, process-liveness detection, the capped redial, and the idle hold. CI passes.

[linear-code]

ENG-24547

[gwdp]

• [eas-cli] Add --ssh to workflow:run #4033
• [eas-cli] Add workflow:ssh command #4032
• [worker] Wire SSH session into build #4031
• [build-tools] Add worker-side SSH session helper #4030 👈 (View in Graphite)
• [eas-build-job] Add optional ssh field on job payloads #4083
• [eas-build-job] Add SSH build phase #4029
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.95%. Comparing base (3f8ffac) to head (37ebacc).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4030      +/-   ##
==========================================
+ Coverage   63.74%   63.95%   +0.21%     
==========================================
  Files        1030     1033       +3     
  Lines       47477    47747     +270     
  Branches     9990    10043      +53     
==========================================
+ Hits        30260    30530     +270     
  Misses      17116    17116              
  Partials      101      101              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[claude]

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

_{Tip: disable this comment in your organization's Code Review settings.}

[github-actions]

🤖 AI code review

Decision: Request changes

Overall PR risk: Low. The change is additive: it introduces a new SSH relay session helper (upterm host spawn, GraphQL session registration, redial/idle supervision) without editing any existing function body, and the cross-cutting review found no caller in this diff wiring the new helper into an existing build or job-run path, so the new code stays inert until a later change calls it. The most serious verified issue is a critical secret-leak path: redactSpawnErrorForLog can let the unredacted SSH session secret reach the build log through the output array on a spawn error. Two further warnings cover a missing SIGKILL escalation that can leak upterm host processes, and a stale-connection-state bug that can make a dead redial look alive and block further reconnects. Given the critical secrets finding, changes are requested before merge.

🔴 Critical (1)

• redactSpawnErrorForLog copies the unredacted output array of the spawn error — packages/build-tools/src/utils/upterm.ts:102 (secrets) · id:6d0ee6ce557e
  Confidence: Medium — the spread copies output verbatim; the leak reaches the log when the build logger writes debug records.
  Impact if shipped: High — the upterm session secret can reach the build log. That secret gives an SSH shell on the build worker.
  Suggested remediation: Redact every string in output too, for example ...(Array.isArray(spawnErr.output) ? { output: spawnErr.output.map(item => (typeof item === 'string' ? redactConnectionSecrets(item) : item)) } : {}). A safer option is to build the logged object from an explicit list of fields, instead of spreading the error.

  Evidence and reasoning

  redactSpawnErrorForLog spreads the spawn error first, then overwrites only message, stdout, and stderr.

  A rejected @expo/turtle-spawn promise carries the SpawnResult fields on the error. SpawnResult includes output: string[], which holds the same stdout and stderr strings. This repo already reads that field on a spawn error: packages/eas-cli/src/run/ios/xcrun.ts:26 checks Array.isArray(e.output), and packages/eas-build-cache-provider/src/helpers.ts:4 types a spawn error as Error & SpawnResult. Those properties are own and enumerable, so ...spawnErr copies output into the returned object without redaction.

  The host process runs with stdio: ['ignore', 'pipe', 'pipe'], so upterm stdout is captured. The file redacts stdout on its own, because that stream carries the session id (see the comment at line 80). The same text stays readable in output[0].

  The sink is line 278: ctx.logger.debug({ err: redactSpawnErrorForLog(err) }, ...). packages/logger/src/index.ts:10 sets serializers: bunyan.stdSerializers. The err serializer returns the value unchanged when the value has no stack. The spread drops stack, because stack is not enumerable on an Error. So bunyan writes the whole plain object, including the unredacted output array.

  The session id is the value stored as SshConnectionConfig.secret. A reader of the log can use it to open the SSH session and get a login shell with the job environment.

🟡 Warning (2)

• killProcessGroup has no SIGKILL escalation after SIGTERM — packages/build-tools/src/utils/processes.ts:13 (correctness) · id:218f27b04044
  Confidence: High — direct comparison with the sibling teardown helper in the same package shows the wait-and-escalate step is missing.
  Impact if shipped: Medium — an upterm host process, or its long-running keep-alive child, can survive stop and redial if it ignores SIGTERM, leaking processes on the build machine for up to 6 hours.
  Suggested remediation: After sending SIGTERM, poll isChildProcessAlive for a bounded time, then send SIGKILL to the process group (or the single pid) if the process is still alive, mirroring stopDetachedProcessAsync.

  Evidence and reasoning

  packages/build-tools/src/steps/utils/remoteDeviceRunSession.ts (stopDetachedProcessAsync) sets the pattern for killing a detached process group in this package: send SIGTERM to the negated pid, poll isProcessRunning for up to 5 seconds, and escalate to SIGKILL on the group (falling back to the single pid) if the process is still alive.

  killProcessGroup in processes.ts solves the same problem for the upterm host process used by stopAsync and redialAsync in upterm.ts, but it sends SIGTERM only once and falls back to child.kill() (still a default-signal kill) only when process.kill throws, not when the process is merely still alive. The caller in upterm.ts races the exit against a 5-second timeout and then only logs a debug message; it never escalates the signal. Because dialAsync() calls killProcessGroup on every redial, up to MAX_SSH_REDIALS times per session, a single unresponsive process group can leave an orphaned upterm host and its child process running for the rest of the build.

• Failed redial leaves a stale connectionConfig while isAlive() still reports true — packages/build-tools/src/utils/upterm.ts:312 (correctness) · id:2c84785b8069
  Confidence: Medium — the code path is directly traceable, but the trigger needs the upterm process to keep running after it fails to register a session in time.
  Impact if shipped: Medium — the SSH relay session can get stuck reporting as connected while it is actually dead, blocking further reconnect attempts.
  Suggested remediation: In dialAsync, clear currentProcess (or stop the new process) when waitForConnectionConfigAsync throws, so isAlive() reflects the real connection state and a later ensureConnectedAsync retry re-enters the redial branch.

  Evidence and reasoning

  dialAsync (line 281) sets currentProcess = uptermProcess before it awaits waitForConnectionConfigAsync. If that wait times out and throws, dialAsync rejects, but currentProcess still points at the new process. redialAsync (line 312-315) only assigns the outer connectionConfig variable after dialAsync resolves, so on a throw the getter keeps returning the old connectionConfig, which points at a session that stopCurrentProcessAsync already killed at the top of the same dialAsync call.

  In packages/build-tools/src/utils/turtleSshSession.ts, ensureConnectedAsync calls host.redialAsync() and, on a throw, retries after a backoff. On the retry it first checks if (!host.isAlive()). If the new, unconfirmed process from the failed redial is still running, isAlive() returns true, so the retry skips redialAsync() and calls createOrUpdateSessionAsync with the stale host.connectionConfig, marking reconnecting: false even though the session behind that config is dead. This can report the SSH relay as restored when it is not, and it blocks further redial attempts because isAlive() keeps returning true for the leftover process.

---

This review is advisory — it never blocks a merge and never auto-approves.

[sjchmiela]

this feels slightly misleading cause ti may happen that a user disconnects and another connects and we won't print anything because the count is still 1

[gwdp]

Good point,, but the limitation is that upterm session current --output json only hands back a count, it does len(connected_clients) server-side and drops the list, so from the worker a 1-for-1 swap looks identical to a steady connection.

To log real join/leave events I think I can read upterm's connected_clients (they carry a public-key fingerprint) off its gRPC admin api instead of the CLI. I might defer now as a known limitation and go for a follow-up if that works for you?

[gwdp]

Applied changes but want to do a little more testing before requesting review.

[sjchmiela]

just sharing -- what i recently did for logs was https://github.com/expo/universe/blob/1ae9b878798169e2de9e05af590e549061aa2f51/server/www/src/graphql/mutations/RealtimeLogsMutation.ts#L26-L34

i think it works nicely

[gwdp]

Hm... that's a good one, I think I do prefer your way 🤔 (baking a change)

[gwdp]

https://github.com/expo/universe/pull/30238 🙈

[github-actions]

✅ Thank you for adding the changelog entry!