From 6fccb1e445e0a8c692a85f6c888bcc0e00d043b0 Mon Sep 17 00:00:00 2001 From: Nathan Herald Date: Thu, 30 Jul 2026 17:55:36 +0200 Subject: [PATCH 1/4] fix: recover unlinked live daemon registry --- CHANGELOG.md | 9 ++ README.md | 11 ++ completions/pty.bash | 5 +- completions/pty.fish | 2 + completions/pty.zsh | 5 + docs/disk-layout.md | 15 +++ src/cli.ts | 151 ++++++++++++++++++++++++ src/completions.ts | 7 ++ src/recovery.ts | 178 ++++++++++++++++++++++++++++ src/server.ts | 236 +++++++++++++++++++++++++++++++++++++ src/sessions.ts | 33 ++++++ tests/help.test.ts | 2 +- tests/recovery.test.ts | 261 +++++++++++++++++++++++++++++++++++++++++ 13 files changed, 913 insertions(+), 2 deletions(-) create mode 100644 src/recovery.ts create mode 100644 tests/recovery.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 37225b6..4b7c280 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,15 @@ ## Unreleased +### Storage format + +- Supporting live daemons now advertise a `recovery` capability in session + metadata. `pty recover --snapshot ` uses that captured + capability to authenticate a signal-free listener/registry rebind after an + external unlink. Recovery preserves the daemon, PTY child, existing clients, + generation, and launch identity; stale, tampered, legacy, or foreign-path + attempts fail closed without relaunching. + ### Read-only session listing - `listSessions()` and `pty list` are now strictly observational: they no diff --git a/README.md b/README.md index 66f81fb..d2a01a0 100644 --- a/README.md +++ b/README.md @@ -102,6 +102,7 @@ pty emit myserver user.note --text "checkpoint reached" # with a text payloa pty restart myserver # restart an exited session (must have been preserved) pty kill myserver # terminate a running session +pty --root /state/pty recover myserver --snapshot ./myserver.json # rebind a live supporting daemon after external unlink pty rm myserver # remove an exited session's metadata pty gc # reconcile: kill orphan children, respawn permanents, sweep vanished pty gc --dry-run # preview what gc would do without changing anything @@ -174,6 +175,16 @@ Two per-session flags override the configured default either way: against the dead metadata to respawn them). `pty kill` also preserves — it is stop-and-keep, deliberately distinct from `pty rm`. +If an external cleanup unlinks a live session's socket, pid, and metadata +paths, do not rerun its launch command: that can create a second provider. +New daemons advertise a recovery capability in metadata when their selected +root is private to the daemon user. Capture that complete metadata before the +unlink, then use `pty recover --snapshot ` +against the same `PTY_ROOT`. Recovery authenticates the original daemon and +rebinds its listener without signaling, restarting, or disconnecting existing +clients. Missing, legacy, stale, tampered, wrong-root, and occupied-path +snapshots fail closed. + ```sh pty run -d -- npm test # shipped default: reaped when it finishes PTY_REAP_ON_EXIT=false pty run -d -- npm test # preserved: peekable until gc sweeps it diff --git a/completions/pty.bash b/completions/pty.bash index d9c17d8..9fc7451 100644 --- a/completions/pty.bash +++ b/completions/pty.bash @@ -5,7 +5,7 @@ _pty() { COMPREPLY=() cur="${COMP_WORDS[COMP_CWORD]}" prev="${COMP_WORDS[COMP_CWORD-1]}" - commands="run attach a exec peek send events list ls stats restart kill rm remove gc tag tag-multi emit rename up down test remote-serve" + commands="run attach a exec peek send events list ls stats restart kill recover rm remove gc tag tag-multi emit rename up down test remote-serve" if [[ ${COMP_CWORD} -eq 1 ]]; then if [[ "${cur}" == -* ]]; then @@ -81,6 +81,9 @@ _pty() { COMPREPLY=($(compgen -W "${names}" -- "${cur}")) fi ;; + recover) + COMPREPLY=($(compgen -W "--snapshot" -- "${cur}")) + ;; rm|remove) if [[ "${cur}" == -* ]]; then COMPREPLY=($(compgen -W "" -- "${cur}")) diff --git a/completions/pty.fish b/completions/pty.fish index 78c6c27..754a14d 100644 --- a/completions/pty.fish +++ b/completions/pty.fish @@ -53,6 +53,7 @@ complete -c pty -n __pty_needs_command -a ls -d 'List sessions' complete -c pty -n __pty_needs_command -a stats -d 'Live CPU / memory / PIDs' complete -c pty -n __pty_needs_command -a restart -d 'SIGTERM + respawn' complete -c pty -n __pty_needs_command -a kill -d 'SIGTERM a running session' +complete -c pty -n __pty_needs_command -a recover -d 'Rebind a supporting live daemon after registry unlink' complete -c pty -n __pty_needs_command -a rm -d 'Remove exited metadata' complete -c pty -n __pty_needs_command -a remove -d 'Remove exited metadata' complete -c pty -n __pty_needs_command -a gc -d 'Reconciliation pass' @@ -114,6 +115,7 @@ complete -c pty -n '__pty_using_command restart' -l yes -s y -d 'Skip confirmati complete -c pty -n '__pty_using_command restart' -l force -d 'Attach after restart even from inside another pty' complete -c pty -n '__pty_using_command restart' -a '(__pty_sessions)' -d 'Session' complete -c pty -n '__pty_using_command kill' -a '(__pty_sessions)' -d 'Session' +complete -c pty -n '__pty_using_command recover' -l snapshot -d 'Captured capability-bearing metadata file' complete -c pty -n '__pty_using_command rm remove' -a '(__pty_sessions)' -d 'Session' complete -c pty -n '__pty_using_command gc' -l dry-run -s n -d 'Preview without changing anything' complete -c pty -n '__pty_using_command gc' -l idle-days -d 'Reap permanents with no attach in N days' diff --git a/completions/pty.zsh b/completions/pty.zsh index 84cb4ac..8eefa6b 100644 --- a/completions/pty.zsh +++ b/completions/pty.zsh @@ -26,6 +26,7 @@ _pty() { 'stats:Live CPU / memory / PIDs' 'restart:SIGTERM + respawn' 'kill:SIGTERM a running session' + 'recover:Rebind a supporting live daemon after registry unlink' 'rm:Remove exited metadata' 'remove:Alias for rm' 'gc:Reconciliation pass' @@ -132,6 +133,10 @@ _pty() { _arguments \ '1:session:_pty_sessions' ;; + recover) + _arguments \ + '--snapshot[Captured capability-bearing metadata file]' + ;; rm|remove) _arguments \ '1:session:_pty_sessions' diff --git a/docs/disk-layout.md b/docs/disk-layout.md index 69d3e56..f4c8be2 100644 --- a/docs/disk-layout.md +++ b/docs/disk-layout.md @@ -15,6 +15,7 @@ For non-Node tools that want to read pty's state without paying Node startup. Th | `.sock` | daemon IPC socket (Unix) | 2 | | `.pid` | daemon pid (decimal) | 2 | | `.lock` | creation-race lock | 2 | +| `.recovery/` | authenticated request/result exchange for supported live daemons | 2 | | `theme` | last-selected TUI theme | 2 | | `gc.log` | stdout/stderr of `pty gc` when run by launchd/cron (only present after auto-running gc is installed) | 2 | | `.json.tmp..` | atomic-write tmp — readers MUST ignore | n/a | @@ -35,6 +36,14 @@ Pretty-printed JSON. Source of truth: `SessionMetadata` in `src/sessions.ts`. { generation?: string; // opaque daemon generation; guards cleanup ownership daemonPid?: number; // daemon owning this generation, retained after child exit + recovery?: { // signal-free live-registry recovery capability + protocol: 1; + secret: string; // opaque request-authentication key + processStartToken: string; + launchIdentity: string; + rootDevice: number; + rootInode: number; + }; command: string; // resolved binary path args: string[]; displayCommand: string; // command as the user typed it @@ -60,6 +69,12 @@ Pretty-printed JSON. Source of truth: `SessionMetadata` in `src/sessions.ts`. removes files still owned by its generation, and `pty rm` waits for that daemon to finish deferred shutdown before it reports success. Readers should treat the generation token as opaque. +- `recovery` is present only when the daemon can prove its OS process-start + identity and the selected root is owned by the daemon user with no + group/other permissions. A snapshot containing this capability can authenticate + `pty recover` after the socket, pid, and metadata paths are externally + unlinked. The root is mode `0700`; treat the embedded secret as opaque and + do not publish snapshots. Successful recovery rotates the secret. - Reserved tag keys (`ptyfile*`, `strategy`, anything starting with `:`) are pty/tool-internal; hidden from `pty list` unless `--tags`. - User-facing tags that drive pty behavior but are visible by default: - `strategy=permanent` — `pty gc` respawns the session when its daemon exits (the historic supervisor's role; now stateless and run on a cron). diff --git a/src/cli.ts b/src/cli.ts index ae6684a..f0db551 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -20,8 +20,10 @@ import { validateName, validateDisplayName, acquireLock, + acquireRecoveryLock, isLockOwnedByPid, releaseLock, + releaseRecoveryLock, updateTags, setDisplayName, allRefs, @@ -30,6 +32,9 @@ import { writeMetadata, atomicWriteFileSync, getSessionDir, + getSocketPath, + getPidPath, + getMetadataPath, DEFAULT_SESSION_DIR, type SessionInfo, type SessionMetadata, @@ -44,6 +49,17 @@ import { readPtyFile, type PtySessionDef } from "./ptyfile.ts"; import { extractFilterTags as extractFilterTagsImpl, matchesAllTags, isReservedTagKey } from "./tags.ts"; import { parseDuration, formatDuration } from "./duration.ts"; import { serveRemoteControl, runRemoteServeStdio, fetchRemoteList, dialAndRoute, RouteRefusedError, PTY_REMOTE_ALPN, FABRIC_BIN } from "./remote.ts"; +import { + RECOVERY_PROTOCOL, + atomicWritePrivate, + readBoundedJson, + readProcessStartToken, + recoveryRequestPath, + recoveryResultPath, + signRecoveryRequest, + verifyRecoveryResult, + type RecoveryResult, +} from "./recovery.ts"; // Name this process so it shows up meaningfully in ps/top/htop/btm instead of // "MainThread" (V8's default main-thread name under Node 24+). `process.title` @@ -266,6 +282,17 @@ SIGTERM a running session's daemon. Metadata is kept — restart or \`pty rm\` i Examples: pty kill myserver`, + recover: `Usage: pty recover --snapshot + +Ask the original supporting daemon to republish an externally unlinked socket +and registry without signaling or restarting its PTY child. + +The snapshot must have been captured from the same selected PTY_ROOT before +the registry was unlinked and must advertise a recovery capability. + +Example: + pty --root /state/pty recover myserver --snapshot ./myserver.json`, + rm: `Usage: pty rm Remove an exited session's files (socket/pid/json/events) (alias: pty remove). @@ -468,6 +495,7 @@ Lifecycle: pty restart SIGTERM + respawn using stored metadata (prompts if running) pty restart -y Same, no prompt pty kill SIGTERM a running session's daemon + pty recover --snapshot Rebind a supporting live daemon after registry unlink pty rm Remove an exited session's metadata (alias: pty remove) pty gc Reconciliation pass: orphan-kill, abandoned-reap, permanent-respawn, exited-sweep @@ -1277,6 +1305,23 @@ async function main(): Promise { break; } + case "recover": { + const recoverName = args[1]; + const snapshotIndex = args.indexOf("--snapshot"); + const snapshotPath = snapshotIndex >= 0 ? args[snapshotIndex + 1] : null; + if (!recoverName || !snapshotPath) { + console.error("Usage: pty recover --snapshot "); + process.exit(1); + } + try { + await cmdRecover(recoverName, snapshotPath); + } catch (error) { + console.error(`pty recover: ${error instanceof Error ? error.message : String(error)}`); + process.exit(1); + } + break; + } + case "gc": { const gcArgs = args.slice(1); const dryRun = gcArgs.some((a) => a === "--dry-run" || a === "-n"); @@ -2468,6 +2513,112 @@ async function cmdKill(name: string): Promise { } } +async function cmdRecover(name: string, snapshotPath: string): Promise { + validateName(name); + const root = path.resolve(getSessionDir()); + const snapshot = readBoundedJson(path.resolve(snapshotPath)); + const capability = snapshot.recovery; + if ( + capability?.protocol !== RECOVERY_PROTOCOL || + typeof capability.secret !== "string" || + !snapshot.generation || + !snapshot.daemonPid + ) { + throw new Error("snapshot does not advertise supported recovery"); + } + const rootStat = fs.statSync(root); + if (rootStat.dev !== capability.rootDevice || rootStat.ino !== capability.rootInode) { + throw new Error("selected PTY_ROOT does not match the captured snapshot"); + } + if (readProcessStartToken(snapshot.daemonPid) !== capability.processStartToken) { + throw new Error("daemon PID/start identity no longer matches the snapshot"); + } + if (!acquireRecoveryLock(name)) { + throw new Error(`session "${name}" is being created by another process`); + } + + const requestPath = recoveryRequestPath(root, name); + const resultPath = recoveryResultPath(root, name); + try { + for (const target of [ + getSocketPath(name), + getPidPath(name), + getMetadataPath(name), + ]) { + if (fs.existsSync(target)) { + throw new Error("recovery target is no longer empty"); + } + } + try { fs.unlinkSync(resultPath); } catch {} + const nonce = randomBytes(16).toString("hex"); + const request = signRecoveryRequest(capability.secret, { + protocol: RECOVERY_PROTOCOL, + name, + daemonPid: snapshot.daemonPid, + generation: snapshot.generation, + processStartToken: capability.processStartToken, + launchIdentity: capability.launchIdentity, + rootDevice: capability.rootDevice, + rootInode: capability.rootInode, + lockOwnerPid: process.pid, + nonce, + metadata: snapshot, + }); + atomicWritePrivate(requestPath, request); + + const deadline = Date.now() + 7000; + let nextNotify = Date.now() + 250; + let result: RecoveryResult | null = null; + while (Date.now() < deadline) { + try { + const candidate = readBoundedJson(resultPath); + if (candidate.nonce === nonce) { + result = candidate; + break; + } + } catch {} + if (Date.now() >= nextNotify) { + atomicWritePrivate(requestPath, request); + nextNotify = Date.now() + 250; + } + await new Promise((resolve) => setTimeout(resolve, 25)); + } + if (!result) throw new Error("supporting daemon did not answer recovery request"); + if (!verifyRecoveryResult(capability.secret, result)) { + throw new Error("daemon recovery response authentication failed"); + } + if (!result.ok) throw new Error(result.error ?? "daemon refused recovery"); + if ( + result.daemonPid !== snapshot.daemonPid || + result.generation !== snapshot.generation || + result.processStartToken !== capability.processStartToken || + result.launchIdentity !== capability.launchIdentity + ) { + throw new Error("daemon recovery response changed identity"); + } + + const current = readMetadata(name); + if ( + !current || + current.daemonPid !== snapshot.daemonPid || + current.generation !== snapshot.generation || + current.recovery?.processStartToken !== capability.processStartToken || + current.recovery.launchIdentity !== capability.launchIdentity + ) { + throw new Error("republished metadata changed identity"); + } + const stats = await queryStats(name); + if (stats.daemon.pid !== snapshot.daemonPid) { + throw new Error("republished socket reached a different daemon"); + } + console.log(`Session "${name}" registry recovered without restart.`); + } finally { + try { fs.unlinkSync(requestPath); } catch {} + try { fs.unlinkSync(resultPath); } catch {} + releaseRecoveryLock(name, process.pid); + } +} + function renameUsage(): void { // Single source of truth: the same text `pty rename --help` prints, to stderr // for the error paths. diff --git a/src/completions.ts b/src/completions.ts index 528083e..da81577 100644 --- a/src/completions.ts +++ b/src/completions.ts @@ -166,6 +166,13 @@ const COMMANDS: readonly CommandSpec[] = [ desc: "SIGTERM a running session", dynamic: "sessions", }, + { + name: "recover", + desc: "Rebind a supporting live daemon after registry unlink", + flags: [ + { name: "snapshot", desc: "Captured capability-bearing metadata file" }, + ], + }, { name: "rm", aliases: ["remove"], diff --git a/src/recovery.ts b/src/recovery.ts new file mode 100644 index 0000000..b1b1206 --- /dev/null +++ b/src/recovery.ts @@ -0,0 +1,178 @@ +import * as fs from "node:fs"; +import * as path from "node:path"; +import { execFileSync } from "node:child_process"; +import { createHash, createHmac, timingSafeEqual } from "node:crypto"; +import type { SessionMetadata } from "./sessions.ts"; + +export const RECOVERY_PROTOCOL = 1; +export const RECOVERY_MAX_BYTES = 1024 * 1024; + +export interface RecoveryCapability { + protocol: 1; + secret: string; + processStartToken: string; + launchIdentity: string; + rootDevice: number; + rootInode: number; +} + +export interface RecoveryRequestPayload { + protocol: 1; + name: string; + daemonPid: number; + generation: string; + processStartToken: string; + launchIdentity: string; + rootDevice: number; + rootInode: number; + lockOwnerPid: number; + nonce: string; + metadata: SessionMetadata; +} + +export interface RecoveryRequest extends RecoveryRequestPayload { + auth: string; +} + +export interface RecoveryResultPayload { + protocol: 1; + name: string; + nonce: string; + ok: boolean; + error?: string; + daemonPid?: number; + generation?: string; + processStartToken?: string; + launchIdentity?: string; +} + +export interface RecoveryResult extends RecoveryResultPayload { + auth: string; +} + +export function recoveryDir(root: string): string { + return path.join(root, ".recovery"); +} + +export function recoveryRequestPath(root: string, name: string): string { + return path.join(recoveryDir(root), `${name}.request.json`); +} + +export function recoveryResultPath(root: string, name: string): string { + return path.join(recoveryDir(root), `${name}.result.json`); +} + +export function ensureRecoveryDir(root: string): void { + fs.mkdirSync(recoveryDir(root), { recursive: true, mode: 0o700 }); +} + +export function readProcessStartToken(pid: number): string | null { + if (!Number.isSafeInteger(pid) || pid <= 0) return null; + try { + if (process.platform === "linux") { + const stat = fs.readFileSync(`/proc/${pid}/stat`, "utf8"); + const tail = stat.slice(stat.lastIndexOf(")") + 2).trim().split(/\s+/); + const startTime = tail[19]; + return startTime ? `linux:${startTime}` : null; + } + if (process.platform === "darwin") { + const started = execFileSync( + "ps", + ["-o", "lstart=", "-p", String(pid)], + { encoding: "utf8", stdio: ["ignore", "pipe", "ignore"] }, + ).trim(); + return started ? `darwin:${started}` : null; + } + } catch {} + return null; +} + +export function stableStringify(value: unknown): string { + if (value === null || typeof value !== "object") return JSON.stringify(value); + if (Array.isArray(value)) return `[${value.map(stableStringify).join(",")}]`; + const record = value as Record; + return `{${Object.keys(record).sort().filter((key) => record[key] !== undefined) + .map((key) => `${JSON.stringify(key)}:${stableStringify(record[key])}`).join(",")}}`; +} + +export function launchIdentity(metadata: Pick< + SessionMetadata, + "command" | "args" | "displayCommand" | "cwd" | "rows" | "cols" | + "ephemeral" | "isolateEnv" | "extraEnv" | "env" +>): string { + return createHash("sha256").update(stableStringify({ + command: metadata.command, + args: metadata.args, + displayCommand: metadata.displayCommand, + cwd: metadata.cwd, + rows: metadata.rows, + cols: metadata.cols, + ephemeral: metadata.ephemeral === true, + isolateEnv: metadata.isolateEnv === true, + extraEnv: metadata.extraEnv, + env: metadata.env, + })).digest("hex"); +} + +function mac(secret: string, payload: unknown): string { + return createHmac("sha256", Buffer.from(secret, "hex")) + .update(stableStringify(payload)) + .digest("hex"); +} + +export function signRecoveryRequest( + secret: string, + payload: RecoveryRequestPayload, +): RecoveryRequest { + return { ...payload, auth: mac(secret, payload) }; +} + +export function verifyRecoveryRequest(secret: string, request: RecoveryRequest): boolean { + const { auth, ...payload } = request; + const expected = Buffer.from(mac(secret, payload), "hex"); + const actual = Buffer.from(typeof auth === "string" ? auth : "", "hex"); + return actual.length === expected.length && timingSafeEqual(actual, expected); +} + +export function signRecoveryResult( + secret: string, + payload: RecoveryResultPayload, +): RecoveryResult { + return { ...payload, auth: mac(secret, payload) }; +} + +export function verifyRecoveryResult(secret: string, result: RecoveryResult): boolean { + const { auth, ...payload } = result; + const expected = Buffer.from(mac(secret, payload), "hex"); + const actual = Buffer.from(typeof auth === "string" ? auth : "", "hex"); + return actual.length === expected.length && timingSafeEqual(actual, expected); +} + +export function readBoundedJson(file: string): T { + const stat = fs.lstatSync(file); + if (!stat.isFile() || stat.isSymbolicLink() || stat.size > RECOVERY_MAX_BYTES) { + throw new Error("recovery file must be a bounded regular file"); + } + return JSON.parse(fs.readFileSync(file, "utf8")) as T; +} + +export function atomicWritePrivate(file: string, value: unknown): void { + const tmp = `${file}.tmp.${process.pid}.${Math.random().toString(16).slice(2)}`; + try { + fs.writeFileSync(tmp, JSON.stringify(value, null, 2), { mode: 0o600 }); + fs.renameSync(tmp, file); + } catch (error) { + try { fs.unlinkSync(tmp); } catch {} + throw error; + } +} + +export function publishPrivateNoReplace(file: string, value: string): void { + const tmp = `${file}.tmp.${process.pid}.${Math.random().toString(16).slice(2)}`; + try { + fs.writeFileSync(tmp, value, { mode: 0o600 }); + fs.linkSync(tmp, file); + } finally { + try { fs.unlinkSync(tmp); } catch {} + } +} diff --git a/src/server.ts b/src/server.ts index 762597f..c2770a2 100644 --- a/src/server.ts +++ b/src/server.ts @@ -1,5 +1,6 @@ import * as net from "node:net"; import * as fs from "node:fs"; +import * as path from "node:path"; import { execFileSync } from "node:child_process"; import { randomBytes } from "node:crypto"; import * as pty from "node-pty"; @@ -22,6 +23,7 @@ import { getSocketPath, getPidPath, getMetadataPath, + getSessionDir, ensureSessionDir, cleanupOwnedSocket, cleanupOwnedAll, @@ -32,6 +34,23 @@ import { type SessionMetadata, } from "./sessions.ts"; import { EventWriter, clearEvents, EventType, type EventRecord } from "./events.ts"; +import { + RECOVERY_PROTOCOL, + atomicWritePrivate, + ensureRecoveryDir, + launchIdentity, + publishPrivateNoReplace, + readBoundedJson, + readProcessStartToken, + recoveryDir, + recoveryRequestPath, + recoveryResultPath, + signRecoveryResult, + verifyRecoveryRequest, + type RecoveryCapability, + type RecoveryRequest, + type RecoveryResultPayload, +} from "./recovery.ts"; interface Client { socket: net.Socket; @@ -224,6 +243,7 @@ export class PtyServer { private serialize: SerializeAddon; private ptyProcess: pty.IPty; private socketServer: net.Server; + private retiredSocketServers: net.Server[] = []; private clients = new Map(); private exited = false; private exitCode = 0; @@ -252,6 +272,10 @@ export class PtyServer { private lastResizeTime = 0; private eventWriter: EventWriter; private generation: string; + private recoveryCapability: RecoveryCapability | null = null; + private recoveryRoot = ""; + private recoveryInFlight = false; + private recoveryWatcher: fs.FSWatcher | null = null; private lastTitle = ""; readonly ready: Promise; // Resolves when the child process's onExit has fired — used by close() to @@ -533,6 +557,36 @@ export class PtyServer { // Create Unix socket server ensureSessionDir(); + this.recoveryRoot = path.resolve(getSessionDir()); + const rootStat = fs.statSync(this.recoveryRoot); + const processStartToken = readProcessStartToken(process.pid); + const rootIsPrivate = + (rootStat.mode & 0o077) === 0 && + (typeof process.getuid !== "function" || rootStat.uid === process.getuid()); + if (processStartToken !== null && rootIsPrivate) { + const identity = launchIdentity({ + command: options.command, + args: options.args, + displayCommand: options.displayCommand, + cwd: options.cwd, + rows: options.rows, + cols: options.cols, + ephemeral: options.ephemeral, + isolateEnv: options.isolateEnv, + extraEnv: options.extraEnv, + env: options.env, + }); + this.recoveryCapability = { + protocol: RECOVERY_PROTOCOL, + secret: randomBytes(32).toString("hex"), + processStartToken, + launchIdentity: identity, + rootDevice: rootStat.dev, + rootInode: rootStat.ino, + }; + ensureRecoveryDir(this.recoveryRoot); + this.startRecoveryWatcher(); + } clearEvents(this.name); const socketPath = getSocketPath(this.name); @@ -561,6 +615,7 @@ export class PtyServer { writeMetadata(this.name, { generation: this.generation, daemonPid: process.pid, + ...(this.recoveryCapability ? { recovery: this.recoveryCapability } : {}), command: options.command, args: options.args, displayCommand: options.displayCommand, @@ -593,6 +648,179 @@ export class PtyServer { }); } + private startRecoveryWatcher(): void { + const requestPath = recoveryRequestPath(this.recoveryRoot, this.name); + this.recoveryWatcher = fs.watch( + recoveryDir(this.recoveryRoot), + { persistent: false }, + (_event, filename) => { + if ( + filename === path.basename(requestPath) && + fs.existsSync(requestPath) && + !this.recoveryInFlight + ) { + void this.handleRecoveryRequest(); + } + }, + ); + } + + private recoveryMetadata( + observed: SessionMetadata, + capability: RecoveryCapability, + ): SessionMetadata { + return { + ...observed, + generation: this.generation, + daemonPid: process.pid, + recovery: capability, + }; + } + + private async handleRecoveryRequest(): Promise { + const capability = this.recoveryCapability; + if (!capability || this.recoveryInFlight) return; + this.recoveryInFlight = true; + const requestPath = recoveryRequestPath(this.recoveryRoot, this.name); + const resultPath = recoveryResultPath(this.recoveryRoot, this.name); + let request: RecoveryRequest | null = null; + let result: RecoveryResultPayload | null = null; + try { + request = readBoundedJson(requestPath); + const currentRoot = fs.statSync(this.recoveryRoot); + const currentStart = readProcessStartToken(process.pid); + const metadataCapability = request.metadata?.recovery; + const lockPath = path.join(this.recoveryRoot, `${this.name}.lock`); + const lockOwner = Number(fs.readFileSync(lockPath, "utf8").trim()); + const exact = + request.protocol === RECOVERY_PROTOCOL && + request.name === this.name && + request.daemonPid === process.pid && + request.generation === this.generation && + request.processStartToken === capability.processStartToken && + request.launchIdentity === capability.launchIdentity && + request.rootDevice === capability.rootDevice && + request.rootInode === capability.rootInode && + currentRoot.dev === capability.rootDevice && + currentRoot.ino === capability.rootInode && + currentStart === capability.processStartToken && + request.lockOwnerPid === lockOwner && + metadataCapability?.protocol === capability.protocol && + metadataCapability.secret === capability.secret && + metadataCapability.processStartToken === capability.processStartToken && + metadataCapability.launchIdentity === capability.launchIdentity && + verifyRecoveryRequest(capability.secret, request); + if (!exact) throw new Error("recovery identity or authentication mismatch"); + + const socketPath = getSocketPath(this.name); + const pidPath = getPidPath(this.name); + const metadataPath = getMetadataPath(this.name); + for (const target of [socketPath, pidPath, metadataPath]) { + if (fs.existsSync(target)) throw new Error("recovery target is no longer empty"); + } + + const replacement = net.createServer((socket) => this.handleClient(socket)); + await new Promise((resolve, reject) => { + replacement.once("error", reject); + replacement.listen(socketPath, resolve); + }); + replacement.on("error", (error) => { + console.error(`Socket server error: ${error.message}`); + }); + let socketIdentity: { dev: number; ino: number } | null = null; + let publishedPid = false; + let publishedMetadata = false; + let rotatedCapability: RecoveryCapability | null = null; + try { + fs.chmodSync(socketPath, 0o600); + const socketStat = fs.lstatSync(socketPath); + socketIdentity = { dev: socketStat.dev, ino: socketStat.ino }; + if (fs.existsSync(pidPath) || fs.existsSync(metadataPath)) { + throw new Error("recovery sidecar appeared during publication"); + } + const rotated: RecoveryCapability = { + ...capability, + secret: randomBytes(32).toString("hex"), + }; + rotatedCapability = rotated; + const recoveredMetadata = this.recoveryMetadata(request.metadata, rotated); + publishPrivateNoReplace(pidPath, process.pid.toString()); + publishedPid = true; + publishPrivateNoReplace(metadataPath, JSON.stringify(recoveredMetadata, null, 2)); + publishedMetadata = true; + const finalSocket = fs.lstatSync(socketPath); + if (finalSocket.dev !== socketIdentity.dev || finalSocket.ino !== socketIdentity.ino) { + throw new Error("recovery pathname was replaced during publication"); + } + + const previous = this.socketServer; + this.socketServer = replacement; + this.recoveryCapability = rotated; + // Node remembers a Unix server's pathname and unlinks it on close. + // The old listener still remembers the same string even though its + // inode was externally unlinked; closing it now would unlink the new + // listener. Keep the unreachable fd unref'd until daemon shutdown. + previous.unref(); + this.retiredSocketServers.push(previous); + result = { + protocol: RECOVERY_PROTOCOL, + name: this.name, + nonce: request.nonce, + ok: true, + daemonPid: process.pid, + generation: this.generation, + processStartToken: capability.processStartToken, + launchIdentity: capability.launchIdentity, + }; + } catch (error) { + try { replacement.close(); } catch {} + if (publishedMetadata && rotatedCapability) { + try { + const current = readMetadata(this.name); + if (current?.recovery?.secret === rotatedCapability.secret) { + fs.unlinkSync(metadataPath); + } + } catch {} + } + if (publishedPid) { + try { + if (fs.readFileSync(pidPath, "utf8").trim() === String(process.pid)) { + fs.unlinkSync(pidPath); + } + } catch {} + } + if (socketIdentity) { + try { + const current = fs.lstatSync(socketPath); + if (current.dev === socketIdentity.dev && current.ino === socketIdentity.ino) { + fs.unlinkSync(socketPath); + } + } catch {} + } + throw error; + } + } catch (error) { + result = { + protocol: RECOVERY_PROTOCOL, + name: this.name, + nonce: request?.nonce ?? "", + ok: false, + error: error instanceof Error ? error.message : "recovery refused", + }; + } finally { + try { + if (result) { + atomicWritePrivate( + resultPath, + signRecoveryResult(capability.secret, result), + ); + } + } catch {} + try { fs.unlinkSync(requestPath); } catch {} + this.recoveryInFlight = false; + } + } + private handleClient(socket: net.Socket): void { const client: Client = { socket, @@ -931,6 +1159,7 @@ export class PtyServer { writeMetadata(this.name, { generation: this.generation, daemonPid: process.pid, + ...(this.recoveryCapability ? { recovery: this.recoveryCapability } : {}), command: this.options.command, args: this.options.args, displayCommand: this.options.displayCommand, @@ -952,6 +1181,10 @@ export class PtyServer { /** Clean up resources. Does not call process.exit(). */ close(): Promise { + if (this.recoveryRoot) { + try { this.recoveryWatcher?.close(); } catch {} + this.recoveryWatcher = null; + } // Update exit metadata with final output — by the time close() runs, // all PTY data has been delivered to the terminal buffer. This overwrites // the initial save from onExit which may have had incomplete lastLines. @@ -963,6 +1196,9 @@ export class PtyServer { for (const client of this.clients.values()) { client.socket.destroy(); } + for (const retired of this.retiredSocketServers.splice(0)) { + try { retired.close(); } catch {} + } this.socketServer.close(async () => { cleanupOwnedSocket(this.name, { generation: this.generation, diff --git a/src/sessions.ts b/src/sessions.ts index 9d2bdb2..501f4b4 100644 --- a/src/sessions.ts +++ b/src/sessions.ts @@ -130,6 +130,9 @@ export interface SessionMetadata { * sidecar pidfile, this survives socket cleanup long enough for `pty rm` * to wait until deferred daemon shutdown is complete. */ daemonPid?: number; + /** Capability advertised only by daemons that support authenticated, + * signal-free recovery of an unlinked registry. Treat `secret` as opaque. */ + recovery?: import("./recovery.ts").RecoveryCapability; command: string; args: string[]; displayCommand: string; // original command as the user typed it @@ -1652,6 +1655,36 @@ export function acquireLock(name: string): boolean { return tryCreate(); } +/** Fail-closed lock acquisition for recovery. + * + * Unlike normal creation, recovery must not probe or steal an existing lock: + * any competing owner is grounds to refuse, and the recovery path promises no + * process signal (including a liveness-only signal 0 probe). */ +export function acquireRecoveryLock(name: string): boolean { + ensureSessionDir(); + try { + const fd = fs.openSync(getLockPath(name), "wx", 0o600); + try { + fs.writeSync(fd, process.pid.toString()); + } finally { + fs.closeSync(fd); + } + return true; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "EEXIST") return false; + throw error; + } +} + +/** Release only the recovery lock still owned by the expected PID. */ +export function releaseRecoveryLock(name: string, ownerPid: number): void { + try { + if (fs.readFileSync(getLockPath(name), "utf8").trim() === String(ownerPid)) { + fs.unlinkSync(getLockPath(name)); + } + } catch {} +} + export function releaseLock(name: string): void { try { fs.unlinkSync(getLockPath(name)); diff --git a/tests/help.test.ts b/tests/help.test.ts index 698550e..e423125 100644 --- a/tests/help.test.ts +++ b/tests/help.test.ts @@ -12,7 +12,7 @@ const cliSource = fs.readFileSync(path.join(__dirname, "..", "src", "cli.ts"), " // Canonical subcommands that must each ship focused `--help`. const COMMANDS = [ "run", "attach", "exec", "peek", "send", "events", "list", "stats", - "restart", "kill", "rm", "gc", "tag", "tag-multi", "emit", "rename", + "restart", "kill", "recover", "rm", "gc", "tag", "tag-multi", "emit", "rename", "up", "down", "test", "remote-serve", ]; // Aliases that must resolve to the same help. diff --git a/tests/recovery.test.ts b/tests/recovery.test.ts new file mode 100644 index 0000000..aba28f6 --- /dev/null +++ b/tests/recovery.test.ts @@ -0,0 +1,261 @@ +import * as fs from "node:fs"; +import * as net from "node:net"; +import * as os from "node:os"; +import * as path from "node:path"; +import { spawnSync } from "node:child_process"; +import { afterEach, describe, expect, it } from "vitest"; +import { PacketReader, MessageType, encodeAttach } from "../src/protocol.ts"; +import { queryStats } from "../src/client.ts"; +import { acquireRecoveryLock, type SessionMetadata } from "../src/sessions.ts"; +import { + RECOVERY_MAX_BYTES, + readProcessStartToken, + recoveryRequestPath, +} from "../src/recovery.ts"; +import { terminateAndWait } from "./setup/processes.ts"; + +const projectRoot = path.resolve(import.meta.dirname, ".."); +const cli = path.join(projectRoot, "dist", "cli.js"); +const roots: string[] = []; +const daemonPids: number[] = []; + +function makeRoot(): string { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "pty-recover-")); + roots.push(root); + process.env.PTY_ROOT = root; + return root; +} + +function run(root: string, args: string[]) { + return spawnSync(process.execPath, [cli, "--root", root, ...args], { + encoding: "utf8", + timeout: 15_000, + env: { ...process.env, PTY_SESSION: "", PTY_ROOT_LEGACY_SILENT: "1" }, + }); +} + +function metadata(root: string, name: string): SessionMetadata { + return JSON.parse(fs.readFileSync(path.join(root, `${name}.json`), "utf8")); +} + +function writeSnapshot(root: string, name: string, value: SessionMetadata): string { + const file = path.join(root, `${name}.snapshot`); + fs.writeFileSync(file, JSON.stringify(value)); + return file; +} + +function unlinkRegistry(root: string, name: string): void { + for (const suffix of ["sock", "pid", "json"]) { + fs.unlinkSync(path.join(root, `${name}.${suffix}`)); + } +} + +async function waitFor(check: () => boolean | Promise, timeout = 5000): Promise { + const deadline = Date.now() + timeout; + while (Date.now() < deadline) { + if (await check()) return; + await new Promise((resolve) => setTimeout(resolve, 25)); + } + throw new Error("timed out waiting for condition"); +} + +async function attachCollector(socketPath: string) { + const socket = net.createConnection(socketPath); + const reader = new PacketReader(); + let output = ""; + socket.on("data", (chunk) => { + for (const packet of reader.feed(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk))) { + if (packet.type === MessageType.DATA || packet.type === MessageType.SCREEN) { + output += packet.payload.toString(); + } + } + }); + await new Promise((resolve, reject) => { + socket.once("connect", resolve); + socket.once("error", reject); + }); + socket.write(encodeAttach(24, 80)); + return { socket, output: () => output }; +} + +function startProvider(root: string, name: string) { + const marker = path.join(root, `${name}.launches`); + const script = [ + "const fs=require('fs');", + "const marker=process.argv[1];", + "fs.appendFileSync(marker,'launch\\n');", + "let n=0; setInterval(()=>process.stdout.write(`tick:${++n}\\n`),50);", + ].join(""); + const started = run(root, [ + "run", "-d", "--id", name, "--no-display-name", "--", + process.execPath, "-e", script, marker, + ]); + expect(started.status, started.stderr || started.stdout).toBe(0); + const pid = Number(fs.readFileSync(path.join(root, `${name}.pid`), "utf8")); + daemonPids.push(pid); + return { marker, pid }; +} + +afterEach(async () => { + await terminateAndWait(daemonPids.splice(0)); + delete process.env.PTY_ROOT; + for (const root of roots.splice(0)) { + fs.rmSync(root, { recursive: true, force: true, maxRetries: 3, retryDelay: 50 }); + } +}); + +describe("live daemon registry recovery", () => { + it("refuses an existing creation lock without any liveness signal", () => { + const root = makeRoot(); + fs.writeFileSync(path.join(root, "locked.lock"), "2147483647"); + const originalKill = process.kill; + let calls = 0; + process.kill = ((..._args: Parameters) => { + calls++; + throw new Error("recovery must not signal"); + }) as typeof process.kill; + try { + expect(acquireRecoveryLock("locked")).toBe(false); + expect(calls).toBe(0); + } finally { + process.kill = originalKill; + } + }); + + it("does not advertise a recovery secret from a non-private root", () => { + const root = makeRoot(); + fs.chmodSync(root, 0o755); + startProvider(root, "public-root"); + expect(metadata(root, "public-root").recovery).toBeUndefined(); + }); + + it("rebinds the original daemon while preserving provider and attached client", async () => { + const root = makeRoot(); + const name = "positive"; + const { marker, pid } = startProvider(root, name); + const before = metadata(root, name); + const snapshot = writeSnapshot(root, name, before); + const clientA = await attachCollector(path.join(root, `${name}.sock`)); + await waitFor(() => clientA.output().includes("tick:2")); + + unlinkRegistry(root, name); + const outputBefore = clientA.output().length; + await waitFor(() => clientA.output().length > outputBefore); + await expect(queryStats(name)).rejects.toThrow(); + + const recovered = run(root, ["recover", name, "--snapshot", snapshot]); + expect(recovered.status, recovered.stderr || recovered.stdout).toBe(0); + const after = metadata(root, name); + const stats = await queryStats(name); + expect(stats.daemon.pid).toBe(pid); + expect(after.daemonPid).toBe(pid); + expect(after.generation).toBe(before.generation); + expect(after.recovery?.processStartToken).toBe(before.recovery?.processStartToken); + expect(after.recovery?.launchIdentity).toBe(before.recovery?.launchIdentity); + expect(after.recovery?.secret).not.toBe(before.recovery?.secret); + expect(fs.readFileSync(marker, "utf8").trim().split("\n")).toHaveLength(1); + const starts = fs.readFileSync(path.join(root, `${name}.events.jsonl`), "utf8") + .split("\n").filter((line) => line.includes('"session_start"')); + expect(starts).toHaveLength(1); + const clientB = await attachCollector(path.join(root, `${name}.sock`)); + await waitFor(() => clientB.output().includes("tick:")); + const preservedAt = clientA.output().length; + await waitFor(() => clientA.output().length > preservedAt); + clientA.socket.destroy(); + clientB.socket.destroy(); + }, 20_000); + + it("refuses malformed, unsupported, locked, wrong-root, and tampered requests", async () => { + const root = makeRoot(); + const name = "tampered"; + const { pid } = startProvider(root, name); + const before = metadata(root, name); + unlinkRegistry(root, name); + const requestPath = recoveryRequestPath(root, name); + + fs.writeFileSync(requestPath, "{not-json"); + await waitFor(() => !fs.existsSync(requestPath)); + fs.writeFileSync(requestPath, "x".repeat(RECOVERY_MAX_BYTES + 1)); + await waitFor(() => !fs.existsSync(requestPath)); + const symlinkTarget = path.join(root, "must-survive"); + fs.writeFileSync(symlinkTarget, "owned elsewhere"); + fs.symlinkSync(symlinkTarget, requestPath); + await waitFor(() => !fs.existsSync(requestPath)); + expect(fs.readFileSync(symlinkTarget, "utf8")).toBe("owned elsewhere"); + expect(readProcessStartToken(pid)).toBe(before.recovery?.processStartToken); + + const unsupported = { ...before, recovery: undefined }; + expect(run(root, ["recover", name, "--snapshot", writeSnapshot(root, "unsupported", unsupported)]).status) + .not.toBe(0); + + for (const [label, changed] of [ + ["pid", { ...before, daemonPid: pid + 1 }], + ["generation", { ...before, generation: "wrong" }], + ["start", { ...before, recovery: { ...before.recovery!, processStartToken: "wrong" } }], + ["launch", { ...before, recovery: { ...before.recovery!, launchIdentity: "wrong" } }], + ["secret", { ...before, recovery: { ...before.recovery!, secret: "00".repeat(32) } }], + ] as const) { + const result = run(root, [ + "recover", name, "--snapshot", writeSnapshot(root, label, changed), + ]); + expect(result.status, label).not.toBe(0); + expect(readProcessStartToken(pid)).toBe(before.recovery?.processStartToken); + } + + fs.writeFileSync(path.join(root, `${name}.lock`), String(process.pid)); + try { + const locked = run(root, [ + "recover", name, "--snapshot", writeSnapshot(root, "locked", before), + ]); + expect(locked.status).not.toBe(0); + expect(readProcessStartToken(pid)).toBe(before.recovery?.processStartToken); + } finally { + fs.unlinkSync(path.join(root, `${name}.lock`)); + } + + const otherRoot = makeRoot(); + const wrongRoot = run(otherRoot, [ + "recover", name, "--snapshot", writeSnapshot(root, "wrong-root", before), + ]); + expect(wrongRoot.status).not.toBe(0); + expect(readProcessStartToken(pid)).toBe(before.recovery?.processStartToken); + }, 20_000); + + it("never replaces a foreign pathname", async () => { + const root = makeRoot(); + const name = "foreign"; + startProvider(root, name); + const snapshot = writeSnapshot(root, name, metadata(root, name)); + unlinkRegistry(root, name); + const foreign = net.createServer(); + await new Promise((resolve) => foreign.listen(path.join(root, `${name}.sock`), resolve)); + try { + const result = run(root, ["recover", name, "--snapshot", snapshot]); + expect(result.status).not.toBe(0); + expect(fs.lstatSync(path.join(root, `${name}.sock`)).isSocket()).toBe(true); + const probe = net.createConnection(path.join(root, `${name}.sock`)); + await new Promise((resolve, reject) => { + probe.once("connect", resolve); + probe.once("error", reject); + }); + probe.destroy(); + } finally { + await new Promise((resolve) => foreign.close(() => resolve())); + } + }, 15_000); + + it("rejects replay of the rotated snapshot", () => { + const root = makeRoot(); + const name = "replay"; + startProvider(root, name); + const first = metadata(root, name); + const oldSnapshot = writeSnapshot(root, "old", first); + unlinkRegistry(root, name); + expect(run(root, ["recover", name, "--snapshot", oldSnapshot]).status).toBe(0); + const current = metadata(root, name); + const currentSnapshot = writeSnapshot(root, "current", current); + unlinkRegistry(root, name); + expect(run(root, ["recover", name, "--snapshot", oldSnapshot]).status).not.toBe(0); + expect(run(root, ["recover", name, "--snapshot", currentSnapshot]).status).toBe(0); + }, 20_000); +}); From 141ea8b0635a01b6a6faece5177ffdc93b1fb258 Mon Sep 17 00:00:00 2001 From: Nathan Herald Date: Thu, 30 Jul 2026 18:33:42 +0200 Subject: [PATCH 2/4] fix: close live recovery safety gaps --- CHANGELOG.md | 7 ++- README.md | 5 ++ docs/disk-layout.md | 8 ++- src/cli.ts | 57 +++++++++++++++----- src/recovery.ts | 116 ++++++++++++++++++++++++++++++++++++++- src/server.ts | 80 ++++++++++++++++++++------- src/sessions.ts | 53 +++++++++++++++--- tests/recovery.test.ts | 120 +++++++++++++++++++++++++++++++++++++++-- 8 files changed, 401 insertions(+), 45 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b7c280..aa60581 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,12 @@ capability to authenticate a signal-free listener/registry rebind after an external unlink. Recovery preserves the daemon, PTY child, existing clients, generation, and launch identity; stale, tampered, legacy, or foreign-path - attempts fail closed without relaunching. + attempts fail closed without relaunching. A retained signed metadata revision + prevents older snapshots from rolling back later tags, display names, attach + state, or lifecycle metadata. Recovery locks are resumable only by the same + authenticated daemon identity after an interrupted CLI, and both the root and + `.recovery` directory are identity/permission checked immediately before + authenticated request exchange. ### Read-only session listing diff --git a/README.md b/README.md index d2a01a0..98b8c69 100644 --- a/README.md +++ b/README.md @@ -184,6 +184,11 @@ against the same `PTY_ROOT`. Recovery authenticates the original daemon and rebinds its listener without signaling, restarting, or disconnecting existing clients. Missing, legacy, stale, tampered, wrong-root, and occupied-path snapshots fail closed. +Recovery also rejects a snapshot captured before any later metadata mutation, +and rechecks that both the root and its recovery directory are still private +before exchanging authenticated state. An interrupted recover command can be +resumed with the same valid snapshot; it never probes, signals, or relaunches +the supporting daemon. ```sh pty run -d -- npm test # shipped default: reaped when it finishes diff --git a/docs/disk-layout.md b/docs/disk-layout.md index f4c8be2..ffbdeb0 100644 --- a/docs/disk-layout.md +++ b/docs/disk-layout.md @@ -43,6 +43,9 @@ Pretty-printed JSON. Source of truth: `SessionMetadata` in `src/sessions.ts`. launchIdentity: string; rootDevice: number; rootInode: number; + recoveryDirDevice: number; + recoveryDirInode: number; + metadataRevision: string; // exact revision bound to retained recovery state }; command: string; // resolved binary path args: string[]; @@ -74,7 +77,10 @@ Pretty-printed JSON. Source of truth: `SessionMetadata` in `src/sessions.ts`. group/other permissions. A snapshot containing this capability can authenticate `pty recover` after the socket, pid, and metadata paths are externally unlinked. The root is mode `0700`; treat the embedded secret as opaque and - do not publish snapshots. Successful recovery rotates the secret. + do not publish snapshots. The root and `.recovery` directory identities and + permissions are revalidated before recovery state is exchanged. A signed + retained revision rejects older snapshots after tags, display name, attach + state, or other metadata changes. Successful recovery rotates the secret. - Reserved tag keys (`ptyfile*`, `strategy`, anything starting with `:`) are pty/tool-internal; hidden from `pty list` unless `--tags`. - User-facing tags that drive pty behavior but are visible by default: - `strategy=permanent` — `pty gc` respawns the session when its daemon exits (the historic supervisor's role; now stateless and run on a cron). diff --git a/src/cli.ts b/src/cli.ts index f0db551..e6ecb5d 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -51,9 +51,12 @@ import { parseDuration, formatDuration } from "./duration.ts"; import { serveRemoteControl, runRemoteServeStdio, fetchRemoteList, dialAndRoute, RouteRefusedError, PTY_REMOTE_ALPN, FABRIC_BIN } from "./remote.ts"; import { RECOVERY_PROTOCOL, + assertPrivateRecoveryPaths, atomicWritePrivate, readBoundedJson, readProcessStartToken, + recoveryLockContents, + recoveryLockIdentity, recoveryRequestPath, recoveryResultPath, signRecoveryRequest, @@ -2521,34 +2524,59 @@ async function cmdRecover(name: string, snapshotPath: string): Promise { if ( capability?.protocol !== RECOVERY_PROTOCOL || typeof capability.secret !== "string" || + typeof capability.metadataRevision !== "string" || + capability.metadataRevision.length === 0 || !snapshot.generation || !snapshot.daemonPid ) { throw new Error("snapshot does not advertise supported recovery"); } - const rootStat = fs.statSync(root); - if (rootStat.dev !== capability.rootDevice || rootStat.ino !== capability.rootInode) { - throw new Error("selected PTY_ROOT does not match the captured snapshot"); - } + assertPrivateRecoveryPaths(root, capability); if (readProcessStartToken(snapshot.daemonPid) !== capability.processStartToken) { throw new Error("daemon PID/start identity no longer matches the snapshot"); } - if (!acquireRecoveryLock(name)) { + const lockIdentity = recoveryLockIdentity({ + name, + daemonPid: snapshot.daemonPid, + processStartToken: capability.processStartToken, + rootDevice: capability.rootDevice, + rootInode: capability.rootInode, + recoveryDirDevice: capability.recoveryDirDevice, + recoveryDirInode: capability.recoveryDirInode, + }); + const lockContents = recoveryLockContents(snapshot.daemonPid, lockIdentity); + if (!acquireRecoveryLock(name, lockContents)) { throw new Error(`session "${name}" is being created by another process`); } const requestPath = recoveryRequestPath(root, name); const resultPath = recoveryResultPath(root, name); try { - for (const target of [ + const targets = [ getSocketPath(name), getPidPath(name), getMetadataPath(name), - ]) { - if (fs.existsSync(target)) { + ]; + if (targets.some((target) => fs.existsSync(target))) { + const current = readMetadata(name); + if ( + !targets.every((target) => fs.existsSync(target)) || + !current || + current.daemonPid !== snapshot.daemonPid || + current.generation !== snapshot.generation || + current.recovery?.processStartToken !== capability.processStartToken || + current.recovery.launchIdentity !== capability.launchIdentity + ) { throw new Error("recovery target is no longer empty"); } + const stats = await queryStats(name); + if (stats.daemon.pid !== snapshot.daemonPid) { + throw new Error("republished socket reached a different daemon"); + } + console.log(`Session "${name}" registry recovered without restart.`); + return; } + assertPrivateRecoveryPaths(root, capability); try { fs.unlinkSync(resultPath); } catch {} const nonce = randomBytes(16).toString("hex"); const request = signRecoveryRequest(capability.secret, { @@ -2560,10 +2588,11 @@ async function cmdRecover(name: string, snapshotPath: string): Promise { launchIdentity: capability.launchIdentity, rootDevice: capability.rootDevice, rootInode: capability.rootInode, - lockOwnerPid: process.pid, + lockIdentity, nonce, metadata: snapshot, }); + assertPrivateRecoveryPaths(root, capability); atomicWritePrivate(requestPath, request); const deadline = Date.now() + 7000; @@ -2578,6 +2607,7 @@ async function cmdRecover(name: string, snapshotPath: string): Promise { } } catch {} if (Date.now() >= nextNotify) { + assertPrivateRecoveryPaths(root, capability); atomicWritePrivate(requestPath, request); nextNotify = Date.now() + 250; } @@ -2613,9 +2643,12 @@ async function cmdRecover(name: string, snapshotPath: string): Promise { } console.log(`Session "${name}" registry recovered without restart.`); } finally { - try { fs.unlinkSync(requestPath); } catch {} - try { fs.unlinkSync(resultPath); } catch {} - releaseRecoveryLock(name, process.pid); + try { + assertPrivateRecoveryPaths(root, capability); + try { fs.unlinkSync(requestPath); } catch {} + try { fs.unlinkSync(resultPath); } catch {} + } catch {} + releaseRecoveryLock(name, lockContents); } } diff --git a/src/recovery.ts b/src/recovery.ts index b1b1206..ed396bb 100644 --- a/src/recovery.ts +++ b/src/recovery.ts @@ -14,6 +14,9 @@ export interface RecoveryCapability { launchIdentity: string; rootDevice: number; rootInode: number; + recoveryDirDevice: number; + recoveryDirInode: number; + metadataRevision: string; } export interface RecoveryRequestPayload { @@ -25,7 +28,7 @@ export interface RecoveryRequestPayload { launchIdentity: string; rootDevice: number; rootInode: number; - lockOwnerPid: number; + lockIdentity: string; nonce: string; metadata: SessionMetadata; } @@ -50,6 +53,30 @@ export interface RecoveryResult extends RecoveryResultPayload { auth: string; } +export interface RecoveryRevisionPayload { + protocol: 1; + name: string; + generation: string; + metadataRevision: string; +} + +export interface RecoveryRevision extends RecoveryRevisionPayload { + auth: string; +} + +export interface RecoveryPathIdentity { + rootDevice: number; + rootInode: number; + recoveryDirDevice: number; + recoveryDirInode: number; +} + +export interface RecoveryLockIdentityPayload extends RecoveryPathIdentity { + name: string; + daemonPid: number; + processStartToken: string; +} + export function recoveryDir(root: string): string { return path.join(root, ".recovery"); } @@ -62,10 +89,51 @@ export function recoveryResultPath(root: string, name: string): string { return path.join(recoveryDir(root), `${name}.result.json`); } +export function recoveryRevisionPath(root: string, name: string): string { + return path.join(recoveryDir(root), `${name}.revision.json`); +} + export function ensureRecoveryDir(root: string): void { fs.mkdirSync(recoveryDir(root), { recursive: true, mode: 0o700 }); } +function requirePrivateOwnedDirectory(target: string, label: string): fs.Stats { + const stat = fs.lstatSync(target); + if ( + stat.isSymbolicLink() || + !stat.isDirectory() || + (stat.mode & 0o077) !== 0 || + (typeof process.getuid === "function" && stat.uid !== process.getuid()) + ) { + throw new Error(`${label} must be an owned private non-symlink directory`); + } + return stat; +} + +export function assertPrivateRecoveryPaths( + root: string, + expected?: RecoveryPathIdentity, +): RecoveryPathIdentity { + const rootStat = requirePrivateOwnedDirectory(root, "PTY_ROOT"); + const recoveryStat = requirePrivateOwnedDirectory(recoveryDir(root), "PTY_ROOT recovery directory"); + const actual = { + rootDevice: rootStat.dev, + rootInode: rootStat.ino, + recoveryDirDevice: recoveryStat.dev, + recoveryDirInode: recoveryStat.ino, + }; + if ( + expected && + (actual.rootDevice !== expected.rootDevice || + actual.rootInode !== expected.rootInode || + actual.recoveryDirDevice !== expected.recoveryDirDevice || + actual.recoveryDirInode !== expected.recoveryDirInode) + ) { + throw new Error("recovery root identity changed"); + } + return actual; +} + export function readProcessStartToken(pid: number): string | null { if (!Number.isSafeInteger(pid) || pid <= 0) return null; try { @@ -114,6 +182,26 @@ export function launchIdentity(metadata: Pick< })).digest("hex"); } +export function metadataRevision(metadata: SessionMetadata): string { + const recovery = metadata.recovery + ? { ...metadata.recovery, metadataRevision: undefined } + : undefined; + return createHash("sha256").update(stableStringify({ + ...metadata, + recovery, + })).digest("hex"); +} + +export function stampRecoveryMetadata(metadata: SessionMetadata): SessionMetadata { + if (!metadata.recovery) return metadata; + const stamped: SessionMetadata = { + ...metadata, + recovery: { ...metadata.recovery, metadataRevision: "" }, + }; + stamped.recovery!.metadataRevision = metadataRevision(stamped); + return stamped; +} + function mac(secret: string, payload: unknown): string { return createHmac("sha256", Buffer.from(secret, "hex")) .update(stableStringify(payload)) @@ -148,6 +236,32 @@ export function verifyRecoveryResult(secret: string, result: RecoveryResult): bo return actual.length === expected.length && timingSafeEqual(actual, expected); } +export function signRecoveryRevision( + secret: string, + payload: RecoveryRevisionPayload, +): RecoveryRevision { + return { ...payload, auth: mac(secret, payload) }; +} + +export function verifyRecoveryRevision(secret: string, revision: RecoveryRevision): boolean { + const { auth, ...payload } = revision; + const expected = Buffer.from(mac(secret, payload), "hex"); + const actual = Buffer.from(typeof auth === "string" ? auth : "", "hex"); + return actual.length === expected.length && timingSafeEqual(actual, expected); +} + +export function recoveryLockIdentity( + payload: RecoveryLockIdentityPayload, +): string { + return createHash("sha256") + .update(stableStringify({ purpose: "recovery-lock", ...payload })) + .digest("hex"); +} + +export function recoveryLockContents(daemonPid: number, identity: string): string { + return `${daemonPid}\nrecovery:${identity}\n`; +} + export function readBoundedJson(file: string): T { const stat = fs.lstatSync(file); if (!stat.isFile() || stat.isSymbolicLink() || stat.size > RECOVERY_MAX_BYTES) { diff --git a/src/server.ts b/src/server.ts index c2770a2..72e0665 100644 --- a/src/server.ts +++ b/src/server.ts @@ -36,19 +36,28 @@ import { import { EventWriter, clearEvents, EventType, type EventRecord } from "./events.ts"; import { RECOVERY_PROTOCOL, + assertPrivateRecoveryPaths, atomicWritePrivate, ensureRecoveryDir, launchIdentity, + metadataRevision, publishPrivateNoReplace, readBoundedJson, readProcessStartToken, recoveryDir, + recoveryLockContents, + recoveryLockIdentity, recoveryRequestPath, + recoveryRevisionPath, recoveryResultPath, + signRecoveryRevision, signRecoveryResult, + stampRecoveryMetadata, verifyRecoveryRequest, + verifyRecoveryRevision, type RecoveryCapability, type RecoveryRequest, + type RecoveryRevision, type RecoveryResultPayload, } from "./recovery.ts"; @@ -558,12 +567,11 @@ export class PtyServer { // Create Unix socket server ensureSessionDir(); this.recoveryRoot = path.resolve(getSessionDir()); - const rootStat = fs.statSync(this.recoveryRoot); const processStartToken = readProcessStartToken(process.pid); - const rootIsPrivate = - (rootStat.mode & 0o077) === 0 && - (typeof process.getuid !== "function" || rootStat.uid === process.getuid()); - if (processStartToken !== null && rootIsPrivate) { + try { + ensureRecoveryDir(this.recoveryRoot); + const paths = assertPrivateRecoveryPaths(this.recoveryRoot); + if (processStartToken === null) throw new Error("process start identity unavailable"); const identity = launchIdentity({ command: options.command, args: options.args, @@ -581,12 +589,11 @@ export class PtyServer { secret: randomBytes(32).toString("hex"), processStartToken, launchIdentity: identity, - rootDevice: rootStat.dev, - rootInode: rootStat.ino, + ...paths, + metadataRevision: "", }; - ensureRecoveryDir(this.recoveryRoot); this.startRecoveryWatcher(); - } + } catch {} clearEvents(this.name); const socketPath = getSocketPath(this.name); @@ -669,12 +676,12 @@ export class PtyServer { observed: SessionMetadata, capability: RecoveryCapability, ): SessionMetadata { - return { + return stampRecoveryMetadata({ ...observed, generation: this.generation, daemonPid: process.pid, recovery: capability, - }; + }); } private async handleRecoveryRequest(): Promise { @@ -686,12 +693,24 @@ export class PtyServer { let request: RecoveryRequest | null = null; let result: RecoveryResultPayload | null = null; try { + assertPrivateRecoveryPaths(this.recoveryRoot, capability); request = readBoundedJson(requestPath); - const currentRoot = fs.statSync(this.recoveryRoot); const currentStart = readProcessStartToken(process.pid); const metadataCapability = request.metadata?.recovery; const lockPath = path.join(this.recoveryRoot, `${this.name}.lock`); - const lockOwner = Number(fs.readFileSync(lockPath, "utf8").trim()); + const lockContents = recoveryLockContents(process.pid, request.lockIdentity); + const expectedLockIdentity = recoveryLockIdentity({ + name: request.name, + daemonPid: request.daemonPid, + processStartToken: request.processStartToken, + rootDevice: request.rootDevice, + rootInode: request.rootInode, + recoveryDirDevice: capability.recoveryDirDevice, + recoveryDirInode: capability.recoveryDirInode, + }); + const revision = readBoundedJson( + recoveryRevisionPath(this.recoveryRoot, this.name), + ); const exact = request.protocol === RECOVERY_PROTOCOL && request.name === this.name && @@ -701,14 +720,23 @@ export class PtyServer { request.launchIdentity === capability.launchIdentity && request.rootDevice === capability.rootDevice && request.rootInode === capability.rootInode && - currentRoot.dev === capability.rootDevice && - currentRoot.ino === capability.rootInode && currentStart === capability.processStartToken && - request.lockOwnerPid === lockOwner && + request.lockIdentity === expectedLockIdentity && + fs.readFileSync(lockPath, "utf8") === lockContents && metadataCapability?.protocol === capability.protocol && metadataCapability.secret === capability.secret && metadataCapability.processStartToken === capability.processStartToken && metadataCapability.launchIdentity === capability.launchIdentity && + metadataCapability.rootDevice === capability.rootDevice && + metadataCapability.rootInode === capability.rootInode && + metadataCapability.recoveryDirDevice === capability.recoveryDirDevice && + metadataCapability.recoveryDirInode === capability.recoveryDirInode && + metadataCapability.metadataRevision === metadataRevision(request.metadata) && + revision.protocol === RECOVERY_PROTOCOL && + revision.name === this.name && + revision.generation === this.generation && + revision.metadataRevision === metadataCapability.metadataRevision && + verifyRecoveryRevision(capability.secret, revision) && verifyRecoveryRequest(capability.secret, request); if (!exact) throw new Error("recovery identity or authentication mismatch"); @@ -722,6 +750,7 @@ export class PtyServer { const replacement = net.createServer((socket) => this.handleClient(socket)); await new Promise((resolve, reject) => { replacement.once("error", reject); + assertPrivateRecoveryPaths(this.recoveryRoot, capability); replacement.listen(socketPath, resolve); }); replacement.on("error", (error) => { @@ -738,12 +767,14 @@ export class PtyServer { if (fs.existsSync(pidPath) || fs.existsSync(metadataPath)) { throw new Error("recovery sidecar appeared during publication"); } + assertPrivateRecoveryPaths(this.recoveryRoot, capability); const rotated: RecoveryCapability = { ...capability, secret: randomBytes(32).toString("hex"), + metadataRevision: "", }; - rotatedCapability = rotated; const recoveredMetadata = this.recoveryMetadata(request.metadata, rotated); + rotatedCapability = recoveredMetadata.recovery!; publishPrivateNoReplace(pidPath, process.pid.toString()); publishedPid = true; publishPrivateNoReplace(metadataPath, JSON.stringify(recoveredMetadata, null, 2)); @@ -752,10 +783,20 @@ export class PtyServer { if (finalSocket.dev !== socketIdentity.dev || finalSocket.ino !== socketIdentity.ino) { throw new Error("recovery pathname was replaced during publication"); } + assertPrivateRecoveryPaths(this.recoveryRoot, capability); + atomicWritePrivate( + recoveryRevisionPath(this.recoveryRoot, this.name), + signRecoveryRevision(rotatedCapability.secret, { + protocol: RECOVERY_PROTOCOL, + name: this.name, + generation: this.generation, + metadataRevision: rotatedCapability.metadataRevision, + }), + ); const previous = this.socketServer; this.socketServer = replacement; - this.recoveryCapability = rotated; + this.recoveryCapability = rotatedCapability; // Node remembers a Unix server's pathname and unlinks it on close. // The old listener still remembers the same string even though its // inode was externally unlinked; closing it now would unlink the new @@ -809,14 +850,15 @@ export class PtyServer { }; } finally { try { + assertPrivateRecoveryPaths(this.recoveryRoot, capability); if (result) { atomicWritePrivate( resultPath, signRecoveryResult(capability.secret, result), ); } + fs.unlinkSync(requestPath); } catch {} - try { fs.unlinkSync(requestPath); } catch {} this.recoveryInFlight = false; } } diff --git a/src/sessions.ts b/src/sessions.ts index 501f4b4..a2d59b7 100644 --- a/src/sessions.ts +++ b/src/sessions.ts @@ -4,6 +4,13 @@ import * as path from "node:path"; import * as os from "node:os"; import * as net from "node:net"; import { createHash } from "node:crypto"; +import { + assertPrivateRecoveryPaths, + atomicWritePrivate, + recoveryRevisionPath, + signRecoveryRevision, + stampRecoveryMetadata, +} from "./recovery.ts"; // Circular import: events.ts imports getEventsPath/ensureSessionDir from // this file. Cycle is safe — `appendEventSync` is only called at runtime // from inside functions, never at module-init time. @@ -236,7 +243,27 @@ function randomHex(bytes: number): string { export function writeMetadata(name: string, metadata: SessionMetadata): void { ensureSessionDir(); - atomicWriteFileSync(getMetadataPath(name), JSON.stringify(metadata, null, 2)); + const stamped = stampRecoveryMetadata(metadata); + atomicWriteFileSync(getMetadataPath(name), JSON.stringify(stamped, null, 2)); + const capability = stamped.recovery; + if (!capability || !stamped.generation) return; + try { + const root = path.resolve(getSessionDir()); + assertPrivateRecoveryPaths(root, capability); + atomicWritePrivate( + recoveryRevisionPath(root, name), + signRecoveryRevision(capability.secret, { + protocol: capability.protocol, + name, + generation: stamped.generation, + metadataRevision: capability.metadataRevision, + }), + ); + } catch { + // Metadata remains usable if recovery storage is no longer trustworthy. + // The retained old/missing signed revision makes any later recovery fail + // closed rather than accepting this untracked mutation. + } } /** Set or clear the displayName on an existing session. Atomic read-modify-write. @@ -1524,6 +1551,9 @@ export function cleanupAll(name: string): void { try { fs.unlinkSync(getEventsPath(name)); } catch {} + try { + fs.unlinkSync(recoveryRevisionPath(path.resolve(getSessionDir()), name)); + } catch {} releaseLock(name); } @@ -1577,6 +1607,9 @@ export function cleanupOwnedAll(name: string, owner: SessionGenerationOwner): bo try { fs.unlinkSync(getEventsPath(name)); } catch {} + try { + fs.unlinkSync(recoveryRevisionPath(path.resolve(getSessionDir()), name)); + } catch {} return true; } finally { releaseLock(name); @@ -1660,26 +1693,32 @@ export function acquireLock(name: string): boolean { * Unlike normal creation, recovery must not probe or steal an existing lock: * any competing owner is grounds to refuse, and the recovery path promises no * process signal (including a liveness-only signal 0 probe). */ -export function acquireRecoveryLock(name: string): boolean { +export function acquireRecoveryLock(name: string, contents: string): boolean { ensureSessionDir(); try { const fd = fs.openSync(getLockPath(name), "wx", 0o600); try { - fs.writeSync(fd, process.pid.toString()); + fs.writeSync(fd, contents); } finally { fs.closeSync(fd); } return true; } catch (error) { - if ((error as NodeJS.ErrnoException).code === "EEXIST") return false; + if ((error as NodeJS.ErrnoException).code === "EEXIST") { + try { + return fs.readFileSync(getLockPath(name), "utf8") === contents; + } catch { + return false; + } + } throw error; } } -/** Release only the recovery lock still owned by the expected PID. */ -export function releaseRecoveryLock(name: string, ownerPid: number): void { +/** Release only the recovery lock still owned by the authenticated identity. */ +export function releaseRecoveryLock(name: string, contents: string): void { try { - if (fs.readFileSync(getLockPath(name), "utf8").trim() === String(ownerPid)) { + if (fs.readFileSync(getLockPath(name), "utf8") === contents) { fs.unlinkSync(getLockPath(name)); } } catch {} diff --git a/tests/recovery.test.ts b/tests/recovery.test.ts index aba28f6..8bdc737 100644 --- a/tests/recovery.test.ts +++ b/tests/recovery.test.ts @@ -2,14 +2,19 @@ import * as fs from "node:fs"; import * as net from "node:net"; import * as os from "node:os"; import * as path from "node:path"; -import { spawnSync } from "node:child_process"; +import { spawn, spawnSync } from "node:child_process"; +import { pathToFileURL } from "node:url"; import { afterEach, describe, expect, it } from "vitest"; import { PacketReader, MessageType, encodeAttach } from "../src/protocol.ts"; import { queryStats } from "../src/client.ts"; import { acquireRecoveryLock, type SessionMetadata } from "../src/sessions.ts"; import { + RECOVERY_PROTOCOL, RECOVERY_MAX_BYTES, readProcessStartToken, + recoveryLockContents, + recoveryLockIdentity, + recoveryResultPath, recoveryRequestPath, } from "../src/recovery.ts"; import { terminateAndWait } from "./setup/processes.ts"; @@ -115,7 +120,7 @@ describe("live daemon registry recovery", () => { throw new Error("recovery must not signal"); }) as typeof process.kill; try { - expect(acquireRecoveryLock("locked")).toBe(false); + expect(acquireRecoveryLock("locked", "recovery-owner")).toBe(false); expect(calls).toBe(0); } finally { process.kill = originalKill; @@ -133,10 +138,10 @@ describe("live daemon registry recovery", () => { const root = makeRoot(); const name = "positive"; const { marker, pid } = startProvider(root, name); - const before = metadata(root, name); - const snapshot = writeSnapshot(root, name, before); const clientA = await attachCollector(path.join(root, `${name}.sock`)); await waitFor(() => clientA.output().includes("tick:2")); + const before = metadata(root, name); + const snapshot = writeSnapshot(root, name, before); unlinkRegistry(root, name); const outputBefore = clientA.output().length; @@ -153,6 +158,7 @@ describe("live daemon registry recovery", () => { expect(after.recovery?.processStartToken).toBe(before.recovery?.processStartToken); expect(after.recovery?.launchIdentity).toBe(before.recovery?.launchIdentity); expect(after.recovery?.secret).not.toBe(before.recovery?.secret); + expect(after.recovery?.metadataRevision).not.toBe(before.recovery?.metadataRevision); expect(fs.readFileSync(marker, "utf8").trim().split("\n")).toHaveLength(1); const starts = fs.readFileSync(path.join(root, `${name}.events.jsonl`), "utf8") .split("\n").filter((line) => line.includes('"session_start"')); @@ -165,6 +171,112 @@ describe("live daemon registry recovery", () => { clientB.socket.destroy(); }, 20_000); + it("refuses a stale metadata snapshot instead of rolling back live mutations", async () => { + const root = makeRoot(); + const name = "stale-metadata"; + startProvider(root, name); + const stale = metadata(root, name); + const staleSnapshot = writeSnapshot(root, "stale", stale); + + expect(run(root, ["tag", name, "role=current", "strategy=permanent"]).status).toBe(0); + expect(run(root, ["rename", name, "Current Display"]).status).toBe(0); + const client = await attachCollector(path.join(root, `${name}.sock`)); + await waitFor(() => metadata(root, name).lastAttachAt !== undefined); + const current = metadata(root, name); + const currentSnapshot = writeSnapshot(root, "current", current); + expect(current.recovery?.metadataRevision).not.toBe(stale.recovery?.metadataRevision); + + unlinkRegistry(root, name); + const refused = run(root, ["recover", name, "--snapshot", staleSnapshot]); + expect(refused.status).not.toBe(0); + expect(fs.existsSync(path.join(root, `${name}.json`))).toBe(false); + + const recovered = run(root, ["recover", name, "--snapshot", currentSnapshot]); + expect(recovered.status, recovered.stderr || recovered.stdout).toBe(0); + const after = metadata(root, name); + expect(after.tags).toEqual(current.tags); + expect(after.displayName).toBe(current.displayName); + expect(after.lastAttachAt).toBe(current.lastAttachAt); + client.socket.destroy(); + }, 20_000); + + it("resumes an authenticated lock after its recoverer is killed", async () => { + const root = makeRoot(); + const name = "interrupted-lock"; + const { marker, pid } = startProvider(root, name); + const client = await attachCollector(path.join(root, `${name}.sock`)); + await waitFor(() => client.output().includes("tick:2")); + const before = metadata(root, name); + const snapshot = writeSnapshot(root, name, before); + unlinkRegistry(root, name); + + const capability = before.recovery!; + const identity = recoveryLockIdentity({ + name, + daemonPid: pid, + processStartToken: capability.processStartToken, + rootDevice: capability.rootDevice, + rootInode: capability.rootInode, + recoveryDirDevice: capability.recoveryDirDevice, + recoveryDirInode: capability.recoveryDirInode, + }); + const contents = recoveryLockContents(pid, identity); + const sessionsModule = pathToFileURL(path.join(projectRoot, "dist", "sessions.js")).href; + const lockHolder = spawn(process.execPath, [ + "-e", + "import(process.argv[1]).then(m=>{process.env.PTY_ROOT=process.argv[2];if(!m.acquireRecoveryLock(process.argv[3],process.argv[4]))process.exit(2);process.stdout.write('locked\\n');setInterval(()=>{},1000)})", + sessionsModule, + root, + name, + contents, + ], { stdio: ["ignore", "pipe", "pipe"] }); + await new Promise((resolve, reject) => { + lockHolder.stdout!.once("data", () => resolve()); + lockHolder.once("error", reject); + lockHolder.once("exit", (code) => { + if (code !== null) reject(new Error(`lock holder exited ${code}`)); + }); + }); + lockHolder.kill("SIGKILL"); + await new Promise((resolve) => lockHolder.once("exit", () => resolve())); + expect(fs.readFileSync(path.join(root, `${name}.lock`), "utf8")).toBe(contents); + + const outputBefore = client.output().length; + const recovered = run(root, ["recover", name, "--snapshot", snapshot]); + expect(recovered.status, recovered.stderr || recovered.stdout).toBe(0); + await waitFor(() => client.output().length > outputBefore); + expect((await queryStats(name)).daemon.pid).toBe(pid); + expect(fs.readFileSync(marker, "utf8").trim().split("\n")).toHaveLength(1); + expect(fs.existsSync(path.join(root, `${name}.lock`))).toBe(false); + client.socket.destroy(); + }, 20_000); + + it("refuses permission downgrades before writing secret-bearing recovery state", () => { + for (const downgraded of ["root", "recovery-dir"] as const) { + const root = makeRoot(); + const name = `privacy-${downgraded}`; + startProvider(root, name); + const snapshot = writeSnapshot(root, name, metadata(root, name)); + unlinkRegistry(root, name); + const recoveryDir = path.join(root, ".recovery"); + const before = fs.readdirSync(recoveryDir).sort(); + fs.chmodSync(downgraded === "root" ? root : recoveryDir, 0o755); + try { + const refused = run(root, ["recover", name, "--snapshot", snapshot]); + expect(refused.status).not.toBe(0); + expect(fs.readdirSync(recoveryDir).sort()).toEqual(before); + expect(fs.existsSync(recoveryRequestPath(root, name))).toBe(false); + expect(fs.existsSync(recoveryResultPath(root, name))).toBe(false); + expect(fs.existsSync(path.join(root, `${name}.lock`))).toBe(false); + expect(fs.existsSync(path.join(root, `${name}.sock`))).toBe(false); + expect(fs.existsSync(path.join(root, `${name}.pid`))).toBe(false); + expect(fs.existsSync(path.join(root, `${name}.json`))).toBe(false); + } finally { + fs.chmodSync(downgraded === "root" ? root : recoveryDir, 0o700); + } + } + }, 20_000); + it("refuses malformed, unsupported, locked, wrong-root, and tampered requests", async () => { const root = makeRoot(); const name = "tampered"; From 2b3d08d600f279e0cf525891808fd6e6183eb552 Mon Sep 17 00:00:00 2001 From: Nathan Herald Date: Thu, 30 Jul 2026 19:10:18 +0200 Subject: [PATCH 3/4] fix: publish recovery revision before metadata --- CHANGELOG.md | 4 ++- docs/disk-layout.md | 5 ++- src/server.ts | 20 +++++++----- src/sessions.ts | 25 ++++++++++----- tests/recovery.test.ts | 69 +++++++++++++++++++++++++++++++++++++++++- 5 files changed, 104 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aa60581..cedfc39 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,9 @@ state, or lifecycle metadata. Recovery locks are resumable only by the same authenticated daemon identity after an interrupted CLI, and both the root and `.recovery` directory are identity/permission checked immediately before - authenticated request exchange. + authenticated request exchange. Metadata mutations advance their signed + recovery revision before publishing the new metadata, so an unlink during + publication can deny recovery but cannot authorize an older snapshot. ### Read-only session listing diff --git a/docs/disk-layout.md b/docs/disk-layout.md index ffbdeb0..edd2239 100644 --- a/docs/disk-layout.md +++ b/docs/disk-layout.md @@ -80,7 +80,10 @@ Pretty-printed JSON. Source of truth: `SessionMetadata` in `src/sessions.ts`. do not publish snapshots. The root and `.recovery` directory identities and permissions are revalidated before recovery state is exchanged. A signed retained revision rejects older snapshots after tags, display name, attach - state, or other metadata changes. Successful recovery rotates the secret. + state, or other metadata changes. The signed revision advances before changed + metadata is renamed into place: a partial publication may disable recovery, + but never re-authorizes the previous snapshot. Successful recovery rotates + the secret. - Reserved tag keys (`ptyfile*`, `strategy`, anything starting with `:`) are pty/tool-internal; hidden from `pty list` unless `--tags`. - User-facing tags that drive pty behavior but are visible by default: - `strategy=permanent` — `pty gc` respawns the session when its daemon exits (the historic supervisor's role; now stateless and run on a cron). diff --git a/src/server.ts b/src/server.ts index 72e0665..e38fa24 100644 --- a/src/server.ts +++ b/src/server.ts @@ -775,14 +775,10 @@ export class PtyServer { }; const recoveredMetadata = this.recoveryMetadata(request.metadata, rotated); rotatedCapability = recoveredMetadata.recovery!; - publishPrivateNoReplace(pidPath, process.pid.toString()); - publishedPid = true; - publishPrivateNoReplace(metadataPath, JSON.stringify(recoveredMetadata, null, 2)); - publishedMetadata = true; - const finalSocket = fs.lstatSync(socketPath); - if (finalSocket.dev !== socketIdentity.dev || finalSocket.ino !== socketIdentity.ino) { - throw new Error("recovery pathname was replaced during publication"); - } + // Advance the authoritative signed revision before any rotated + // capability-bearing metadata becomes visible. A later publication + // failure intentionally leaves recovery unavailable rather than + // allowing the old snapshot/secret to roll metadata back. assertPrivateRecoveryPaths(this.recoveryRoot, capability); atomicWritePrivate( recoveryRevisionPath(this.recoveryRoot, this.name), @@ -793,6 +789,14 @@ export class PtyServer { metadataRevision: rotatedCapability.metadataRevision, }), ); + publishPrivateNoReplace(pidPath, process.pid.toString()); + publishedPid = true; + publishPrivateNoReplace(metadataPath, JSON.stringify(recoveredMetadata, null, 2)); + publishedMetadata = true; + const finalSocket = fs.lstatSync(socketPath); + if (finalSocket.dev !== socketIdentity.dev || finalSocket.ino !== socketIdentity.ino) { + throw new Error("recovery pathname was replaced during publication"); + } const previous = this.socketServer; this.socketServer = replacement; diff --git a/src/sessions.ts b/src/sessions.ts index a2d59b7..efec5da 100644 --- a/src/sessions.ts +++ b/src/sessions.ts @@ -241,13 +241,21 @@ function randomHex(bytes: number): string { return out; } -export function writeMetadata(name: string, metadata: SessionMetadata): void { +export interface WriteMetadataHooks { + /** @internal Deterministic seam for proving recovery revision publication + * precedes the metadata rename. */ + afterRecoveryRevisionPublished?: () => void; +} + +export function writeMetadata( + name: string, + metadata: SessionMetadata, + hooks: WriteMetadataHooks = {}, +): void { ensureSessionDir(); const stamped = stampRecoveryMetadata(metadata); - atomicWriteFileSync(getMetadataPath(name), JSON.stringify(stamped, null, 2)); const capability = stamped.recovery; - if (!capability || !stamped.generation) return; - try { + if (capability && stamped.generation) { const root = path.resolve(getSessionDir()); assertPrivateRecoveryPaths(root, capability); atomicWritePrivate( @@ -259,11 +267,12 @@ export function writeMetadata(name: string, metadata: SessionMetadata): void { metadataRevision: capability.metadataRevision, }), ); - } catch { - // Metadata remains usable if recovery storage is no longer trustworthy. - // The retained old/missing signed revision makes any later recovery fail - // closed rather than accepting this untracked mutation. + hooks.afterRecoveryRevisionPublished?.(); } + // For capability-bearing metadata the signed revision is authoritative + // first. If this rename fails, the advanced revision intentionally makes the + // old visible metadata unrecoverable rather than authorizing stale rollback. + atomicWriteFileSync(getMetadataPath(name), JSON.stringify(stamped, null, 2)); } /** Set or clear the displayName on an existing session. Atomic read-modify-write. diff --git a/tests/recovery.test.ts b/tests/recovery.test.ts index 8bdc737..c7fb792 100644 --- a/tests/recovery.test.ts +++ b/tests/recovery.test.ts @@ -7,15 +7,24 @@ import { pathToFileURL } from "node:url"; import { afterEach, describe, expect, it } from "vitest"; import { PacketReader, MessageType, encodeAttach } from "../src/protocol.ts"; import { queryStats } from "../src/client.ts"; -import { acquireRecoveryLock, type SessionMetadata } from "../src/sessions.ts"; +import { + acquireRecoveryLock, + writeMetadata, + type SessionMetadata, +} from "../src/sessions.ts"; import { RECOVERY_PROTOCOL, RECOVERY_MAX_BYTES, + metadataRevision, readProcessStartToken, + readBoundedJson, recoveryLockContents, recoveryLockIdentity, + recoveryRevisionPath, recoveryResultPath, recoveryRequestPath, + verifyRecoveryRevision, + type RecoveryRevision, } from "../src/recovery.ts"; import { terminateAndWait } from "./setup/processes.ts"; @@ -134,6 +143,64 @@ describe("live daemon registry recovery", () => { expect(metadata(root, "public-root").recovery).toBeUndefined(); }); + it("advances the signed revision before publishing mutated metadata", () => { + const root = makeRoot(); + const name = "ordered-revision"; + const recoveryDir = path.join(root, ".recovery"); + fs.mkdirSync(recoveryDir, { mode: 0o700 }); + const rootStat = fs.lstatSync(root); + const recoveryStat = fs.lstatSync(recoveryDir); + const initial: SessionMetadata = { + generation: "ordered-generation", + daemonPid: process.pid, + recovery: { + protocol: RECOVERY_PROTOCOL, + secret: "11".repeat(32), + processStartToken: "test-start", + launchIdentity: "22".repeat(32), + rootDevice: rootStat.dev, + rootInode: rootStat.ino, + recoveryDirDevice: recoveryStat.dev, + recoveryDirInode: recoveryStat.ino, + metadataRevision: "", + }, + command: "/bin/sh", + args: [], + displayCommand: "sh", + cwd: root, + createdAt: new Date().toISOString(), + }; + writeMetadata(name, initial); + const before = metadata(root, name); + const mutated: SessionMetadata = { + ...before, + tags: { role: "current", strategy: "permanent" }, + displayName: "Current Display", + lastAttachAt: new Date().toISOString(), + }; + let seamObserved = false; + + writeMetadata(name, mutated, { + afterRecoveryRevisionPublished: () => { + seamObserved = true; + // The old metadata is still the only visible snapshot at this exact + // seam, but its revision has already stopped being authoritative. + expect(metadata(root, name)).toEqual(before); + const revision = readBoundedJson( + recoveryRevisionPath(root, name), + ); + expect(verifyRecoveryRevision(initial.recovery!.secret, revision)).toBe(true); + expect(revision.metadataRevision).toBe(metadataRevision(mutated)); + expect(revision.metadataRevision).not.toBe(before.recovery!.metadataRevision); + }, + }); + + expect(seamObserved).toBe(true); + expect(metadata(root, name).tags).toEqual(mutated.tags); + expect(metadata(root, name).displayName).toBe(mutated.displayName); + expect(metadata(root, name).lastAttachAt).toBe(mutated.lastAttachAt); + }); + it("rebinds the original daemon while preserving provider and attached client", async () => { const root = makeRoot(); const name = "positive"; From f70f85baa89145136a63cc9909fc2423bd3d1e7d Mon Sep 17 00:00:00 2001 From: Nathan Herald Date: Thu, 30 Jul 2026 19:23:59 +0200 Subject: [PATCH 4/4] test: prove interrupted metadata publication fails closed --- tests/recovery.test.ts | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/tests/recovery.test.ts b/tests/recovery.test.ts index c7fb792..c8f68a2 100644 --- a/tests/recovery.test.ts +++ b/tests/recovery.test.ts @@ -201,6 +201,47 @@ describe("live daemon registry recovery", () => { expect(metadata(root, name).lastAttachAt).toBe(mutated.lastAttachAt); }); + it("fails closed when metadata publication stops after revision advancement", () => { + const root = makeRoot(); + const name = "interrupted-revision"; + startProvider(root, name); + const before = metadata(root, name); + const snapshot = writeSnapshot(root, name, before); + const mutated: SessionMetadata = { + ...before, + tags: { role: "must-not-publish" }, + }; + + expect(() => writeMetadata(name, mutated, { + afterRecoveryRevisionPublished: () => { + throw new Error("stop before metadata publication"); + }, + })).toThrow("stop before metadata publication"); + + // Publication stopped in the exact revision-before-metadata window: the + // old metadata remains visible, but its signed revision is no longer + // authoritative. + expect(metadata(root, name)).toEqual(before); + const advanced = readBoundedJson( + recoveryRevisionPath(root, name), + ); + expect(verifyRecoveryRevision(before.recovery!.secret, advanced)).toBe(true); + expect(advanced.metadataRevision).toBe(metadataRevision(mutated)); + expect(advanced.metadataRevision).not.toBe(before.recovery!.metadataRevision); + + unlinkRegistry(root, name); + for (let attempt = 0; attempt < 2; attempt++) { + const refused = run(root, ["recover", name, "--snapshot", snapshot]); + expect(refused.status).not.toBe(0); + expect(fs.existsSync(path.join(root, `${name}.sock`))).toBe(false); + expect(fs.existsSync(path.join(root, `${name}.pid`))).toBe(false); + expect(fs.existsSync(path.join(root, `${name}.json`))).toBe(false); + expect(readBoundedJson( + recoveryRevisionPath(root, name), + )).toEqual(advanced); + } + }, 20_000); + it("rebinds the original daemon while preserving provider and attached client", async () => { const root = makeRoot(); const name = "positive";