diff --git a/browse/src/security.ts b/browse/src/security.ts index cb46efe12..6475aea73 100644 --- a/browse/src/security.ts +++ b/browse/src/security.ts @@ -33,7 +33,8 @@ import { spawn } from 'child_process'; import * as fs from 'fs'; import * as path from 'path'; 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 ────────────────────────────── @@ -346,17 +347,17 @@ export interface SessionState { } /** - * Atomic write of session state (temp + rename pattern). Writes are safe - * across process boundaries. + * Atomic write of session state (via lib/fs-atomic). Writes are safe + * 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 { - try { - mkdirSecure(SECURITY_DIR); - const tmp = `${STATE_FILE}.tmp.${process.pid}`; - writeSecureFile(tmp, JSON.stringify(state, null, 2)); - fs.renameSync(tmp, STATE_FILE); - } catch (err) { - console.error('[security] writeSessionState failed:', (err as Error).message); + try { mkdirSecure(SECURITY_DIR); } catch { /* write below fails and logs */ } + if (atomicWriteQuiet(STATE_FILE, JSON.stringify(state, null, 2), { mode: 0o600 })) { + // Windows ACL hardening (POSIX chmod is redundant with mode above). + restrictFilePermissions(STATE_FILE); + } else { + console.error('[security] writeSessionState failed'); } } diff --git a/browse/src/terminal-agent-control.ts b/browse/src/terminal-agent-control.ts index 094ba668f..a8c42d63c 100644 --- a/browse/src/terminal-agent-control.ts +++ b/browse/src/terminal-agent-control.ts @@ -17,7 +17,8 @@ import * as fs from 'fs'; import * as path from 'path'; 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 @@ -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 { try { mkdirSecure(stateDir); } catch {} const target = agentRecordPath(stateDir); - const tmp = `${target}.tmp-${process.pid}`; - writeSecureFile(tmp, JSON.stringify(record)); - fs.renameSync(tmp, target); + atomicWriteSync(target, JSON.stringify(record), { mode: 0o600 }); + // Windows ACL hardening (POSIX chmod is redundant with mode above). + restrictFilePermissions(target); } export function clearAgentRecord(stateDir: string): void { diff --git a/browse/src/terminal-agent.ts b/browse/src/terminal-agent.ts index 4814c7508..d73fb21c2 100644 --- a/browse/src/terminal-agent.ts +++ b/browse/src/terminal-agent.ts @@ -23,7 +23,8 @@ import * as fs from 'fs'; import * as path from 'path'; 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 { writeAgentRecord, clearAgentRecord } from './terminal-agent-control'; import { extractPtyCookie } from './pty-session-cookie'; @@ -267,12 +268,9 @@ function writeClaudeAvailable(): void { checked_at: new Date().toISOString(), }; const target = path.join(stateDir, 'claude-available.json'); - const tmp = path.join(stateDir, `.tmp-claude-${process.pid}`); - try { - writeSecureFile(tmp, JSON.stringify(status, null, 2)); - fs.renameSync(tmp, target); - } catch { - safeUnlink(tmp); + // Fire-and-forget state file: a failed write must not break boot. + if (atomicWriteQuiet(target, JSON.stringify(status, null, 2), { mode: 0o600 })) { + restrictFilePermissions(target); // Windows ACL hardening } } @@ -881,12 +879,10 @@ function handleTabState(msg: { })), }; const target = path.join(stateDir, 'tabs.json'); - const tmp = path.join(stateDir, `.tmp-tabs-${process.pid}`); - try { - writeSecureFile(tmp, JSON.stringify(payload, null, 2)); - fs.renameSync(tmp, target); - } catch { - safeUnlink(tmp); + // Fire-and-forget state file: atomic write (via lib/fs-atomic) so + // claude never reads a half-written JSON document; failures swallowed. + if (atomicWriteQuiet(target, JSON.stringify(payload, null, 2), { mode: 0o600 })) { + restrictFilePermissions(target); // Windows ACL hardening } } @@ -896,17 +892,12 @@ function handleTabState(msg: { const active = msg.active; if (active && active.url && !active.url.startsWith('chrome://') && !active.url.startsWith('chrome-extension://')) { const ctxFile = path.join(stateDir, 'active-tab.json'); - const tmp = path.join(stateDir, `.tmp-tab-${process.pid}`); - try { - writeSecureFile(tmp, JSON.stringify({ - tabId: active.tabId ?? null, - url: active.url, - title: active.title ?? '', - })); - fs.renameSync(tmp, ctxFile); - } catch { - safeUnlink(tmp); - } + const ok = atomicWriteQuiet(ctxFile, JSON.stringify({ + tabId: active.tabId ?? null, + url: active.url, + title: active.title ?? '', + }), { mode: 0o600 }); + if (ok) restrictFilePermissions(ctxFile); // Windows ACL hardening } } @@ -916,17 +907,13 @@ function handleTabSwitch(msg: { tabId?: number; url?: string; title?: string }): const stateDir = path.dirname(STATE_FILE); const ctxFile = path.join(stateDir, 'active-tab.json'); - const tmp = path.join(stateDir, `.tmp-tab-${process.pid}`); - try { - writeSecureFile(tmp, JSON.stringify({ - tabId: msg.tabId ?? null, - url, - title: msg.title ?? '', - })); - fs.renameSync(tmp, ctxFile); - } catch { - safeUnlink(tmp); - } + // Fire-and-forget: atomic write via lib/fs-atomic, failures swallowed. + const ok = atomicWriteQuiet(ctxFile, JSON.stringify({ + tabId: msg.tabId ?? null, + url, + title: msg.title ?? '', + }), { mode: 0o600 }); + if (ok) restrictFilePermissions(ctxFile); // Windows ACL hardening // Best-effort sync to parent server so its activeTabId tracking matches. // 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. + // Throws on failure — a boot without a discoverable port file is broken. const dir = path.dirname(PORT_FILE); try { mkdirSecure(dir); } catch {} - const tmp = `${PORT_FILE}.tmp-${process.pid}`; - writeSecureFile(tmp, String(port)); - fs.renameSync(tmp, PORT_FILE); + atomicWriteSync(PORT_FILE, String(port), { mode: 0o600 }); + restrictFilePermissions(PORT_FILE); // Windows ACL hardening // Write identity-based agent record (pid + per-boot gen). Replaces the // v1.43- `pkill -f terminal-agent\.ts` regex teardown that could kill diff --git a/browse/test/terminal-agent.test.ts b/browse/test/terminal-agent.test.ts index 8dd54e200..e579bd7e3 100644 --- a/browse/test/terminal-agent.test.ts +++ b/browse/test/terminal-agent.test.ts @@ -175,11 +175,13 @@ describe('Source-level guard: terminal-agent', () => { expect(AGENT_SRC).toContain("msg?.type === 'tabState'"); expect(AGENT_SRC).toContain('function handleTabState'); const fn = AGENT_SRC.slice(AGENT_SRC.indexOf('function handleTabState')); - // Atomic write via tmp + rename for both files (so claude never reads - // a half-written JSON document). + // Atomic write for both files (so claude never reads a half-written + // 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("'active-tab.json'"); - expect(fn).toContain('renameSync'); + expect(fn).toContain('atomicWriteQuiet'); // Skip chrome:// and chrome-extension:// pages — they're not useful // targets for browse commands. expect(fn).toContain("startsWith('chrome://')");