fix(browse): migrate hand-rolled atomic writes to lib/fs-atomic

Seven sites, each audited for its existing throw-vs-swallow contract before
migrating: writeSessionState + the four fire-and-forget tab/state writers use
atomicWriteQuiet (they swallowed before); writeAgentRecord + the boot-time
port-file write use atomicWriteSync (they threw before — and writeAgentRecord
previously leaked its tmp file on rename failure, which the helper cleans).
All carry {mode: 0o600} plus restrictFilePermissions after successful writes,
preserving the Windows ACL hardening that writeSecureFile provided (mode bits
are POSIX-only). server.ts untouched: its three state writes route through
tmpStatePath(), pinned by server-tmp-state-path.test.ts.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Garry Tan 2026-08-14 19:56:41 -07:00
parent ef0fa9e9ff
commit 2b27d89ae3
No known key found for this signature in database
GPG Key ID: C1F69E85C74EFE1D
4 changed files with 47 additions and 56 deletions

View File

@ -33,7 +33,8 @@ import { spawn } from 'child_process';
import * as fs from 'fs'; import * as fs from 'fs';
import * as path from 'path'; import * as path from 'path';
import * as os from 'os'; import * as os from 'os';
import { writeSecureFile, appendSecureFile, mkdirSecure } from './file-permissions'; import { restrictFilePermissions, appendSecureFile, mkdirSecure } from './file-permissions';
import { atomicWriteQuiet } from '../../lib/fs-atomic';
// ─── Thresholds + verdict types ────────────────────────────── // ─── Thresholds + verdict types ──────────────────────────────
@ -346,17 +347,17 @@ export interface SessionState {
} }
/** /**
* Atomic write of session state (temp + rename pattern). Writes are safe * Atomic write of session state (via lib/fs-atomic). Writes are safe
* across process boundaries. * across process boundaries. Swallow-with-log polarity: a failed write
* must never take down the caller (security state is best-effort cache).
*/ */
export function writeSessionState(state: SessionState): void { export function writeSessionState(state: SessionState): void {
try { try { mkdirSecure(SECURITY_DIR); } catch { /* write below fails and logs */ }
mkdirSecure(SECURITY_DIR); if (atomicWriteQuiet(STATE_FILE, JSON.stringify(state, null, 2), { mode: 0o600 })) {
const tmp = `${STATE_FILE}.tmp.${process.pid}`; // Windows ACL hardening (POSIX chmod is redundant with mode above).
writeSecureFile(tmp, JSON.stringify(state, null, 2)); restrictFilePermissions(STATE_FILE);
fs.renameSync(tmp, STATE_FILE); } else {
} catch (err) { console.error('[security] writeSessionState failed');
console.error('[security] writeSessionState failed:', (err as Error).message);
} }
} }

View File

