diff --git a/docs/specs/dor-cli.md b/docs/specs/dor-cli.md index 16df26ff..c9aae7d4 100644 --- a/docs/specs/dor-cli.md +++ b/docs/specs/dor-cli.md @@ -66,13 +66,12 @@ Public PTY env: folder. Unset under the standalone app (no workspace concept) and for an empty VS Code window. - `DORMOUSE_CONTROL_SOCKET` and `DORMOUSE_CONTROL_TOKEN` — private control - endpoint credentials. The token is the socket's sole authenticator, so it is - a CSPRNG value (24 random bytes, hex-encoded — `randomBytes` in the VS Code - host, the OS CSPRNG via `getrandom` in the standalone host) and the server - compares it in constant time (SHA-256 digests through `timingSafeEqual`), - never a short-circuiting string compare. Source of truth: - [`dor-control-server.js`](../../standalone/sidecar/dor-control-server.js) - (`tokenMatches`). + endpoint credentials, set together or not at all (see + [Control-channel security](#control-channel-security)). The token is the + shared secret both ends of the channel prove knowledge of, so it is a CSPRNG + value (24 random bytes, hex-encoded — `randomBytes` in the VS Code host, the + OS CSPRNG via `getrandom` in the standalone host). It is never sent on the + wire. `DORMOUSE_CLI_BIN` is host-internal spawn configuration. Terminals should rely on `PATH`, not on that variable. @@ -162,9 +161,14 @@ exports. `standalone/package.json` runs `pnpm stage:dor-cli` before Tauri dev/build. Rust resolves the staged/bundled CLI paths, starts the Node sidecar with -`DORMOUSE_HOST`, `DORMOUSE_NODE`, `DORMOUSE_CLI_BIN`, `DORMOUSE_CLI_JS`, -`DORMOUSE_CONTROL_SOCKET`, and `DORMOUSE_CONTROL_TOKEN`, then the shared PTY -core prepends `DORMOUSE_CLI_BIN` and sets `DORMOUSE_SURFACE_ID` per PTY. +`DORMOUSE_HOST`, `DORMOUSE_NODE`, `DORMOUSE_CLI_BIN`, `DORMOUSE_CLI_JS`, and +`DORMOUSE_CONTROL_TOKEN`, then the shared PTY core prepends `DORMOUSE_CLI_BIN` +and sets `DORMOUSE_SURFACE_ID` per PTY. Rust does **not** set +`DORMOUSE_CONTROL_SOCKET`: the sidecar chooses the path itself and puts both +control variables back into its own `process.env` — which is what `pty-core` +merges into every spawned shell — only once the socket is bound. Until then it +holds incoming stdin commands, so no PTY can be spawned into the window where +the channel's fate is undecided. Control direction: @@ -184,7 +188,13 @@ dor process `vscode-ext/package.json` runs `pnpm stage:dor-cli` before bundling the extension host and `pty-host.js`. The extension host computes the staged CLI paths under `context.extensionPath/dor-cli`, starts `pty-host.js`, and sends the -same dor env on each PTY spawn. +same dor env on each PTY spawn. `getDorRuntimeEnv` deliberately omits both +control variables: the token reaches `pty-host.js` through the fork env alone, +and the host pairs it with a bound socket path onto each spawn's env itself. The +`ready` message that releases the extension host's queued messages is held until +the channel has settled, so no spawn can race the bind. Source of truth: +[`pty-manager.ts`](../../vscode-ext/src/pty-manager.ts), +[`pty-host.js`](../../vscode-ext/src/pty-host.js). `DORMOUSE_NODE` points at VS Code's own runtime (`process.execPath`, re-execed as Node by VS Code's extension-host environment), not a user-installed Node. @@ -207,6 +217,52 @@ Because VS Code can host multiple Dormouse webviews in one extension host, the request includes `DORMOUSE_SURFACE_ID`; `message-router.ts` routes to the webview that owns that surface when one is available. +### Control-channel security + +The control channel carries the whole surface API — `dor send` types arbitrary +keystrokes into any pane, `dor read` returns its screen and scrollback, `dor +kill` destroys it — so the threat it defends against is another local principal +(a second account on the box, or any process running as the user) getting +between `dor` and its host. All three defences live in +[`dor-control-server.js`](../../standalone/sidecar/dor-control-server.js), which +both hosts load, and its client half in +[`control-client.ts`](../../dor/src/control-client.ts). + +**The server picks the path, and picks it unguessably.** On POSIX the socket is +`/dormouse-dor-/<8 random bytes>.sock`. The parent directory is +created `0700` and re-checked on every use — a real directory, not a symlink, +owned by this uid, at exactly mode `0700`; a directory of ours that is merely +loose gets tightened, anything else stands the channel down. This mirrors +`peerDirIsSafe()` in +[`peer-link.ts`](../../vscode-ext/src/peer-link.ts). 8 random bytes rather than +16 because macOS caps `sun_path` near 104 bytes and its `os.tmpdir()` spends +~50 of them. On Windows the name is `\\.\pipe\dormouse-dor-<8 random bytes>`: +the pipe namespace is machine-wide and has no directory to harden, so +unpredictability is what is left. Neither spelling derives from the PID, which +is enumerable and recycled. + +**The server proves itself first.** The token is a bearer credential, so it +never goes on the wire in either direction. The server speaks first with a +challenge nonce; the client answers with `HMAC-SHA256(token, "dor-control/client +")` and a nonce of its own; the server answers that with +`HMAC-SHA256(token, "dor-control/server ")` before the client sends any +request. A peer that fails its half gets hung up on with no reply — a wrong +answer and a port scan deserve the same nothing. Whoever merely bound the path +learns two nonces. Both sides compare proofs in constant time (SHA-256 digests +through `timingSafeEqual`), never a short-circuiting string compare. + +**A lost bind is fatal to the channel, never to the host.** PTY work must +survive a dead control channel, so neither host exits — but a host that keeps +handing `DORMOUSE_CONTROL_TOKEN` to every shell it spawns is feeding clients and +tokens to whoever won the race. So the token stops at the process that owns the +server: `pty-host.js` and the sidecar delete both control variables from their +own environment on startup (`pty-core` merges `process.env` into every shell) +and put them back only when `ready` resolves. When the bind is lost — a squatted +Windows pipe name, an unsafe socket directory, a socket file that cannot be +cleared — the variables stay gone and `dor` reports the endpoint as unavailable +rather than dialling a stranger. Both hosts hold their spawn path until `ready` +settles (2s ceiling) so the first terminal cannot race the bind. + ## Handle Model Dormouse supports multiple Workspaces within one Window (`docs/specs/glossary.md`): @@ -515,6 +571,17 @@ in `dor/test/cli-output.test.mjs`. ## Future +- **Surface a dead control channel in the UI.** A lost bind currently leaves one + `[dor-control]` line on the host's stderr, and the only thing a user sees is + `dor` reporting "Dormouse control endpoint is not available in this terminal + yet" — which reads like a startup race rather than a channel that will never + come up (and, on Windows, possibly a name somebody else took). It wants a + visible notice, but the two hosts have no shared place to put one, so the + design question is where: the Baseboard carries the standalone update notice + (`docs/specs/auto-update.md`) and has no VS Code counterpart. The plumbing that + would feed it exists — both hosts already know the outcome at `ready` (see + [Control-channel security](#control-channel-security)). + - **`dor skill` follow-ons** — skill-ecosystem publication (plugin marketplaces, npm) distributes the bootstrap stub, never a copy of the content. A user-level `--global` install variant waits until a story needs diff --git a/dor/src/control-client.ts b/dor/src/control-client.ts index be4c8ad2..b785a4db 100644 --- a/dor/src/control-client.ts +++ b/dor/src/control-client.ts @@ -1,3 +1,4 @@ +import { createHash, createHmac, randomBytes, timingSafeEqual } from 'node:crypto'; import { createConnection } from 'node:net'; import type { AgentBrowserSurfaceRequest, @@ -30,6 +31,30 @@ export interface SocketControlClientOptions { timeoutMs?: number; } +// Must match standalone/sidecar/dor-control-server.js, the other half of this +// handshake. The two live in different packages (a bundled ESM CLI and a plain +// CJS module loaded by both hosts) with no shared build, so the constants and +// the proof construction are duplicated rather than imported. +const CLIENT_PROOF_DOMAIN = 'dor-control/client'; +const SERVER_PROOF_DOMAIN = 'dor-control/server'; + +// Deliberately says nothing about which half of the handshake failed: from here +// a squatter, a torn-down host, and a stale socket file are the same event, and +// the user's next move is the same for all three. +const HANDSHAKE_FAILURE = + 'the process holding the Dormouse control socket could not prove it is Dormouse'; + +function proveToken(token: string, domain: string, nonce: string): string { + return createHmac('sha256', token).update(`${domain} ${nonce}`).digest('hex'); +} + +function proofMatches(provided: unknown, expected: string): boolean { + if (typeof provided !== 'string') return false; + const a = createHash('sha256').update(provided).digest(); + const b = createHash('sha256').update(expected).digest(); + return timingSafeEqual(a, b); +} + export class SocketControlClient implements ControlClient { private readonly socketPath: string; private readonly token: string; @@ -85,12 +110,25 @@ export class SocketControlClient implements ControlClient { return this.request(SURFACE_CONTROL_METHODS.resolveOpen, request); } + /** + * One request over one socket, preceded by a mutual handshake. + * + * The token is a bearer credential for the whole surface-control API (`send` + * types into any pane, `read` returns its scrollback), so it never goes on the + * wire: the peer must first prove it holds the token over a nonce we did not + * choose, and we answer over a nonce it did not choose. Whoever merely bound + * the socket path learns two nonces and nothing else. + */ private request(method: SurfaceControlMethod, params: unknown): Promise { const requestId = `dor-${this.idBase}-${++this.nextRequestId}`; return new Promise((resolve, reject) => { const socket = createConnection({ path: this.socketPath }); let responseBuffer = ''; let settled = false; + // 'challenge' → 'welcome' → 'response': the three lines the server sends, + // in order, over one connection. + let phase: 'challenge' | 'welcome' | 'response' = 'challenge'; + const nonce = randomBytes(16).toString('hex'); const settle = (callback: () => void) => { if (settled) return; @@ -105,39 +143,78 @@ export class SocketControlClient implements ControlClient { }, this.timeoutMs); socket.setEncoding('utf8'); - socket.on('connect', () => { - socket.write(`${JSON.stringify({ - requestId, - token: this.token, - surfaceId: this.surfaceId, - method, - params, - })}\n`); - }); + // Deliberately nothing on 'connect': the server speaks first. socket.on('data', (chunk) => { responseBuffer += chunk; - const newlineIndex = responseBuffer.indexOf('\n'); - if (newlineIndex === -1) return; - const line = responseBuffer.slice(0, newlineIndex); - settle(() => { + let newlineIndex = responseBuffer.indexOf('\n'); + while (newlineIndex !== -1 && !settled) { + const line = responseBuffer.slice(0, newlineIndex); + responseBuffer = responseBuffer.slice(newlineIndex + 1); + let frame: { kind?: unknown; nonce?: unknown; proof?: unknown }; + if (phase === 'response') { + settle(() => { + try { + const response = JSON.parse(line) as DorControlResult; + if (response.ok) { + resolve(response.result as T); + } else { + reject(new Error(response.error || `${method} failed`)); + } + } catch (error) { + reject(error instanceof Error ? error : new Error(String(error))); + } + }); + return; + } try { - const response = JSON.parse(line) as DorControlResult; - if (response.ok) { - resolve(response.result as T); - } else { - reject(new Error(response.error || `${method} failed`)); + frame = JSON.parse(line); + } catch { + settle(() => reject(new Error(HANDSHAKE_FAILURE))); + return; + } + if (phase === 'challenge') { + if (frame?.kind !== 'challenge' || typeof frame.nonce !== 'string' || !frame.nonce) { + settle(() => reject(new Error(HANDSHAKE_FAILURE))); + return; + } + // Answering a challenge proves nothing about the challenger, which + // is why this is all that is sent until the welcome comes back. + socket.write(`${JSON.stringify({ + kind: 'hello', + nonce, + proof: proveToken(this.token, CLIENT_PROOF_DOMAIN, frame.nonce), + })}\n`); + phase = 'welcome'; + } else { + if ( + frame?.kind !== 'welcome' || + !proofMatches(frame.proof, proveToken(this.token, SERVER_PROOF_DOMAIN, nonce)) + ) { + settle(() => reject(new Error(HANDSHAKE_FAILURE))); + return; } - } catch (error) { - reject(error instanceof Error ? error : new Error(String(error))); + socket.write(`${JSON.stringify({ + requestId, + surfaceId: this.surfaceId, + method, + params, + })}\n`); + phase = 'response'; } - }); + newlineIndex = responseBuffer.indexOf('\n'); + } }); socket.on('error', (error) => { settle(() => reject(error)); }); socket.on('end', () => { if (settled) return; - settle(() => reject(new Error(`connection closed before ${method} response`))); + // A peer that drops us mid-handshake is the same event as one that + // answers it wrongly — the server hangs up on a bad hello rather than + // replying — so report it the same way. + settle(() => + reject(new Error(phase === 'response' ? `connection closed before ${method} response` : HANDSHAKE_FAILURE)), + ); }); }); } diff --git a/dor/src/node-runtime.d.ts b/dor/src/node-runtime.d.ts index c926a118..500a8677 100644 --- a/dor/src/node-runtime.d.ts +++ b/dor/src/node-runtime.d.ts @@ -12,6 +12,26 @@ declare module 'node:net' { export function createConnection(options: { path: string }): Socket; } +declare module 'node:crypto' { + // Opaque stand-in for Buffer: this package ships without @types/node (see the + // hand-written shims around it), and nothing here needs more than "the thing + // digest() returns, which timingSafeEqual accepts". + export interface BinaryDigest { + readonly length: number; + } + + export interface Hash { + update(data: string): Hash; + digest(): BinaryDigest; + digest(encoding: 'hex'): string; + } + + export function createHash(algorithm: string): Hash; + export function createHmac(algorithm: string, key: string): Hash; + export function randomBytes(size: number): { toString(encoding: 'hex'): string }; + export function timingSafeEqual(a: BinaryDigest, b: BinaryDigest): boolean; +} + declare module 'node:fs' { export function existsSync(path: string): boolean; export function readFileSync(path: string, encoding: 'utf8'): string; diff --git a/dor/test/control-client.test.mjs b/dor/test/control-client.test.mjs new file mode 100644 index 00000000..98d5a9b8 --- /dev/null +++ b/dor/test/control-client.test.mjs @@ -0,0 +1,119 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { createServer } from 'node:net'; +import { mkdtemp, rm } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { createRequire } from 'node:module'; +import { SocketControlClient } from '../dist/control-client.js'; + +// The other half of the handshake, exactly as both hosts load it. +const require = createRequire(import.meta.url); +const { createDorControlServer } = require('../../standalone/sidecar/dor-control-server.js'); + +const skipOnWindows = { skip: process.platform === 'win32' ? 'unix sockets only' : false }; + +async function withTempSocket(run) { + const dir = await mkdtemp(join(tmpdir(), 'dor-control-')); + try { + return await run(join(dir, 'control.sock')); + } finally { + await rm(dir, { recursive: true, force: true }); + } +} + +test('a dor client and the host control server complete a request', skipOnWindows, async () => { + await withTempSocket(async (socketPath) => { + const forwarded = []; + const server = createDorControlServer({ + socketPath, + token: 'shared-secret', + send(event, data) { + forwarded.push({ event, data }); + server.respond({ requestId: data.requestId, ok: true, result: { surfaces: [] } }); + }, + }); + await server.ready; + try { + const client = new SocketControlClient({ + socketPath, + token: 'shared-secret', + surfaceId: 'surface-1', + timeoutMs: 5000, + }); + assert.deepEqual(await client.listSurfaces({}), { surfaces: [] }); + assert.equal(forwarded.length, 1); + assert.equal(forwarded[0].data.surfaceId, 'surface-1'); + assert.equal(forwarded[0].data.method, 'surface.list'); + } finally { + server.close(); + } + }); +}); + +test('a dor client refuses a host whose token does not match', skipOnWindows, async () => { + await withTempSocket(async (socketPath) => { + const server = createDorControlServer({ socketPath, token: 'shared-secret', send() {} }); + await server.ready; + try { + const client = new SocketControlClient({ socketPath, token: 'wrong-secret', timeoutMs: 5000 }); + await assert.rejects(client.listSurfaces({}), /could not prove it is Dormouse/); + } finally { + server.close(); + } + }); +}); + +// The reason the handshake exists: whoever holds the socket path used to receive +// DORMOUSE_CONTROL_TOKEN as the first bytes of the first `dor` invocation, and +// that token grants the whole surface API (`send` types into any pane, `read` +// returns its scrollback). +test('a squatter that cannot prove the token never receives it', skipOnWindows, async () => { + await withTempSocket(async (socketPath) => { + const received = []; + const squatter = createServer((socket) => { + socket.setEncoding('utf8'); + socket.on('error', () => {}); + socket.on('data', (chunk) => received.push(chunk)); + // Play the part convincingly: a challenge, then a welcome whose proof is + // the best a peer without the token can do. + socket.write(`${JSON.stringify({ kind: 'challenge', nonce: 'attacker-nonce' })}\n`); + setTimeout(() => { + socket.write(`${JSON.stringify({ kind: 'welcome', proof: 'f'.repeat(64) })}\n`); + }, 10).unref(); + }); + await new Promise((resolve) => squatter.listen(socketPath, resolve)); + try { + const client = new SocketControlClient({ socketPath, token: 'shared-secret', timeoutMs: 5000 }); + await assert.rejects(client.listSurfaces({}), /could not prove it is Dormouse/); + const wire = received.join(''); + assert.ok(!wire.includes('shared-secret'), `token leaked to the squatter: ${wire}`); + // Only the hello — the request, with whatever it would have carried, was + // never sent. + assert.equal(received.join('').trim().split('\n').length, 1); + assert.equal(JSON.parse(wire.trim()).kind, 'hello'); + } finally { + await new Promise((resolve) => squatter.close(resolve)); + } + }); +}); + +test('a peer that does not open with a challenge gets nothing at all', skipOnWindows, async () => { + await withTempSocket(async (socketPath) => { + const received = []; + const squatter = createServer((socket) => { + socket.setEncoding('utf8'); + socket.on('error', () => {}); + socket.on('data', (chunk) => received.push(chunk)); + socket.write(`${JSON.stringify({ ok: true, result: {} })}\n`); + }); + await new Promise((resolve) => squatter.listen(socketPath, resolve)); + try { + const client = new SocketControlClient({ socketPath, token: 'shared-secret', timeoutMs: 5000 }); + await assert.rejects(client.listSurfaces({}), /could not prove it is Dormouse/); + assert.deepEqual(received, []); + } finally { + await new Promise((resolve) => squatter.close(resolve)); + } + }); +}); diff --git a/standalone/scripts/dev-agent-browser.mjs b/standalone/scripts/dev-agent-browser.mjs index c0996888..6efcb18d 100644 --- a/standalone/scripts/dev-agent-browser.mjs +++ b/standalone/scripts/dev-agent-browser.mjs @@ -22,11 +22,10 @@ const dorEntrypoint = path.join(sidecarDir, 'dor-cli', 'dist', 'dor.js'); const hostPort = Number(process.env.DORMOUSE_BROWSER_DEV_HOST_PORT || 1422); const vitePort = Number(process.env.DORMOUSE_BROWSER_DEV_VITE_PORT || 1420); const browserSession = process.env.DORMOUSE_BROWSER_DEV_AB_SESSION || 'dormouse-dev-standalone'; -// Windows can't bind an AF_UNIX socket at an arbitrary temp path here (listen -// fails EACCES); the real standalone host uses a named pipe, so mirror that. -const controlSocket = process.platform === 'win32' - ? `\\\\.\\pipe\\dormouse-${process.pid}-browser-dor` - : path.join(os.tmpdir(), `dormouse-${process.pid}-browser-dor.sock`); +// Only the token: the sidecar picks the control socket path itself (hardened +// per-user directory on POSIX, unguessable pipe name on Windows) and reports it +// on its own stderr as `[dor-control] listening on …`, which this harness +// forwards. See docs/specs/dor-cli.md -> Control-channel security. const controlToken = Math.random().toString(36).slice(2); // The remote Host persists its enrollment + ACL here, under the harness's own // temp dir so a dev run never touches the installed app's state. @@ -201,13 +200,11 @@ function startSidecar() { DORMOUSE_NODE: process.execPath, DORMOUSE_CLI_BIN: dorBinDir, DORMOUSE_CLI_JS: dorEntrypoint, - DORMOUSE_CONTROL_SOCKET: controlSocket, DORMOUSE_CONTROL_TOKEN: controlToken, DORMOUSE_STATE_DIR: stateDir, }, }); log(`sidecar pid=${sidecar.pid}`); - log(`dor control socket: ${controlSocket}`); log(`remote host state dir: ${stateDir}`); createInterface({ input: sidecar.stdout }).on('line', (line) => { diff --git a/standalone/sidecar/dor-control-server.js b/standalone/sidecar/dor-control-server.js index d47481cc..8de9fdce 100644 --- a/standalone/sidecar/dor-control-server.js +++ b/standalone/sidecar/dor-control-server.js @@ -1,20 +1,123 @@ const crypto = require('node:crypto'); const fs = require('node:fs'); const net = require('node:net'); +const os = require('node:os'); +const path = require('node:path'); -// Constant-time control-token check. A short-circuiting `!==` compare leaks the -// token byte-by-byte to a co-resident local process that can time the response, +// Handshake proof domains. Separating the two directions keeps a proof the +// server emitted from being replayable as a client's answer and vice versa. +const CLIENT_PROOF_DOMAIN = 'dor-control/client'; +const SERVER_PROOF_DOMAIN = 'dor-control/server'; + +// A client that connects and then says nothing holds a socket (and, on Windows, +// a pipe instance) open forever. Nothing legitimate needs longer than this to +// answer a challenge it already has the token for. +const HANDSHAKE_BUDGET_MS = 10_000; + +function proveToken(token, domain, nonce) { + return crypto.createHmac('sha256', token).update(`${domain} ${nonce}`).digest('hex'); +} + +// Constant-time proof check. A short-circuiting `!==` compare leaks the expected +// proof byte-by-byte to a co-resident local process that can time the response, // so hash both sides to fixed-length digests (side-stepping timingSafeEqual's -// length-mismatch throw, which would itself leak the token length) and compare -// those. Mirrors the SHA-256 + timingSafeEqual pattern the selfhost server uses -// in server/src/state.ts. -function tokenMatches(provided, expected) { +// length-mismatch throw, which would itself leak the length) and compare those. +// Mirrors the SHA-256 + timingSafeEqual pattern the selfhost server uses in +// server/src/state.ts. +function proofMatches(provided, expected) { if (typeof provided !== 'string') return false; const a = crypto.createHash('sha256').update(provided).digest(); const b = crypto.createHash('sha256').update(expected).digest(); return crypto.timingSafeEqual(a, b); } +/** + * The per-user directory the POSIX control socket lives in. + * + * `os.tmpdir()` is world-writable, so a socket sitting directly in it can be + * created first by any other principal on the box. A private directory they + * cannot write to takes that away before the handshake has to. Mirrors + * `peerDirPath()` in vscode-ext/src/peer-link.ts. + */ +function controlDirPath() { + return path.join(os.tmpdir(), `dormouse-dor-${process.getuid?.() ?? 0}`); +} + +function lstatOrNull(target) { + try { + return fs.lstatSync(target); + } catch { + return null; + } +} + +/** + * Make the per-user socket directory and report whether it is safe to use. + * + * Anything but a plain directory of ours at mode 0700 is somebody else's, + * possibly on purpose, and no amount of retrying makes it ours — so the caller + * stands the control channel down rather than binding inside it. Returns the + * directory when it is safe, `null` when it is not. + * + * The same predicate as `peerDirIsSafe()` in vscode-ext/src/peer-link.ts, which + * carries the matching pointer back here: sync-vs-async fs and the return type + * are the only differences, so a correction to the hardening rule belongs in + * both copies. + */ +function ensureControlDir(dir = controlDirPath()) { + try { + fs.mkdirSync(dir, { recursive: true, mode: 0o700 }); + } catch { + // Already there, or unwritable — the checks below decide either way. + } + const uid = process.getuid?.(); + let info = lstatOrNull(dir); + // Ours but loose — a permissive umask, or a directory from before this check + // existed. Tightening something we already own is safe and keeps the test + // below exact rather than "0700 or better". + if (info?.isDirectory() && info.uid === uid && (info.mode & 0o777) !== 0o700) { + try { + fs.chmodSync(dir, 0o700); + } catch { + // Not ours to tighten; the re-stat below fails the check. + } + info = lstatOrNull(dir); + } + const safe = + !!info && + info.isDirectory() && + // `lstat` does not follow, so a symlink reports as one rather than as + // whatever it points at — which is the whole reason it is `lstat`. + !info.isSymbolicLink() && + info.uid === uid && + (info.mode & 0o777) === 0o700; + return safe ? dir : null; +} + +/** + * A fresh, unguessable path for this host process's control channel. + * + * Random rather than derived from the PID: a PID is enumerable and recycled, so + * a PID-derived name lets another principal create the path (POSIX) or take the + * pipe name (Windows) before Dormouse gets there. 8 bytes keeps the POSIX path + * clear of the ~104-byte sun_path cap on macOS, whose `os.tmpdir()` is already + * ~50 bytes on its own. + * + * Returns `null` when the POSIX directory cannot be made private — the caller + * must treat that as a dead control channel, not as a reason to bind anyway. + */ +function resolveControlSocketPath(dir) { + const unique = crypto.randomBytes(8).toString('hex'); + if (process.platform === 'win32') { + // Named pipes are not filesystem objects and carry their own ACL, so there + // is no directory to harden here; unpredictability is what is left. + return `\\\\.\\pipe\\dormouse-dor-${unique}`; + } + const safeDir = ensureControlDir(dir); + if (!safeDir) return null; + return path.join(safeDir, `${unique}.sock`); +} + // The server timeout must outlast the dor client's own deadline so the client // always controls the outcome — its longest is `dor ensure --restart` at 60s, so // 65s clears it. (A shorter server timeout would fire first and send the client a @@ -22,8 +125,23 @@ function tokenMatches(provided, expected) { // legitimately working, e.g. waiting on shell integration or a server restart.) // In practice socket close reaps pending entries the instant the client gives up; // this timer only releases a pending entry if the webview never answers at all. -function createDorControlServer({ socketPath, token, send, timeoutMs = 65000 }) { - if (!socketPath || !token) return null; +// +// `socketPath` and `socketDir` are test seams: production callers leave both +// unset and take the hardened path this module picks, which they then hand to +// spawned shells. +function createDorControlServer({ socketPath, socketDir, token, send, timeoutMs = 65000 }) { + if (!token) return null; + + const effectiveSocketPath = socketPath || resolveControlSocketPath(socketDir); + if (!effectiveSocketPath) { + const error = new Error( + `${socketDir || controlDirPath()} is not a private directory of this user; the dor control channel is off`, + ); + console.error(`[dor-control] ${error.message}`); + const failed = Promise.reject(error); + failed.catch(() => {}); + return { close() {}, ready: failed, respond() {}, socketPath: null }; + } const pending = new Map(); let resolveReady; @@ -32,9 +150,18 @@ function createDorControlServer({ socketPath, token, send, timeoutMs = 65000 }) resolveReady = resolve; rejectReady = reject; }); + ready.catch(() => { + // Callers gate the environment of spawned shells on `ready` and keep the + // sidecar/pty-host alive for normal PTY work; this handler only stops an + // unhandled rejection from taking the process down first. + }); const server = net.createServer((socket) => { socket.setEncoding('utf8'); let buffer = ''; + let authenticated = false; + const challenge = crypto.randomBytes(16).toString('hex'); + const handshakeTimer = setTimeout(() => socket.destroy(), HANDSHAKE_BUDGET_MS); + handshakeTimer.unref?.(); // A `dor` client that times out destroys its socket; without this handler // the resulting ECONNRESET would surface as an uncaught exception and take @@ -45,6 +172,7 @@ function createDorControlServer({ socketPath, token, send, timeoutMs = 65000 }) // release any entries owned by this socket right away rather than letting // them linger until their own timeout fires against a dead socket. socket.on('close', () => { + clearTimeout(handshakeTimer); for (const [requestId, entry] of pending) { if (entry.socket !== socket) continue; if (entry.timeout) clearTimeout(entry.timeout); @@ -54,14 +182,49 @@ function createDorControlServer({ socketPath, token, send, timeoutMs = 65000 }) socket.on('data', (chunk) => { buffer += chunk; - const newlineIndex = buffer.indexOf('\n'); - if (newlineIndex === -1) return; - const line = buffer.slice(0, newlineIndex); - buffer = buffer.slice(newlineIndex + 1); - handleRequest(socket, line); + let newlineIndex = buffer.indexOf('\n'); + while (newlineIndex !== -1) { + const line = buffer.slice(0, newlineIndex); + buffer = buffer.slice(newlineIndex + 1); + if (!authenticated) { + // Nothing is answered — not even an error — until the peer has proven + // it holds the token. A wrong answer is indistinguishable from a port + // scan, and neither deserves a reply. + if (!acceptHello(socket, line, challenge)) { + socket.destroy(); + return; + } + authenticated = true; + clearTimeout(handshakeTimer); + } else { + handleRequest(socket, line); + } + newlineIndex = buffer.indexOf('\n'); + } }); + + // The server speaks first, on purpose: a client that has not yet seen proof + // of the token must not volunteer one into whatever bound this path. + socket.write(`${JSON.stringify({ kind: 'challenge', nonce: challenge })}\n`); }); + function acceptHello(socket, line, challenge) { + let hello; + try { + hello = JSON.parse(line); + } catch { + return false; + } + if (!hello || hello.kind !== 'hello' || typeof hello.nonce !== 'string' || !hello.nonce) return false; + if (!proofMatches(hello.proof, proveToken(token, CLIENT_PROOF_DOMAIN, challenge))) return false; + // Our half, over the nonce *it* chose: answering a challenge proves nothing + // about the challenger, so the client sends no request until it has this. + socket.write( + `${JSON.stringify({ kind: 'welcome', proof: proveToken(token, SERVER_PROOF_DOMAIN, hello.nonce) })}\n`, + ); + return true; + } + function handleRequest(socket, line) { let request; try { @@ -71,11 +234,6 @@ function createDorControlServer({ socketPath, token, send, timeoutMs = 65000 }) return; } - if (!tokenMatches(request.token, token)) { - writeResponse(socket, { requestId: request.requestId, ok: false, error: 'invalid Dormouse control token' }); - return; - } - if (typeof request.requestId !== 'string' || typeof request.method !== 'string') { writeResponse(socket, { ok: false, error: 'invalid Dormouse control request' }); return; @@ -118,7 +276,7 @@ function createDorControlServer({ socketPath, token, send, timeoutMs = 65000 }) } if (process.platform !== 'win32') { try { - fs.unlinkSync(socketPath); + fs.unlinkSync(effectiveSocketPath); } catch (error) { if (error.code !== 'ENOENT') { console.error(`[dor-control] failed to remove socket: ${error.message}`); @@ -127,28 +285,37 @@ function createDorControlServer({ socketPath, token, send, timeoutMs = 65000 }) } } + // Clear whatever is sitting on the path. In production the name is freshly + // random, so this only ever finds nothing; it is the `socketPath` test seam + // that reaches the failure branch. A failure means somebody else's file is on + // the path — fatal to the control channel but, like a lost `listen`, never to + // the host: throwing from this constructor would take the sidecar (and every + // PTY in it) down with it. Crash leftovers are *not* reclaimed here; they sit + // in the control dir under their own random names until the OS sweeps tmp. if (process.platform !== 'win32') { try { - fs.unlinkSync(socketPath); + fs.unlinkSync(effectiveSocketPath); } catch (error) { - if (error.code !== 'ENOENT') throw error; + if (error.code !== 'ENOENT') { + console.error(`[dor-control] ${error.message}`); + rejectReady(error); + // Not `close`: nothing is listening and nothing is pending, and its + // unlink would delete the very file we just failed to claim. + return { close() {}, ready, respond() {}, socketPath: null }; + } } } - server.listen(socketPath, () => { - console.error(`[dor-control] listening on ${socketPath}`); + server.listen(effectiveSocketPath, () => { + console.error(`[dor-control] listening on ${effectiveSocketPath}`); resolveReady(); }); server.on('error', (error) => { console.error(`[dor-control] ${error.message}`); rejectReady(error); }); - ready.catch(() => { - // `ready` is used by tests; production logs listen failures through the - // server error handler and keeps the sidecar alive for normal PTY work. - }); - return { close, ready, respond }; + return { close, ready, respond, socketPath: effectiveSocketPath }; } function writeResponse(socket, response) { @@ -162,4 +329,12 @@ function writeResponse(socket, response) { } } -module.exports = { createDorControlServer }; +module.exports = { + createDorControlServer, + controlDirPath, + ensureControlDir, + resolveControlSocketPath, + proveToken, + CLIENT_PROOF_DOMAIN, + SERVER_PROOF_DOMAIN, +}; diff --git a/standalone/sidecar/dor-control-server.test.js b/standalone/sidecar/dor-control-server.test.js index 74e014bb..3a9a127b 100644 --- a/standalone/sidecar/dor-control-server.test.js +++ b/standalone/sidecar/dor-control-server.test.js @@ -1,9 +1,18 @@ const test = require('node:test'); const assert = require('node:assert/strict'); +const fs = require('node:fs'); const net = require('node:net'); +const os = require('node:os'); const path = require('node:path'); -const { createDorControlServer } = require('./dor-control-server'); +const { + createDorControlServer, + ensureControlDir, + resolveControlSocketPath, + proveToken, + CLIENT_PROOF_DOMAIN, + SERVER_PROOF_DOMAIN, +} = require('./dor-control-server'); function testSocketPath(name) { const suffix = `${process.pid}-${Date.now()}-${Math.random().toString(36).slice(2)}`; @@ -13,25 +22,45 @@ function testSocketPath(name) { return path.join('/tmp', `dormouse-${name}-${suffix}.sock`); } -function sendSocketRequest(socketPath, payload) { +/** + * A `dor`-shaped client: read the challenge, answer it, verify the welcome, and + * only then send the request. `hello`/`request` overrides let a test play the + * peer that gets the handshake wrong. + */ +function sendSocketRequest(socketPath, request, options = {}) { + const { token = 'secret', hello, expectLines = 3 } = options; return new Promise((resolve, reject) => { const socket = net.createConnection({ path: socketPath }); + const nonce = 'client-nonce'; + const lines = []; let buffer = ''; socket.setEncoding('utf8'); - socket.on('connect', () => { - socket.write(`${JSON.stringify(payload)}\n`); - }); socket.on('data', (chunk) => { buffer += chunk; - }); - socket.on('end', () => { - try { - resolve(JSON.parse(buffer.trim())); - } catch (error) { - reject(error); + let index = buffer.indexOf('\n'); + while (index !== -1) { + const line = buffer.slice(0, index); + buffer = buffer.slice(index + 1); + lines.push(JSON.parse(line)); + if (lines.length === 1) { + socket.write(`${JSON.stringify(hello ?? { + kind: 'hello', + nonce, + proof: proveToken(token, CLIENT_PROOF_DOMAIN, lines[0].nonce), + })}\n`); + } else if (lines.length === 2 && request) { + socket.write(`${JSON.stringify(request)}\n`); + } + if (lines.length >= expectLines) { + resolve({ lines, nonce, closed: false }); + socket.destroy(); + return; + } + index = buffer.indexOf('\n'); } }); + socket.on('close', () => resolve({ lines, nonce, closed: true })); socket.on('error', reject); }); } @@ -53,12 +82,12 @@ test('dor control server forwards valid requests and writes responses', async () }); assert.ok(server); + assert.equal(server.socketPath, socketPath); await server.ready; try { - const responsePromise = sendSocketRequest(socketPath, { + const exchange = sendSocketRequest(socketPath, { requestId: 'request-1', - token: 'secret', surfaceId: 'pane-1', method: 'surface.list', params: { pane: 'focused' }, @@ -82,7 +111,15 @@ test('dor control server forwards valid requests and writes responses', async () result: { surfaces: [] }, }); - assert.deepEqual(await responsePromise, { + const { lines, nonce } = await exchange; + assert.equal(lines[0].kind, 'challenge'); + // The server proves itself over the nonce the *client* chose, so a squatter + // that merely bound the path cannot fake this half. + assert.deepEqual(lines[1], { + kind: 'welcome', + proof: proveToken('secret', SERVER_PROOF_DOMAIN, nonce), + }); + assert.deepEqual(lines[2], { requestId: 'request-1', ok: true, result: { surfaces: [] }, @@ -92,8 +129,36 @@ test('dor control server forwards valid requests and writes responses', async () } }); -test('dor control server rejects invalid tokens', async () => { - const socketPath = testSocketPath('token'); +test('dor control server speaks first and never sees the raw token', async () => { + const socketPath = testSocketPath('challenge'); + const server = createDorControlServer({ socketPath, token: 'secret', send() {} }); + assert.ok(server); + await server.ready; + + try { + const received = await new Promise((resolve, reject) => { + const socket = net.createConnection({ path: socketPath }); + socket.setEncoding('utf8'); + // Say nothing at all — a squatter's whole hope is that the client + // volunteers the token on connect. + socket.on('data', (chunk) => { + resolve(chunk); + socket.destroy(); + }); + socket.on('error', reject); + }); + + const frame = JSON.parse(received.trim()); + assert.equal(frame.kind, 'challenge'); + assert.match(frame.nonce, /^[0-9a-f]{32}$/); + assert.ok(!received.includes('secret')); + } finally { + server.close(); + } +}); + +test('dor control server hangs up on a client that cannot prove the token', async () => { + const socketPath = testSocketPath('bad-hello'); const sent = []; const server = createDorControlServer({ socketPath, @@ -107,25 +172,24 @@ test('dor control server rejects invalid tokens', async () => { await server.ready; try { - const response = await sendSocketRequest(socketPath, { - requestId: 'request-1', - token: 'wrong', - method: 'surface.list', - }); + const { lines, closed } = await sendSocketRequest( + socketPath, + { requestId: 'request-1', method: 'surface.list' }, + { token: 'wrong' }, + ); + assert.equal(closed, true); + // Challenge only: no welcome, no response, and nothing reached the webview. + assert.equal(lines.length, 1); + assert.equal(lines[0].kind, 'challenge'); assert.deepEqual(sent, []); - assert.deepEqual(response, { - requestId: 'request-1', - ok: false, - error: 'invalid Dormouse control token', - }); } finally { server.close(); } }); -test('dor control server rejects a missing (non-string) token', async () => { - const socketPath = testSocketPath('token-missing'); +test('dor control server hangs up on a hello that is not a hello', async () => { + const socketPath = testSocketPath('no-hello'); const sent = []; const server = createDorControlServer({ socketPath, @@ -139,18 +203,109 @@ test('dor control server rejects a missing (non-string) token', async () => { await server.ready; try { - const response = await sendSocketRequest(socketPath, { - requestId: 'request-1', - method: 'surface.list', + // The pre-handshake wire shape: a request that carries the token the way the + // old protocol did must not be honoured. + const { lines, closed } = await sendSocketRequest(socketPath, null, { + hello: { requestId: 'request-1', token: 'secret', method: 'surface.list' }, }); + assert.equal(closed, true); + assert.equal(lines.length, 1); assert.deepEqual(sent, []); - assert.deepEqual(response, { - requestId: 'request-1', - ok: false, - error: 'invalid Dormouse control token', - }); } finally { server.close(); } }); + +test('dor control server refuses to start without a token', () => { + assert.equal(createDorControlServer({ token: '', send() {} }), null); + assert.equal(createDorControlServer({ token: undefined, send() {} }), null); +}); + +test('the chosen socket path is unguessable and, on POSIX, privately owned', { skip: process.platform === 'win32' }, () => { + const productionPath = resolveControlSocketPath(); + assert.ok(productionPath); + // macOS caps sun_path near 104 bytes, and it is the platform with no slack: + // its os.tmpdir() is a ~48-byte per-user path where CI's Linux `/tmp` is 4. + // Check the production spelling under both, or a name that grows by 20 bytes + // passes here and fails to bind on a Mac. The isolated directory below is + // intentionally different and must not make this depend on a test-only + // prefix, the PID, or Math.random's variable-length rendering. + for (const tmp of [os.tmpdir(), `/var/folders/ab/${'c'.repeat(30)}/T`]) { + const candidate = path.join(tmp, path.relative(os.tmpdir(), productionPath)); + assert.ok( + Buffer.byteLength(candidate) < 104, + `${candidate} is ${Buffer.byteLength(candidate)} bytes`, + ); + } + + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'dor-test-')); + try { + const first = resolveControlSocketPath(dir); + const second = resolveControlSocketPath(dir); + assert.ok(first); + // A PID-derived name is enumerable; two draws must not collide. + assert.notEqual(first, second); + assert.equal(path.dirname(first), dir); + assert.match(path.basename(first), /^[0-9a-f]{16}\.sock$/); + assert.equal(fs.lstatSync(dir).mode & 0o777, 0o700); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } +}); + +test('a loosely-permissioned directory of ours is tightened rather than used as-is', { skip: process.platform === 'win32' }, () => { + const dir = path.join(os.tmpdir(), `dormouse-dor-test-${process.pid}-${Math.random().toString(36).slice(2)}`); + fs.mkdirSync(dir, { recursive: true, mode: 0o777 }); + fs.chmodSync(dir, 0o777); + try { + assert.equal(ensureControlDir(dir), dir); + assert.equal(fs.lstatSync(dir).mode & 0o777, 0o700); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } +}); + +test('a socket directory that is not a directory of ours is refused', { skip: process.platform === 'win32' }, () => { + const dir = path.join(os.tmpdir(), `dormouse-dor-test-${process.pid}-${Math.random().toString(36).slice(2)}`); + // A symlink is the shape an attacker plants to redirect the bind elsewhere; + // `mkdir -p` succeeds through it, so only the lstat catches it. + fs.mkdirSync(`${dir}-target`, { recursive: true, mode: 0o700 }); + fs.symlinkSync(`${dir}-target`, dir); + try { + assert.equal(ensureControlDir(dir), null); + assert.equal(resolveControlSocketPath(dir), null); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + fs.rmSync(`${dir}-target`, { recursive: true, force: true }); + } +}); + +test('a control server that cannot get a private directory reports a dead channel', { skip: process.platform === 'win32' }, async () => { + const dir = path.join(os.tmpdir(), `dormouse-dor-test-${process.pid}-${Math.random().toString(36).slice(2)}`); + fs.mkdirSync(`${dir}-target`, { recursive: true, mode: 0o700 }); + fs.symlinkSync(`${dir}-target`, dir); + try { + const server = createDorControlServer({ socketDir: dir, token: 'secret', send() {} }); + assert.ok(server); + // No path at all — the caller has nothing to put in a shell's environment, + // which is the point: no bind, no token handout. + assert.equal(server.socketPath, null); + await assert.rejects(server.ready, /not a private directory/); + server.close(); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + fs.rmSync(`${dir}-target`, { recursive: true, force: true }); + } +}); + +test('a lost bind is fatal to the channel, not to the host', { skip: process.platform === 'win32' }, async () => { + // A path under a directory that does not exist: `listen` fails the way a + // squatted Windows pipe name does, and the constructor must survive it — a + // throw here would take the sidecar and every PTY in it down. + const socketPath = path.join(os.tmpdir(), `dormouse-dor-missing-${process.pid}`, 'control.sock'); + const server = createDorControlServer({ socketPath, token: 'secret', send() {} }); + assert.ok(server); + await assert.rejects(server.ready); + server.close(); +}); diff --git a/standalone/sidecar/main.js b/standalone/sidecar/main.js index e4669e14..e246273b 100644 --- a/standalone/sidecar/main.js +++ b/standalone/sidecar/main.js @@ -50,9 +50,18 @@ const remoteHost = createSidecarRemoteHost({ mgr, }); +// The control token arrives from Rust in our own environment, and `pty-core` +// merges `process.env` into every shell it spawns — so it has to come out of +// there and go back only once the channel is actually listening. A lost bind +// (a squatted Windows pipe name, an unsafe socket directory) is not fatal to +// PTY work, but it must not leave Dormouse handing the token, and the surface +// API it opens, to whoever won the path. See docs/specs/dor-cli.md. +const dorControlToken = process.env.DORMOUSE_CONTROL_TOKEN; +delete process.env.DORMOUSE_CONTROL_TOKEN; +delete process.env.DORMOUSE_CONTROL_SOCKET; + const dorControl = createDorControlServer({ - socketPath: process.env.DORMOUSE_CONTROL_SOCKET, - token: process.env.DORMOUSE_CONTROL_TOKEN, + token: dorControlToken, send, }); @@ -67,7 +76,42 @@ async function respondAsync(event, requestId, run) { const rl = readline.createInterface({ input: process.stdin }); +// Hold commands until the control channel has settled, so the very first +// `pty:spawn` cannot race the bind and produce a shell with no `dor` (or, worse, +// with a token for a channel that never came up). `listen` calls back or errors +// within a tick or two, and the 2s ceiling means a runtime that somehow does +// neither costs a short delay rather than a sidecar that never spawns anything. +let controlSettled = !dorControl; +const queuedLines = []; + +if (dorControl) { + dorControl.ready.then( + () => { + process.env.DORMOUSE_CONTROL_SOCKET = dorControl.socketPath; + process.env.DORMOUSE_CONTROL_TOKEN = dorControlToken; + }, + () => { + console.error('[dor-control] control channel is off; `dor` will not be available in new terminals'); + }, + ); + Promise.race([ + dorControl.ready.catch(() => {}), + new Promise((resolve) => setTimeout(resolve, 2000).unref?.()), + ]).then(() => { + controlSettled = true; + while (queuedLines.length > 0) handleLine(queuedLines.shift()); + }); +} + rl.on('line', (line) => { + if (!controlSettled) { + queuedLines.push(line); + return; + } + handleLine(line); +}); + +function handleLine(line) { try { const { event, data } = JSON.parse(line); switch (event) { @@ -158,7 +202,7 @@ rl.on('line', (line) => { } catch (err) { console.error(`[sidecar] Failed to parse message:`, err.message); } -}); +} let shuttingDown = false; async function shutdown() { diff --git a/standalone/src-tauri/src/lib.rs b/standalone/src-tauri/src/lib.rs index d9fb1174..bbcef313 100644 --- a/standalone/src-tauri/src/lib.rs +++ b/standalone/src-tauri/src/lib.rs @@ -1096,28 +1096,18 @@ fn ensure_console_subsystem_node(gui_node: &Path, app: &AppHandle) -> Result String { - let pid = std::process::id(); - #[cfg(windows)] - { - format!(r"\\.\pipe\dormouse-{pid}-dor") - } - #[cfg(not(windows))] - { - env::temp_dir() - .join(format!("dormouse-{pid}-dor.sock")) - .to_string_lossy() - .into_owned() - } -} - fn dor_control_token() -> String { - // Must be unguessable: it is the sole authenticator for the private `dor` - // control socket. A PID+timestamp value is locally discoverable (`ps`) and - // bounded by the app's launch window, so draw 24 bytes from the OS CSPRNG and - // hex-encode them — matching the VS Code host's randomBytes(24).toString('hex') - // in pty-manager.ts. Aborting on CSPRNG failure is deliberate: never fall back - // to a weak token. + // Must be unguessable: it is the shared secret both ends of the private `dor` + // control channel prove knowledge of (never sent on the wire — see + // standalone/sidecar/dor-control-server.js). A PID+timestamp value is locally + // discoverable (`ps`) and bounded by the app's launch window, so draw 24 bytes + // from the OS CSPRNG and hex-encode them — matching the VS Code host's + // randomBytes(24).toString('hex') in pty-manager.ts. Aborting on CSPRNG failure + // is deliberate: never fall back to a weak token. + // + // The socket path is not set here: the sidecar picks it (hardened per-user + // directory on POSIX, unguessable pipe name on Windows) and exports + // DORMOUSE_CONTROL_SOCKET into spawned shells itself, only once it is bound. let mut bytes = [0u8; 24]; getrandom::fill(&mut bytes).expect("OS CSPRNG unavailable for dor control token"); bytes.iter().map(|b| format!("{b:02x}")).collect() @@ -1172,7 +1162,6 @@ fn start_sidecar(app: &AppHandle) -> Result { let node_path = resolve_node_binary_path()?; let dor_cli_paths = resolve_dor_cli_paths(&sidecar_path, manifest_dir); let dor_node_path = resolve_dor_node_path(&node_path, app); - let dor_control_socket = dor_control_socket_path(); let dor_control_token = dor_control_token(); let state_dir = remote_host_state_dir(app); append_log(format!( @@ -1189,7 +1178,6 @@ fn start_sidecar(app: &AppHandle) -> Result { "[dor] CLI entrypoint: {}", dor_cli_paths.entrypoint.display() )); - append_log(format!("[dor] control socket: {dor_control_socket}")); append_log(format!( "[remote-host] state dir: {}", state_dir.as_deref().unwrap_or("(none)") @@ -1201,7 +1189,6 @@ fn start_sidecar(app: &AppHandle) -> Result { .env("DORMOUSE_NODE", &dor_node_path) .env("DORMOUSE_CLI_BIN", &dor_cli_paths.bin_dir) .env("DORMOUSE_CLI_JS", &dor_cli_paths.entrypoint) - .env("DORMOUSE_CONTROL_SOCKET", &dor_control_socket) .env("DORMOUSE_CONTROL_TOKEN", &dor_control_token) .env("DORMOUSE_STATE_DIR", state_dir.as_deref().unwrap_or("")) .stdin(Stdio::piped()) diff --git a/vscode-ext/src/peer-link.ts b/vscode-ext/src/peer-link.ts index 94f7b394..ab521dca 100644 --- a/vscode-ext/src/peer-link.ts +++ b/vscode-ext/src/peer-link.ts @@ -176,6 +176,11 @@ function peerDirPath(): string { * * Windows named pipes are not filesystem objects and carry their own ACL, so * there is nothing here for them to check. + * + * The same predicate is duplicated as `ensureControlDir()` in + * standalone/sidecar/dor-control-server.js (sync fs, returns the directory + * rather than a boolean) for the `dor` control socket. Nothing tests the two + * against each other, so a correction to the hardening rule belongs in both. */ async function peerDirIsSafe(): Promise { if (process.platform === 'win32') return true; diff --git a/vscode-ext/src/pty-host.js b/vscode-ext/src/pty-host.js index 05d18527..8f53e60e 100644 --- a/vscode-ext/src/pty-host.js +++ b/vscode-ext/src/pty-host.js @@ -10,17 +10,27 @@ const mgr = create((event, data) => { process.send({ type: event, ...data }); }, nodePty); +// The control token reaches this process through the fork env and stops here: +// `pty-core` merges our own `process.env` into every shell it spawns, so leaving +// it there would hand it out even when the channel never came up. It goes back +// into a shell's environment only alongside a socket path that is listening — +// see docs/specs/dor-cli.md. +const dorControlToken = process.env.DORMOUSE_CONTROL_TOKEN; +delete process.env.DORMOUSE_CONTROL_TOKEN; +delete process.env.DORMOUSE_CONTROL_SOCKET; + const dorControl = createDorControlServer({ - socketPath: process.env.DORMOUSE_CONTROL_SOCKET, - token: process.env.DORMOUSE_CONTROL_TOKEN, + token: dorControlToken, send(event, data) { process.send({ type: event, ...data }); }, }); +let dorControlEnv = null; + process.on('message', (msg) => { switch (msg.type) { - case 'spawn': mgr.spawn(msg.id, { cols: msg.cols, rows: msg.rows, cwd: msg.cwd, shell: msg.shell, args: msg.args, env: msg.env }); break; + case 'spawn': mgr.spawn(msg.id, { cols: msg.cols, rows: msg.rows, cwd: msg.cwd, shell: msg.shell, args: msg.args, env: { ...msg.env, ...dorControlEnv } }); break; case 'input': mgr.write(msg.id, msg.data); break; case 'resize': mgr.resize(msg.id, msg.cols, msg.rows); break; case 'kill': mgr.kill(msg.id); break; @@ -43,4 +53,31 @@ function shutdown() { process.on('disconnect', shutdown); process.on('SIGTERM', shutdown); -process.send({ type: 'ready' }); +// `ready` is what releases pty-manager's queued messages, so holding it until +// the control channel has settled means no spawn can race the bind — a shell +// either gets a listening socket or gets no control env at all. `listen` calls +// back or errors within a tick or two; the 2s ceiling keeps a runtime that +// somehow does neither from wedging terminal creation outright. +function announceReady() { + process.send({ type: 'ready' }); +} + +if (dorControl) { + dorControl.ready.then( + () => { + dorControlEnv = { + DORMOUSE_CONTROL_SOCKET: dorControl.socketPath, + DORMOUSE_CONTROL_TOKEN: dorControlToken, + }; + }, + () => { + console.error('[dor-control] control channel is off; `dor` will not be available in new terminals'); + }, + ); + Promise.race([ + dorControl.ready.catch(() => {}), + new Promise((resolve) => setTimeout(resolve, 2000).unref?.()), + ]).then(announceReady); +} else { + announceReady(); +} diff --git a/vscode-ext/src/pty-manager.ts b/vscode-ext/src/pty-manager.ts index 85a759e8..c296a25b 100644 --- a/vscode-ext/src/pty-manager.ts +++ b/vscode-ext/src/pty-manager.ts @@ -1,6 +1,5 @@ import { fork, ChildProcess, type Serializable } from 'child_process'; import * as path from 'path'; -import * as os from 'os'; import * as vscode from 'vscode'; import { randomBytes } from 'crypto'; import { log } from './log'; @@ -178,10 +177,12 @@ let childReady = false; let pendingMessages: any[] = []; const callbackSet = new Set(); const dorControlRequestListeners = new Set<(request: DorControlRequest) => void>(); +// The socket path is chosen by the pty-host, not here: it has to land in a +// hardened per-user directory (POSIX) or under an unguessable pipe name +// (Windows), and only the process that binds it knows whether it came up. The +// host reports it back through the spawn env — see pty-host.js and +// docs/specs/dor-cli.md. const dorControlToken = randomBytes(24).toString('hex'); -const dorControlSocket = process.platform === 'win32' - ? `\\\\.\\pipe\\dormouse-vscode-${process.pid}-dor` - : path.join(os.tmpdir(), `dormouse-vscode-${process.pid}-dor.sock`); // Always run the pty host under the editor's own Node — Electron's bundled // runtime (process.execPath, re-execed as Node via ELECTRON_RUN_AS_NODE, which @@ -222,8 +223,6 @@ function getDorRuntimeEnv(extensionPath: string): Record { // OSC 633 shell-integration scripts, copied next to the bundled pty-host by // the build (see package.json `build`). Mirrors how DORMOUSE_CLI_BIN is set. DORMOUSE_SHELL_INTEGRATION_DIR: path.join(extensionPath, 'dist', 'shell-integration'), - DORMOUSE_CONTROL_SOCKET: dorControlSocket, - DORMOUSE_CONTROL_TOKEN: dorControlToken, }; dorRuntimeEnvCache = { path: extensionPath, env }; return env; @@ -243,6 +242,10 @@ function ensureChild(extensionPath: string): ChildProcess { env: { ...process.env, ...dorEnv, + // Only the fork gets the token; `getDorRuntimeEnv` deliberately omits it, + // so it reaches a shell only after pty-host.js has a listening socket to + // pair it with. + DORMOUSE_CONTROL_TOKEN: dorControlToken, }, });