fix(ssh-controlmaster): handle read-only ControlPath under bind-mounted ~/.ssh
The devbox bind-mounts ~/.ssh read-only. A user ~/.ssh/config with a per-host ControlPath under it (the CGNAT idiom `ControlPath ~/.ssh/cm/%r@%h:%p`) is unwritable there, so a plain `ssh <host> pwd` exits 255 trying to bind the master socket — blocking `pi --ssh <host>` with "Could not resolve remote pwd". A system default cannot override a user's per-host value (SSH first-value-wins), so this must be handled in the extension. - controlPathWritable(): expands ~, tests whether the socket's parent dir is writable (or missing but creatable via nearest existing ancestor). Pure fs check — OS-agnostic, no host-OS detection. - negotiateMaster / negotiateMasterWithPassword: reuse the system master only when its ControlPath is writable; otherwise start our own /tmp master whose command-line `-o ControlPath` overrides the user's unwritable path. - Remote pwd probe: `-o ControlPath=none -o ControlMaster=no` so a read-only system ControlPath cannot make the initial probe fail. No behaviour change for configs without ControlMaster. Updates README.md + AGENTS.md to match.
This commit is contained in:
@@ -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 <host>`
|
||||
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-<pid>.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-<pid>.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 <remote> 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`
|
||||
|
||||
|
||||
@@ -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 <host>` 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-<pid>.sock <remote>` and shuts it down cleanly on exit
|
||||
4. All tool calls multiplex over the socket with `-o ControlMaster=no -o ControlPath=<socket>` — near-zero per-call overhead
|
||||
5. The system prompt is patched to tell the LLM it's operating on `<remoteCwd> (via SSH ControlMaster: <remote>)`
|
||||
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-<pid>.sock <remote>` 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=<socket>` — near-zero per-call overhead
|
||||
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
|
||||
|
||||
The status bar shows `⚡ own master` or `⚡ system master` so you can see which path was taken.
|
||||
|
||||
|
||||
@@ -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<boolean> {
|
||||
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}`);
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user