diff --git a/AGENTS.md b/AGENTS.md index 8b23d1c..7577259 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -98,11 +98,22 @@ on a remote machine via SSH when `--ssh user@host` is passed. - **ControlMaster negotiation via `ssh -G`.** Before starting any connection, `readSshConfig(remote)` runs `ssh -G ` and inspects the `controlmaster` and `controlpath` fields. If the effective - config already has `ControlMaster auto` or `yes`, the system socket is reused - (`ownsmaster: false`). Otherwise, the extension starts its own master at - `/tmp/pi-cm-.sock` (`ownsmaster: true`). The `session_shutdown` handler - only calls `ssh -O exit` when `ownsmaster` is true — it never tears down a - connection it didn't create. + config already has `ControlMaster auto` or `yes` **and the `ControlPath` + directory is writable** (`controlPathWritable()` expands `~`, then checks the + socket's parent dir exists+writable, or is missing but creatable), the system + socket is reused (`ownsmaster: false`). Otherwise — no system master, **or** a + system `ControlPath` on a read-only mount (the devbox bind-mounts `~/.ssh` + read-only, so a user's `ControlPath ~/.ssh/cm/...` is unwritable) — the + extension starts its own master at `/tmp/pi-cm-.sock` (`ownsmaster: + true`); its command-line `-o ControlPath` overrides the user's unwritable + path. The `session_shutdown` handler only calls `ssh -O exit` when + `ownsmaster` is true — it never tears down a connection it didn't create. + +- **Read-only-`~/.ssh` safe pwd probe.** + The remote working dir is resolved with `ssh -o ControlPath=none -o + ControlMaster=no pwd` — a direct connection that ignores multiplexing + 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. - **Password auth via `--ssh-ask-pass`.** When the flag is set, `ctx.ui.input()` prompts for a password before @@ -119,9 +130,11 @@ on a remote machine via SSH when `--ssh user@host` is passed. when the paths diverge. - **`ownsmaster` vs system master and `--ssh-ask-pass`.** - If the system already has a ControlMaster configured for the target host, - `--ssh-ask-pass` is silently ignored — the system master handles auth - independently and the socket is just reused. + If the system already has a ControlMaster configured for the target host + *and its `ControlPath` is writable*, `--ssh-ask-pass` is silently ignored — + the system master handles auth independently and the socket is just reused. + When the system `ControlPath` is unwritable (read-only `~/.ssh`) we start our + own master instead, so the password flag is honored on that path. ### `confirm-destructive.ts` diff --git a/README.md b/README.md index de504de..b9a099d 100644 --- a/README.md +++ b/README.md @@ -95,11 +95,12 @@ pi -e ~/src/src_local/pi-extensions/extensions/ssh-controlmaster.ts --ssh user@h **How it works:** 1. On `session_start`, runs `ssh -G ` to read the effective config for that host -2. If `~/.ssh/config` already configures `ControlMaster auto` or `yes` for the host, the existing system socket is reused — no second connection is opened and pi does **not** tear down the master on exit (it was the system's to manage) -3. Otherwise pi establishes its own master: `ssh -fN -o ControlMaster=yes -o ControlPersist=yes -o ControlPath=/tmp/pi-cm-.sock ` and shuts it down cleanly on exit -4. All tool calls multiplex over the socket with `-o ControlMaster=no -o ControlPath=` — near-zero per-call overhead -5. The system prompt is patched to tell the LLM it's operating on ` (via SSH ControlMaster: )` -6. User `!` shell commands are also routed over SSH +2. If `~/.ssh/config` already configures `ControlMaster auto` or `yes` for the host **and its `ControlPath` directory is writable**, the existing system socket is reused — no second connection is opened and pi does **not** tear down the master on exit (it was the system's to manage) +3. Otherwise (no system master, **or** its `ControlPath` is on a read-only mount — e.g. `~/.ssh/cm` when `~/.ssh` is bind-mounted read-only) pi establishes its own master: `ssh -fN -o ControlMaster=yes -o ControlPersist=yes -o ControlPath=/tmp/pi-cm-.sock ` and shuts it down cleanly on exit. The command-line `-o ControlPath` overrides the user's unwritable path. +4. The remote `pwd` is resolved with a direct connection (`-o ControlPath=none -o ControlMaster=no`) so a read-only system `ControlPath` can't make the initial probe fail +5. All tool calls multiplex over the socket with `-o ControlMaster=no -o ControlPath=` — near-zero per-call overhead +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 The status bar shows `⚡ own master` or `⚡ system master` so you can see which path was taken. diff --git a/extensions/ssh-controlmaster.ts b/extensions/ssh-controlmaster.ts index 221d848..d0aa0b4 100644 --- a/extensions/ssh-controlmaster.ts +++ b/extensions/ssh-controlmaster.ts @@ -28,9 +28,10 @@ */ import { spawn } from "node:child_process"; -import { writeFile, unlink } from "node:fs/promises"; -import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { constants as fsConstants } from "node:fs"; +import { access, writeFile, unlink } from "node:fs/promises"; +import { homedir, tmpdir } from "node:os"; +import { dirname, join } from "node:path"; import type { ExtensionAPI } from "@earendil-works/pi-coding-agent"; import { type BashOperations, @@ -59,6 +60,43 @@ function ownSocketPath(): string { return join(tmpdir(), `pi-cm-${process.pid}.sock`); } +/** + * Is the directory that would hold the ControlMaster socket actually writable? + * The devbox commonly bind-mounts ~/.ssh READ-ONLY, while the user's + * ~/.ssh/config may point ControlPath at ~/.ssh/cm/... — unwritable here. + * Reusing such a system socket path is doomed (the master can't bind, and even + * a plain `ssh` fails), so we detect it and fall back to our own socket in + * /tmp instead. OS-agnostic: it tests the actual filesystem, not the host OS. + * + * Returns true if the socket's parent dir exists and is writable, OR is + * missing but a `mkdir -p` could create it (nearest ancestor is writable). + */ +async function controlPathWritable(controlPath: string): Promise { + if (!controlPath) return false; + const expanded = controlPath.replace(/^~(?=\/|$)/, homedir()); + const dir = dirname(expanded); + try { + await access(dir, fsConstants.W_OK); + return true; // dir exists and is writable + } catch (e) { + if ((e as NodeJS.ErrnoException).code !== "ENOENT") return false; // exists but RO + // Dir is missing: could `mkdir -p` create it? Walk up to the nearest + // existing ancestor and check that it is writable. + let probe = dirname(dir); + for (;;) { + try { + await access(probe, fsConstants.W_OK); + return true; + } catch (e2) { + if ((e2 as NodeJS.ErrnoException).code !== "ENOENT") return false; + const parent = dirname(probe); + if (parent === probe) return false; // reached root, nothing writable + probe = parent; + } + } + } +} + function askpassScriptPath(): string { return join(tmpdir(), `pi-askpass-${process.pid}.sh`); } @@ -163,11 +201,14 @@ async function negotiateMaster( const cfg = await readSshConfig(remote); const systemHasMaster = cfg.master === "auto" || cfg.master === "yes"; - if (systemHasMaster && cfg.path) { + if (systemHasMaster && cfg.path && (await controlPathWritable(cfg.path))) { return { socketPath: cfg.path, ownsmaster: false }; } - // No system master — create our own + // Either no system master, or the system ControlPath is on a read-only mount + // (e.g. ~/.ssh/cm with ~/.ssh bind-mounted RO) — create our own at a writable + // /tmp socket. Our `-o ControlPath` on the command line overrides the user's + // unwritable path, so this works regardless of ~/.ssh/config. const socketPath = ownSocketPath(); await startControlMaster(remote, socketPath); return { socketPath, ownsmaster: true }; @@ -180,11 +221,13 @@ async function negotiateMasterWithPassword( const cfg = await readSshConfig(remote); const systemHasMaster = cfg.master === "auto" || cfg.master === "yes"; - if (systemHasMaster && cfg.path) { + if (systemHasMaster && cfg.path && (await controlPathWritable(cfg.path))) { // System master handles auth on its own — password flag is ignored return { socketPath: cfg.path, ownsmaster: false }; } + // No usable system master (none configured, or ControlPath is read-only) — + // create our own at a writable /tmp socket. const socketPath = ownSocketPath(); await startControlMasterWithPassword(remote, socketPath, password); return { socketPath, ownsmaster: true }; @@ -403,7 +446,20 @@ export default function (pi: ExtensionAPI) { [remote, remoteCwd] = arg.split(":"); } else { remote = arg; - remoteCwd = await run(["ssh", remote, "pwd"]).catch((e) => { + // Resolve remote $HOME with a DIRECT connection (ControlPath=none): the + // user's ~/.ssh/config may set ControlMaster auto + a ControlPath on a + // 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}`); }); }