From ecc88f16210e0bade1b9ceaf573ab76e14a293d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=ED=98=84=EC=84=B1?= Date: Tue, 7 Jul 2026 19:16:50 +0900 Subject: [PATCH 1/9] feat(browse): add writeSecureFileAtomic for torn-write-safe state Adds an atomic variant of writeSecureFile: write to a same-directory temp file (owner-only), best-effort fsync, then rename over the destination. A crash or concurrent reader sees either the old or new file, never a partial. Needed by the upcoming auto-cookie persistence layer, which rewrites its state file on every mutating command; a torn write there would drop a live auth cookie on the next load. Co-Authored-By: Claude Opus 4.8 --- browse/src/file-permissions.ts | 48 ++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/browse/src/file-permissions.ts b/browse/src/file-permissions.ts index d3d404acd..e6e25691d 100644 --- a/browse/src/file-permissions.ts +++ b/browse/src/file-permissions.ts @@ -39,6 +39,7 @@ import { execFileSync } from 'child_process'; import * as fs from 'fs'; import * as os from 'os'; +import * as path from 'path'; let warnedOnce = false; @@ -123,6 +124,53 @@ export function writeSecureFile( restrictFilePermissions(filePath); } +/** + * Atomically write a file and restrict it to owner-only access, cross-platform. + * + * `writeSecureFile` writes in place, so a crash (or a concurrent reader) + * mid-write can observe a truncated/half-written file. For state that is + * rewritten repeatedly by a long-lived daemon — e.g. the auto-cookie state, + * which checkpoints on every mutating command — a torn write means the next + * load reads corrupt JSON and silently drops a live auth cookie. This variant + * writes to a same-directory temp file (owner-only), best-effort fsyncs it, + * then `rename`s over the destination. `rename` within a directory is atomic + * on POSIX and on NTFS (ReplaceFile semantics via Node), so a reader sees + * either the old file or the new one, never a partial. + * + * Same-directory is load-bearing: a temp file on another filesystem would make + * `rename` fall back to copy+unlink, which is NOT atomic. The temp name embeds + * the pid so two daemons racing the same destination don't clobber each other's + * temp file before their own rename (the destination itself still needs a lock + * — this only guarantees each write is individually atomic). + */ +export function writeSecureFileAtomic( + filePath: string, + data: string | NodeJS.ArrayBufferView, +): void { + const dir = path.dirname(filePath); + const tmp = path.join(dir, `.${path.basename(filePath)}.${process.pid}.tmp`); + let fd: number | null = null; + try { + fd = fs.openSync(tmp, 'w', 0o600); + fs.writeSync(fd, data as any); + try { fs.fsyncSync(fd); } catch { /* fsync is best-effort — rename still gives atomicity */ } + } finally { + if (fd !== null) { + try { fs.closeSync(fd); } catch { /* already closed / best-effort */ } + } + } + // On Windows, openSync with mode does not set an ACL; restrict explicitly + // before the rename so the destination never exists unrestricted. + restrictFilePermissions(tmp); + try { + fs.renameSync(tmp, filePath); + } catch (err) { + // Rename failed — clean up the temp file so it doesn't accumulate. + try { fs.unlinkSync(tmp); } catch { /* best-effort */ } + throw err; + } +} + /** * Append to a file with owner-only permissions, cross-platform. * Replaces `fs.appendFileSync(path, data, { mode: 0o600 })` + Windows ACL. From f66aedb65609db9aed9b2b40b49bbcfc3f3966e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=ED=98=84=EC=84=B1?= Date: Tue, 7 Jul 2026 19:17:04 +0900 Subject: [PATCH 2/9] feat(browse): auto-cookie persistence module (opt-in, persistent-cookie-only) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Core of an opt-in layer that lets a device-trust cookie survive a headless daemon restart. The daemon uses a non-persistent Playwright context, so a restart drops all in-memory cookies and a __Host--bound web device identity re-registers on the next visit. This module (no wiring yet): - isAutoCookiePersistEnabled: gated on BROWSE_AUTO_COOKIE_PERSIST (off by default — auto-persisting auth cookies to plaintext-on-disk is a security escalation from ephemeral-profile semantics, so it must be opt-in). - filterPersistableCookies: persistent (expires !== -1, unexpired) cookies only; drops session cookies, internal-network domains, and honors an optional BROWSE_AUTO_COOKIE_DOMAINS allowlist. Fields (incl. __Host- host-only domain and partitionKey) preserved verbatim. - load/save as a Playwright storageState-shaped object ({cookies, origins:[]}); never persists localStorage/pages. Hash short-circuit skips no-op writes. - Per-workspace state file keyed by hash(realpath(projectDir)) + a mkdir-atomic exclusive lock with conservative dead-pid stale reclaim, so concurrent daemons (separate worktrees) never last-writer-wins the same slot. - Writes go through writeSecureFileAtomic; a one-time plaintext warning fires when enabled. Validated against Playwright 1.58.2: a persistent __Host- cookie survives the cookies() -> JSON -> newContext({storageState}) round-trip intact. Co-Authored-By: Claude Opus 4.8 --- browse/src/auto-cookie-persist.ts | 350 ++++++++++++++++++++++++++++++ 1 file changed, 350 insertions(+) create mode 100644 browse/src/auto-cookie-persist.ts diff --git a/browse/src/auto-cookie-persist.ts b/browse/src/auto-cookie-persist.ts new file mode 100644 index 000000000..0dfb41bec --- /dev/null +++ b/browse/src/auto-cookie-persist.ts @@ -0,0 +1,350 @@ +/** + * Automatic cookie persistence for the headless browse daemon (opt-in). + * + * Why this exists + * ---------------- + * `BrowserManager.launch()` uses `chromium.launch()` + a NON-persistent + * `browser.newContext()`, which Playwright defines as never writing browsing + * data to disk. So when the daemon restarts (idle timeout, `/stop`, crash), + * every in-memory cookie is gone. For a site whose device identity is bound to + * a long-lived cookie (e.g. a `__Host-`-prefixed device-trust cookie), that + * restart forces a fresh device registration on the next visit — the login + * looks transparent, but the device cookie silently vanished. + * + * `recreateContext()` (viewport / user-agent change) already saves and restores + * state in-memory, so it preserves cookies; only a full daemon restart loses + * them. This layer closes that gap by persisting cookies to a small state file + * and reloading them at launch. + * + * Design (validated against Playwright 1.58.2) + * -------------------------------------------- + * - OPT-IN. Off unless BROWSE_AUTO_COOKIE_PERSIST is truthy. Auto-persisting + * auth cookies to plaintext-on-disk for every user is a security escalation + * from the current ephemeral-profile semantics, so it must be a conscious + * choice. When enabled the daemon logs a one-time plaintext warning. + * - PERSISTENT COOKIES ONLY. Session cookies (expires === -1) are excluded by + * definition — turning a browser-session login into a durable one is wrong. + * Expired cookies are dropped so a logout/expiry is never resurrected. + * - Cookies are captured/restored as a Playwright `storageState`-shaped object + * ({ cookies, origins: [] }); we intentionally never persist localStorage, + * sessionStorage, IndexedDB, or open pages — only cookies. + * - Fields are preserved verbatim (domain/path/httpOnly/secure/sameSite/ + * partitionKey); `__Host-` host-only cookies restore correctly via + * `newContext({ storageState })` / `addCookies` with their bare domain. + * - PER-WORKSPACE FILE + EXCLUSIVE LOCK. The state file is keyed by a hash of + * the resolved project dir, and a sibling lock dir (mkdir-atomic) prevents + * two concurrent daemons (separate git worktrees) from racing the same slot. + * - ATOMIC WRITES. Every checkpoint is a temp-file + rename, so a crash or a + * concurrent reader never sees a torn file (see writeSecureFileAtomic). + * - MANUAL STATES ARE UNTOUCHED. This is a separate internal file, not a + * reserved `state save|load `; users can't clobber it and it can't + * clobber theirs. + */ + +import * as fs from 'fs'; +import * as path from 'path'; +import { createHash } from 'crypto'; +import type { BrowserContext, Cookie } from 'playwright'; +import type { BrowseConfig } from './config'; +import { mkdirSecure, writeSecureFileAtomic } from './file-permissions'; +import { safeUnlinkQuiet, isProcessAlive } from './error-handling'; + +const STATE_KIND = 'gstack-browse-auto-cookie-state'; +const STATE_VERSION = 1; + +/** Persisted wrapper written to /browse-auto-cookies/.json */ +interface AutoCookieState { + version: number; + kind: string; + workspaceId: string; + savedAt: string; + storageState: { cookies: Cookie[]; origins: [] }; +} + +/** Lock ownership metadata (pid + start time), written into the lock dir. */ +interface LockMeta { + pid: number; + startedAt: string; +} + +let warnedOnce = false; + +/** + * Truthy check for the opt-in flag. Accepts 1/true/yes/on (case-insensitive); + * everything else (unset, 0, false, empty) is off. + */ +export function isAutoCookiePersistEnabled( + env: Record = process.env, +): boolean { + const v = (env.BROWSE_AUTO_COOKIE_PERSIST || '').trim().toLowerCase(); + return v === '1' || v === 'true' || v === 'yes' || v === 'on'; +} + +/** + * Optional host allowlist from BROWSE_AUTO_COOKIE_DOMAINS (comma-separated). + * Supports exact hosts (example.com) and leading-wildcard (*.example.com, + * which also matches the apex example.com). Empty/unset ⇒ null ⇒ all hosts. + */ +export function parseDomainAllowlist( + env: Record = process.env, +): string[] | null { + const raw = (env.BROWSE_AUTO_COOKIE_DOMAINS || '').trim(); + if (!raw) return null; + const list = raw.split(',').map(s => s.trim().toLowerCase()).filter(Boolean); + return list.length > 0 ? list : null; +} + +function hostMatchesAllowlist(domain: string, allowlist: string[]): boolean { + // Cookie domains may carry a leading dot (domain cookies); normalize it off. + const host = (domain.startsWith('.') ? domain.slice(1) : domain).toLowerCase(); + for (const pattern of allowlist) { + if (pattern.startsWith('*.')) { + const base = pattern.slice(2); + if (host === base || host.endsWith(`.${base}`)) return true; + } else if (host === pattern) { + return true; + } + } + return false; +} + +/** + * Stable per-workspace id. Hash of the resolved (realpath) project dir so two + * different checkouts of the same repo don't share a slot, and a moved/renamed + * dir gets its own. Falls back to the raw projectDir if realpath fails. + */ +export function computeProfileId(config: BrowseConfig): string { + let base = config.projectDir; + try { + base = fs.realpathSync(config.projectDir); + } catch { + // Dir may not exist yet in some test setups — hash the declared path. + } + return createHash('sha256').update(base).digest('hex').slice(0, 16); +} + +function autoDir(config: BrowseConfig): string { + return path.join(config.stateDir, 'browse-auto-cookies'); +} +function statePath(config: BrowseConfig): string { + return path.join(autoDir(config), `${computeProfileId(config)}.json`); +} +function lockPath(config: BrowseConfig): string { + return path.join(autoDir(config), `${computeProfileId(config)}.lock`); +} + +/** + * Keep only cookies that are safe and sensible to persist: + * - persistent (expires !== -1) and not already expired, + * - well-formed (string name/value, non-empty string domain), + * - not internal-network (mirrors the manual `state load` safety filter), + * - within the optional host allowlist. + */ +export function filterPersistableCookies( + cookies: Cookie[], + allowlist: string[] | null, + now: number = Date.now(), +): Cookie[] { + const nowSec = now / 1000; + return cookies.filter((c) => { + if (!c || typeof c.name !== 'string' || typeof c.value !== 'string') return false; + if (typeof c.domain !== 'string' || !c.domain) return false; + // Session cookie — Playwright reports expires === -1. Never persist. + if (typeof c.expires !== 'number' || c.expires === -1) return false; + if (c.expires <= nowSec) return false; // already expired + const d = c.domain.startsWith('.') ? c.domain.slice(1) : c.domain; + if (d === 'localhost' || d.endsWith('.internal') || d === '169.254.169.254') return false; + if (allowlist && !hostMatchesAllowlist(c.domain, allowlist)) return false; + return true; + }); +} + +/** + * Deterministic content hash of a cookie set — used to skip no-op writes. + * Sorted by (domain, path, name) and reduced to the identity-bearing fields so + * that a re-serialization with reordered cookies doesn't churn the file. + */ +export function cookieSetHash(cookies: Cookie[]): string { + const norm = cookies + .map(c => ({ + name: c.name, value: c.value, domain: c.domain, path: c.path, + expires: c.expires, httpOnly: c.httpOnly, secure: c.secure, + sameSite: c.sameSite, partitionKey: c.partitionKey ?? null, + })) + .sort((a, b) => + a.domain.localeCompare(b.domain) || a.path.localeCompare(b.path) || a.name.localeCompare(b.name)); + return createHash('sha256').update(JSON.stringify(norm)).digest('hex'); +} + +// ─── Locking (mkdir-atomic; conservative stale reclaim) ───────────────────── + +/** + * Try to acquire the per-workspace lock. `mkdir` of the lock dir is the atomic + * primitive — it fails if the dir already exists. If a lock is held by a dead + * pid (crash without cleanup) we reclaim it; a lock held by a live pid means a + * peer daemon owns the slot and we back off. Returns true on acquisition. + */ +export function acquireLock(config: BrowseConfig): boolean { + const dir = lockPath(config); + const meta: LockMeta = { pid: process.pid, startedAt: new Date().toISOString() }; + // Ensure the parent (browse-auto-cookies/) exists; the lock dir itself is + // created non-recursively so its creation stays the atomic acquire primitive. + try { mkdirSecure(autoDir(config)); } catch { /* best-effort — mkdirSync below reports the real failure */ } + try { + fs.mkdirSync(dir, { recursive: false }); + } catch (err: any) { + if (err?.code !== 'EEXIST') return false; + // Lock exists — reclaim only if its owner is provably dead. + if (!reclaimIfStale(dir)) return false; + try { + fs.mkdirSync(dir, { recursive: false }); + } catch { + return false; // lost a race to another reclaimer + } + } + try { + fs.writeFileSync(path.join(dir, 'owner.json'), JSON.stringify(meta), { mode: 0o600 }); + } catch { + // Metadata is advisory; the dir's existence is the actual lock. + } + return true; +} + +/** Reclaim a lock dir iff its recorded owner pid is no longer alive. */ +function reclaimIfStale(dir: string): boolean { + try { + const raw = fs.readFileSync(path.join(dir, 'owner.json'), 'utf-8'); + const meta = JSON.parse(raw) as LockMeta; + if (typeof meta.pid === 'number' && meta.pid > 0 && isProcessAlive(meta.pid)) { + return false; // live owner — do not steal + } + } catch { + // No/!parseable metadata — treat as stale (best-effort reclaim). + } + safeUnlinkQuiet(path.join(dir, 'owner.json')); + try { + fs.rmdirSync(dir); + return true; + } catch { + return false; + } +} + +/** Release the lock if we own it (pid match). Safe to call unconditionally. */ +export function releaseLock(config: BrowseConfig): void { + const dir = lockPath(config); + try { + const raw = fs.readFileSync(path.join(dir, 'owner.json'), 'utf-8'); + const meta = JSON.parse(raw) as LockMeta; + if (meta.pid !== process.pid) return; // not ours — don't remove + } catch { + // No metadata — fall through and attempt removal (we likely created it). + } + safeUnlinkQuiet(path.join(dir, 'owner.json')); + try { fs.rmdirSync(dir); } catch { /* best-effort */ } +} + +// ─── Load / Save ──────────────────────────────────────────────────────────── + +/** + * Read persisted cookies for restore, as a Playwright storageState-shaped + * object suitable for `newContext({ storageState })`. Returns null when the + * feature is off, no file exists, the file is corrupt/foreign, or the workspace + * id doesn't match. Never throws — a bad state file must not block launch. + */ +export function loadAutoCookieState( + config: BrowseConfig, + now: number = Date.now(), +): { cookies: Cookie[]; origins: [] } | null { + if (!isAutoCookiePersistEnabled()) return null; + const file = statePath(config); + let parsed: AutoCookieState; + try { + if (!fs.existsSync(file)) return null; + parsed = JSON.parse(fs.readFileSync(file, 'utf-8')) as AutoCookieState; + } catch { + return null; // corrupt — ignore (a later save will overwrite it) + } + if (!parsed || parsed.kind !== STATE_KIND || parsed.version !== STATE_VERSION) return null; + if (parsed.workspaceId !== computeProfileId(config)) return null; + const cookies = parsed.storageState?.cookies; + if (!Array.isArray(cookies)) return null; + // Re-filter on load: a cookie may have expired while the daemon was down, + // and the allowlist may have tightened since the last save. + const usable = filterPersistableCookies(cookies, parseDomainAllowlist(), now); + return { cookies: usable, origins: [] }; +} + +/** + * Snapshot the live context's cookies and persist the persistable subset. + * No-ops when disabled, when there's no context, or when the filtered set is + * unchanged since the last write (hash compare). Returns a small status for + * logging/tests. Best-effort: never throws into the caller (checkpoints run on + * hot paths and during shutdown). + * + * `lastHash` lets the caller skip redundant writes; pass the value returned by + * the previous call. An empty filtered set is still written once (to clear a + * prior file) so a logout is not resurrected on next load. + */ +export async function saveAutoCookieState( + config: BrowseConfig, + context: BrowserContext | null, + lastHash: string | null, +): Promise<{ wrote: boolean; hash: string | null; count: number }> { + try { + if (!isAutoCookiePersistEnabled() || !context) { + return { wrote: false, hash: lastHash, count: 0 }; + } + const all = await context.cookies(); + const persistable = filterPersistableCookies(all, parseDomainAllowlist()); + const hash = cookieSetHash(persistable); + if (hash === lastHash) { + return { wrote: false, hash, count: persistable.length }; + } + const state: AutoCookieState = { + version: STATE_VERSION, + kind: STATE_KIND, + workspaceId: computeProfileId(config), + savedAt: new Date().toISOString(), + storageState: { cookies: persistable, origins: [] }, + }; + mkdirSecure(autoDir(config)); + writeSecureFileAtomic(statePath(config), JSON.stringify(state)); + return { wrote: true, hash, count: persistable.length }; + } catch { + // Persistence is a convenience layer — a failure here must never break a + // command or block shutdown. + return { wrote: false, hash: lastHash, count: 0 }; + } +} + +/** One-time plaintext warning when the feature is enabled. */ +export function warnPlaintextOnce(config: BrowseConfig): void { + if (warnedOnce || !isAutoCookiePersistEnabled()) return; + warnedOnce = true; + console.warn( + `[browse] BROWSE_AUTO_COOKIE_PERSIST is ON — persistent cookies are written ` + + `in PLAINTEXT to ${autoDir(config)} (owner-only file perms only). ` + + `These are auth credentials; anyone who can read the file can impersonate ` + + `the logged-in sessions. Disable by unsetting the env var.` + ); +} + +/** Opportunistic cleanup of leftover temp files from a crashed atomic write. */ +export function cleanStaleTempFiles(config: BrowseConfig): void { + const dir = autoDir(config); + try { + for (const name of fs.readdirSync(dir)) { + if (name.startsWith('.') && name.endsWith('.tmp')) { + safeUnlinkQuiet(path.join(dir, name)); + } + } + } catch { + // Dir may not exist yet — nothing to clean. + } +} + +/** Test-only: reset the once-per-process warning gate. */ +export function __resetWarnedForTests(): void { + warnedOnce = false; +} From e568f96df9842eb12d641f3804a9fad084505e32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=ED=98=84=EC=84=B1?= Date: Tue, 7 Jul 2026 19:17:17 +0900 Subject: [PATCH 3/9] feat(browse): wire auto-cookie persistence into launch + daemon lifecycle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Connects the persistence module to the daemon: - browser-manager.ts: getContext() accessor, and launch() seeds the fresh context via newContext({ storageState }) from the persisted cookies BEFORE the first tab — a restart transparently restores the device cookie. recreateContext() is untouched (it restores in-memory state itself), so there is no double-restore. - server.ts: acquire the per-workspace lock once at daemon start (a live peer holding it disables persistence for this daemon, loudly, instead of racing). Three save triggers — a debounced checkpoint after mutating commands, a periodic safety-net checkpoint, and a final flush in shutdown() right before the context closes (the graceful path all intentional shutdowns funnel through). The crash path (emergencyCleanup) deliberately never saves, to avoid serializing a dying context. Lock is released on shutdown. All triggers are no-ops unless BROWSE_AUTO_COOKIE_PERSIST is set, so default behavior is byte-for-byte unchanged. Co-Authored-By: Claude Opus 4.8 --- browse/src/browser-manager.ts | 29 +++++++++++++ browse/src/server.ts | 78 +++++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+) diff --git a/browse/src/browser-manager.ts b/browse/src/browser-manager.ts index 2bc1c597d..c96530faa 100644 --- a/browse/src/browser-manager.ts +++ b/browse/src/browser-manager.ts @@ -254,6 +254,14 @@ export class BrowserManager { getConnectionMode(): 'launched' | 'headed' { return this.connectionMode; } + /** + * The live browser context, or null before launch / after close. Exposed for + * the auto-cookie-persist layer (server.ts) to snapshot cookies on a + * checkpoint without reaching into private state. Callers must null-check — + * a checkpoint can fire between close() and the next launch(). + */ + getContext(): import('playwright').BrowserContext | null { return this.context; } + // ─── Watch Mode Methods ───────────────────────────────── isWatching(): boolean { return this.watching; } @@ -401,6 +409,27 @@ export class BrowserManager { if (this.customUserAgent) { contextOptions.userAgent = this.customUserAgent; } + + // Auto-cookie persistence (opt-in): seed the fresh context with previously + // persisted cookies so a device-trust cookie survives a daemon restart. + // Read-only + best-effort — the writer path (server.ts) owns the lock; a + // concurrent read of an atomically-written file is always a complete file. + // recreateContext() does NOT go through here (it restores in-memory state + // itself), so there's no double-restore. + try { + const { resolveConfig } = await import('./config'); + const { loadAutoCookieState, warnPlaintextOnce } = await import('./auto-cookie-persist'); + const cfg = resolveConfig(); + warnPlaintextOnce(cfg); + const restored = loadAutoCookieState(cfg); + if (restored && restored.cookies.length > 0) { + contextOptions.storageState = restored; + console.log(`[browse] Restored ${restored.cookies.length} persisted cookie(s) from auto-state`); + } + } catch { + // Never block launch on the persistence layer. + } + this.context = await this.browser.newContext(contextOptions); if (Object.keys(this.extraHeaders).length > 0) { diff --git a/browse/src/server.ts b/browse/src/server.ts index 301781acc..a1c6ea765 100644 --- a/browse/src/server.ts +++ b/browse/src/server.ts @@ -37,6 +37,13 @@ import { } from './token-registry'; import { validateTempPath } from './path-security'; import { resolveConfig, ensureStateDir, readVersionHash, resolveChromiumProfile, cleanSingletonLocks } from './config'; +import { + isAutoCookiePersistEnabled, + acquireLock as acquireAutoCookieLock, + releaseLock as releaseAutoCookieLock, + saveAutoCookieState, + cleanStaleTempFiles as cleanAutoCookieTempFiles, +} from './auto-cookie-persist'; import { emitActivity, subscribe, getActivityAfter, getActivityHistory, getSubscriberCount } from './activity'; import { createSseEndpoint } from './sse-helpers'; import { initAuditLog, writeAuditEntry } from './audit'; @@ -648,6 +655,47 @@ function idleCheckTick() { } const idleCheckInterval = setInterval(idleCheckTick, 60_000); +// ─── Auto-cookie persistence (opt-in) ────────────────────────── +// Checkpoints the live context's persistent cookies to a per-workspace state +// file so a device-trust cookie survives a daemon restart. Three triggers: +// (1) a periodic checkpoint below, (2) a debounced checkpoint after mutating +// commands (scheduleAutoCookieCheckpoint, called from handleCommandInternalImpl), +// and (3) a final flush in shutdown() before the browser closes. The crash +// path (emergencyCleanup) deliberately does NOT save — serializing a dying +// context risks a corrupt write; we rely on the most recent good checkpoint. +let autoCookieLockHeld = false; +let autoCookieLastHash: string | null = null; +let autoCookieDebounce: ReturnType | null = null; + +async function autoCookieCheckpoint(): Promise { + if (!autoCookieLockHeld) return; // never write without owning the lock + const res = await saveAutoCookieState(config, activeBrowserManager.getContext(), autoCookieLastHash); + autoCookieLastHash = res.hash; +} + +// Debounced checkpoint: coalesces bursts of mutating commands into one write a +// short while after the last one. unref'd so it never keeps the process alive. +function scheduleAutoCookieCheckpoint(): void { + if (!autoCookieLockHeld) return; + if (autoCookieDebounce) clearTimeout(autoCookieDebounce); + autoCookieDebounce = setTimeout(() => { + autoCookieDebounce = null; + void autoCookieCheckpoint(); + }, 1500); + if (typeof autoCookieDebounce.unref === 'function') autoCookieDebounce.unref(); +} + +// Periodic safety-net checkpoint (covers crash-before-shutdown where the +// debounce/flush never runs). unref'd — must not hold the daemon open. +const autoCookieInterval = setInterval(() => { void autoCookieCheckpoint(); }, 60_000); +if (typeof autoCookieInterval.unref === 'function') autoCookieInterval.unref(); + +// Commands that can mutate cookies/browser state and thus warrant a checkpoint. +const AUTO_COOKIE_MUTATING_COMMANDS = new Set([ + 'goto', 'click', 'fill', 'type', 'press', 'select', 'reload', 'back', 'forward', + 'chain', 'state', 'cookie-picker', 'set-cookie', +]); + // Test-only surface for server-factory.test.ts. Lets the dual-instance // idle-timer behavior be exercised deterministically without mutating // Date.now (which would interact with the leaked module-level setInterval). @@ -959,6 +1007,14 @@ async function handleCommandInternalImpl( const command = canonicalizeCommand(rawCommand); const isAliased = command !== rawCommand; + // Auto-cookie checkpoint (opt-in): a mutating command may have changed + // cookies. Schedule a debounced checkpoint — the 1.5s debounce means the + // capture runs after the command has actually applied. No-op unless the lock + // is held. Skip nested chain subcommands: the outer chain already scheduled. + if ((opts?.chainDepth ?? 0) === 0 && AUTO_COOKIE_MUTATING_COMMANDS.has(command)) { + scheduleAutoCookieCheckpoint(); + } + // ─── Recursion guard: reject nested chains ────────────────── if (command === 'chain' && (opts?.chainDepth ?? 0) > 0) { return { status: 400, result: JSON.stringify({ error: 'Nested chain commands are not allowed' }), json: true }; @@ -1607,9 +1663,17 @@ export function buildFetchHandler(cfg: ServerConfig): ServerHandle { if (cfgBrowserManager.isWatching()) cfgBrowserManager.stopWatch(); clearInterval(flushInterval); clearInterval(idleCheckInterval); + clearInterval(autoCookieInterval); + if (autoCookieDebounce) { clearTimeout(autoCookieDebounce); autoCookieDebounce = null; } if (agentWatchdogInterval) clearInterval(agentWatchdogInterval); await flushBuffers(); + // Final cookie checkpoint BEFORE the context closes — this is the graceful + // path all intentional shutdowns funnel through (idle timeout, SIGINT, + // /stop). Must run while the context still exists. Then release the lock. + await autoCookieCheckpoint(); + if (autoCookieLockHeld) { releaseAutoCookieLock(config); autoCookieLockHeld = false; } + await cfgBrowserManager.close(); cleanSingletonLocks(resolveChromiumProfile()); @@ -1640,6 +1704,20 @@ export function buildFetchHandler(cfg: ServerConfig): ServerHandle { // reports connectionMode === 'launched'. activeBrowserManager = cfgBrowserManager; + // Acquire the per-workspace auto-cookie lock once, at daemon start. Holding + // it for the daemon's lifetime is what serializes concurrent daemons (e.g. + // separate git worktrees) — only the lock holder writes the shared slot. + // If a live peer owns it, disable auto-persistence for this daemon (loud) so + // two daemons never last-writer-wins each other's cookies. Reads still work + // without the lock (atomic writes make a concurrent read safe). + if (isAutoCookiePersistEnabled()) { + cleanAutoCookieTempFiles(config); + autoCookieLockHeld = acquireAutoCookieLock(config); + if (!autoCookieLockHeld) { + console.warn('[browse] Auto-cookie persistence disabled for this daemon: another instance holds the workspace lock.'); + } + } + // Wire the cfg-instance's onDisconnect to run shutdown when the user // closes the headed browser window. CHAIN any caller-provided handler // instead of overwriting it: gbrowser may have set its own onDisconnect From 2f2ed32b46f08a12dabda68ada82c0dfc1982676 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=ED=98=84=EC=84=B1?= Date: Tue, 7 Jul 2026 19:17:28 +0900 Subject: [PATCH 4/9] =?UTF-8?q?test(browse):=20auto-cookie=20persistence?= =?UTF-8?q?=20=E2=80=94=20unit,=20round-trip,=20launch,=20wiring?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Unit: opt-in gate, persistent-only/expiry/internal-network/allowlist filter, order-independent content hash, atomic-write helper, mkdir-atomic lock (acquire / same-pid contend / dead-pid reclaim / release). - Real Playwright round-trip: a persistent __Host- cookie saves and reloads via newContext({ storageState }); session/expired cookies are excluded; an unchanged set skips the second write. - Real BrowserManager.launch() integration: a pre-seeded auto-state file is restored into the launched context (simulates a daemon restart). - Static wiring tripwires: save gated on the opt-in flag, shutdown flush runs before the browser closes, and the crash path never checkpoints. Co-Authored-By: Claude Opus 4.8 --- browse/test/auto-cookie-persist.test.ts | 294 ++++++++++++++++++++++++ 1 file changed, 294 insertions(+) create mode 100644 browse/test/auto-cookie-persist.test.ts diff --git a/browse/test/auto-cookie-persist.test.ts b/browse/test/auto-cookie-persist.test.ts new file mode 100644 index 000000000..0771c2623 --- /dev/null +++ b/browse/test/auto-cookie-persist.test.ts @@ -0,0 +1,294 @@ +/** + * Tests for opt-in automatic cookie persistence (auto-cookie-persist.ts). + * + * Unit tests cover the cookie filter (persistent-only, expiry, internal-network, + * allowlist), the content hash, the mkdir-atomic lock (acquire / contend / + * stale-reclaim / release), and the atomic-write helper. A Playwright + * round-trip test proves a persistent __Host- cookie is saved and restored via + * newContext({ storageState }), and that session/expired cookies are excluded. + * + * Static-grep tests pin the load-bearing wiring in server.ts / browser-manager.ts + * (opt-in gate, no-save-on-crash-path, shutdown flush before close) so a refactor + * can't silently regress them. + */ + +import { describe, test, expect, afterEach } from 'bun:test'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import type { Cookie } from 'playwright'; +import { chromium } from 'playwright'; +import type { BrowseConfig } from '../src/config'; +import { + isAutoCookiePersistEnabled, + parseDomainAllowlist, + filterPersistableCookies, + cookieSetHash, + computeProfileId, + acquireLock, + releaseLock, + loadAutoCookieState, + saveAutoCookieState, +} from '../src/auto-cookie-persist'; +import { writeSecureFileAtomic } from '../src/file-permissions'; + +const META = fs.readFileSync(path.join(import.meta.dir, '../src/server.ts'), 'utf-8'); +const BM = fs.readFileSync(path.join(import.meta.dir, '../src/browser-manager.ts'), 'utf-8'); + +function tmpConfig(): BrowseConfig { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'auto-cookie-')); + const stateDir = path.join(dir, '.gstack'); + fs.mkdirSync(stateDir, { recursive: true }); + return { + projectDir: dir, + stateDir, + stateFile: path.join(stateDir, 'browse.json'), + consoleLog: '', networkLog: '', dialogLog: '', auditLog: '', + }; +} + +const future = Math.floor(Date.now() / 1000) + 60 * 24 * 60 * 60; +function cookie(over: Partial = {}): Cookie { + return { + name: 'c', value: 'v', domain: 'example.com', path: '/', + expires: future, httpOnly: true, secure: true, sameSite: 'Lax', ...over, + } as Cookie; +} + +const savedEnv = { ...process.env }; +afterEach(() => { + process.env = { ...savedEnv }; +}); + +describe('opt-in gate', () => { + test('enabled only for truthy flag values', () => { + for (const v of ['1', 'true', 'TRUE', 'yes', 'on']) { + expect(isAutoCookiePersistEnabled({ BROWSE_AUTO_COOKIE_PERSIST: v })).toBe(true); + } + for (const v of [undefined, '', '0', 'false', 'no', 'off']) { + expect(isAutoCookiePersistEnabled({ BROWSE_AUTO_COOKIE_PERSIST: v as any })).toBe(false); + } + }); + + test('loadAutoCookieState returns null when disabled', () => { + const cfg = tmpConfig(); + delete process.env.BROWSE_AUTO_COOKIE_PERSIST; + expect(loadAutoCookieState(cfg)).toBeNull(); + }); + + test('saveAutoCookieState is a no-op when disabled', async () => { + const cfg = tmpConfig(); + delete process.env.BROWSE_AUTO_COOKIE_PERSIST; + const res = await saveAutoCookieState(cfg, null, null); + expect(res.wrote).toBe(false); + expect(fs.existsSync(path.join(cfg.stateDir, 'browse-auto-cookies'))).toBe(false); + }); +}); + +describe('cookie filter', () => { + test('keeps persistent, drops session cookies', () => { + const kept = filterPersistableCookies([cookie(), cookie({ name: 's', expires: -1 })], null); + expect(kept.map(c => c.name)).toEqual(['c']); + }); + + test('drops already-expired cookies', () => { + const past = Math.floor(Date.now() / 1000) - 10; + const kept = filterPersistableCookies([cookie({ name: 'old', expires: past }), cookie()], null); + expect(kept.map(c => c.name)).toEqual(['c']); + }); + + test('rejects internal-network domains', () => { + const kept = filterPersistableCookies([ + cookie({ name: 'lh', domain: 'localhost' }), + cookie({ name: 'meta', domain: '169.254.169.254' }), + cookie({ name: 'int', domain: 'foo.internal' }), + cookie({ name: 'ok', domain: 'example.com' }), + ], null); + expect(kept.map(c => c.name)).toEqual(['ok']); + }); + + test('applies host allowlist incl. leading-wildcard + apex', () => { + const cookies = [ + cookie({ name: 'a', domain: 'example.com' }), + cookie({ name: 'b', domain: 'sub.example.com' }), + cookie({ name: 'c', domain: 'other.com' }), + ]; + const kept = filterPersistableCookies(cookies, ['*.example.com']); + expect(kept.map(c => c.name).sort()).toEqual(['a', 'b']); // apex + subdomain, not other.com + }); + + test('parseDomainAllowlist splits and lowercases; empty ⇒ null', () => { + expect(parseDomainAllowlist({ BROWSE_AUTO_COOKIE_DOMAINS: 'A.com, *.B.com' })) + .toEqual(['a.com', '*.b.com']); + expect(parseDomainAllowlist({ BROWSE_AUTO_COOKIE_DOMAINS: '' })).toBeNull(); + expect(parseDomainAllowlist({})).toBeNull(); + }); +}); + +describe('content hash', () => { + test('order-independent', () => { + const a = cookie({ name: 'a', domain: 'a.com' }); + const b = cookie({ name: 'b', domain: 'b.com' }); + expect(cookieSetHash([a, b])).toBe(cookieSetHash([b, a])); + }); + test('changes when a value changes', () => { + const a = cookie({ name: 'a', value: '1' }); + const a2 = cookie({ name: 'a', value: '2' }); + expect(cookieSetHash([a])).not.toBe(cookieSetHash([a2])); + }); +}); + +describe('atomic write helper', () => { + test('writes content and leaves no temp file', () => { + const cfg = tmpConfig(); + const f = path.join(cfg.stateDir, 'atomic.json'); + writeSecureFileAtomic(f, JSON.stringify({ ok: 1 })); + expect(JSON.parse(fs.readFileSync(f, 'utf-8'))).toEqual({ ok: 1 }); + const leftovers = fs.readdirSync(cfg.stateDir).filter(n => n.endsWith('.tmp')); + expect(leftovers).toEqual([]); + }); +}); + +describe('workspace lock', () => { + test('acquire, contend (same process re-acquire fails), release', () => { + const cfg = tmpConfig(); + expect(acquireLock(cfg)).toBe(true); + // Second acquire while a live pid (this process) owns it → refused. + expect(acquireLock(cfg)).toBe(false); + releaseLock(cfg); + // After release the dir is gone and we can re-acquire. + expect(acquireLock(cfg)).toBe(true); + releaseLock(cfg); + }); + + test('reclaims a lock owned by a dead pid', () => { + const cfg = tmpConfig(); + const lockDir = path.join(cfg.stateDir, 'browse-auto-cookies', `${computeProfileId(cfg)}.lock`); + fs.mkdirSync(lockDir, { recursive: true }); + // PID 2^31-1 is not a live process → stale, reclaimable. + fs.writeFileSync(path.join(lockDir, 'owner.json'), JSON.stringify({ pid: 2147483646, startedAt: 'x' })); + expect(acquireLock(cfg)).toBe(true); + releaseLock(cfg); + }); +}); + +describe('save → load round-trip (real Playwright)', () => { + test('persistent __Host- cookie survives; session/expired excluded', async () => { + process.env.BROWSE_AUTO_COOKIE_PERSIST = '1'; + delete process.env.BROWSE_AUTO_COOKIE_DOMAINS; + const cfg = tmpConfig(); + const origin = 'https://luseed.example.com'; + const browser = await chromium.launch({ headless: true }); + try { + const ctx = await browser.newContext(); + await ctx.addCookies([ + { name: '__Host-lsd', value: 'tok.raw', url: origin, httpOnly: true, secure: true, sameSite: 'Lax', expires: future }, + { name: 'authjs.session-token', value: 'sess', url: origin, httpOnly: true, secure: true, sameSite: 'Lax', expires: future }, + { name: 'ephem', value: 'x', url: origin, httpOnly: false, secure: true, sameSite: 'Lax' }, // session + ]); + const res = await saveAutoCookieState(cfg, ctx, null); + await ctx.close(); + expect(res.wrote).toBe(true); + expect(res.count).toBe(2); // ephem (session) filtered out + + const loaded = loadAutoCookieState(cfg); + expect(loaded).not.toBeNull(); + const names = loaded!.cookies.map(c => c.name).sort(); + expect(names).toEqual(['__Host-lsd', 'authjs.session-token']); + + // The restored storageState actually rehydrates a fresh context. + const ctx2 = await browser.newContext({ storageState: loaded! }); + const back = await ctx2.cookies(); + await ctx2.close(); + expect(back.some(c => c.name === '__Host-lsd')).toBe(true); + } finally { + await browser.close(); + } + }); + + test('unchanged cookie set skips the second write (hash short-circuit)', async () => { + process.env.BROWSE_AUTO_COOKIE_PERSIST = '1'; + const cfg = tmpConfig(); + const origin = 'https://luseed.example.com'; + const browser = await chromium.launch({ headless: true }); + try { + const ctx = await browser.newContext(); + await ctx.addCookies([{ name: '__Host-lsd', value: 'tok', url: origin, httpOnly: true, secure: true, sameSite: 'Lax', expires: future }]); + const first = await saveAutoCookieState(cfg, ctx, null); + expect(first.wrote).toBe(true); + const second = await saveAutoCookieState(cfg, ctx, first.hash); + expect(second.wrote).toBe(false); // identical set → no write + await ctx.close(); + } finally { + await browser.close(); + } + }); +}); + +describe('BrowserManager.launch() integration (real browser + real daemon config)', () => { + test('launch() restores a persisted __Host- cookie into the fresh context', async () => { + process.env.BROWSE_AUTO_COOKIE_PERSIST = '1'; + delete process.env.BROWSE_AUTO_COOKIE_DOMAINS; + const cfg = tmpConfig(); + // launch() calls resolveConfig() internally, which honors BROWSE_STATE_FILE — + // point it at this isolated workspace. + process.env.BROWSE_STATE_FILE = cfg.stateFile; + + // Pre-seed the auto-state file exactly as a prior daemon's shutdown flush + // would have (persistent __Host- cookie for an https origin). + const origin = 'https://luseed.example.com'; + const browser0 = await chromium.launch({ headless: true }); + const seedCtx = await browser0.newContext(); + await seedCtx.addCookies([ + { name: '__Host-lsd', value: 'device.token', url: origin, httpOnly: true, secure: true, sameSite: 'Lax', expires: future }, + ]); + const wrote = await saveAutoCookieState(cfg, seedCtx, null); + await seedCtx.close(); + await browser0.close(); + expect(wrote.wrote).toBe(true); + + // Launch a real BrowserManager against the same workspace — it should load + // the file and seed the new context, simulating a daemon restart. + const { BrowserManager } = await import('../src/browser-manager'); + const bm = new BrowserManager(); + try { + await bm.launch(); + const ctx = bm.getContext(); + expect(ctx).not.toBeNull(); + const cookies = await ctx!.cookies(); + expect(cookies.some(c => c.name === '__Host-lsd' && c.value === 'device.token')).toBe(true); + } finally { + await bm.close(); + } + }, 30_000); // real browser launch can take >5s (matches handoff integration timing) +}); + +describe('wiring (static)', () => { + test('save is gated on the opt-in flag at daemon start', () => { + expect(META).toContain('isAutoCookiePersistEnabled()'); + expect(META).toContain('acquireAutoCookieLock(config)'); + }); + + test('final checkpoint runs in shutdown BEFORE the browser closes', () => { + const shutIdx = META.indexOf('async function shutdown('); + const flushIdx = META.indexOf('await autoCookieCheckpoint();', shutIdx); + const closeIdx = META.indexOf('await cfgBrowserManager.close();', shutIdx); + expect(flushIdx).toBeGreaterThan(shutIdx); + expect(closeIdx).toBeGreaterThan(flushIdx); // flush precedes close + }); + + test('crash path (emergencyCleanup) does NOT checkpoint cookies', () => { + const start = META.indexOf('function emergencyCleanup()'); + const end = META.indexOf('\n}', start); + const body = META.slice(start, end); + expect(body).not.toContain('autoCookieCheckpoint'); + expect(body).not.toContain('saveAutoCookieState'); + }); + + test('launch() seeds the context from persisted cookies before newContext', () => { + const loadIdx = BM.indexOf('loadAutoCookieState(cfg)'); + const newCtxIdx = BM.indexOf('this.context = await this.browser.newContext(contextOptions);'); + expect(loadIdx).toBeGreaterThan(0); + expect(newCtxIdx).toBeGreaterThan(loadIdx); // restore assigned into contextOptions before use + }); +}); From 14041062936764cd5263a1cca1d9b07006c7ee97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=ED=98=84=EC=84=B1?= Date: Tue, 7 Jul 2026 20:26:36 +0900 Subject: [PATCH 5/9] fix(browse): surface auto-cookie state read errors instead of swallowing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit loadAutoCookieState treated every read failure as the benign corrupt-file case and returned null silently. Split the file read from the JSON parse: a read error (e.g. EACCES) now logs a warning — the device cookie won't restore and that should be visible — while a JSON parse failure stays silent (a later save overwrites it). Addresses a slop-scan default-return/filesystem finding. Co-Authored-By: Claude Opus 4.8 --- browse/src/auto-cookie-persist.ts | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/browse/src/auto-cookie-persist.ts b/browse/src/auto-cookie-persist.ts index 0dfb41bec..0d2b631fc 100644 --- a/browse/src/auto-cookie-persist.ts +++ b/browse/src/auto-cookie-persist.ts @@ -258,12 +258,21 @@ export function loadAutoCookieState( ): { cookies: Cookie[]; origins: [] } | null { if (!isAutoCookiePersistEnabled()) return null; const file = statePath(config); - let parsed: AutoCookieState; + let raw: string; try { if (!fs.existsSync(file)) return null; - parsed = JSON.parse(fs.readFileSync(file, 'utf-8')) as AutoCookieState; + raw = fs.readFileSync(file, 'utf-8'); + } catch (err: any) { + // A read error here (e.g. EACCES) is NOT the benign corrupt-file case — the + // device cookie silently won't restore, so surface it rather than swallow. + console.warn(`[browse] auto-cookie state unreadable (${err?.code || err?.message}); starting without persisted cookies`); + return null; + } + let parsed: AutoCookieState; + try { + parsed = JSON.parse(raw) as AutoCookieState; } catch { - return null; // corrupt — ignore (a later save will overwrite it) + return null; // corrupt JSON — ignore (a later save overwrites it) } if (!parsed || parsed.kind !== STATE_KIND || parsed.version !== STATE_VERSION) return null; if (parsed.workspaceId !== computeProfileId(config)) return null; From c0d85f7c0e412959b01ddf991107b6d39ebaa98f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=ED=98=84=EC=84=B1?= Date: Tue, 7 Jul 2026 20:59:22 +0900 Subject: [PATCH 6/9] fix(browse): reject NaN/Infinity expires and case-fold internal domains filterPersistableCookies trusted expires as a finite number and matched internal-network domains case-sensitively. A cookie with expires=NaN or Infinity slipped past the `=== -1` session check (NaN <= nowSec is false), and an uppercase `LOCALHOST` / `Foo.INTERNAL` bypassed the internal-domain block. Add a `Number.isFinite` guard and a shared `normalizeCookieDomain` that lower-cases before the internal-domain comparison. Allowlist matching already lower-cases internally, so behavior there is unchanged. Co-Authored-By: Claude Opus 4.8 --- browse/src/auto-cookie-persist.ts | 9 +++++++-- browse/test/auto-cookie-persist.test.ts | 11 +++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/browse/src/auto-cookie-persist.ts b/browse/src/auto-cookie-persist.ts index 0d2b631fc..6c7bfd290 100644 --- a/browse/src/auto-cookie-persist.ts +++ b/browse/src/auto-cookie-persist.ts @@ -133,6 +133,11 @@ function lockPath(config: BrowseConfig): string { return path.join(autoDir(config), `${computeProfileId(config)}.lock`); } +function normalizeCookieDomain(domain: string): string { + const d = domain.startsWith('.') ? domain.slice(1) : domain; + return d.toLowerCase(); +} + /** * Keep only cookies that are safe and sensible to persist: * - persistent (expires !== -1) and not already expired, @@ -150,9 +155,9 @@ export function filterPersistableCookies( if (!c || typeof c.name !== 'string' || typeof c.value !== 'string') return false; if (typeof c.domain !== 'string' || !c.domain) return false; // Session cookie — Playwright reports expires === -1. Never persist. - if (typeof c.expires !== 'number' || c.expires === -1) return false; + if (typeof c.expires !== 'number' || c.expires === -1 || !Number.isFinite(c.expires)) return false; if (c.expires <= nowSec) return false; // already expired - const d = c.domain.startsWith('.') ? c.domain.slice(1) : c.domain; + const d = normalizeCookieDomain(c.domain); if (d === 'localhost' || d.endsWith('.internal') || d === '169.254.169.254') return false; if (allowlist && !hostMatchesAllowlist(c.domain, allowlist)) return false; return true; diff --git a/browse/test/auto-cookie-persist.test.ts b/browse/test/auto-cookie-persist.test.ts index 0771c2623..a18af4684 100644 --- a/browse/test/auto-cookie-persist.test.ts +++ b/browse/test/auto-cookie-persist.test.ts @@ -100,13 +100,24 @@ describe('cookie filter', () => { test('rejects internal-network domains', () => { const kept = filterPersistableCookies([ cookie({ name: 'lh', domain: 'localhost' }), + cookie({ name: 'lh2', domain: 'LOCALHOST' }), cookie({ name: 'meta', domain: '169.254.169.254' }), cookie({ name: 'int', domain: 'foo.internal' }), + cookie({ name: 'int2', domain: 'Foo.INTERNAL' }), cookie({ name: 'ok', domain: 'example.com' }), ], null); expect(kept.map(c => c.name)).toEqual(['ok']); }); + test('rejects malformed expires values', () => { + const kept = filterPersistableCookies([ + cookie({ name: 'nan', expires: Number.NaN }), + cookie({ name: 'inf', expires: Number.POSITIVE_INFINITY }), + cookie({ name: 'ok' }), + ], null); + expect(kept.map(c => c.name)).toEqual(['ok']); + }); + test('applies host allowlist incl. leading-wildcard + apex', () => { const cookies = [ cookie({ name: 'a', domain: 'example.com' }), From fd976e9b66197dad6bb21fc1fd53767ad8a2eda6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=ED=98=84=EC=84=B1?= Date: Tue, 7 Jul 2026 21:00:38 +0900 Subject: [PATCH 7/9] fix(browse): distinguish pid reuse when reclaiming a stale cookie lock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit reclaimIfStale treated any live pid matching the recorded owner as proof the lock is still held. After a crash the OS can recycle that pid for an unrelated process, so a liveness check alone would refuse to reclaim a lock whose real owner is long gone, permanently disabling auto-persistence for the workspace. Cross-check the owner's recorded lock-acquire time against the live process start time via `ps -o lstart=`: if the live process started after the lock was taken, the pid was reused and the lock is stale. Fallback is conservative — if start-time can't be read (ps fails, timeout, non-Unix, unparseable), we keep the lock rather than risk stealing an active peer's slot. Co-Authored-By: Claude Opus 4.8 --- browse/src/auto-cookie-persist.ts | 29 ++++++++++++++++++++++++- browse/test/auto-cookie-persist.test.ts | 18 +++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/browse/src/auto-cookie-persist.ts b/browse/src/auto-cookie-persist.ts index 6c7bfd290..fe3d0056c 100644 --- a/browse/src/auto-cookie-persist.ts +++ b/browse/src/auto-cookie-persist.ts @@ -215,12 +215,39 @@ export function acquireLock(config: BrowseConfig): boolean { return true; } +function readProcessStartTimeMs(pid: number): number | null { + if (process.platform === 'win32') return null; + try { + const result = Bun.spawnSync(['ps', '-p', String(pid), '-o', 'lstart='], { + stdout: 'pipe', stderr: 'pipe', timeout: 2000, + }); + if (result.exitCode !== 0) return null; + const parsed = Date.parse(result.stdout.toString().trim()); + return Number.isFinite(parsed) ? parsed : null; + } catch { + return null; + } +} + +function isRecordedOwnerStillAlive(meta: LockMeta): boolean { + if (typeof meta.pid !== 'number' || meta.pid <= 0) return false; + if (meta.pid === process.pid) return true; + if (!isProcessAlive(meta.pid)) return false; + + const recordedMs = Date.parse(meta.startedAt); + const liveStartMs = readProcessStartTimeMs(meta.pid); + if (Number.isFinite(recordedMs) && liveStartMs !== null && liveStartMs > recordedMs) { + return false; + } + return true; +} + /** Reclaim a lock dir iff its recorded owner pid is no longer alive. */ function reclaimIfStale(dir: string): boolean { try { const raw = fs.readFileSync(path.join(dir, 'owner.json'), 'utf-8'); const meta = JSON.parse(raw) as LockMeta; - if (typeof meta.pid === 'number' && meta.pid > 0 && isProcessAlive(meta.pid)) { + if (isRecordedOwnerStillAlive(meta)) { return false; // live owner — do not steal } } catch { diff --git a/browse/test/auto-cookie-persist.test.ts b/browse/test/auto-cookie-persist.test.ts index a18af4684..39e890e86 100644 --- a/browse/test/auto-cookie-persist.test.ts +++ b/browse/test/auto-cookie-persist.test.ts @@ -181,6 +181,24 @@ describe('workspace lock', () => { expect(acquireLock(cfg)).toBe(true); releaseLock(cfg); }); + + test('reclaims a lock whose pid was reused by a newer process', () => { + if (process.platform === 'win32') return; + const cfg = tmpConfig(); + const lockDir = path.join(cfg.stateDir, 'browse-auto-cookies', `${computeProfileId(cfg)}.lock`); + fs.mkdirSync(lockDir, { recursive: true }); + const proc = Bun.spawn(['sleep', '5'], { stdout: 'ignore', stderr: 'ignore' }); + try { + fs.writeFileSync(path.join(lockDir, 'owner.json'), JSON.stringify({ + pid: proc.pid, + startedAt: '1970-01-01T00:00:00.000Z', + })); + expect(acquireLock(cfg)).toBe(true); + releaseLock(cfg); + } finally { + proc.kill(); + } + }); }); describe('save → load round-trip (real Playwright)', () => { From 53c2750cf1f28e2650b1bc03ac5579bfb259c1b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=ED=98=84=EC=84=B1?= Date: Tue, 7 Jul 2026 21:00:59 +0900 Subject: [PATCH 8/9] fix(browse): add random suffix to atomic-write temp filenames writeSecureFileAtomic embedded only the pid in the staging filename, so two writes from the same process racing the same destination (the periodic checkpoint firing while a debounced checkpoint runs, both serialized but sharing a pid) could collide on the temp path and clobber each other's in-flight staging file before rename. Append 4 random bytes so every write gets a unique staging path. The destination still needs its own lock; this only guarantees each individual write is atomic. Co-Authored-By: Claude Opus 4.8 --- browse/src/file-permissions.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/browse/src/file-permissions.ts b/browse/src/file-permissions.ts index e6e25691d..b7dc08bb5 100644 --- a/browse/src/file-permissions.ts +++ b/browse/src/file-permissions.ts @@ -37,6 +37,7 @@ */ import { execFileSync } from 'child_process'; +import { randomBytes } from 'crypto'; import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; @@ -139,16 +140,16 @@ export function writeSecureFile( * * Same-directory is load-bearing: a temp file on another filesystem would make * `rename` fall back to copy+unlink, which is NOT atomic. The temp name embeds - * the pid so two daemons racing the same destination don't clobber each other's - * temp file before their own rename (the destination itself still needs a lock - * — this only guarantees each write is individually atomic). + * the pid plus random bytes so racing writes never share a staging path (the + * destination itself still needs a lock — this only guarantees each write is + * individually atomic). */ export function writeSecureFileAtomic( filePath: string, data: string | NodeJS.ArrayBufferView, ): void { const dir = path.dirname(filePath); - const tmp = path.join(dir, `.${path.basename(filePath)}.${process.pid}.tmp`); + const tmp = path.join(dir, `.${path.basename(filePath)}.${process.pid}.${randomBytes(4).toString('hex')}.tmp`); let fd: number | null = null; try { fd = fs.openSync(tmp, 'w', 0o600); From 13ff147233634f75fb65264b5fb5c4e83be3ac16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=ED=98=84=EC=84=B1?= Date: Tue, 7 Jul 2026 21:01:35 +0900 Subject: [PATCH 9/9] fix(browse): checkpoint on real cookie mutators and after the command runs The debounced auto-cookie checkpoint had two gaps. First, the trigger set was a hand-maintained list that missed the actual cookie-mutating commands (cookie, cookie-import, load-html) while listing a non-existent set-cookie. Drive the decision off WRITE_COMMANDS (the real mutator registry) plus a small extra set for js/eval and the meta commands (chain, state, newtab, tab-each) that can change the cookie jar. Second, the schedule call ran before dispatch, so a slow navigation or login redirect snapshotted the pre-command jar; move it to after the command attempt (success and error paths, plus the scoped-newtab early return) so the debounce captures the post-mutation state. Also wire cookie-picker import/remove: those flow through the /cookie-picker route, not /command, so they never scheduled a save. Pass scheduleAutoCookieCheckpoint as an onCookieMutation callback and fire it after addCookies / removal. Serialize checkpoints with an in-flight promise guard so an overlapping periodic + debounced + shutdown checkpoint can't race two concurrent saveAutoCookieState writes onto the same slot. The shutdown flush awaits any in-flight checkpoint first, then runs its own, so no save is lost. Co-Authored-By: Claude Opus 4.8 --- browse/src/cookie-picker-routes.ts | 3 ++ browse/src/server.ts | 44 +++++++++++++++++-------- browse/test/auto-cookie-persist.test.ts | 19 +++++++++++ 3 files changed, 53 insertions(+), 13 deletions(-) diff --git a/browse/src/cookie-picker-routes.ts b/browse/src/cookie-picker-routes.ts index 07ab5a2c2..e6b60befa 100644 --- a/browse/src/cookie-picker-routes.ts +++ b/browse/src/cookie-picker-routes.ts @@ -110,6 +110,7 @@ export async function handleCookiePickerRoute( req: Request, bm: BrowserManager, authToken?: string, + onCookieMutation?: () => void, ): Promise { const pathname = url.pathname; const port = parseInt(url.port, 10) || 9400; @@ -268,6 +269,7 @@ export async function handleCookiePickerRoute( // Add to Playwright context const page = bm.getActiveSession().getPage(); await page.context().addCookies(result.cookies); + onCookieMutation?.(); // Track what was imported for (const domain of Object.keys(result.domainCounts)) { @@ -305,6 +307,7 @@ export async function handleCookiePickerRoute( importedDomains.delete(domain); importedCounts.delete(domain); } + onCookieMutation?.(); console.log(`[cookie-picker] Removed cookies for ${domains.length} domains`); diff --git a/browse/src/server.ts b/browse/src/server.ts index a1c6ea765..7ecd66769 100644 --- a/browse/src/server.ts +++ b/browse/src/server.ts @@ -666,11 +666,24 @@ const idleCheckInterval = setInterval(idleCheckTick, 60_000); let autoCookieLockHeld = false; let autoCookieLastHash: string | null = null; let autoCookieDebounce: ReturnType | null = null; +let autoCookieCheckpointInFlight: Promise | null = null; async function autoCookieCheckpoint(): Promise { + while (autoCookieCheckpointInFlight) { + await autoCookieCheckpointInFlight; + } if (!autoCookieLockHeld) return; // never write without owning the lock - const res = await saveAutoCookieState(config, activeBrowserManager.getContext(), autoCookieLastHash); - autoCookieLastHash = res.hash; + const run = (async () => { + if (!autoCookieLockHeld) return; + const res = await saveAutoCookieState(config, activeBrowserManager.getContext(), autoCookieLastHash); + if (autoCookieLockHeld) autoCookieLastHash = res.hash; + })(); + autoCookieCheckpointInFlight = run; + try { + await run; + } finally { + if (autoCookieCheckpointInFlight === run) autoCookieCheckpointInFlight = null; + } } // Debounced checkpoint: coalesces bursts of mutating commands into one write a @@ -692,10 +705,14 @@ if (typeof autoCookieInterval.unref === 'function') autoCookieInterval.unref(); // Commands that can mutate cookies/browser state and thus warrant a checkpoint. const AUTO_COOKIE_MUTATING_COMMANDS = new Set([ - 'goto', 'click', 'fill', 'type', 'press', 'select', 'reload', 'back', 'forward', - 'chain', 'state', 'cookie-picker', 'set-cookie', + 'chain', 'state', 'newtab', 'tab-each', + 'js', 'eval', ]); +function shouldAutoCookieCheckpoint(command: string): boolean { + return WRITE_COMMANDS.has(command) || AUTO_COOKIE_MUTATING_COMMANDS.has(command); +} + // Test-only surface for server-factory.test.ts. Lets the dual-instance // idle-timer behavior be exercised deterministically without mutating // Date.now (which would interact with the leaked module-level setInterval). @@ -1006,14 +1023,7 @@ async function handleCommandInternalImpl( // so the trail records what the agent actually typed. const command = canonicalizeCommand(rawCommand); const isAliased = command !== rawCommand; - - // Auto-cookie checkpoint (opt-in): a mutating command may have changed - // cookies. Schedule a debounced checkpoint — the 1.5s debounce means the - // capture runs after the command has actually applied. No-op unless the lock - // is held. Skip nested chain subcommands: the outer chain already scheduled. - if ((opts?.chainDepth ?? 0) === 0 && AUTO_COOKIE_MUTATING_COMMANDS.has(command)) { - scheduleAutoCookieCheckpoint(); - } + const shouldCheckpointAutoCookies = (opts?.chainDepth ?? 0) === 0 && shouldAutoCookieCheckpoint(command); // ─── Recursion guard: reject nested chains ────────────────── if (command === 'chain' && (opts?.chainDepth ?? 0) > 0) { @@ -1113,6 +1123,7 @@ async function handleCommandInternalImpl( // ─── newtab with ownership for scoped tokens ────────────── if (command === 'newtab' && tokenInfo && tokenInfo.clientId !== 'root') { const newId = await browserManager.newTab(args[0] || undefined, tokenInfo.clientId); + if (shouldCheckpointAutoCookies) scheduleAutoCookieCheckpoint(); return { status: 200, json: true, result: JSON.stringify({ @@ -1240,6 +1251,11 @@ async function handleCommandInternalImpl( }; } + // Auto-cookie checkpoint (opt-in): schedule after the command attempt has + // actually run, so slow navigations/login redirects do not snapshot the + // pre-command cookie jar. No-op unless the lock is held. + if (shouldCheckpointAutoCookies) scheduleAutoCookieCheckpoint(); + // ─── Centralized content wrapping (single location for all commands) ─── // Scoped tokens: content filter + enhanced envelope + datamarking // Root tokens: basic untrusted content wrapper (backward compat) @@ -1311,6 +1327,8 @@ async function handleCommandInternalImpl( } return { status: 200, result }; } catch (err: any) { + if (shouldCheckpointAutoCookies) scheduleAutoCookieCheckpoint(); + // Restore original active tab even on error if (savedTabId !== null) { try { browserManager.switchTab(savedTabId, { bringToFront: false }); } catch (restoreErr: any) { @@ -1803,7 +1821,7 @@ export function buildFetchHandler(cfg: ServerConfig): ServerHandle { // Cookie picker routes — HTML page unauthenticated, data/action routes require auth if (url.pathname.startsWith('/cookie-picker')) { - return handleCookiePickerRoute(url, req, browserManager, authToken); + return handleCookiePickerRoute(url, req, browserManager, authToken, scheduleAutoCookieCheckpoint); } // Welcome page — served when GStack Browser launches in headed mode diff --git a/browse/test/auto-cookie-persist.test.ts b/browse/test/auto-cookie-persist.test.ts index 39e890e86..538ca6bad 100644 --- a/browse/test/auto-cookie-persist.test.ts +++ b/browse/test/auto-cookie-persist.test.ts @@ -34,6 +34,7 @@ import { writeSecureFileAtomic } from '../src/file-permissions'; const META = fs.readFileSync(path.join(import.meta.dir, '../src/server.ts'), 'utf-8'); const BM = fs.readFileSync(path.join(import.meta.dir, '../src/browser-manager.ts'), 'utf-8'); +const PICKER = fs.readFileSync(path.join(import.meta.dir, '../src/cookie-picker-routes.ts'), 'utf-8'); function tmpConfig(): BrowseConfig { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'auto-cookie-')); @@ -320,4 +321,22 @@ describe('wiring (static)', () => { expect(loadIdx).toBeGreaterThan(0); expect(newCtxIdx).toBeGreaterThan(loadIdx); // restore assigned into contextOptions before use }); + + test('debounced command checkpoint runs after dispatch and covers real mutators', () => { + expect(META).toContain('WRITE_COMMANDS.has(command)'); + expect(META).toContain("'js', 'eval'"); + expect(META).toContain("'chain', 'state', 'newtab', 'tab-each'"); + expect(META).not.toContain("'set-cookie'"); + const canonicalIdx = META.indexOf('const command = canonicalizeCommand(rawCommand);'); + const dispatchIdx = META.indexOf('if (READ_COMMANDS.has(command))', canonicalIdx); + const scheduleIdx = META.indexOf('// Auto-cookie checkpoint (opt-in): schedule after', dispatchIdx); + expect(dispatchIdx).toBeGreaterThan(canonicalIdx); + expect(scheduleIdx).toBeGreaterThan(dispatchIdx); + }); + + test('cookie picker imports/removals schedule auto-cookie persistence', () => { + expect(META).toContain('handleCookiePickerRoute(url, req, browserManager, authToken, scheduleAutoCookieCheckpoint)'); + expect(PICKER).toContain('onCookieMutation?: () => void'); + expect(PICKER.match(/onCookieMutation\?\.\(\)/g)?.length).toBe(2); + }); });