From 0d0303184285f0c6eeda04e3111fff37cd210192 Mon Sep 17 00:00:00 2001 From: dormouse-bot <287024035+dormouse-bot@users.noreply.github.com> Date: Fri, 21 Aug 2026 16:10:21 +0000 Subject: [PATCH 1/5] fix(dor): harden the control socket path and make the host prove itself MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The control channel grants the whole surface API — `dor send` types into any pane, `dor read` returns its scrollback — and reached it over a PID-derived path in a shared namespace, with the client writing DORMOUSE_CONTROL_TOKEN as the first bytes on the wire to a peer that had proven nothing. - The server picks the path: a per-uid 0700 directory (created and re-checked like peer-link's peerDirIsSafe) with a random name on POSIX, an unguessable pipe name on Windows. - The server proves itself first. The token never goes on the wire; both halves are HMACs over the other side's nonce, compared in constant time. A bad hello is hung up on, not answered. - A lost bind stays fatal to the channel only. Both hosts drop the control variables from the environment they hand to spawned shells and restore them only once the socket is listening, so a squatter cannot get Dormouse to keep feeding it clients and tokens. Closes #431 --- docs/specs/dor-cli.md | 89 ++++++- dor/src/control-client.ts | 121 ++++++++-- dor/src/node-runtime.d.ts | 20 ++ dor/test/control-client.test.mjs | 119 +++++++++ standalone/sidecar/dor-control-server.js | 225 +++++++++++++++--- standalone/sidecar/dor-control-server.test.js | 213 ++++++++++++++--- standalone/sidecar/main.js | 50 +++- standalone/src-tauri/src/lib.rs | 35 +-- vscode-ext/src/pty-host.js | 45 +++- vscode-ext/src/pty-manager.ts | 15 +- 10 files changed, 797 insertions(+), 135 deletions(-) create mode 100644 dor/test/control-client.test.mjs 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/sidecar/dor-control-server.js b/standalone/sidecar/dor-control-server.js index d47481cc..a561dcba 100644 --- a/standalone/sidecar/dor-control-server.js +++ b/standalone/sidecar/dor-control-server.js @@ -1,20 +1,118 @@ 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. + */ +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 +120,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 +145,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 +167,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 +177,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 +229,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 +271,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 +280,34 @@ function createDorControlServer({ socketPath, token, send, timeoutMs = 65000 }) } } + // Clear a socket file left behind by a crash. A failure here means somebody + // else's file is sitting on the path — which is fatal to the control channel + // but, like a lost `listen`, must not be fatal to the host: throwing from this + // constructor would take the sidecar (and every PTY in it) down with it. 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 +321,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..eee49e1e 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,95 @@ 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 dir = path.join(os.tmpdir(), `dormouse-dor-test-${process.pid}-${Math.random().toString(36).slice(2)}`); + 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$/); + // macOS caps sun_path near 104 bytes; a path that overruns it fails to bind. + assert.ok(Buffer.byteLength(first) < 104, `${first} is ${Buffer.byteLength(first)} bytes`); + 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/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, }, }); From 65d7073936b858068af9513546fe5d98ddb3a057 Mon Sep 17 00:00:00 2001 From: dormouse-bot <287024035+dormouse-bot@users.noreply.github.com> Date: Fri, 21 Aug 2026 16:20:34 +0000 Subject: [PATCH 2/5] fix(dor): stop the ab dev harness computing a control socket path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sidecar now picks and binds the control socket path itself, deleting DORMOUSE_CONTROL_SOCKET from its own environment at startup. The harness's PID-derived value was therefore dead, but it was still printed on a `dor control socket:` log line — the wrong path in the one console someone debugging `dor` reads. The real path arrives on the forwarded sidecar stderr as `[dor-control] listening on …`. --- standalone/scripts/dev-agent-browser.mjs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) 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) => { From eef58bace1548e90be4b61f44dda39467f83abc9 Mon Sep 17 00:00:00 2001 From: dormouse-bot <287024035+dormouse-bot@users.noreply.github.com> Date: Fri, 21 Aug 2026 16:23:09 +0000 Subject: [PATCH 3/5] docs(dor): correct the pre-listen unlink comment, cross-link the dir check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The socket name is freshly random on every launch, so the pre-`listen` `unlinkSync` can only find nothing in production — a crash leftover sits at a different random name and nothing reclaims it. The comment above it claimed crash recovery it can no longer perform; say instead what the branch is for (the `socketPath` test seam) and where leftovers actually go. `ensureControlDir` and `peerDirIsSafe` are the same hardening predicate in two languages, and the reference was one-way. Add the pointer back from peer-link.ts so a future correction to the rule finds both copies. --- standalone/sidecar/dor-control-server.js | 16 ++++++++++++---- vscode-ext/src/peer-link.ts | 5 +++++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/standalone/sidecar/dor-control-server.js b/standalone/sidecar/dor-control-server.js index a561dcba..8de9fdce 100644 --- a/standalone/sidecar/dor-control-server.js +++ b/standalone/sidecar/dor-control-server.js @@ -58,6 +58,11 @@ function lstatOrNull(target) { * 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 { @@ -280,10 +285,13 @@ function createDorControlServer({ socketPath, socketDir, token, send, timeoutMs } } - // Clear a socket file left behind by a crash. A failure here means somebody - // else's file is sitting on the path — which is fatal to the control channel - // but, like a lost `listen`, must not be fatal to the host: throwing from this - // constructor would take the sidecar (and every PTY in it) down with it. + // 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(effectiveSocketPath); 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; From 0d6475fb13d7542e800f47ddd0f9aac53163c146 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 21 Aug 2026 16:59:17 -0700 Subject: [PATCH 4/5] test(dor): check the production socket path length --- standalone/sidecar/dor-control-server.test.js | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/standalone/sidecar/dor-control-server.test.js b/standalone/sidecar/dor-control-server.test.js index eee49e1e..c48745b5 100644 --- a/standalone/sidecar/dor-control-server.test.js +++ b/standalone/sidecar/dor-control-server.test.js @@ -223,7 +223,18 @@ test('dor control server refuses to start without a token', () => { }); test('the chosen socket path is unguessable and, on POSIX, privately owned', { skip: process.platform === 'win32' }, () => { - const dir = path.join(os.tmpdir(), `dormouse-dor-test-${process.pid}-${Math.random().toString(36).slice(2)}`); + const productionPath = resolveControlSocketPath(); + assert.ok(productionPath); + // macOS caps sun_path near 104 bytes. Check the production spelling: the + // isolated test directory below is intentionally different and must not make + // this assertion depend on a test-only prefix, the PID, or Math.random's + // variable-length rendering. + assert.ok( + Buffer.byteLength(productionPath) < 104, + `${productionPath} is ${Buffer.byteLength(productionPath)} bytes`, + ); + + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'dor-test-')); try { const first = resolveControlSocketPath(dir); const second = resolveControlSocketPath(dir); @@ -232,8 +243,6 @@ test('the chosen socket path is unguessable and, on POSIX, privately owned', { s assert.notEqual(first, second); assert.equal(path.dirname(first), dir); assert.match(path.basename(first), /^[0-9a-f]{16}\.sock$/); - // macOS caps sun_path near 104 bytes; a path that overruns it fails to bind. - assert.ok(Buffer.byteLength(first) < 104, `${first} is ${Buffer.byteLength(first)} bytes`); assert.equal(fs.lstatSync(dir).mode & 0o777, 0o700); } finally { fs.rmSync(dir, { recursive: true, force: true }); From 2581895cab1c8dfdb8b03ae31b87c45219c5d194 Mon Sep 17 00:00:00 2001 From: dormouse-bot <287024035+dormouse-bot@users.noreply.github.com> Date: Sat, 22 Aug 2026 00:11:27 +0000 Subject: [PATCH 5/5] test(dor): check the socket path length under the macOS tmpdir shape too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI runs only on ubuntu-latest, where os.tmpdir() is /tmp — 59 bytes of slack against the 104-byte sun_path cap, versus ~15 on macOS. Re-spell the production path under a synthetic /var/folders shape so a widened random name fails here instead of only on a Mac. --- standalone/sidecar/dor-control-server.test.js | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/standalone/sidecar/dor-control-server.test.js b/standalone/sidecar/dor-control-server.test.js index c48745b5..3a9a127b 100644 --- a/standalone/sidecar/dor-control-server.test.js +++ b/standalone/sidecar/dor-control-server.test.js @@ -225,14 +225,19 @@ test('dor control server refuses to start without a token', () => { 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. Check the production spelling: the - // isolated test directory below is intentionally different and must not make - // this assertion depend on a test-only prefix, the PID, or Math.random's - // variable-length rendering. - assert.ok( - Buffer.byteLength(productionPath) < 104, - `${productionPath} is ${Buffer.byteLength(productionPath)} bytes`, - ); + // 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 {