@ -17,7 +17,8 @@
import * as fs from 'fs'; import * as fs from 'fs';
import * as path from 'path'; import * as path from 'path';
import { safeUnlink, safeKill, isProcessAlive } from './error-handling'; import { safeUnlink, safeKill, isProcessAlive } from './error-handling';
import { writeSecureFile, mkdirSecure } from './file-permissions'; import { restrictFilePermissions, mkdirSecure } from './file-permissions';
import { atomicWriteSync } from '../../lib/fs-atomic';
/** /**
* Locate the terminal-agent script on disk. In dev (cli.ts running via * Locate the terminal-agent script on disk. In dev (cli.ts running via
@ -108,13 +109,13 @@ export function readAgentRecord(stateDir: string): AgentRecord | null {
} }
} }
/** Atomic write. Caller must ensure stateDir exists; agent does this at boot. */ /** Atomic write (throws on failure — boot must not proceed on a bad record). */
export function writeAgentRecord(stateDir: string, record: AgentRecord): void { export function writeAgentRecord(stateDir: string, record: AgentRecord): void {
try { mkdirSecure(stateDir); } catch {} try { mkdirSecure(stateDir); } catch {}
const target = agentRecordPath(stateDir); const target = agentRecordPath(stateDir);
const tmp = `${target}.tmp-${process.pid}`; atomicWriteSync(target, JSON.stringify(record), { mode: 0o600 });
writeSecureFile(tmp, JSON.stringify(record)); // Windows ACL hardening (POSIX chmod is redundant with mode above).
fs.renameSync(tmp, target); restrictFilePermissions(target);
} }
export function clearAgentRecord(stateDir: string): void { export function clearAgentRecord(stateDir: string): void {

View File

@ -23,7 +23,8 @@
import * as fs from 'fs'; import * as fs from 'fs';
import * as path from 'path'; import * as path from 'path';
import * as crypto from 'crypto'; import * as crypto from 'crypto';
import { writeSecureFile, mkdirSecure } from './file-permissions'; import { writeSecureFile, restrictFilePermissions, mkdirSecure } from './file-permissions';
import { atomicWriteSync, atomicWriteQuiet } from '../../lib/fs-atomic';
import { safeUnlink } from './error-handling'; import { safeUnlink } from './error-handling';
import { writeAgentRecord, clearAgentRecord } from './terminal-agent-control'; import { writeAgentRecord, clearAgentRecord } from './terminal-agent-control';
import { extractPtyCookie } from './pty-session-cookie'; import { extractPtyCookie } from './pty-session-cookie';
@ -267,12 +268,9 @@ function writeClaudeAvailable(): void {
checked_at: new Date().toISOString(), checked_at: new Date().toISOString(),
}; };
const target = path.join(stateDir, 'claude-available.json'); const target = path.join(stateDir, 'claude-available.json');
const tmp = path.join(stateDir, `.tmp-claude-${process.pid}`); // Fire-and-forget state file: a failed write must not break boot.
try { if (atomicWriteQuiet(target, JSON.stringify(status, null, 2), { mode: 0o600 })) {
writeSecureFile(tmp, JSON.stringify(status, null, 2)); restrictFilePermissions(target); // Windows ACL hardening
fs.renameSync(tmp, target);
} catch {
safeUnlink(tmp);
} }
} }
@ -881,12 +879,10 @@ function handleTabState(msg: {
})), })),
}; };
const target = path.join(stateDir, 'tabs.json'); const target = path.join(stateDir, 'tabs.json');
const tmp = path.join(stateDir, `.tmp-tabs-${process.pid}`); // Fire-and-forget state file: atomic write (via lib/fs-atomic) so
try { // claude never reads a half-written JSON document; failures swallowed.
writeSecureFile(tmp, JSON.stringify(payload, null, 2)); if (atomicWriteQuiet(target, JSON.stringify(payload, null, 2), { mode: 0o600 })) {
fs.renameSync(tmp, target); restrictFilePermissions(target); // Windows ACL hardening
} catch {
safeUnlink(tmp);
} }
} }
@ -896,17 +892,12 @@ function handleTabState(msg: {
const active = msg.active; const active = msg.active;
if (active && active.url && !active.url.startsWith('chrome://') && !active.url.startsWith('chrome-extension://')) { if (active && active.url && !active.url.startsWith('chrome://') && !active.url.startsWith('chrome-extension://')) {
const ctxFile = path.join(stateDir, 'active-tab.json'); const ctxFile = path.join(stateDir, 'active-tab.json');
const tmp = path.join(stateDir, `.tmp-tab-${process.pid}`); const ok = atomicWriteQuiet(ctxFile, JSON.stringify({
try { tabId: active.tabId ?? null,
writeSecureFile(tmp, JSON.stringify({ url: active.url,
tabId: active.tabId ?? null, title: active.title ?? '',
url: active.url, }), { mode: 0o600 });
title: active.title ?? '', if (ok) restrictFilePermissions(ctxFile); // Windows ACL hardening
}));
fs.renameSync(tmp, ctxFile);
} catch {
safeUnlink(tmp);
}
} }
} }
@ -916,17 +907,13 @@ function handleTabSwitch(msg: { tabId?: number; url?: string; title?: string }):
const stateDir = path.dirname(STATE_FILE); const stateDir = path.dirname(STATE_FILE);
const ctxFile = path.join(stateDir, 'active-tab.json'); const ctxFile = path.join(stateDir, 'active-tab.json');
const tmp = path.join(stateDir, `.tmp-tab-${process.pid}`); // Fire-and-forget: atomic write via lib/fs-atomic, failures swallowed.
try { const ok = atomicWriteQuiet(ctxFile, JSON.stringify({
writeSecureFile(tmp, JSON.stringify({ tabId: msg.tabId ?? null,
tabId: msg.tabId ?? null, url,
url, title: msg.title ?? '',
title: msg.title ?? '', }), { mode: 0o600 });
})); if (ok) restrictFilePermissions(ctxFile); // Windows ACL hardening
fs.renameSync(tmp, ctxFile);
} catch {
safeUnlink(tmp);
}
// Best-effort sync to parent server so its activeTabId tracking matches. // Best-effort sync to parent server so its activeTabId tracking matches.
// No await; this is fire-and-forget. // No await; this is fire-and-forget.
@ -964,11 +951,11 @@ function main() {
} }
// Write port file atomically so the parent server can pick it up. // Write port file atomically so the parent server can pick it up.
// Throws on failure — a boot without a discoverable port file is broken.
const dir = path.dirname(PORT_FILE); const dir = path.dirname(PORT_FILE);
try { mkdirSecure(dir); } catch {} try { mkdirSecure(dir); } catch {}
const tmp = `${PORT_FILE}.tmp-${process.pid}`; atomicWriteSync(PORT_FILE, String(port), { mode: 0o600 });
writeSecureFile(tmp, String(port)); restrictFilePermissions(PORT_FILE); // Windows ACL hardening
fs.renameSync(tmp, PORT_FILE);
// Write identity-based agent record (pid + per-boot gen). Replaces the // Write identity-based agent record (pid + per-boot gen). Replaces the
// v1.43- `pkill -f terminal-agent\.ts` regex teardown that could kill // v1.43- `pkill -f terminal-agent\.ts` regex teardown that could kill

View File

@ -175,11 +175,13 @@ describe('Source-level guard: terminal-agent', () => {
expect(AGENT_SRC).toContain("msg?.type === 'tabState'"); expect(AGENT_SRC).toContain("msg?.type === 'tabState'");
expect(AGENT_SRC).toContain('function handleTabState'); expect(AGENT_SRC).toContain('function handleTabState');
const fn = AGENT_SRC.slice(AGENT_SRC.indexOf('function handleTabState')); const fn = AGENT_SRC.slice(AGENT_SRC.indexOf('function handleTabState'));
// Atomic write via tmp + rename for both files (so claude never reads // Atomic write for both files (so claude never reads a half-written
// a half-written JSON document). // JSON document) — via the shared lib/fs-atomic helper, which owns the
// tmp + rename dance. Quiet variant: state-file writes are
// fire-and-forget and must never take down the agent.
expect(fn).toContain("'tabs.json'"); expect(fn).toContain("'tabs.json'");
expect(fn).toContain("'active-tab.json'"); expect(fn).toContain("'active-tab.json'");
expect(fn).toContain('renameSync'); expect(fn).toContain('atomicWriteQuiet');
// Skip chrome:// and chrome-extension:// pages — they're not useful // Skip chrome:// and chrome-extension:// pages — they're not useful
// targets for browse commands. // targets for browse commands.
expect(fn).toContain("startsWith('chrome://')"); expect(fn).toContain("startsWith('chrome://')");