fix(ssh-controlmaster): prevent session_start from hanging on unreachable host
pi awaits session_start before the agent loop accepts input, so a blocking ssh call there presents as "TUI is up but prompts are silently ignored". The remote pwd probe and ControlMaster start had no ConnectTimeout, no BatchMode, and no wall-clock cap, so an unreachable host (classic case: `pi-dev --ssh <lan-host>` runs pi *inside* the devbox container, which has no route to LAN hosts without a ProxyJump) blocked startup on the OS TCP timeout (~75s+) while silently swallowing keystrokes. Hardening: - CONNECT_OPTS (ConnectTimeout=8 + ServerAlive keepalives) on every ssh invocation: pwd probe, master start (both key and password paths), sshExec, and bash exec. - run() gains a timeoutMs option (kills child + rejects); pwd probe and both master-start paths bounded by STARTUP_TIMEOUT_MS (15s). - BATCH_OPTS (BatchMode=yes) on key-auth calls so ssh never waits on a /dev/tty password/passphrase/host-key prompt behind the TUI; omitted under --ssh-ask-pass so the SSH_ASKPASS path still works. askPass flag is now read before the probe to pick the right opts. - Probe failure sets an "unreachable" status + error toast and returns, instead of throwing an unhandled rejection from the handler. Docs: README "How it works" + AGENTS.md technical note updated.
This commit is contained in:
@@ -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
|
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.
|
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 <lan-host>` from the
|
||||||
|
container now fails fast instead of stalling on the OS TCP timeout.
|
||||||
|
|
||||||
- **Password auth via `--ssh-ask-pass`.**
|
- **Password auth via `--ssh-ask-pass`.**
|
||||||
When the flag is set, `ctx.ui.input()` prompts for a password before
|
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
|
connecting. The password is passed to SSH via `SSH_ASKPASS`: a temp script at
|
||||||
|
|||||||
@@ -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 `<remoteCwd> (via SSH ControlMaster: <remote>)`
|
6. The system prompt is patched to tell the LLM it's operating on `<remoteCwd> (via SSH ControlMaster: <remote>)`
|
||||||
7. User `!` shell commands are also routed over SSH
|
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.
|
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.
|
**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.
|
||||||
|
|||||||
@@ -55,6 +55,22 @@ interface SshState {
|
|||||||
|
|
||||||
// ── Helpers ──────────────────────────────────────────────────────────────────
|
// ── 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 {
|
function ownSocketPath(): string {
|
||||||
// Keep path short — macOS has a ~104-char Unix socket path limit
|
// Keep path short — macOS has a ~104-char Unix socket path limit
|
||||||
return join(tmpdir(), `pi-cm-${process.pid}.sock`);
|
return join(tmpdir(), `pi-cm-${process.pid}.sock`);
|
||||||
@@ -118,7 +134,7 @@ async function startControlMasterWithPassword(
|
|||||||
await new Promise<void>((resolve, reject) => {
|
await new Promise<void>((resolve, reject) => {
|
||||||
const child = spawn(
|
const child = spawn(
|
||||||
"ssh",
|
"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",
|
stdio: "ignore",
|
||||||
env: {
|
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) => {
|
child.on("close", (code) => {
|
||||||
|
clearTimeout(timer);
|
||||||
if (code === 0) resolve();
|
if (code === 0) resolve();
|
||||||
else reject(new Error(`ControlMaster exited with code ${code}`));
|
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<string> {
|
* 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<string> {
|
||||||
return new Promise((resolve, reject) => {
|
return new Promise((resolve, reject) => {
|
||||||
const child = spawn(args[0], args.slice(1), { stdio: ["ignore", "pipe", "pipe"] });
|
const child = spawn(args[0], args.slice(1), { stdio: ["ignore", "pipe", "pipe"] });
|
||||||
const out: Buffer[] = [];
|
const out: Buffer[] = [];
|
||||||
const err: 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.stdout.on("data", (d: Buffer) => out.push(d));
|
||||||
child.stderr.on("data", (d: Buffer) => err.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) => {
|
child.on("close", (code) => {
|
||||||
|
if (timer) clearTimeout(timer);
|
||||||
if (code === 0) resolve(Buffer.concat(out).toString().trim());
|
if (code === 0) resolve(Buffer.concat(out).toString().trim());
|
||||||
else reject(new Error(`${args.join(" ")} exited ${code}: ${Buffer.concat(err).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<void> {
|
|||||||
return new Promise((resolve, reject) => {
|
return new Promise((resolve, reject) => {
|
||||||
// -fN: fork to background after auth, don't run a remote command
|
// -fN: fork to background after auth, don't run a remote command
|
||||||
// ControlPersist=yes: keep master alive in the background indefinitely
|
// 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(
|
const child = spawn(
|
||||||
"ssh",
|
"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" },
|
{ 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) => {
|
child.on("close", (code) => {
|
||||||
|
clearTimeout(timer);
|
||||||
if (code === 0) resolve();
|
if (code === 0) resolve();
|
||||||
else reject(new Error(`ControlMaster exited with code ${code}`));
|
else reject(new Error(`ControlMaster exited with code ${code}`));
|
||||||
});
|
});
|
||||||
@@ -272,7 +311,7 @@ function sshExec(remote: string, socketPath: string, command: string): Promise<B
|
|||||||
return new Promise((resolve, reject) => {
|
return new Promise((resolve, reject) => {
|
||||||
const child = spawn(
|
const child = spawn(
|
||||||
"ssh",
|
"ssh",
|
||||||
["-o", "ControlMaster=no", "-o", `ControlPath=${socketPath}`, remote, command],
|
["-o", "ControlMaster=no", "-o", `ControlPath=${socketPath}`, ...CONNECT_OPTS, remote, command],
|
||||||
{ stdio: ["ignore", "pipe", "pipe"] },
|
{ stdio: ["ignore", "pipe", "pipe"] },
|
||||||
);
|
);
|
||||||
const out: Buffer[] = [];
|
const out: Buffer[] = [];
|
||||||
@@ -341,7 +380,7 @@ function createRemoteBashOps(
|
|||||||
const cmd = `cd ${JSON.stringify(r(cwd))} && ${command}`;
|
const cmd = `cd ${JSON.stringify(r(cwd))} && ${command}`;
|
||||||
const child = spawn(
|
const child = spawn(
|
||||||
"ssh",
|
"ssh",
|
||||||
["-o", "ControlMaster=no", "-o", `ControlPath=${socketPath}`, remote, cmd],
|
["-o", "ControlMaster=no", "-o", `ControlPath=${socketPath}`, ...CONNECT_OPTS, remote, cmd],
|
||||||
{ stdio: ["ignore", "pipe", "pipe"] },
|
{ stdio: ["ignore", "pipe", "pipe"] },
|
||||||
);
|
);
|
||||||
let timedOut = false;
|
let timedOut = false;
|
||||||
@@ -442,6 +481,10 @@ export default function (pi: ExtensionAPI) {
|
|||||||
let remote: string;
|
let remote: string;
|
||||||
let remoteCwd: 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(":")) {
|
if (arg.includes(":")) {
|
||||||
[remote, remoteCwd] = arg.split(":");
|
[remote, remoteCwd] = arg.split(":");
|
||||||
} else {
|
} else {
|
||||||
@@ -451,22 +494,35 @@ export default function (pi: ExtensionAPI) {
|
|||||||
// read-only mount, which makes a plain `ssh pwd` fail trying to bind the
|
// read-only mount, which makes a plain `ssh pwd` fail trying to bind the
|
||||||
// master socket. ControlPath=none sidesteps multiplexing for this one
|
// master socket. ControlPath=none sidesteps multiplexing for this one
|
||||||
// probe; the real (writable) master is established by negotiateMaster().
|
// probe; the real (writable) master is established by negotiateMaster().
|
||||||
remoteCwd = await run([
|
//
|
||||||
"ssh",
|
// Hardened: CONNECT_OPTS + a wall-clock timeout guarantee this can't hang
|
||||||
"-o",
|
// the UI when the host is unreachable (e.g. a LAN target with no route
|
||||||
"ControlPath=none",
|
// from inside a container). BatchMode is added for key auth so ssh never
|
||||||
"-o",
|
// waits silently on a /dev/tty prompt; it is omitted under --ssh-ask-pass.
|
||||||
"ControlMaster=no",
|
const probeOpts = [
|
||||||
remote,
|
"-o", "ControlPath=none",
|
||||||
"pwd",
|
"-o", "ControlMaster=no",
|
||||||
]).catch((e) => {
|
...CONNECT_OPTS,
|
||||||
throw new Error(`Could not resolve remote pwd: ${e.message}`);
|
...(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…`));
|
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;
|
let password: string | undefined;
|
||||||
|
|
||||||
if (askPass) {
|
if (askPass) {
|
||||||
|
|||||||
Reference in New Issue
Block a user