diff --git a/AGENTS.md b/AGENTS.md index 7577259..cb1a6c6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -115,6 +115,25 @@ on a remote machine via SSH when `--ssh user@host` is passed. so a read-only system `ControlPath` (which a plain `ssh … pwd` would try, and fail, to bind a master socket into) cannot make the initial probe exit 255. +- **Hang-proofing (no ssh call can block the UI).** + `session_start` is `await`ed by pi before the agent loop accepts input, so a + blocking ssh call there manifests as *"the TUI is up but my prompts are + silently ignored."* Guards: a shared `CONNECT_OPTS` (`ConnectTimeout=8` + + `ServerAliveInterval=5`/`ServerAliveCountMax=3`) is appended to **every** ssh + invocation (pwd probe, master start, `sshExec`, bash exec); the `pwd` probe + uses `run(..., { timeoutMs: STARTUP_TIMEOUT_MS })` (15 s) and both master-start + paths kill the child + reject on the same wall-clock cap. Key-auth calls also + carry `BATCH_OPTS` (`BatchMode=yes`) so ssh refuses any `/dev/tty` + password/passphrase/host-key prompt rather than waiting on it invisibly; + `--ssh-ask-pass` omits `BatchMode` (it would disable `SSH_ASKPASS`). The + `askPass` flag is read **before** the probe so the probe picks the right opts. + On probe failure the handler sets an `✗ unreachable` status + error toast and + `return`s (instead of throwing an unhandled rejection). Common trigger: a LAN + host reachable from the macOS host but **not** from inside the pi-devbox + container (`pi-dev` = `docker compose … exec devbox pi`); the container has no + route without a `ProxyJump host` entry, so `pi --ssh ` from the + container now fails fast instead of stalling on the OS TCP timeout. + - **Password auth via `--ssh-ask-pass`.** When the flag is set, `ctx.ui.input()` prompts for a password before connecting. The password is passed to SSH via `SSH_ASKPASS`: a temp script at diff --git a/README.md b/README.md index b9a099d..dbb8010 100644 --- a/README.md +++ b/README.md @@ -102,6 +102,8 @@ pi -e ~/src/src_local/pi-extensions/extensions/ssh-controlmaster.ts --ssh user@h 6. The system prompt is patched to tell the LLM it's operating on ` (via SSH ControlMaster: )` 7. User `!` shell commands are also routed over SSH +**Hang-proofing:** every SSH invocation carries `ConnectTimeout=8` + `ServerAlive*` keepalives, and the startup probe / master start are additionally bounded by a 15 s wall-clock timeout. Key-auth calls also set `BatchMode=yes` so ssh can never wait silently on a `/dev/tty` password/passphrase or host-key prompt behind pi's TUI (`--ssh-ask-pass` omits `BatchMode` so the `SSH_ASKPASS` path still works). If the host can't be reached — e.g. a LAN target with **no route from inside a container** (`pi --ssh` needs a `ProxyJump` to reach LAN hosts from a devbox) — the probe fails fast with an `✗ unreachable` status and an error toast instead of hanging startup and silently swallowing your prompts. + The status bar shows `⚡ own master` or `⚡ system master` so you can see which path was taken. **Status bar:** Shows `SSH ⚡ user@host:/path` when the master is ready, `⟳ connecting…` during setup, and an error state if the master fails to start. diff --git a/extensions/ssh-controlmaster.ts b/extensions/ssh-controlmaster.ts index d0aa0b4..169efcd 100644 --- a/extensions/ssh-controlmaster.ts +++ b/extensions/ssh-controlmaster.ts @@ -55,6 +55,22 @@ interface SshState { // ── Helpers ────────────────────────────────────────────────────────────────── +// SSH options that bound connection setup and detect dead links, so neither the +// startup probe nor a runtime tool call can hang the UI indefinitely. An +// unreachable host (e.g. a LAN target with no route from inside a container) +// now fails fast instead of blocking on the OS TCP timeout (~75s+). +const CONNECT_OPTS = [ + "-o", "ConnectTimeout=8", // cap TCP/handshake setup + "-o", "ServerAliveInterval=5", // probe a dead link rather than block forever + "-o", "ServerAliveCountMax=3", +]; +// Key-auth only: refuse any tty/askpass prompt so ssh can never silently wait +// on /dev/tty behind pi's TUI. NOT used with --ssh-ask-pass (would disable the +// SSH_ASKPASS password path). +const BATCH_OPTS = ["-o", "BatchMode=yes"]; +// Wall-clock cap for ssh calls made during session_start. +const STARTUP_TIMEOUT_MS = 15000; + function ownSocketPath(): string { // Keep path short — macOS has a ~104-char Unix socket path limit return join(tmpdir(), `pi-cm-${process.pid}.sock`); @@ -118,7 +134,7 @@ async function startControlMasterWithPassword( await new Promise((resolve, reject) => { const child = spawn( "ssh", - ["-fN", "-o", "ControlMaster=yes", "-o", `ControlPath=${socketPath}`, "-o", "ControlPersist=yes", remote], + ["-fN", "-o", "ControlMaster=yes", "-o", `ControlPath=${socketPath}`, "-o", "ControlPersist=yes", ...CONNECT_OPTS, remote], { stdio: "ignore", env: { @@ -129,8 +145,13 @@ async function startControlMasterWithPassword( }, }, ); - child.on("error", reject); + const timer = setTimeout(() => { + child.kill("SIGKILL"); + reject(new Error(`ControlMaster (password) start timed out after ${STARTUP_TIMEOUT_MS}ms`)); + }, STARTUP_TIMEOUT_MS); + child.on("error", (e) => { clearTimeout(timer); reject(e); }); child.on("close", (code) => { + clearTimeout(timer); if (code === 0) resolve(); else reject(new Error(`ControlMaster exited with code ${code}`)); }); @@ -140,16 +161,27 @@ async function startControlMasterWithPassword( } } -/** Run a one-shot command, return stdout as string. Rejects on non-zero exit. */ -function run(args: string[]): Promise { +/** + * Run a one-shot command, return stdout as string. Rejects on non-zero exit. + * Pass `timeoutMs` to guarantee the call cannot hang forever — on timeout the + * child is killed and the promise rejects. + */ +function run(args: string[], opts: { timeoutMs?: number } = {}): Promise { return new Promise((resolve, reject) => { const child = spawn(args[0], args.slice(1), { stdio: ["ignore", "pipe", "pipe"] }); const out: Buffer[] = []; const err: Buffer[] = []; + const timer = opts.timeoutMs + ? setTimeout(() => { + child.kill("SIGKILL"); + reject(new Error(`${args.join(" ")} timed out after ${opts.timeoutMs}ms`)); + }, opts.timeoutMs) + : undefined; child.stdout.on("data", (d: Buffer) => out.push(d)); child.stderr.on("data", (d: Buffer) => err.push(d)); - child.on("error", reject); + child.on("error", (e) => { if (timer) clearTimeout(timer); reject(e); }); child.on("close", (code) => { + if (timer) clearTimeout(timer); if (code === 0) resolve(Buffer.concat(out).toString().trim()); else reject(new Error(`${args.join(" ")} exited ${code}: ${Buffer.concat(err).toString().trim()}`)); }); @@ -237,13 +269,20 @@ function startControlMaster(remote: string, socketPath: string): Promise { return new Promise((resolve, reject) => { // -fN: fork to background after auth, don't run a remote command // ControlPersist=yes: keep master alive in the background indefinitely + // BatchMode=yes: key-auth only — never wait on a tty password/passphrase + // prompt (which would hang invisibly behind pi's TUI). const child = spawn( "ssh", - ["-fN", "-o", "ControlMaster=yes", "-o", `ControlPath=${socketPath}`, "-o", "ControlPersist=yes", remote], + ["-fN", "-o", "ControlMaster=yes", "-o", `ControlPath=${socketPath}`, "-o", "ControlPersist=yes", ...CONNECT_OPTS, ...BATCH_OPTS, remote], { stdio: "ignore" }, ); - child.on("error", reject); + const timer = setTimeout(() => { + child.kill("SIGKILL"); + reject(new Error(`ControlMaster start timed out after ${STARTUP_TIMEOUT_MS}ms`)); + }, STARTUP_TIMEOUT_MS); + child.on("error", (e) => { clearTimeout(timer); reject(e); }); child.on("close", (code) => { + clearTimeout(timer); if (code === 0) resolve(); else reject(new Error(`ControlMaster exited with code ${code}`)); }); @@ -272,7 +311,7 @@ function sshExec(remote: string, socketPath: string, command: string): Promise { const child = spawn( "ssh", - ["-o", "ControlMaster=no", "-o", `ControlPath=${socketPath}`, remote, command], + ["-o", "ControlMaster=no", "-o", `ControlPath=${socketPath}`, ...CONNECT_OPTS, remote, command], { stdio: ["ignore", "pipe", "pipe"] }, ); const out: Buffer[] = []; @@ -341,7 +380,7 @@ function createRemoteBashOps( const cmd = `cd ${JSON.stringify(r(cwd))} && ${command}`; const child = spawn( "ssh", - ["-o", "ControlMaster=no", "-o", `ControlPath=${socketPath}`, remote, cmd], + ["-o", "ControlMaster=no", "-o", `ControlPath=${socketPath}`, ...CONNECT_OPTS, remote, cmd], { stdio: ["ignore", "pipe", "pipe"] }, ); let timedOut = false; @@ -442,6 +481,10 @@ export default function (pi: ExtensionAPI) { let remote: string; let remoteCwd: string; + // Read this up front: it decides whether the pwd probe may use BatchMode + // (key auth) or must allow the SSH_ASKPASS password path. + const askPass = pi.getFlag("ssh-ask-pass") as boolean; + if (arg.includes(":")) { [remote, remoteCwd] = arg.split(":"); } else { @@ -451,22 +494,35 @@ export default function (pi: ExtensionAPI) { // read-only mount, which makes a plain `ssh pwd` fail trying to bind the // master socket. ControlPath=none sidesteps multiplexing for this one // probe; the real (writable) master is established by negotiateMaster(). - remoteCwd = await run([ - "ssh", - "-o", - "ControlPath=none", - "-o", - "ControlMaster=no", - remote, - "pwd", - ]).catch((e) => { - throw new Error(`Could not resolve remote pwd: ${e.message}`); - }); + // + // Hardened: CONNECT_OPTS + a wall-clock timeout guarantee this can't hang + // the UI when the host is unreachable (e.g. a LAN target with no route + // from inside a container). BatchMode is added for key auth so ssh never + // waits silently on a /dev/tty prompt; it is omitted under --ssh-ask-pass. + const probeOpts = [ + "-o", "ControlPath=none", + "-o", "ControlMaster=no", + ...CONNECT_OPTS, + ...(askPass ? [] : BATCH_OPTS), + ]; + try { + remoteCwd = await run(["ssh", ...probeOpts, remote, "pwd"], { + timeoutMs: STARTUP_TIMEOUT_MS, + }); + } catch (e) { + ctx.ui.setStatus("ssh", ctx.ui.theme.fg("error", `SSH: ${remote} ✗ unreachable`)); + ctx.ui.notify( + `SSH: could not reach ${remote} (${(e as Error).message}). ` + + `Check the host is routable from here — e.g. \`pi --ssh\` from inside a ` + + `container needs a ProxyJump to LAN hosts.`, + "error", + ); + return; + } } ctx.ui.setStatus("ssh", ctx.ui.theme.fg("accent", `SSH: ${remote} ⟳ connecting…`)); - const askPass = pi.getFlag("ssh-ask-pass") as boolean; let password: string | undefined; if (askPass) {