From 01df279a3aeb08febad68cd481c6a9181cdc98b5 Mon Sep 17 00:00:00 2001 From: Dongkeun Lee Date: Tue, 28 Jul 2026 11:40:56 -0400 Subject: [PATCH 01/18] Harden automation browser isolation --- .agents/skills/bootstrap-diagnostics/SKILL.md | 4 +- AGENTS.md | 4 +- bin/chrome-devtools-axi | 7 + bin/fm-bootstrap.sh | 21 +- bin/fm-browser-isolation.mjs | 977 ++++++++++++++++++ bin/fm-browser-isolation.sh | 9 + bin/fm-session-start.sh | 4 +- bin/fm-spawn.sh | 103 +- bin/fm-teardown.sh | 29 +- docs/browser-isolation.md | 50 + docs/macos-permissions.md | 3 + docs/scripts.md | 2 + tests/fm-browser-isolation-macos-smoke.sh | 245 +++++ tests/fm-browser-isolation.test.sh | 391 +++++++ 14 files changed, 1831 insertions(+), 18 deletions(-) create mode 100755 bin/chrome-devtools-axi create mode 100755 bin/fm-browser-isolation.mjs create mode 100755 bin/fm-browser-isolation.sh create mode 100644 docs/browser-isolation.md create mode 100755 tests/fm-browser-isolation-macos-smoke.sh create mode 100755 tests/fm-browser-isolation.test.sh diff --git a/.agents/skills/bootstrap-diagnostics/SKILL.md b/.agents/skills/bootstrap-diagnostics/SKILL.md index 584ec6022d..19ab9cb170 100644 --- a/.agents/skills/bootstrap-diagnostics/SKILL.md +++ b/.agents/skills/bootstrap-diagnostics/SKILL.md @@ -2,7 +2,7 @@ name: bootstrap-diagnostics description: >- Agent-only handling playbook for session-start bootstrap diagnostics. - Use whenever the session-start digest's bootstrap section prints any diagnostic or capability line - MISSING, MISSING_MANUAL, BACKEND_INVALID, ACCOUNT_ROUTING, NEEDS_GH_AUTH, TANGLE, CREW_HARNESS_OVERRIDE, CREW_DISPATCH, FLEET_SYNC, SECONDMATE_SYNC, SECONDMATE_LIVENESS, TASKS_AXI, NUDGE_SECONDMATES, REPORT_RETENTION, or FMX - or when a standalone bin/fm-bootstrap.sh run prints one. + Use whenever the session-start digest's bootstrap section prints any diagnostic or capability line - MISSING, MISSING_MANUAL, BACKEND_INVALID, ACCOUNT_ROUTING, NEEDS_GH_AUTH, TANGLE, CREW_HARNESS_OVERRIDE, CREW_DISPATCH, FLEET_SYNC, SECONDMATE_SYNC, SECONDMATE_LIVENESS, TASKS_AXI, NUDGE_SECONDMATES, REPORT_RETENTION, BROWSER_GC, or FMX - or when a standalone bin/fm-bootstrap.sh run prints one. A silent bootstrap section means all good and needs no skill load. user-invocable: false metadata: @@ -48,5 +48,7 @@ The inline rules in `AGENTS.md` section 3 still bind: detect, then consent, then A secondmate that was skipped, already current, or whose advance changed no instructions is not listed and must not be disturbed. - `REPORT_RETENTION: unavailable: ` - the machine-global report-retention LaunchAgent is absent, stale, unloaded, or has not reported a recent successful prune, so opportunistic bounded pruning remains available but post-minimum-age cleanup is not guaranteed to run while Firstmate is idle. Surface the failure, wait for the captain's consent, then run `bin/fm-bootstrap.sh install report-retention`; never install or activate it without that consent. +- `BROWSER_GC: reaped ` - the locked session-start sweep removed task-owned or legacy AXI browser processes and temp profiles that survived graceful teardown; no action is needed. +- `BROWSER_GC: unavailable: ` - the browser orphan backstop could not prove cleanup, so surface the reason and inspect `bin/fm-browser-isolation.sh` before relying on the next sweep; task-scoped teardown remains fail-closed independently. - `FMX: X mode on ...` / `FMX: X mode off ...` - bootstrap confirmed or removed the local X-mode poll artifacts (`docs/configuration.md` "X mode (.env)"). Only when a running watcher needs the cadence transition applied immediately, restart the home-scoped watcher through the emitted harness supervision protocol; bootstrap deliberately never restarts the watcher itself. diff --git a/AGENTS.md b/AGENTS.md index eeed517dc9..f138e8f750 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -145,7 +145,7 @@ It composes today's `fm-lock.sh`, `fm-bootstrap.sh`, and `fm-wake-drain.sh` - ca 1. **Lock** - acquires the per-home session lock first, before anything mutates shared state. 2. **Bootstrap** - detect-only diagnostics (tool/version problems, GitHub auth, the worktree-tangle check, harness override, dispatch-profile validation, backlog-backend status) always run and always print. When the lock could not be acquired, the worktree-tangle check uses read-only advisory wording without a checkout repair command. - The five MUTATING sweeps - report-retention ownership, fleet sync, the local secondmate fast-forward sweep, the secondmate liveness sweep, and X-mode artifact writes - run only when this session actually holds the lock from step 1. + The six MUTATING sweeps - report-retention ownership, fleet sync, the local secondmate fast-forward sweep, the secondmate liveness sweep, browser orphan cleanup, and X-mode artifact writes - run only when this session actually holds the lock from step 1. The secondmate liveness sweep probes each live secondmate's endpoint for a real agent process, reports handled or deferred outcomes as `SECONDMATE_LIVENESS:` lines, and leaves any required routing decision to `secondmate-provisioning` (`bin/fm-bootstrap.sh`; `bin/fm-backend.sh`'s `fm_backend_agent_alive`). 3. **Wake queue** - when locked, drains the durable wake queue and prints the records prominently as this turn's first work queue, exactly as `bin/fm-wake-drain.sh` did before; a lapsed watcher chain still surfaces here via the same guard banner. When the lock could not be acquired, the queue is left untouched because another session owns it, and the guard's tangle/watcher-liveness alarms still print in read-only advisory mode without drain, supervision repair, or checkout repair commands. @@ -810,7 +810,7 @@ It performs only fast-forward self-updates of firstmate and registered secondmat These skills are not captain-invocable; they are conditional operating references you must load at the trigger points below. -- `bootstrap-diagnostics` - load whenever the session-start digest's bootstrap section prints any diagnostic or capability line (`MISSING:`, `MISSING_MANUAL:`, `BACKEND_INVALID:`, `ACCOUNT_ROUTING:`, `NEEDS_GH_AUTH`, `TANGLE:`, `CREW_HARNESS_OVERRIDE:`, `CREW_DISPATCH:`, `FLEET_SYNC:`, `SECONDMATE_SYNC:`, `SECONDMATE_LIVENESS:`, `TASKS_AXI:`, `NUDGE_SECONDMATES:`, `REPORT_RETENTION:`, or `FMX:`); silence needs no load. +- `bootstrap-diagnostics` - load whenever the session-start digest's bootstrap section prints any diagnostic or capability line (`MISSING:`, `MISSING_MANUAL:`, `BACKEND_INVALID:`, `ACCOUNT_ROUTING:`, `NEEDS_GH_AUTH`, `TANGLE:`, `CREW_HARNESS_OVERRIDE:`, `CREW_DISPATCH:`, `FLEET_SYNC:`, `SECONDMATE_SYNC:`, `SECONDMATE_LIVENESS:`, `TASKS_AXI:`, `NUDGE_SECONDMATES:`, `REPORT_RETENTION:`, `BROWSER_GC:`, or `FMX:`); silence needs no load. - `harness-adapters` - load before spawning or recovering a crewmate or secondmate, handling a trust or permission dialog, sending a harness-specific skill invocation, interrupting or exiting an agent, resuming an exited agent, or verifying a new harness adapter. - `operating-fundamentals` - load when intaking any captain ask, deciding whether to dispatch or work inline, supervising under load, handling a blocked lane or finished crewmate, protecting shared validation capacity, acting on an explicit captain order, about to make a consequential config/system change or an escalation, or about to assert a fleet fact. - `crew-steering` - load before writing or materially revising any crewmate brief and before live-steering a crewmate. diff --git a/bin/chrome-devtools-axi b/bin/chrome-devtools-axi new file mode 100755 index 0000000000..53f924a003 --- /dev/null +++ b/bin/chrome-devtools-axi @@ -0,0 +1,7 @@ +#!/usr/bin/env bash +# PATH-scoped chrome-devtools-axi entrypoint for firstmate-launched agents. +# fm-spawn.sh puts this directory ahead of the real AXI only inside a crew task. +set -eu + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +exec "$SCRIPT_DIR/fm-browser-isolation.sh" run "$@" diff --git a/bin/fm-bootstrap.sh b/bin/fm-bootstrap.sh index fa63bd898c..b2f6795094 100755 --- a/bin/fm-bootstrap.sh +++ b/bin/fm-bootstrap.sh @@ -17,6 +17,7 @@ # "NUDGE_SECONDMATES: fm-...", # "REPORT_RETENTION: unavailable: ", # "SECONDMATE_LIVENESS: secondmate : ", +# "BROWSER_GC: reaped|unavailable: ", # "FMX: X mode on ..." or "FMX: X mode off ...". # A NUDGE_SECONDMATES line lists the RUNNING secondmate task selectors # (fm-) whose worktree was fast-forwarded to firstmate's own @@ -76,9 +77,10 @@ # refresh relays any completed fm-fleet-sync.sh output before the # aggregate timeout skip line with timeout and elapsed seconds. # Set FM_FLEET_PRUNE=0 to skip branch pruning during that refresh. -# Set FM_BOOTSTRAP_DETECT_ONLY=1 to skip the five MUTATING sweeps +# Set FM_BOOTSTRAP_DETECT_ONLY=1 to skip the six MUTATING sweeps # (report_retention_ensure, secondmate_sync, -# secondmate_liveness_sweep, x_mode_setup, fleet_sync) while still +# secondmate_liveness_sweep, browser_gc_sweep, x_mode_setup, +# fleet_sync) while still # printing every read-only detect line # above; the TANGLE line switches to advisory-only wording with no # checkout command. Used by @@ -127,6 +129,20 @@ report_retention_ensure() { fi } +browser_gc_sweep() { + local out + if [ "${FM_GATE_REFUSE_BYPASS:-0}" = 1 ] && [ "${FM_BROWSER_GC_BOOTSTRAP_TEST:-0}" != 1 ]; then + return 0 + fi + [ -x "$SCRIPT_DIR/fm-browser-isolation.sh" ] || return 0 + if out=$("$SCRIPT_DIR/fm-browser-isolation.sh" sweep "$FM_HOME" "$STATE" 2>&1); then + [ -z "$out" ] || printf '%s\n' "$out" + else + [ -n "$out" ] || out="browser cleanup could not be verified" + echo "BROWSER_GC: unavailable: ${out%%$'\n'*}" + fi +} + checkout_refresh_ensure() { local out [ "$(uname)" = Darwin ] || return 0 @@ -878,6 +894,7 @@ if [ "${FM_BOOTSTRAP_DETECT_ONLY:-0}" != 1 ]; then report_retention_ensure secondmate_sync secondmate_liveness_sweep + browser_gc_sweep x_mode_setup fleet_sync fi diff --git a/bin/fm-browser-isolation.mjs b/bin/fm-browser-isolation.mjs new file mode 100755 index 0000000000..7cef7ee8e8 --- /dev/null +++ b/bin/fm-browser-isolation.mjs @@ -0,0 +1,977 @@ +#!/usr/bin/env node +/** + * Task-scoped browser lifecycle for firstmate-launched agents. + * + * Commands: + * prepare [mcp-path] + * run + * reap + * sweep + * classify + * + * The spawn path prepares a private browser root under /tmp/fm-/browser + * and puts bin/chrome-devtools-axi ahead of the real AXI on the crew PATH. + * The wrapper launches a separate Chrome for Testing/Canary executable lazily, + * attaches AXI through a loopback debugging endpoint, and keeps the captain's + * stable Chrome app and profile out of the process tree. + * + * Teardown reaps the recorded bridge and browser before deleting the task + * profile. The locked bootstrap sweep also removes task roots whose owning + * home no longer has metadata and legacy AXI orphans identified by exact + * bridge/headless/temp-profile markers. Set FM_BROWSER_TMP_ROOT only in tests. + */ + +import { spawn, spawnSync } from "node:child_process"; +import { + closeSync, + existsSync, + lstatSync, + mkdirSync, + openSync, + readFileSync, + readdirSync, + realpathSync, + rmSync, + statSync, + writeFileSync, +} from "node:fs"; +import http from "node:http"; +import net from "node:net"; +import os from "node:os"; +import path from "node:path"; + +const TASK_ID_RE = /^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$/; +const LEGACY_PROFILE_NAME_RE = /^puppeteer_dev_chrome_profile-[A-Za-z0-9._-]+$/; +const USER_DATA_DIR_ARG_RE = + /--user-data-dir(?:=|\s+)(["']?)(\/[^\s"']+)\1(?:\s|$)/; +const HEADLESS_ARG_RE = /(?:^|\s)--headless(?:=[^\s]+)?(?:\s|$)/; +const CHROME_PROCESS_RE = + /(?:Google Chrome|Chromium|(?:^|[\s/])(?:chrome|chromium)(?:\s|$))/i; +const BRIDGE_PROCESS_RE = + /(?:^|\s)(?:[^\s]*\/)?chrome-devtools-axi-bridge\.js(?:\s|$)/; +const OWNER_FILE = "owner.json"; +const BROWSER_FILE = "browser.json"; +const BROWSER_FAILURE_FILE = "browser.failed"; +const AXI_PORT_FILE = "axi-port"; +const LOCK_FILE = "lifecycle.lock"; +const START_TIMEOUT_MS = 20_000; +const STOP_GRACE_MS = 2_000; +const CREDENTIAL_ISOLATION_ARGS = ["--use-mock-keychain", "--password-store=basic"]; + +function fail(message) { + throw new Error(message); +} + +function sleep(ms) { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +function processAlive(pid) { + if (!Number.isSafeInteger(pid) || pid <= 1) return false; + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +} + +function canonicalTmpRoot() { + const configured = process.env.FM_BROWSER_TMP_ROOT || "/tmp"; + const resolved = realpathSync(configured); + const metadata = lstatSync(resolved); + if (!metadata.isDirectory() || metadata.isSymbolicLink()) { + fail(`unsafe browser temp root: ${configured}`); + } + if (!process.env.FM_BROWSER_TMP_ROOT && resolved !== "/tmp" && resolved !== "/private/tmp") { + fail(`unexpected system temp root: ${resolved}`); + } + return resolved; +} + +function validateTaskId(taskId) { + if (!TASK_ID_RE.test(taskId)) fail(`invalid browser task id: ${taskId}`); + return taskId; +} + +function expectedTaskTmp(taskId) { + return path.join(canonicalTmpRoot(), `fm-${validateTaskId(taskId)}`); +} + +function validateTaskTmp(taskId, taskTmp, requireExisting = true) { + const expected = expectedTaskTmp(taskId); + const resolved = requireExisting + ? realpathSync(taskTmp) + : path.join(realpathSync(path.dirname(path.resolve(taskTmp))), path.basename(taskTmp)); + if (resolved !== expected) { + fail(`unsafe browser task temp path for ${taskId}: ${taskTmp}`); + } + if (requireExisting) { + const metadata = lstatSync(resolved); + if (!metadata.isDirectory() || metadata.isSymbolicLink()) { + fail(`unsafe browser task temp directory for ${taskId}: ${taskTmp}`); + } + } + return resolved; +} + +function validateBrowserRoot(taskId, browserRoot) { + const expected = path.join(expectedTaskTmp(taskId), "browser"); + const resolved = realpathSync(browserRoot); + if (resolved !== expected) { + fail(`unsafe browser root for ${taskId}: ${browserRoot}`); + } + const metadata = lstatSync(resolved); + if (!metadata.isDirectory() || metadata.isSymbolicLink()) { + fail(`unsafe browser root directory for ${taskId}: ${browserRoot}`); + } + return resolved; +} + +function readJson(file) { + return JSON.parse(readFileSync(file, "utf8")); +} + +function writeJson(file, value) { + writeFileSync(file, `${JSON.stringify(value)}\n`, { mode: 0o600 }); +} + +function validateExecutable(file, label) { + if (!path.isAbsolute(file)) fail(`${label} must be absolute: ${file}`); + const resolved = realpathSync(file); + const metadata = statSync(resolved); + if (!metadata.isFile()) fail(`${label} is not a regular file: ${file}`); + return resolved; +} + +function validateOwner(owner, root, taskId) { + if (!owner || owner.version !== 1 || owner.taskId !== taskId) { + fail(`browser owner record does not match ${taskId}`); + } + if (owner.root !== root) fail(`browser owner root changed for ${taskId}`); + owner.ownerHome = realpathSync(owner.ownerHome); + owner.realAxi = validateExecutable(owner.realAxi, "chrome-devtools-axi"); + owner.browserExecutable = owner.browserExecutable + ? validateExecutable(owner.browserExecutable, "automation browser executable") + : ""; + if (owner.mcpPath) owner.mcpPath = validateExecutable(owner.mcpPath, "chrome-devtools-mcp"); + return owner; +} + +function loadOwner(root, taskId) { + return validateOwner(readJson(path.join(root, OWNER_FILE)), root, taskId); +} + +function parseInventory(text) { + const rows = []; + for (const line of text.split(/\r?\n/)) { + if (!line.trim()) continue; + const match = line.match(/^\s*(\d+)\s+(\d+)\s+(\d+)\s+(.*)$/); + if (!match) continue; + rows.push({ + pid: Number(match[1]), + ppid: Number(match[2]), + pgid: Number(match[3]), + command: match[4], + }); + } + return rows; +} + +function processInventory() { + const result = spawnSync("ps", ["-axo", "pid=,ppid=,pgid=,command="], { + encoding: "utf8", + timeout: 5_000, + }); + if (result.status !== 0) fail("cannot inspect process inventory for browser cleanup"); + return parseInventory(result.stdout); +} + +function rowForPid(pid, rows = processInventory()) { + return rows.find((row) => row.pid === pid) || null; +} + +function isLegacyBridge(row) { + return BRIDGE_PROCESS_RE.test(row.command); +} + +function legacyProfileFromCommand(command) { + const match = command.match(USER_DATA_DIR_ARG_RE); + if (!match) return null; + const profileName = path.basename(match[2]); + if (!LEGACY_PROFILE_NAME_RE.test(profileName)) return null; + try { + const parent = realpathSync(path.dirname(match[2])); + if (parent !== canonicalTmpRoot()) return null; + return path.join(parent, profileName); + } catch { + return null; + } +} + +function isLegacyBrowser(row) { + return ( + CHROME_PROCESS_RE.test(row.command) && + HEADLESS_ARG_RE.test(row.command) && + legacyProfileFromCommand(row.command) !== null + ); +} + +function taskProfileFromCommand(command) { + const match = command.match( + /--user-data-dir(?:=|\s+)(["']?)((?:\/private)?\/tmp\/fm-[A-Za-z0-9._-]+\/browser\/profile)\1(?:\s|$)/, + ); + return match ? match[2] : null; +} + +function classifyRow(row, protectedPids = new Set()) { + if (protectedPids.has(row.pid) || taskProfileFromCommand(row.command)) { + return "protected-task-browser"; + } + if (isLegacyBridge(row)) return "legacy-bridge"; + if (isLegacyBrowser(row)) return "legacy-browser"; + return "unrelated"; +} + +function processMatchesBrowser(state, row) { + if (!row) return false; + return ( + row.command.includes(state.executable) && + row.command.includes(`--user-data-dir=${state.profile}`) && + HEADLESS_ARG_RE.test(row.command) + ); +} + +function processMatchesBridge(row) { + return row !== null && isLegacyBridge(row); +} + +async function waitForExit(pid, timeoutMs) { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + if (!processAlive(pid)) return true; + await sleep(50); + } + return !processAlive(pid); +} + +async function terminateExactPid(pid, matches, allowGroup = false) { + let row = rowForPid(pid); + if (!row || !matches(row)) return false; + const signalTarget = allowGroup && row.pgid === pid ? -pid : pid; + try { + process.kill(signalTarget, "SIGTERM"); + } catch { + return true; + } + if (await waitForExit(pid, STOP_GRACE_MS)) return true; + row = rowForPid(pid); + if (!row || !matches(row)) return true; + try { + process.kill(signalTarget, "SIGKILL"); + } catch { + return true; + } + return waitForExit(pid, 1_000); +} + +function descendantsOf(pid, rows) { + const descendants = new Set([pid]); + let changed = true; + while (changed) { + changed = false; + for (const row of rows) { + if (descendants.has(row.ppid) && !descendants.has(row.pid)) { + descendants.add(row.pid); + changed = true; + } + } + } + descendants.delete(pid); + return [...descendants]; +} + +async function terminateLegacyBridgeTree(pid) { + const rows = processInventory(); + const root = rowForPid(pid, rows); + if (!root || !isLegacyBridge(root)) return false; + const owned = descendantsOf(pid, rows).reverse(); + for (const childPid of owned) { + try { + process.kill(childPid, "SIGTERM"); + } catch { + // Already gone. + } + } + await terminateExactPid(pid, isLegacyBridge, root.pgid === pid); + await sleep(100); + for (const childPid of owned) { + if (!processAlive(childPid)) continue; + try { + process.kill(childPid, "SIGKILL"); + } catch { + // Already gone. + } + } + return true; +} + +async function terminateLegacyBrowsers(rows) { + const candidates = rows.filter(isLegacyBrowser); + const ownedPids = new Set(); + for (const candidate of candidates) { + ownedPids.add(candidate.pid); + for (const childPid of descendantsOf(candidate.pid, rows)) { + ownedPids.add(childPid); + } + } + for (const pid of ownedPids) { + try { + process.kill(pid, "SIGTERM"); + } catch { + // Already gone. + } + } + await sleep(500); + let killed = 0; + for (const pid of ownedPids) { + const current = rowForPid(pid); + if (!current) { + killed += 1; + continue; + } + const original = rows.find((row) => row.pid === pid); + if (!original || current.command !== original.command) continue; + try { + process.kill(pid, "SIGKILL"); + killed += 1; + } catch { + killed += 1; + } + } + return killed; +} + +function requestJson(port, requestPath, timeoutMs = 1_000) { + return new Promise((resolve, reject) => { + const request = http.get( + { hostname: "127.0.0.1", port, path: requestPath, timeout: timeoutMs }, + (response) => { + let body = ""; + response.setEncoding("utf8"); + response.on("data", (chunk) => { + body += chunk; + }); + response.on("end", () => { + try { + resolve(JSON.parse(body)); + } catch (error) { + reject(error); + } + }); + }, + ); + request.on("timeout", () => { + request.destroy(new Error("timeout")); + }); + request.on("error", reject); + }); +} + +async function browserReachable(port) { + try { + const response = await requestJson(port, "/json/version"); + return typeof response.webSocketDebuggerUrl === "string"; + } catch { + return false; + } +} + +async function allocatePort() { + return new Promise((resolve, reject) => { + const server = net.createServer(); + server.unref(); + server.on("error", reject); + server.listen(0, "127.0.0.1", () => { + const address = server.address(); + const port = typeof address === "object" && address ? address.port : 0; + server.close((error) => { + if (error) reject(error); + else if (port > 0) resolve(port); + else reject(new Error("loopback port allocation failed")); + }); + }); + }); +} + +async function acquireLock(root) { + const lock = path.join(root, LOCK_FILE); + const deadline = Date.now() + 10_000; + while (Date.now() < deadline) { + try { + writeFileSync(lock, `${process.pid}\n`, { flag: "wx", mode: 0o600 }); + return lock; + } catch (error) { + if (error.code !== "EEXIST") throw error; + let ownerPid = 0; + try { + ownerPid = Number(readFileSync(lock, "utf8").trim()); + } catch { + ownerPid = 0; + } + if (!processAlive(ownerPid)) { + try { + rmSync(lock); + } catch { + // Another caller resolved it first. + } + continue; + } + await sleep(50); + } + } + fail(`browser lifecycle lock stayed busy for ${root}`); +} + +function releaseLock(lock) { + try { + const ownerPid = Number(readFileSync(lock, "utf8").trim()); + if (ownerPid === process.pid) rmSync(lock); + } catch { + // Cleanup is best effort after the protected operation has finished. + } +} + +async function ensureAxiPort(root) { + const file = path.join(root, AXI_PORT_FILE); + if (existsSync(file)) { + const current = Number(readFileSync(file, "utf8").trim()); + if (Number.isInteger(current) && current > 1024 && current < 65536) return current; + } + const port = await allocatePort(); + writeFileSync(file, `${port}\n`, { mode: 0o600 }); + return port; +} + +function readBrowserState(root) { + const file = path.join(root, BROWSER_FILE); + if (!existsSync(file)) return null; + try { + const state = readJson(file); + if ( + Number.isSafeInteger(state.pid) && + Number.isSafeInteger(state.port) && + typeof state.profile === "string" && + typeof state.executable === "string" + ) { + return state; + } + } catch { + // Invalid state is treated as stale and replaced only after process checks. + } + return null; +} + +async function stopTaskBrowser(root) { + const state = readBrowserState(root); + if (!state) return false; + const stopped = await terminateExactPid( + state.pid, + (row) => processMatchesBrowser(state, row), + true, + ); + const stillOwned = processInventory().filter( + (row) => row.command.includes(`--user-data-dir=${state.profile}`), + ); + for (const row of stillOwned) { + try { + process.kill(row.pid, "SIGTERM"); + } catch { + // Already gone. + } + } + await sleep(200); + for (const row of stillOwned) { + const current = rowForPid(row.pid); + if (!current || !current.command.includes(`--user-data-dir=${state.profile}`)) continue; + try { + process.kill(row.pid, "SIGKILL"); + } catch { + // Already gone. + } + } + const remaining = processInventory().filter( + (row) => row.command.includes(`--user-data-dir=${state.profile}`), + ); + if (remaining.length > 0) { + fail( + `task browser processes survived cleanup: ${remaining + .map((row) => row.pid) + .join(",")}`, + ); + } + rmSync(path.join(root, BROWSER_FILE), { force: true }); + return stopped || stillOwned.length > 0; +} + +async function ensureBrowser(root, owner) { + if (!owner.browserExecutable) { + fail("no separate automation browser is installed; install Chrome Canary, Chrome for Testing, or Chromium"); + } + if (existsSync(path.join(root, BROWSER_FAILURE_FILE))) { + fail("automation browser is latched after a failure; run chrome-devtools-axi stop before retrying"); + } + const existing = readBrowserState(root); + if (existing) { + const row = rowForPid(existing.pid); + if (processMatchesBrowser(existing, row) && (await browserReachable(existing.port))) { + return existing; + } + await stopTaskBrowser(root); + writeFileSync( + path.join(root, BROWSER_FAILURE_FILE), + "automation browser exited unexpectedly; explicit stop is required before relaunch\n", + { mode: 0o600 }, + ); + fail("automation browser exited unexpectedly; refusing to respawn it implicitly"); + } + + const profile = path.join(root, "profile"); + const home = path.join(root, "home"); + mkdirSync(profile, { recursive: true, mode: 0o700 }); + mkdirSync(home, { recursive: true, mode: 0o700 }); + for (const singleton of ["SingletonCookie", "SingletonLock", "SingletonSocket"]) { + rmSync(path.join(profile, singleton), { force: true }); + } + + const logFile = path.join(root, "browser.log"); + const logFd = openSync(logFile, "a", 0o600); + const args = [ + "--headless=new", + ...CREDENTIAL_ISOLATION_ARGS, + "--no-first-run", + "--no-default-browser-check", + "--noerrdialogs", + "--disable-breakpad", + "--disable-crash-reporter", + "--disable-background-networking", + "--disable-component-update", + "--disable-sync", + "--remote-debugging-address=127.0.0.1", + "--remote-debugging-port=0", + "--remote-allow-origins=*", + `--user-data-dir=${profile}`, + "about:blank", + ]; + const child = spawn(owner.browserExecutable, args, { + detached: true, + env: { ...process.env, HOME: home }, + stdio: ["ignore", logFd, logFd], + }); + closeSync(logFd); + child.unref(); + + const state = { + pid: child.pid, + port: 0, + profile, + executable: owner.browserExecutable, + startedAt: new Date().toISOString(), + }; + writeJson(path.join(root, BROWSER_FILE), state); + + const activePortFile = path.join(profile, "DevToolsActivePort"); + const deadline = Date.now() + START_TIMEOUT_MS; + while (Date.now() < deadline) { + if (!processAlive(child.pid)) break; + if (existsSync(activePortFile)) { + const port = Number(readFileSync(activePortFile, "utf8").split(/\r?\n/, 1)[0]); + if (Number.isInteger(port) && port > 0 && (await browserReachable(port))) { + state.port = port; + writeJson(path.join(root, BROWSER_FILE), state); + rmSync(path.join(root, BROWSER_FAILURE_FILE), { force: true }); + return state; + } + } + await sleep(100); + } + + await stopTaskBrowser(root); + writeFileSync( + path.join(root, BROWSER_FAILURE_FILE), + "automation browser failed to start; explicit stop is required before relaunch\n", + { mode: 0o600 }, + ); + let diagnostic = ""; + try { + diagnostic = readFileSync(logFile, "utf8").trim().split(/\r?\n/).slice(-3).join(" "); + } catch { + diagnostic = ""; + } + fail(`automation browser failed to start${diagnostic ? `: ${diagnostic}` : ""}`); +} + +function axiEnvironment(root, owner, browserState, axiPort) { + const environment = { ...process.env }; + environment.HOME = path.join(root, "home"); + environment.CHROME_DEVTOOLS_AXI_SESSION = `fm-${owner.taskId}`; + environment.CHROME_DEVTOOLS_AXI_PORT = String(axiPort); + if (browserState && browserState.port > 0) { + environment.CHROME_DEVTOOLS_AXI_BROWSER_URL = `http://127.0.0.1:${browserState.port}`; + } else { + delete environment.CHROME_DEVTOOLS_AXI_BROWSER_URL; + } + if (owner.mcpPath) environment.CHROME_DEVTOOLS_AXI_MCP_PATH = owner.mcpPath; + else delete environment.CHROME_DEVTOOLS_AXI_MCP_PATH; + environment.CHROME_DEVTOOLS_AXI_CHROME_ARGS = CREDENTIAL_ISOLATION_ARGS.join(" "); + delete environment.CHROME_DEVTOOLS_AXI_AUTO_CONNECT; + delete environment.CHROME_DEVTOOLS_AXI_USER_DATA_DIR; + delete environment.CHROME_DEVTOOLS_AXI_CHANNEL; + delete environment.CHROME_DEVTOOLS_AXI_HEADED; + return environment; +} + +function invokeAxi(owner, args, environment, stdio = "inherit", timeout) { + return new Promise((resolve, reject) => { + const child = spawn(owner.realAxi, args, { + env: environment, + stdio, + }); + let timedOut = false; + let timer; + let killTimer; + const forwardedSignals = ["SIGINT", "SIGTERM", "SIGHUP"]; + const handlers = new Map(); + + const finish = (callback) => { + if (timer) clearTimeout(timer); + if (killTimer) clearTimeout(killTimer); + for (const [signal, handler] of handlers) { + process.off(signal, handler); + } + callback(); + }; + + for (const signal of forwardedSignals) { + const handler = () => { + try { + child.kill(signal); + } catch { + // The AXI command already exited. + } + if (!killTimer) { + killTimer = setTimeout(() => { + try { + child.kill("SIGKILL"); + } catch { + // The AXI command exited during the signal grace period. + } + }, STOP_GRACE_MS); + killTimer.unref(); + } + }; + handlers.set(signal, handler); + process.on(signal, handler); + } + if (timeout) { + timer = setTimeout(() => { + timedOut = true; + child.kill("SIGTERM"); + killTimer = setTimeout(() => { + try { + child.kill("SIGKILL"); + } catch { + // The AXI command exited during the timeout grace period. + } + }, STOP_GRACE_MS); + killTimer.unref(); + }, timeout); + timer.unref(); + } + child.on("error", (error) => finish(() => reject(error))); + child.on("close", (status, signal) => { + finish(() => { + if (timedOut) resolve(124); + else if (signal) resolve(128 + (os.constants.signals[signal] || 1)); + else resolve(status ?? 1); + }); + }); + }); +} + +function bridgePidFile(root, taskId) { + return path.join( + root, + "home", + ".chrome-devtools-axi", + "sessions", + `fm-${taskId}`, + "bridge.pid", + ); +} + +function readBridgePid(root, taskId) { + try { + const data = readJson(bridgePidFile(root, taskId)); + return Number.isSafeInteger(data.pid) ? data.pid : 0; + } catch { + return 0; + } +} + +async function stopTaskAxi(root, owner) { + const axiPort = await ensureAxiPort(root); + const browserState = readBrowserState(root); + const environment = axiEnvironment(root, owner, browserState, axiPort); + const bridgePid = readBridgePid(root, owner.taskId); + let status = 1; + try { + status = await invokeAxi(owner, ["stop"], environment, "ignore", 5_000); + } catch { + status = 1; + } finally { + if (bridgePid > 0) { + await terminateExactPid(bridgePid, processMatchesBridge, true); + } + } + return status; +} + +async function commandPrepare(args) { + if (args.length < 5 || args.length > 6) { + fail("usage: prepare [mcp-path]"); + } + const [taskId, taskTmpInput, ownerHomeInput, realAxiInput, browserInput, mcpInput = ""] = + args; + validateTaskId(taskId); + const taskTmp = validateTaskTmp(taskId, taskTmpInput); + const root = path.join(taskTmp, "browser"); + mkdirSync(root, { recursive: true, mode: 0o700 }); + const canonicalRoot = validateBrowserRoot(taskId, root); + mkdirSync(path.join(canonicalRoot, "home"), { recursive: true, mode: 0o700 }); + mkdirSync(path.join(canonicalRoot, "profile"), { recursive: true, mode: 0o700 }); + const ownerHome = realpathSync(ownerHomeInput); + const owner = { + version: 1, + taskId, + root: canonicalRoot, + ownerHome, + realAxi: validateExecutable(realAxiInput, "chrome-devtools-axi"), + browserExecutable: browserInput + ? validateExecutable(browserInput, "automation browser executable") + : "", + mcpPath: mcpInput ? validateExecutable(mcpInput, "chrome-devtools-mcp") : "", + }; + writeJson(path.join(canonicalRoot, OWNER_FILE), owner); +} + +async function commandRun(args) { + const taskId = validateTaskId(process.env.FM_BROWSER_TASK_ID || ""); + const root = validateBrowserRoot(taskId, process.env.FM_BROWSER_ROOT || ""); + const owner = loadOwner(root, taskId); + const lock = await acquireLock(root); + try { + const command = args[0] || ""; + const axiPort = await ensureAxiPort(root); + if (command === "stop") { + const status = await stopTaskAxi(root, owner); + await stopTaskBrowser(root); + rmSync(path.join(root, BROWSER_FAILURE_FILE), { force: true }); + process.exitCode = status; + return; + } + const noBrowser = + command === "" || + command === "setup" || + command === "--help" || + command === "-h" || + command === "--version" || + command === "-v" || + command === "-V"; + const browserState = noBrowser ? null : await ensureBrowser(root, owner); + const status = await invokeAxi( + owner, + args, + axiEnvironment(root, owner, browserState, axiPort), + ); + if (status !== 0) { + await stopTaskAxi(root, owner); + await stopTaskBrowser(root); + writeFileSync( + path.join(root, BROWSER_FAILURE_FILE), + `chrome-devtools-axi exited ${status}; explicit stop is required before relaunch\n`, + { mode: 0o600 }, + ); + } + process.exitCode = status; + } finally { + releaseLock(lock); + } +} + +async function reapRoot(taskId, taskTmp, removeRoot = true) { + const expectedTmp = validateTaskTmp(taskId, taskTmp, false); + if (!existsSync(expectedTmp)) return { bridge: 0, browser: 0 }; + const validatedTmp = validateTaskTmp(taskId, expectedTmp); + const root = path.join(validatedTmp, "browser"); + if (!existsSync(root)) return { bridge: 0, browser: 0 }; + const canonicalRoot = validateBrowserRoot(taskId, root); + const owner = loadOwner(canonicalRoot, taskId); + const lock = await acquireLock(canonicalRoot); + let bridge = 0; + let browser = 0; + try { + const bridgePid = readBridgePid(canonicalRoot, taskId); + await stopTaskAxi(canonicalRoot, owner); + if (bridgePid > 0 && !processAlive(bridgePid)) bridge = 1; + if (await stopTaskBrowser(canonicalRoot)) browser = 1; + const profile = path.join(canonicalRoot, "profile"); + const inUse = processInventory().some((row) => row.command.includes(profile)); + if (inUse) fail(`browser profile is still in use for ${taskId}`); + if (removeRoot) rmSync(canonicalRoot, { recursive: true, force: true }); + } finally { + if (existsSync(canonicalRoot)) releaseLock(lock); + } + return { bridge, browser }; +} + +async function commandReap(args) { + if (args.length !== 2) fail("usage: reap "); + await reapRoot(validateTaskId(args[0]), args[1]); +} + +function taskRoots(tmpRoot) { + const roots = []; + for (const entry of readdirSync(tmpRoot, { withFileTypes: true })) { + if (!entry.isDirectory() || !entry.name.startsWith("fm-")) continue; + const browserRoot = path.join(tmpRoot, entry.name, "browser"); + if (!existsSync(path.join(browserRoot, OWNER_FILE))) continue; + roots.push(browserRoot); + } + return roots; +} + +function protectedTaskPids(tmpRoot) { + const protectedPids = new Set(); + for (const root of taskRoots(tmpRoot)) { + const taskId = path.basename(path.dirname(root)).slice(3); + const browserState = readBrowserState(root); + if (browserState?.pid) protectedPids.add(browserState.pid); + const bridgePid = readBridgePid(root, taskId); + if (bridgePid > 0) protectedPids.add(bridgePid); + } + return protectedPids; +} + +function removeLegacyProfiles(tmpRoot) { + const rows = processInventory(); + let removed = 0; + for (const entry of readdirSync(tmpRoot, { withFileTypes: true })) { + if (!LEGACY_PROFILE_NAME_RE.test(entry.name)) continue; + const profile = path.join(tmpRoot, entry.name); + if (rows.some((row) => legacyProfileFromCommand(row.command) === profile)) continue; + const metadata = lstatSync(profile); + if (!metadata.isDirectory() || metadata.isSymbolicLink()) continue; + rmSync(profile, { recursive: true, force: true }); + removed += 1; + } + return removed; +} + +async function commandSweep(args) { + if (args.length !== 2) fail("usage: sweep "); + const ownerHome = realpathSync(args[0]); + const stateDir = realpathSync(args[1]); + const tmpRoot = canonicalTmpRoot(); + let taskRootsReaped = 0; + let bridgesReaped = 0; + let browsersReaped = 0; + + for (const root of taskRoots(tmpRoot)) { + let owner; + try { + owner = readJson(path.join(root, OWNER_FILE)); + } catch { + continue; + } + if (owner.ownerHome !== ownerHome || !TASK_ID_RE.test(owner.taskId || "")) continue; + if (existsSync(path.join(stateDir, `${owner.taskId}.meta`))) continue; + const result = await reapRoot(owner.taskId, path.dirname(root)); + taskRootsReaped += 1; + bridgesReaped += result.bridge; + browsersReaped += result.browser; + } + + const activeRootLocks = []; + let profilesReaped = 0; + try { + for (const root of taskRoots(tmpRoot)) { + const taskId = path.basename(path.dirname(root)).slice(3); + let canonicalRoot; + try { + canonicalRoot = validateBrowserRoot(taskId, root); + } catch { + continue; + } + activeRootLocks.push(await acquireLock(canonicalRoot)); + } + + const protectedPids = protectedTaskPids(tmpRoot); + let rows = processInventory(); + for (const row of rows) { + if (protectedPids.has(row.pid) || !isLegacyBridge(row)) continue; + if (await terminateLegacyBridgeTree(row.pid)) bridgesReaped += 1; + } + rows = processInventory(); + browsersReaped += await terminateLegacyBrowsers( + rows.filter((row) => !protectedPids.has(row.pid)), + ); + profilesReaped = removeLegacyProfiles(tmpRoot); + } finally { + for (const lock of activeRootLocks.reverse()) { + releaseLock(lock); + } + } + + if (taskRootsReaped || bridgesReaped || browsersReaped || profilesReaped) { + process.stdout.write( + `BROWSER_GC: reaped task_roots=${taskRootsReaped} bridges=${bridgesReaped} browser_processes=${browsersReaped} profiles=${profilesReaped}\n`, + ); + } +} + +function commandClassify(args) { + if (args.length !== 1) fail("usage: classify "); + const rows = parseInventory(readFileSync(args[0], "utf8")); + for (const row of rows) { + process.stdout.write(`${row.pid}\t${classifyRow(row)}\n`); + } +} + +async function main() { + const [command, ...args] = process.argv.slice(2); + switch (command) { + case "prepare": + await commandPrepare(args); + break; + case "run": + await commandRun(args); + break; + case "reap": + await commandReap(args); + break; + case "sweep": + await commandSweep(args); + break; + case "classify": + commandClassify(args); + break; + default: + fail("usage: fm-browser-isolation.mjs ..."); + } +} + +main().catch((error) => { + process.stderr.write(`error: ${error.message}\n`); + process.exitCode = 1; +}); diff --git a/bin/fm-browser-isolation.sh b/bin/fm-browser-isolation.sh new file mode 100755 index 0000000000..3b800605ab --- /dev/null +++ b/bin/fm-browser-isolation.sh @@ -0,0 +1,9 @@ +#!/usr/bin/env bash +# Stable shell entrypoint for firstmate's task-scoped browser lifecycle. +# Usage: fm-browser-isolation.sh ... +# The full lifecycle, process discrimination, and safety contract live in the +# adjacent fm-browser-isolation.mjs implementation and docs/browser-isolation.md. +set -eu + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +exec node "$SCRIPT_DIR/fm-browser-isolation.mjs" "$@" diff --git a/bin/fm-session-start.sh b/bin/fm-session-start.sh index f08daa9500..149164d59f 100755 --- a/bin/fm-session-start.sh +++ b/bin/fm-session-start.sh @@ -17,7 +17,7 @@ # standalone with unchanged default behavior - other flows (fm-bootstrap.sh # install after consent, /updatefirstmate, the afk daemon, existing # tests) still call them directly. The one seam this script needed - -# bootstrap running its detect-only diagnostics without its five mutating +# bootstrap running its detect-only diagnostics without its six mutating # sweeps - is an opt-in FM_BOOTSTRAP_DETECT_ONLY=1 flag on fm-bootstrap.sh # itself (default unset/0 = unchanged behavior), not a fork. # @@ -61,7 +61,7 @@ # crew-dispatch validation, tasks-axi and quota-axi tool checks, and tasks-axi # availability - none of which mutate shared state and all of which are safe # to compute from a second session. -# Only the five mutating sweeps and the wake-queue drain are skipped. +# Only the six mutating sweeps and the wake-queue drain are skipped. # The context and fleet-state digests # below are always read-only, so they run unconditionally in both modes. # diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index cec185d8bb..373616378b 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -2961,6 +2961,79 @@ crew_tool_path() { printf '%s' "$out" } +browser_automation_executable() { + local candidate selected='' + if [ -n "${FM_BROWSER_AUTOMATION_EXECUTABLE:-}" ]; then + [ -x "$FM_BROWSER_AUTOMATION_EXECUTABLE" ] || { + echo "error: FM_BROWSER_AUTOMATION_EXECUTABLE is not executable: $FM_BROWSER_AUTOMATION_EXECUTABLE" >&2 + return 1 + } + printf '%s\n' "$FM_BROWSER_AUTOMATION_EXECUTABLE" + return 0 + fi + + case "$(uname)" in + Darwin) + # Canary and Chrome for Testing have app identities distinct from the + # captain's stable Google Chrome. Prefer Canary when installed, then the + # newest Puppeteer-managed Chrome for Testing already on disk. + candidate="/Applications/Google Chrome Canary.app/Contents/MacOS/Google Chrome Canary" + if [ -x "$candidate" ]; then + printf '%s\n' "$candidate" + return 0 + fi + candidate="/Applications/Google Chrome for Testing.app/Contents/MacOS/Google Chrome for Testing" + if [ -x "$candidate" ]; then + printf '%s\n' "$candidate" + return 0 + fi + for candidate in \ + "$HOME"/.cache/puppeteer/chrome/mac_*/chrome-mac-*/Google\ Chrome\ for\ Testing.app/Contents/MacOS/Google\ Chrome\ for\ Testing \ + "$HOME"/Library/Caches/puppeteer/chrome/mac_*/chrome-mac-*/Google\ Chrome\ for\ Testing.app/Contents/MacOS/Google\ Chrome\ for\ Testing + do + [ -x "$candidate" ] || continue + [ -z "$selected" ] || [ "$candidate" -nt "$selected" ] || continue + selected=$candidate + done + ;; + *) + for candidate in \ + "$HOME"/.cache/puppeteer/chrome/linux-*/chrome-linux*/chrome \ + /usr/bin/google-chrome-unstable \ + /usr/bin/chromium \ + /usr/bin/chromium-browser + do + [ -x "$candidate" ] || continue + [ -z "$selected" ] || [ "$candidate" -nt "$selected" ] || continue + selected=$candidate + done + ;; + esac + printf '%s\n' "$selected" +} + +browser_mcp_path() { + local candidate selected='' prefix + if [ -n "${CHROME_DEVTOOLS_AXI_MCP_PATH:-}" ] && [ -f "$CHROME_DEVTOOLS_AXI_MCP_PATH" ]; then + printf '%s\n' "$CHROME_DEVTOOLS_AXI_MCP_PATH" + return 0 + fi + prefix=$(npm prefix -g 2>/dev/null || true) + if [ -n "$prefix" ]; then + candidate="$prefix/lib/node_modules/chrome-devtools-mcp/build/src/bin/chrome-devtools-mcp.js" + if [ -f "$candidate" ]; then + printf '%s\n' "$candidate" + return 0 + fi + fi + for candidate in "$HOME"/.npm/_npx/*/node_modules/chrome-devtools-mcp/build/src/bin/chrome-devtools-mcp.js; do + [ -f "$candidate" ] || continue + [ -z "$selected" ] || [ "$candidate" -nt "$selected" ] || continue + selected=$candidate + done + printf '%s\n' "$selected" +} + # depends on - worktree canonicalization, the per-task temp root, the per-harness # turn-end hook, delivery mode/yolo, and account selection. The body is unchanged # and deliberately NOT re-indented (it contains heredocs whose bodies are written @@ -2988,13 +3061,29 @@ fi # targeted knob: TMPDIR is too broad (affects every program's temp, not just Go's). TASK_TMP="/tmp/fm-$ID" mkdir -p "$TASK_TMP/gotmp" +REAL_CHROME_DEVTOOLS_AXI=$(command -v chrome-devtools-axi 2>/dev/null) || { + echo "error: chrome-devtools-axi is unavailable for task-scoped browser isolation" >&2 + exit 1 +} +[ "$REAL_CHROME_DEVTOOLS_AXI" != "$SCRIPT_DIR/chrome-devtools-axi" ] || { + echo "error: chrome-devtools-axi resolved to firstmate's crew wrapper before task setup" >&2 + exit 1 +} +BROWSER_AUTOMATION_EXECUTABLE=$(browser_automation_executable) || exit 1 +BROWSER_MCP_PATH=$(browser_mcp_path) +"$SCRIPT_DIR/fm-browser-isolation.sh" prepare \ + "$ID" "$TASK_TMP" "$FM_HOME" "$REAL_CHROME_DEVTOOLS_AXI" \ + "$BROWSER_AUTOMATION_EXECUTABLE" "$BROWSER_MCP_PATH" || exit 1 +BROWSER_ROOT="$TASK_TMP/browser" # herdr sets GOTMPDIR natively at agent start. Every other backend exports it into -# the pane shell just before the launch line, further down. CREW_PATH rides the same -# two channels for the same reason. -CREW_PATH=$(crew_tool_path) +# the pane shell just before the launch line, further down. CREW_PATH and the +# task-scoped browser identity ride the same two channels for the same reason. +CREW_PATH="$SCRIPT_DIR:$(crew_tool_path)" if [ "$BACKEND" = herdr ]; then HERDR_AGENT_ENV+=("GOTMPDIR=$TASK_TMP/gotmp") HERDR_AGENT_ENV+=("PATH=$CREW_PATH") + HERDR_AGENT_ENV+=("FM_BROWSER_TASK_ID=$ID") + HERDR_AGENT_ENV+=("FM_BROWSER_ROOT=$BROWSER_ROOT") fi # Per-harness turn-end hook: a file that touches state/.turn-ended when the @@ -3653,9 +3742,9 @@ META_WRITE_LOCK= if [ "$BACKEND" != herdr ]; then build_launch_command fi -# Export the crew PATH and GOTMPDIR into the crewmate's pane shell so the agent and -# every child process (go build, go test, ...) inherit them. Sent before the launch -# command so the env is set when the agent starts; the brief sleep lets it land. +# Export the crew PATH, GOTMPDIR, and browser identity into the crewmate's pane +# shell so the agent and every child process inherit them. Sent before the launch +# command so the environment is set when the agent starts; the brief sleep lets it land. if [ "$BACKEND" = orca ]; then if [ "$(fm_backend_orca_terminal_state "$T" "$ORCA_WORKTREE_ID" "$W")" != present ] \ || ! fm_backend_orca_worktree_terminal_contains "$ORCA_WORKTREE_ID" "$W" "$T"; then @@ -3671,7 +3760,7 @@ fi # same LAUNCH string as its argv and PATH/GOTMPDIR injected via --env, so there is # nothing to type into a pane here. if [ "$BACKEND" != herdr ]; then - spawn_send_text_line "$T" "export PATH='$CREW_PATH' GOTMPDIR=$TASK_TMP/gotmp" + spawn_send_text_line "$T" "export GOTMPDIR=$(shell_quote "$TASK_TMP/gotmp") PATH=$(shell_quote "$CREW_PATH") FM_BROWSER_TASK_ID=$(shell_quote "$ID") FM_BROWSER_ROOT=$(shell_quote "$BROWSER_ROOT")" sleep 0.3 spawn_send_literal "$T" "$LAUNCH" sleep 0.3 diff --git a/bin/fm-teardown.sh b/bin/fm-teardown.sh index ea03eda10b..80ea80cd9a 100755 --- a/bin/fm-teardown.sh +++ b/bin/fm-teardown.sh @@ -1826,9 +1826,12 @@ safe_rm_rf_child_worktree() { } safe_remove_task_tmp() { - local target=$1 base + local target=$1 task_id=${2:-$ID} base [ -n "$target" ] || return 0 - [ "$target" = "/tmp/fm-$ID" ] || return 1 + case "$task_id" in + ''|*[!A-Za-z0-9._-]*) return 1 ;; + esac + [ "$target" = "/tmp/fm-$task_id" ] || return 1 base=$(python3 - <<'PY' import os import stat @@ -1849,7 +1852,14 @@ if not stat.S_ISDIR(os.lstat(base).st_mode): print(base) PY ) || return 1 - removal_tree_operation "$base/fm-$ID" "task temp root" remove + removal_tree_operation "$base/fm-$task_id" "task temp root" remove +} + +reap_task_browser() { + local id=$1 tasktmp=$2 + [ -n "$tasktmp" ] || return 0 + [ -x "$FM_ROOT/bin/fm-browser-isolation.sh" ] || return 0 + "$FM_ROOT/bin/fm-browser-isolation.sh" reap "$id" "$tasktmp" } remove_worktree_compatibility_artifacts() { @@ -3597,7 +3607,7 @@ remove_child_orca_worktree_locked() { } cleanup_firstmate_home_children() { - local home=$1 sub_state child_metas child_meta child_id child_wt child_proj child_kind child_prelock_kind child_home child_home_after child_home_lock child_registry_lock child_backend child_orca_worktree_id child_return_rc child_account_lock child_endpoint_home remaining_child_metas child_registry_prepared child_registry_update child_registry_backup + local home=$1 sub_state child_metas child_meta child_id child_wt child_proj child_kind child_prelock_kind child_home child_home_after child_home_lock child_registry_lock child_backend child_orca_worktree_id child_return_rc child_account_lock child_endpoint_home child_tasktmp remaining_child_metas child_registry_prepared child_registry_update child_registry_backup sub_state="$home/state" child_metas=$(secondmate_state_metadata "$home") || return 1 while IFS= read -r child_meta; do @@ -3668,6 +3678,11 @@ cleanup_firstmate_home_children() { else quiesce_child_endpoint "$child_meta" "$child_id" "$home" "$child_home" || return 1 fi + child_tasktmp=$(meta_value "$child_meta" tasktmp) + reap_task_browser "$child_id" "$child_tasktmp" || { + echo "error: retained child metadata for $child_id because its browser cleanup could not be verified" >&2 + return 1 + } if [ "$child_kind" = secondmate ]; then if [ -n "$child_home" ] && [ -d "$child_home" ]; then cleanup_firstmate_home_children "$child_home" || return 1 @@ -3751,6 +3766,10 @@ EOF fi fi remove_grok_turnend_auth "$sub_state" "$child_id" + safe_remove_task_tmp "$child_tasktmp" "$child_id" || { + echo "error: retained child metadata for $child_id because its task temp root could not be removed" >&2 + return 1 + } rm -f "$sub_state/$child_id.status" "$sub_state/$child_id.turn-ended" "$sub_state/$child_id.check.sh" "$sub_state/$child_id.meta" "$sub_state/$child_id.pi-ext.ts" "$sub_state/$child_id.grok-turnend-token" [ -z "$child_account_lock" ] || fm_account_lifecycle_lock_release "$child_account_lock" >/dev/null 2>&1 || true done </dev/null 2>&1 || true @@ -4184,6 +4204,7 @@ remove_grok_turnend_auth "$STATE" "$ID" fm_backend_clear_transition "$BACKEND" "$STATE" "$T" || true # Remove the per-task temp root (/tmp/fm-/, incl. its gotmp/) recorded by spawn. # Read before the state-file rm below; empty (pre-fix tasks without tasktmp=) is a no-op. +[ -z "$TASK_TMP" ] || reap_task_browser "$ID" "$TASK_TMP" || exit 1 [ -z "$TASK_TMP" ] || safe_remove_task_tmp "$TASK_TMP" || exit 1 rm -f "$STATE/$ID.status" "$STATE/$ID.turn-ended" "$STATE/$ID.check.sh" "$STATE/$ID.meta" "$STATE/$ID.pi-ext.ts" "$STATE/$ID.grok-turnend-token" [ -z "$ACCOUNT_DELETE_LOCK" ] || fm_account_lifecycle_lock_release "$ACCOUNT_DELETE_LOCK" >/dev/null 2>&1 || true diff --git a/docs/browser-isolation.md b/docs/browser-isolation.md new file mode 100644 index 0000000000..df36084fd8 --- /dev/null +++ b/docs/browser-isolation.md @@ -0,0 +1,50 @@ +# Browser isolation and cleanup + +Firstmate-launched crew use a task-scoped automation browser. +The captain's stable Google Chrome process and profile are never an automation target. + +`bin/fm-spawn.sh` selects Chrome Canary, Chrome for Testing, or Chromium as a separate application identity and prepares `/tmp/fm-/browser`. +It then places `bin/chrome-devtools-axi` first on that crew's `PATH`. +The wrapper launches the selected browser with a private profile and attaches the real AXI bridge through a loopback DevTools endpoint. + +## Credential-store boundary + +Every task browser launch includes both `--use-mock-keychain` and `--password-store=basic`. +The same flags are passed through `CHROME_DEVTOOLS_AXI_CHROME_ARGS` as a defense for any AXI fallback launch. +Automation also disables first-run, default-browser, crash-reporting, sync, and native error-dialog paths. +An automation browser must not present any OS dialog to the captain. + +Do not launch Chrome for Testing, Chromium, or Canary directly during firstmate browser diagnostics. +Use the task wrapper so the credential flags and lifecycle owner are inseparable from the browser process. + +## Lifecycle + +One browser and one AXI session are owned by each task. +Successful AXI commands reuse that task browser so page state survives between commands. +`chrome-devtools-axi stop` stops the bridge and browser, verifies that no process still references the task profile, and leaves the task root ready for a clean relaunch. +`fm-teardown.sh` performs the same reap after the agent endpoint is quiescent and before deleting `/tmp/fm-`. + +A browser that exits unexpectedly or an AXI command that fails is cleaned immediately and writes a failure latch. +Later commands refuse to respawn it implicitly. +An explicit `chrome-devtools-axi stop` clears the latch. +This prevents a screenshot loop from recreating browsers while an operator is trying to contain an incident. + +## Crash backstop + +The locked `fm-bootstrap.sh` session-start path sweeps browser state that survived a crashed agent. +It reaps a task root only when its `owner.json` belongs to the current Firstmate home and no matching task metadata exists. + +The legacy cleanup classifier recognizes only: + +- an exact `chrome-devtools-axi-bridge.js` process; +- a Chrome process carrying both `--headless` and a `puppeteer_dev_chrome_profile-*` user-data directory directly under the system temporary root; or +- an unused `puppeteer_dev_chrome_profile-*` directory directly under the system temporary root. + +Stable Chrome, partial-marker commands, headed temp-profile Chrome, headless Chrome with a non-temporary profile, and active task-owned browser roots are preserved. +The sweep prints `BROWSER_GC:` only when it reaps something or cannot verify cleanup. + +## Verification + +Run `tests/fm-browser-isolation.test.sh` for fake-process behavior, failure-latch, teardown, backstop, and discrimination coverage. +On macOS, run `tests/fm-browser-isolation-macos-smoke.sh` manually to launch the real separate browser, capture 15 screenshots, continuously sample on-screen windows, and prove zero bridge and browser processes after stop. +Set `FM_BROWSER_ROUTING_TEST=1` only when a new stable-Chrome window is acceptable; the smoke test then proves `open -na "Google Chrome" --args --new-window ` increases stable Chrome's window count and that the URL never reaches automation. diff --git a/docs/macos-permissions.md b/docs/macos-permissions.md index cd0dcbfbb3..c38c77dc67 100644 --- a/docs/macos-permissions.md +++ b/docs/macos-permissions.md @@ -24,6 +24,9 @@ The default macOS wedge notification uses `osascript` to display a notification, `chrome-devtools-axi` takes page screenshots through the Chrome DevTools Protocol, so those screenshots do not require macOS Screen & System Audio Recording. +Firstmate crew browser launches also stay outside the macOS credential store. +The task-scoped launch and cleanup contract, including the required mock-keychain flags, is documented in [browser-isolation.md](browser-isolation.md). + Native desktop capture and Codex Computer Use are different because they read pixels outside the browser protocol. Native UI control is also different because it uses macOS Accessibility APIs and may use Apple Events for app-specific operations. diff --git a/docs/scripts.md b/docs/scripts.md index 7b5ee77340..af5ab417f1 100644 --- a/docs/scripts.md +++ b/docs/scripts.md @@ -30,6 +30,8 @@ The shared no-mistakes gate refusal used by every directly invocable mutating co | `fm-supervision-instructions.sh` | Render the session-start primary-harness supervision block or the one-line repair instruction | | `fm-home-seed.sh` | Transactionally provision a secondmate home and maintain `data/secondmates.md` | | `fm-spawn.sh` | Spawn, native-resume, or provider-neutrally continue crewmates on the resolved account and runtime backend | +| `chrome-devtools-axi` | Route crew browser commands through the task-scoped credential-safe automation browser | +| `fm-browser-isolation.sh` | Prepare, run, reap, sweep, and classify task-owned automation browser state | | `fm-dispatch-select.sh` | Resolve a matched crew-dispatch rule through quota or the deferred legacy pool-summary branch | | `fm-account-directory.sh` | Select a direct Claude/Codex account directory and install its per-profile Herdr hook | | `fm-account-routing-lib.sh` | Own routing mode plus legacy Agent Fleet lease, exec, resume, and release recovery | diff --git a/tests/fm-browser-isolation-macos-smoke.sh b/tests/fm-browser-isolation-macos-smoke.sh new file mode 100755 index 0000000000..4065647e85 --- /dev/null +++ b/tests/fm-browser-isolation-macos-smoke.sh @@ -0,0 +1,245 @@ +#!/usr/bin/env bash +# Manual macOS integration proof for credential-store isolation, screenshot +# reuse, teardown, and optional stable-Chrome routing. +# +# This test launches only the separate automation browser selected by +# FM_BROWSER_AUTOMATION_EXECUTABLE. It never attaches to or stops stable Chrome. +# Set FM_BROWSER_ROUTING_TEST=1 to exercise the explicit `open -na` routing +# assertion, which creates one real stable-Chrome window for review. +set -eu + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +BROWSER_LIFECYCLE="$ROOT/bin/fm-browser-isolation.sh" +WRAPPER="$ROOT/bin/chrome-devtools-axi" +TASK_ID="credential-smoke-$$" +TASK_TMP="/tmp/fm-$TASK_ID" +MONITOR_PID= +PROFILE= +BRIDGE_PID= + +fail() { + printf 'not ok - %s\n' "$1" >&2 + exit 1 +} + +owned_process_count() { + local marker=$1 + FM_BROWSER_OWNED_MARKER="$marker" node <<'JS' +const { execFileSync } = require("child_process"); +const marker = process.env.FM_BROWSER_OWNED_MARKER; +const commands = execFileSync("ps", ["-axo", "command="], { encoding: "utf8" }) + .split(/\r?\n/); +console.log(commands.filter((command) => command.includes(marker)).length); +JS +} + +stable_window_count() { + # shellcheck disable=SC2016 # Swift source is intentionally a shell literal. + swift -e ' + import CoreGraphics + let rows = CGWindowListCopyWindowInfo( + [.optionOnScreenOnly, .excludeDesktopElements], + kCGNullWindowID + ) as? [[String: Any]] ?? [] + print(rows.filter { + ($0[kCGWindowOwnerName as String] as? String) == "Google Chrome" + }.count) + ' +} + +cleanup() { + local cleanup_rc=0 remaining=0 + touch "$TASK_TMP/monitor.stop" 2>/dev/null || true + if [ -n "$MONITOR_PID" ]; then + wait "$MONITOR_PID" 2>/dev/null || cleanup_rc=1 + fi + if [ -f "$TASK_TMP/browser/owner.json" ]; then + "$BROWSER_LIFECYCLE" reap "$TASK_ID" "$TASK_TMP" >/dev/null 2>&1 || cleanup_rc=1 + fi + if [ -n "$PROFILE" ]; then + remaining=$(owned_process_count "$PROFILE") + [ "$remaining" -eq 0 ] || cleanup_rc=1 + fi + rm -rf "$TASK_TMP" + if [ "$cleanup_rc" -ne 0 ]; then + printf 'not ok - integration cleanup could not prove zero owned processes\n' >&2 + fi + return "$cleanup_rc" +} +trap cleanup EXIT +trap 'exit 130' HUP INT TERM + +[ "$(uname)" = Darwin ] || fail "macOS smoke test requires Darwin" +REAL_AXI=$(command -v chrome-devtools-axi 2>/dev/null) \ + || fail "chrome-devtools-axi is unavailable" +MCP_PATH=${CHROME_DEVTOOLS_AXI_MCP_PATH:-} +if [ -z "$MCP_PATH" ]; then + NPM_PREFIX=$(npm prefix -g 2>/dev/null || true) + if [ -n "$NPM_PREFIX" ]; then + MCP_PATH="$NPM_PREFIX/lib/node_modules/chrome-devtools-mcp/build/src/bin/chrome-devtools-mcp.js" + fi +fi +if [ ! -f "$MCP_PATH" ]; then + MCP_PATH=$( + find "$HOME/.npm/_npx" \ + -type f -path '*/node_modules/chrome-devtools-mcp/build/src/bin/chrome-devtools-mcp.js' \ + 2>/dev/null | + sort | + tail -n 1 + ) +fi +[ -f "$MCP_PATH" ] || fail "chrome-devtools-mcp is unavailable" + +AUTOMATION_BROWSER=${FM_BROWSER_AUTOMATION_EXECUTABLE:-} +if [ -z "$AUTOMATION_BROWSER" ]; then + AUTOMATION_BROWSER=$( + find "$HOME/.cache/puppeteer/chrome" "$HOME/Library/Caches/puppeteer/chrome" \ + -type f -path '*/Google Chrome for Testing.app/Contents/MacOS/Google Chrome for Testing' \ + -perm -111 2>/dev/null | + sort | + tail -n 1 + ) +fi +[ -x "$AUTOMATION_BROWSER" ] || fail "separate Chrome for Testing is unavailable" + +STABLE_PID=$( + ps -axo pid=,command= | + awk -v executable="/Applications/Google Chrome.app/Contents/MacOS/Google Chrome" ' + { + pid = $1 + sub(/^[[:space:]]*[0-9]+[[:space:]]+/, "", $0) + if ($0 == executable) { + print pid + exit + } + } + ' +) +[ -n "$STABLE_PID" ] || fail "captain stable Chrome process was not found" +STABLE_IDENTITY=$(ps -p "$STABLE_PID" -o pid=,lstart=,pgid=) + +mkdir -p "$TASK_TMP/gotmp" "$TASK_TMP/shots" +"$BROWSER_LIFECYCLE" prepare \ + "$TASK_ID" "$TASK_TMP" "$ROOT" "$REAL_AXI" "$AUTOMATION_BROWSER" "$MCP_PATH" + +# shellcheck disable=SC2016 # Swift source is intentionally a shell literal. +swift -e ' + import CoreGraphics + import Foundation + + let taskRoot = CommandLine.arguments[1] + let stopFile = taskRoot + "/monitor.stop" + let readyFile = taskRoot + "/monitor.ready" + let browserOwners: Set = [ + "Chromium", + "Google Chrome Canary", + "Google Chrome for Testing" + ] + try? "ready\n".write(toFile: readyFile, atomically: true, encoding: .utf8) + var samples = 0 + var maximumVisibleAutomationWindows = 0 + while samples < 10 || !FileManager.default.fileExists(atPath: stopFile) { + let rows = CGWindowListCopyWindowInfo( + [.optionOnScreenOnly, .excludeDesktopElements], + kCGNullWindowID + ) as? [[String: Any]] ?? [] + let visible = rows.filter { row in + let owner = row[kCGWindowOwnerName as String] as? String ?? "" + let title = (row[kCGWindowName as String] as? String ?? "").lowercased() + return browserOwners.contains(owner) || title.contains("keychain not found") + }.count + maximumVisibleAutomationWindows = max(maximumVisibleAutomationWindows, visible) + samples += 1 + usleep(100_000) + } + print("dialog_monitor_samples=\(samples)") + print("automation_visible_windows_seen=\(maximumVisibleAutomationWindows)") + exit(maximumVisibleAutomationWindows == 0 ? 0 : 1) +' "$TASK_TMP" >"$TASK_TMP/monitor.out" 2>"$TASK_TMP/monitor.err" & +MONITOR_PID=$! + +READY_ATTEMPTS=0 +while [ ! -f "$TASK_TMP/monitor.ready" ]; do + kill -0 "$MONITOR_PID" 2>/dev/null || fail "native-dialog monitor failed to start" + READY_ATTEMPTS=$((READY_ATTEMPTS + 1)) + [ "$READY_ATTEMPTS" -lt 100 ] || fail "native-dialog monitor did not become ready" + sleep 0.1 +done + +FM_BROWSER_TASK_ID="$TASK_ID" FM_BROWSER_ROOT="$TASK_TMP/browser" \ + "$WRAPPER" open https://example.com/ + +STATE_FILE="$TASK_TMP/browser/browser.json" +PROFILE=$(node -e 'console.log(JSON.parse(require("fs").readFileSync(process.argv[1])).profile)' "$STATE_FILE") +BROWSER_PID=$(node -e 'console.log(JSON.parse(require("fs").readFileSync(process.argv[1])).pid)' "$STATE_FILE") +BROWSER_PORT=$(node -e 'console.log(JSON.parse(require("fs").readFileSync(process.argv[1])).port)' "$STATE_FILE") +BROWSER_COMMAND=$(ps -p "$BROWSER_PID" -o command=) +case "$BROWSER_COMMAND" in + *--use-mock-keychain*) ;; + *) fail "live automation browser lacks --use-mock-keychain" ;; +esac +case "$BROWSER_COMMAND" in + *--password-store=basic*) ;; + *) fail "live automation browser lacks --password-store=basic" ;; +esac + +INDEX=1 +while [ "$INDEX" -le 15 ]; do + FM_BROWSER_TASK_ID="$TASK_ID" FM_BROWSER_ROOT="$TASK_TMP/browser" \ + "$WRAPPER" screenshot "$TASK_TMP/shots/shot-$INDEX.png" >/dev/null + INDEX=$((INDEX + 1)) +done +[ "$(find "$TASK_TMP/shots" -type f -name 'shot-*.png' | wc -l | tr -d ' ')" -eq 15 ] \ + || fail "browser-heavy flow did not produce 15 screenshots" + +if [ "${FM_BROWSER_ROUTING_TEST:-0}" = 1 ]; then + ROUTING_TOKEN="fm-routing-$TASK_ID" + STABLE_WINDOWS_BEFORE=$(stable_window_count) + open -na "Google Chrome" --args --new-window "https://example.com/?$ROUTING_TOKEN" + sleep 3 + STABLE_WINDOWS_AFTER=$(stable_window_count) + [ "$STABLE_WINDOWS_AFTER" -gt "$STABLE_WINDOWS_BEFORE" ] \ + || fail "stable Chrome did not gain the requested new window" + if curl -fsS "http://127.0.0.1:$BROWSER_PORT/json/list" | grep -F "$ROUTING_TOKEN" >/dev/null; then + fail "captain routing test URL landed in the automation browser" + fi + printf 'stable_windows_before=%s\n' "$STABLE_WINDOWS_BEFORE" + printf 'stable_windows_after=%s\n' "$STABLE_WINDOWS_AFTER" + printf 'routing_url_in_automation=0\n' +fi + +OWNED_DURING_RUN=$(owned_process_count "$PROFILE") +BRIDGE_PID=$(node -e 'console.log(JSON.parse(require("fs").readFileSync(process.argv[1])).pid)' \ + "$TASK_TMP/browser/home/.chrome-devtools-axi/sessions/fm-$TASK_ID/bridge.pid") +kill -0 "$BRIDGE_PID" 2>/dev/null || fail "AXI bridge was not live during the run" + +touch "$TASK_TMP/monitor.stop" +wait "$MONITOR_PID" || fail "automation browser displayed a native dialog" +MONITOR_PID= +cat "$TASK_TMP/monitor.out" +grep -Fx 'automation_visible_windows_seen=0' "$TASK_TMP/monitor.out" >/dev/null \ + || fail "native-dialog monitor observed an automation window" +if grep -Ei 'keychain|password store' "$TASK_TMP/browser/browser.log" >/dev/null; then + grep -Ei 'keychain|password store' "$TASK_TMP/browser/browser.log" >&2 + fail "automation browser logged a credential-store error" +fi + +FM_BROWSER_TASK_ID="$TASK_ID" FM_BROWSER_ROOT="$TASK_TMP/browser" \ + "$WRAPPER" stop >/dev/null +OWNED_AFTER_STOP=$(owned_process_count "$PROFILE") +kill -0 "$BRIDGE_PID" 2>/dev/null && fail "AXI bridge survived wrapper stop" +[ "$OWNED_AFTER_STOP" -eq 0 ] || fail "automation browser survived wrapper stop" +"$BROWSER_LIFECYCLE" reap "$TASK_ID" "$TASK_TMP" +[ ! -e "$TASK_TMP/browser" ] || fail "task profile root survived reap" + +STABLE_IDENTITY_AFTER=$(ps -p "$STABLE_PID" -o pid=,lstart=,pgid=) +[ "$STABLE_IDENTITY_AFTER" = "$STABLE_IDENTITY" ] \ + || fail "captain stable Chrome identity changed during the smoke test" + +printf 'screenshots=15\n' +printf 'owned_processes_during_run=%s\n' "$OWNED_DURING_RUN" +printf 'owned_processes_after_stop=%s\n' "$OWNED_AFTER_STOP" +printf 'bridge_alive_after_stop=0\n' +printf 'profile_exists_after_reap=0\n' +printf 'stable_chrome_identity_preserved=1\n' +printf 'ok - macOS credential isolation and teardown smoke test passed\n' diff --git a/tests/fm-browser-isolation.test.sh b/tests/fm-browser-isolation.test.sh new file mode 100755 index 0000000000..6df5c4d749 --- /dev/null +++ b/tests/fm-browser-isolation.test.sh @@ -0,0 +1,391 @@ +#!/usr/bin/env bash +# Behavior tests for task-scoped browser launch, teardown reaping, and legacy +# process discrimination. All process mutations are confined to fake programs +# and profiles under this suite's private temp root. +set -eu + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +BROWSER="$ROOT/bin/fm-browser-isolation.sh" +WRAPPER="$ROOT/bin/chrome-devtools-axi" +SPAWN="$ROOT/bin/fm-spawn.sh" +TEARDOWN="$ROOT/bin/fm-teardown.sh" +BOOTSTRAP="$ROOT/bin/fm-bootstrap.sh" +TMP_ROOT= +TEARDOWN_TASK_TMP= +TEARDOWN_BROWSER_PID= +TEARDOWN_BRIDGE_PID= + +fail() { + printf 'not ok - %s\n' "$1" >&2 + exit 1 +} + +pass() { + printf 'ok - %s\n' "$1" +} + +cleanup() { + for cleanup_pid in \ + "${UNRELATED_PID:-}" \ + "${LEGACY_BRIDGE_PID:-}" \ + "${LEGACY_BROWSER_PID:-}" \ + "${HEADED_BROWSER_PID:-}" \ + "${TEARDOWN_BROWSER_PID:-}" \ + "${TEARDOWN_BRIDGE_PID:-}" + do + [ -n "$cleanup_pid" ] || continue + if kill -0 "$cleanup_pid" 2>/dev/null; then + kill "$cleanup_pid" 2>/dev/null || true + fi + wait "$cleanup_pid" 2>/dev/null || true + done + if [ -n "${TMP_ROOT:-}" ]; then + rm -rf "$TMP_ROOT" + fi + if [ -n "${TEARDOWN_TASK_TMP:-}" ]; then + rm -rf "$TEARDOWN_TASK_TMP" + fi +} +trap cleanup EXIT + +TMP_ROOT=$(mktemp -d "${TMPDIR:-/tmp}/fm-browser-isolation-tests.XXXXXX") +TMP_ROOT=$(cd "$TMP_ROOT" && pwd -P) +mkdir -p "$TMP_ROOT/tmp" "$TMP_ROOT/home" "$TMP_ROOT/state" "$TMP_ROOT/fakes" + +FAKE_BROWSER="$TMP_ROOT/fakes/Google Chrome for Testing" +FAKE_AXI="$TMP_ROOT/fakes/real-chrome-devtools-axi" +FAKE_BRIDGE="$TMP_ROOT/fakes/chrome-devtools-axi-bridge.js" +FAKE_HEADED="$TMP_ROOT/fakes/captain-browser-helper.js" +AXI_RECORD="$TMP_ROOT/axi-record" +export FM_FAKE_AXI_RECORD="$AXI_RECORD" +export FM_FAKE_BRIDGE_SCRIPT="$FAKE_BRIDGE" + +cat > "$FAKE_BROWSER" <<'JS' +#!/usr/bin/env node +const fs = require("fs"); +const http = require("http"); +const path = require("path"); + +const profileArg = process.argv.find((arg) => arg.startsWith("--user-data-dir=")); +const requiredArgs = ["--use-mock-keychain", "--password-store=basic"]; +if ( + !profileArg || + !process.argv.some((arg) => arg.startsWith("--headless")) || + requiredArgs.some((arg) => !process.argv.includes(arg)) +) process.exit(2); +const profile = profileArg.slice("--user-data-dir=".length); +fs.mkdirSync(profile, { recursive: true }); +const server = http.createServer((request, response) => { + response.setHeader("content-type", "application/json"); + response.end(JSON.stringify({ + Browser: "Fake Chrome for Testing", + webSocketDebuggerUrl: `ws://127.0.0.1:${server.address().port}/devtools/browser/fake`, + })); +}); +server.listen(0, "127.0.0.1", () => { + fs.writeFileSync(path.join(profile, "DevToolsActivePort"), `${server.address().port}\n/devtools/browser/fake\n`); +}); +process.on("SIGTERM", () => server.close(() => process.exit(0))); +setInterval(() => {}, 1000); +JS + +cat > "$FAKE_BRIDGE" <<'JS' +#!/usr/bin/env node +setInterval(() => {}, 1000); +JS + +cp "$FAKE_BRIDGE" "$FAKE_HEADED" + +cat > "$FAKE_AXI" <<'JS' +#!/usr/bin/env node +const fs = require("fs"); +const path = require("path"); +const { spawn } = require("child_process"); + +const command = process.argv[2] || ""; +if (command === "stop") process.exit(0); +fs.appendFileSync( + process.env.FM_FAKE_AXI_RECORD, + [ + `home=${process.env.HOME}`, + `session=${process.env.CHROME_DEVTOOLS_AXI_SESSION}`, + `port=${process.env.CHROME_DEVTOOLS_AXI_PORT}`, + `browser_url=${process.env.CHROME_DEVTOOLS_AXI_BROWSER_URL}`, + `auto_connect=${process.env.CHROME_DEVTOOLS_AXI_AUTO_CONNECT || ""}`, + `user_data_dir=${process.env.CHROME_DEVTOOLS_AXI_USER_DATA_DIR || ""}`, + `channel=${process.env.CHROME_DEVTOOLS_AXI_CHANNEL || ""}`, + `chrome_args=${process.env.CHROME_DEVTOOLS_AXI_CHROME_ARGS || ""}`, + ].join("\n") + "\n", +); +if (command === "open") { + const bridge = spawn(process.execPath, [process.env.FM_FAKE_BRIDGE_SCRIPT], { + detached: true, + stdio: "ignore", + }); + bridge.unref(); + const pidFile = path.join( + process.env.HOME, + ".chrome-devtools-axi", + "sessions", + process.env.CHROME_DEVTOOLS_AXI_SESSION, + "bridge.pid", + ); + fs.mkdirSync(path.dirname(pidFile), { recursive: true }); + fs.writeFileSync(pidFile, `${JSON.stringify({ pid: bridge.pid, port: Number(process.env.CHROME_DEVTOOLS_AXI_PORT) })}\n`); +} +JS + +chmod +x "$FAKE_BROWSER" "$FAKE_AXI" "$FAKE_BRIDGE" "$FAKE_HEADED" + +test_process_discrimination() { + local fixture="$TMP_ROOT/processes.txt" out + cat > "$fixture" <<'ROWS' +101 1 101 /Applications/Google Chrome.app/Contents/MacOS/Google Chrome +102 101 101 /Applications/Google Chrome.app/Contents/Frameworks/Google Chrome Helper --type=renderer +103 1 103 /Applications/Google Chrome.app/Contents/MacOS/Google Chrome --headless=new --user-data-dir=/Users/captain/Library/Application Support/Google/Chrome +104 1 104 /Applications/Google Chrome.app/Contents/MacOS/Google Chrome --user-data-dir=/tmp/puppeteer_dev_chrome_profile-headed +105 1 105 codex task text says chrome-devtools-axi-bridge process cleanup +106 1 106 node /opt/axi/chrome-devtools-axi-bridge.js +107 106 106 /Applications/Google Chrome.app/Contents/MacOS/Google Chrome --headless=new --user-data-dir=/tmp/puppeteer_dev_chrome_profile-leaked +108 1 108 /cache/Google Chrome for Testing --headless=new --user-data-dir=/tmp/fm-task-z1/browser/profile +109 1 109 python worker.py --headless=new --user-data-dir=/tmp/puppeteer_dev_chrome_profile-not-a-browser +ROWS + out=$("$BROWSER" classify "$fixture") + [ "$(printf '%s\n' "$out" | awk -F '\t' '$2 == "unrelated" { count += 1 } END { print count + 0 }')" -eq 6 ] \ + || fail "captain-like and partial-marker processes were not all preserved" + [ "$(printf '%s\n' "$out" | awk -F '\t' '$2 == "legacy-bridge" { print $1 }')" = 106 ] \ + || fail "exact AXI bridge was not classified" + [ "$(printf '%s\n' "$out" | awk -F '\t' '$2 == "legacy-browser" { print $1 }')" = 107 ] \ + || fail "headless temp-profile Chrome was not classified" + [ "$(printf '%s\n' "$out" | awk -F '\t' '$2 == "protected-task-browser" { print $1 }')" = 108 ] \ + || fail "new task-owned browser was not protected from the legacy sweep" + pass "classifier requires exact automation markers and preserves captain-like Chrome" +} + +test_task_launch_and_reap() { + local id=browser-owned-z2 tasktmp="$TMP_ROOT/tmp/fm-browser-owned-z2" + local browser_pid bridge_pid canonical_tasktmp + mkdir -p "$tasktmp/gotmp" + FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" \ + "$BROWSER" prepare "$id" "$tasktmp" "$TMP_ROOT/home" "$FAKE_AXI" "$FAKE_BROWSER" "" + FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" \ + FM_BROWSER_TASK_ID="$id" \ + FM_BROWSER_ROOT="$tasktmp/browser" \ + "$WRAPPER" open https://example.test/ + + canonical_tasktmp=$(cd "$tasktmp" && pwd -P) + grep -F "home=$canonical_tasktmp/browser/home" "$AXI_RECORD" >/dev/null \ + || fail "AXI HOME was not isolated under the task browser root" + grep -F "session=fm-$id" "$AXI_RECORD" >/dev/null \ + || fail "AXI session was not keyed to the task" + grep -E '^browser_url=http://127\.0\.0\.1:[0-9]+$' "$AXI_RECORD" >/dev/null \ + || fail "AXI did not attach to the separate automation browser" + grep -Fx 'auto_connect=' "$AXI_RECORD" >/dev/null \ + || fail "AUTO_CONNECT leaked into task AXI" + grep -Fx 'user_data_dir=' "$AXI_RECORD" >/dev/null \ + || fail "AXI direct profile launch leaked around the wrapper" + grep -Fx 'channel=' "$AXI_RECORD" >/dev/null \ + || fail "stable Chrome channel leaked into task AXI" + grep -Fx 'chrome_args=--use-mock-keychain --password-store=basic' "$AXI_RECORD" >/dev/null \ + || fail "AXI fallback launch did not inherit credential-store isolation" + + browser_pid=$(node -e 'console.log(JSON.parse(require("fs").readFileSync(process.argv[1])).pid)' "$tasktmp/browser/browser.json") + bridge_pid=$(node -e 'console.log(JSON.parse(require("fs").readFileSync(process.argv[1])).pid)' "$tasktmp/browser/home/.chrome-devtools-axi/sessions/fm-$id/bridge.pid") + TEARDOWN_BROWSER_PID=$browser_pid + TEARDOWN_BRIDGE_PID=$bridge_pid + kill -0 "$browser_pid" 2>/dev/null || fail "fake automation browser did not remain live" + kill -0 "$bridge_pid" 2>/dev/null || fail "fake AXI bridge did not remain live" + sleep 300 & + UNRELATED_PID=$! + + FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" "$BROWSER" reap "$id" "$tasktmp" + kill -0 "$browser_pid" 2>/dev/null && fail "task browser survived reap" + kill -0 "$bridge_pid" 2>/dev/null && fail "task bridge survived reap" + kill -0 "$UNRELATED_PID" 2>/dev/null || fail "unrelated process was killed by task reap" + [ ! -e "$tasktmp/browser" ] || fail "task browser profile root survived reap" + TEARDOWN_BROWSER_PID= + TEARDOWN_BRIDGE_PID= + pass "task reap removes the exact bridge, browser, and profile without collateral" +} + +test_failed_browser_cannot_respawn() { + local id=browser-failure-z4 tasktmp="$TMP_ROOT/tmp/fm-browser-failure-z4" + local browser_pid second_output + mkdir -p "$tasktmp/gotmp" + FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" \ + "$BROWSER" prepare "$id" "$tasktmp" "$TMP_ROOT/home" "$FAKE_AXI" "$FAKE_BROWSER" "" + FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" \ + FM_BROWSER_TASK_ID="$id" \ + FM_BROWSER_ROOT="$tasktmp/browser" \ + "$WRAPPER" open https://example.test/ + browser_pid=$(node -e 'console.log(JSON.parse(require("fs").readFileSync(process.argv[1])).pid)' "$tasktmp/browser/browser.json") + kill "$browser_pid" + wait "$browser_pid" 2>/dev/null || true + if second_output=$( + FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" \ + FM_BROWSER_TASK_ID="$id" \ + FM_BROWSER_ROOT="$tasktmp/browser" \ + "$WRAPPER" screenshot 2>&1 + ); then + fail "wrapper respawned a browser after its owned browser died" + fi + printf '%s\n' "$second_output" | grep -F 'refusing to respawn it implicitly' >/dev/null \ + || fail "unexpected browser death did not trip the respawn latch" + [ -f "$tasktmp/browser/browser.failed" ] \ + || fail "browser failure latch was not recorded" + [ ! -f "$tasktmp/browser/browser.json" ] \ + || fail "stale browser state survived failed-browser cleanup" + FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" "$BROWSER" reap "$id" "$tasktmp" + pass "unexpected browser death is cleaned and cannot trigger an implicit respawn" +} + +test_real_teardown_path_reaps_owned_browser() { + local id="browser-td-z5-$$" tasktmp="/tmp/fm-browser-td-z5-$$" + local home="$TMP_ROOT/teardown-home" pool="$TMP_ROOT/teardown-pool" + local project="$TMP_ROOT/teardown-project" origin="$TMP_ROOT/teardown-origin.git" + local worktree="$pool/1/worktree" seed="$TMP_ROOT/teardown-seed" + local fakebin="$TMP_ROOT/teardown-fakebin" + local browser_pid bridge_pid output + TEARDOWN_TASK_TMP=$tasktmp + mkdir -p "$tasktmp/gotmp" "$home/state" "$home/data" "$home/config" "$pool/1" "$fakebin" + git init -q --bare "$origin" + git -C "$origin" symbolic-ref HEAD refs/heads/main + git clone -q "$origin" "$seed" 2>/dev/null + git -C "$seed" -c user.name=BrowserIsolationTest \ + -c user.email=browser-isolation@example.invalid \ + commit -q --allow-empty -m init + git -C "$seed" push -q origin main + git clone -q "$origin" "$project" + git -C "$project" worktree add -q -b "fm/$id" "$worktree" main + node - "$pool/treehouse-state.json" "$worktree" "firstmate-$id" <<'JS' +const fs = require("fs"); +const [state, worktree, holder] = process.argv.slice(2); +fs.writeFileSync(state, JSON.stringify({ + worktrees: [{ name: "1", path: worktree, leased: true, lease_holder: holder }], +})); +JS + cat > "$fakebin/treehouse" <<'SH' +#!/usr/bin/env bash +exit 0 +SH + cat > "$fakebin/tmux" <<'SH' +#!/usr/bin/env bash +state="$(dirname "$0")/.tmux-live" +case "${1:-}" in + display-message) + [ -f "$state" ] || exit 1 + case " $* " in + *' #{pane_current_command} '*) printf '%s\n' bash ;; + esac + exit 0 + ;; + list-windows) + [ ! -f "$state" ] || printf 'fm-%s\n' "$FM_FAKE_TASK_ID" + exit 0 + ;; + kill-window) rm -f "$state"; exit 0 ;; +esac +exit 0 +SH + chmod +x "$fakebin/treehouse" "$fakebin/tmux" + : > "$fakebin/.tmux-live" + cat > "$home/state/$id.meta" <&1 + ) || fail "fm-teardown rejected an otherwise-finished browser-owning task: $output" + kill -0 "$browser_pid" 2>/dev/null && fail "browser survived the real fm-teardown path" + kill -0 "$bridge_pid" 2>/dev/null && fail "bridge survived the real fm-teardown path" + [ ! -e "$tasktmp" ] || fail "task temp root survived the real fm-teardown path" + [ ! -e "$home/state/$id.meta" ] || fail "task metadata survived successful teardown" + TEARDOWN_TASK_TMP= + TEARDOWN_BROWSER_PID= + TEARDOWN_BRIDGE_PID= + pass "real fm-teardown path reaps the bridge, browser process tree, and task profile" +} + +test_orphan_sweep() { + local id=browser-orphan-z3 tasktmp="$TMP_ROOT/tmp/fm-browser-orphan-z3" + local legacy_profile="$TMP_ROOT/tmp/puppeteer_dev_chrome_profile-stale" + local headed_profile="$TMP_ROOT/tmp/puppeteer_dev_chrome_profile-headed" out + mkdir -p "$tasktmp/gotmp" "$legacy_profile" "$headed_profile" + FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" \ + "$BROWSER" prepare "$id" "$tasktmp" "$TMP_ROOT/home" "$FAKE_AXI" "$FAKE_BROWSER" "" + + node "$FAKE_BRIDGE" & + LEGACY_BRIDGE_PID=$! + "$FAKE_BROWSER" \ + --headless=new \ + --use-mock-keychain \ + --password-store=basic \ + "--user-data-dir=$legacy_profile" & + LEGACY_BROWSER_PID=$! + node "$FAKE_HEADED" "--user-data-dir=$headed_profile" & + HEADED_BROWSER_PID=$! + sleep 0.2 + kill -0 "$LEGACY_BRIDGE_PID" 2>/dev/null || fail "fake legacy bridge did not start" + kill -0 "$LEGACY_BROWSER_PID" 2>/dev/null || fail "fake legacy browser did not start" + kill -0 "$HEADED_BROWSER_PID" 2>/dev/null || fail "fake headed browser did not start" + sleep 300 & + UNRELATED_PID=$! + + out=$(FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" "$BROWSER" sweep "$TMP_ROOT/home" "$TMP_ROOT/state") + kill -0 "$LEGACY_BRIDGE_PID" 2>/dev/null && fail "legacy AXI bridge survived orphan sweep" + kill -0 "$LEGACY_BROWSER_PID" 2>/dev/null && fail "legacy headless temp-profile browser survived orphan sweep" + wait "$LEGACY_BRIDGE_PID" 2>/dev/null || true + wait "$LEGACY_BROWSER_PID" 2>/dev/null || true + kill -0 "$UNRELATED_PID" 2>/dev/null || fail "unrelated process was killed by orphan sweep" + kill -0 "$HEADED_BROWSER_PID" 2>/dev/null || fail "headed temp-profile browser was killed by orphan sweep" + [ ! -e "$tasktmp/browser" ] || fail "metadata-free task browser root survived sweep" + [ ! -e "$legacy_profile" ] || fail "unused legacy temp profile survived sweep" + [ -d "$headed_profile" ] || fail "active headed temp profile was removed by orphan sweep" + printf '%s\n' "$out" | grep -E '^BROWSER_GC: reaped task_roots=1 bridges=1 browser_processes=1 profiles=1$' >/dev/null \ + || fail "sweep did not report measured cleanup: $out" + pass "backstop kills only exact orphan markers and preserves unrelated processes" +} + +test_spawn_and_teardown_wiring() { + grep -F "\"\$SCRIPT_DIR/fm-browser-isolation.sh\" prepare" "$SPAWN" >/dev/null \ + || fail "spawn does not prepare browser isolation" + grep -F "CREW_PATH=\"\$SCRIPT_DIR:\$(crew_tool_path)\"" "$SPAWN" >/dev/null \ + || fail "spawn does not put the task AXI wrapper first on PATH" + grep -F "HERDR_AGENT_ENV+=(\"FM_BROWSER_TASK_ID=\$ID\")" "$SPAWN" >/dev/null \ + || fail "Herdr does not receive the task browser identity natively" + grep -F "FM_BROWSER_ROOT=\$(shell_quote \"\$BROWSER_ROOT\")" "$SPAWN" >/dev/null \ + || fail "terminal backends do not export the task browser root" + grep -F "reap_task_browser \"\$ID\" \"\$TASK_TMP\"" "$TEARDOWN" >/dev/null \ + || fail "teardown does not reap the task browser before temp-root deletion" + grep -F 'browser_gc_sweep' "$BOOTSTRAP" >/dev/null \ + || fail "bootstrap does not expose the crash-orphan backstop" + grep -F "\"\$SCRIPT_DIR/fm-browser-isolation.sh\" sweep \"\$FM_HOME\" \"\$STATE\"" "$BOOTSTRAP" >/dev/null \ + || fail "bootstrap backstop does not invoke browser orphan cleanup" + pass "spawn and teardown wire the browser lifecycle on every backend channel" +} + +test_process_discrimination +test_task_launch_and_reap +test_failed_browser_cannot_respawn +test_real_teardown_path_reaps_owned_browser +test_orphan_sweep +test_spawn_and_teardown_wiring From 0a7b93e5670e8e18413d13d17821dd91ec81d9fe Mon Sep 17 00:00:00 2001 From: Dongkeun Lee Date: Tue, 28 Jul 2026 11:56:15 -0400 Subject: [PATCH 02/18] no-mistakes(review): Fix home-scoped browser cleanup and process-tree reaping --- bin/fm-browser-isolation.mjs | 168 +++++++++++++++++----- bin/fm-spawn.sh | 32 ++++- bin/fm-teardown.sh | 2 +- docs/browser-isolation-verification.md | 35 +++++ docs/browser-isolation.md | 7 +- tests/fm-browser-isolation-macos-smoke.sh | 66 +++++++-- tests/fm-browser-isolation.test.sh | 99 +++++++++---- 7 files changed, 329 insertions(+), 80 deletions(-) create mode 100644 docs/browser-isolation-verification.md diff --git a/bin/fm-browser-isolation.mjs b/bin/fm-browser-isolation.mjs index 7cef7ee8e8..5077af9112 100755 --- a/bin/fm-browser-isolation.mjs +++ b/bin/fm-browser-isolation.mjs @@ -22,6 +22,7 @@ */ import { spawn, spawnSync } from "node:child_process"; +import { createHash } from "node:crypto"; import { closeSync, existsSync, @@ -98,6 +99,23 @@ function expectedTaskTmp(taskId) { return path.join(canonicalTmpRoot(), `fm-${validateTaskId(taskId)}`); } +function canonicalOwnerHome(ownerHome) { + const resolved = realpathSync(ownerHome); + const metadata = lstatSync(resolved); + if (!metadata.isDirectory() || metadata.isSymbolicLink()) { + fail(`unsafe browser owner home: ${ownerHome}`); + } + return resolved; +} + +function expectedBrowserRoot(taskId, ownerHome) { + const homeKey = createHash("sha256") + .update(canonicalOwnerHome(ownerHome)) + .digest("hex") + .slice(0, 16); + return path.join(canonicalTmpRoot(), `fm-browser-${homeKey}-${validateTaskId(taskId)}`); +} + function validateTaskTmp(taskId, taskTmp, requireExisting = true) { const expected = expectedTaskTmp(taskId); const resolved = requireExisting @@ -115,8 +133,8 @@ function validateTaskTmp(taskId, taskTmp, requireExisting = true) { return resolved; } -function validateBrowserRoot(taskId, browserRoot) { - const expected = path.join(expectedTaskTmp(taskId), "browser"); +function validateBrowserRoot(taskId, browserRoot, ownerHome) { + const expected = expectedBrowserRoot(taskId, ownerHome); const resolved = realpathSync(browserRoot); if (resolved !== expected) { fail(`unsafe browser root for ${taskId}: ${browserRoot}`); @@ -144,12 +162,22 @@ function validateExecutable(file, label) { return resolved; } -function validateOwner(owner, root, taskId) { - if (!owner || owner.version !== 1 || owner.taskId !== taskId) { +function validateOwner(owner, root, taskId, ownerHome, taskTmp = "") { + if (!owner || owner.version !== 2 || owner.taskId !== taskId) { fail(`browser owner record does not match ${taskId}`); } if (owner.root !== root) fail(`browser owner root changed for ${taskId}`); - owner.ownerHome = realpathSync(owner.ownerHome); + owner.ownerHome = canonicalOwnerHome(owner.ownerHome); + if (owner.ownerHome !== canonicalOwnerHome(ownerHome)) { + fail(`browser owner home changed for ${taskId}`); + } + owner.taskTmp = validateTaskTmp(taskId, owner.taskTmp, existsSync(owner.taskTmp)); + if ( + taskTmp && + owner.taskTmp !== validateTaskTmp(taskId, taskTmp, existsSync(taskTmp)) + ) { + fail(`browser task temp changed for ${taskId}`); + } owner.realAxi = validateExecutable(owner.realAxi, "chrome-devtools-axi"); owner.browserExecutable = owner.browserExecutable ? validateExecutable(owner.browserExecutable, "automation browser executable") @@ -158,8 +186,8 @@ function validateOwner(owner, root, taskId) { return owner; } -function loadOwner(root, taskId) { - return validateOwner(readJson(path.join(root, OWNER_FILE)), root, taskId); +function loadOwner(root, taskId, ownerHome, taskTmp = "") { + return validateOwner(readJson(path.join(root, OWNER_FILE)), root, taskId, ownerHome, taskTmp); } function parseInventory(text) { @@ -219,7 +247,7 @@ function isLegacyBrowser(row) { function taskProfileFromCommand(command) { const match = command.match( - /--user-data-dir(?:=|\s+)(["']?)((?:\/private)?\/tmp\/fm-[A-Za-z0-9._-]+\/browser\/profile)\1(?:\s|$)/, + /--user-data-dir(?:=|\s+)(["']?)((?:\/private)?\/tmp\/fm-browser-[a-f0-9]{16}-[A-Za-z0-9._-]+\/profile)\1(?:\s|$)/, ); return match ? match[2] : null; } @@ -275,6 +303,16 @@ async function terminateExactPid(pid, matches, allowGroup = false) { return waitForExit(pid, 1_000); } +function sameProcessIdentity(original, current) { + return ( + current !== null && + original.pid === current.pid && + original.ppid === current.ppid && + original.pgid === current.pgid && + original.command === current.command + ); +} + function descendantsOf(pid, rows) { const descendants = new Set([pid]); let changed = true; @@ -291,6 +329,55 @@ function descendantsOf(pid, rows) { return [...descendants]; } +async function terminateOwnedBrowserTree(state) { + const rows = processInventory(); + const root = rowForPid(state.pid, rows); + if (!processMatchesBrowser(state, root)) return false; + const ownedPids = new Set([root.pid, ...descendantsOf(root.pid, rows)]); + if (root.pgid === root.pid) { + for (const row of rows) { + if (row.pgid === root.pgid) ownedPids.add(row.pid); + } + } + const owned = rows.filter((row) => ownedPids.has(row.pid)); + if (root.pgid === root.pid) { + try { + process.kill(-root.pgid, "SIGTERM"); + } catch { + // The dedicated process group already exited. + } + } else { + for (const row of owned.slice().reverse()) { + try { + process.kill(row.pid, "SIGTERM"); + } catch { + // The exact process already exited. + } + } + } + await sleep(STOP_GRACE_MS); + for (const original of owned) { + if (!sameProcessIdentity(original, rowForPid(original.pid))) continue; + try { + process.kill(original.pid, "SIGKILL"); + } catch { + // The exact process exited after verification. + } + } + await sleep(100); + const survivors = owned.filter((original) => + sameProcessIdentity(original, rowForPid(original.pid)), + ); + if (survivors.length > 0) { + fail( + `task browser process tree survived cleanup: ${survivors + .map((row) => row.pid) + .join(",")}`, + ); + } + return true; +} + async function terminateLegacyBridgeTree(pid) { const rows = processInventory(); const root = rowForPid(pid, rows); @@ -475,11 +562,7 @@ function readBrowserState(root) { async function stopTaskBrowser(root) { const state = readBrowserState(root); if (!state) return false; - const stopped = await terminateExactPid( - state.pid, - (row) => processMatchesBrowser(state, row), - true, - ); + const stopped = await terminateOwnedBrowserTree(state); const stillOwned = processInventory().filter( (row) => row.command.includes(`--user-data-dir=${state.profile}`), ); @@ -745,17 +828,18 @@ async function commandPrepare(args) { args; validateTaskId(taskId); const taskTmp = validateTaskTmp(taskId, taskTmpInput); - const root = path.join(taskTmp, "browser"); + const ownerHome = canonicalOwnerHome(ownerHomeInput); + const root = expectedBrowserRoot(taskId, ownerHome); mkdirSync(root, { recursive: true, mode: 0o700 }); - const canonicalRoot = validateBrowserRoot(taskId, root); + const canonicalRoot = validateBrowserRoot(taskId, root, ownerHome); mkdirSync(path.join(canonicalRoot, "home"), { recursive: true, mode: 0o700 }); mkdirSync(path.join(canonicalRoot, "profile"), { recursive: true, mode: 0o700 }); - const ownerHome = realpathSync(ownerHomeInput); const owner = { - version: 1, + version: 2, taskId, root: canonicalRoot, ownerHome, + taskTmp, realAxi: validateExecutable(realAxiInput, "chrome-devtools-axi"), browserExecutable: browserInput ? validateExecutable(browserInput, "automation browser executable") @@ -767,8 +851,9 @@ async function commandPrepare(args) { async function commandRun(args) { const taskId = validateTaskId(process.env.FM_BROWSER_TASK_ID || ""); - const root = validateBrowserRoot(taskId, process.env.FM_BROWSER_ROOT || ""); - const owner = loadOwner(root, taskId); + const ownerHome = canonicalOwnerHome(process.env.FM_BROWSER_OWNER_HOME || ""); + const root = validateBrowserRoot(taskId, process.env.FM_BROWSER_ROOT || "", ownerHome); + const owner = loadOwner(root, taskId, ownerHome); const lock = await acquireLock(root); try { const command = args[0] || ""; @@ -809,14 +894,12 @@ async function commandRun(args) { } } -async function reapRoot(taskId, taskTmp, removeRoot = true) { +async function reapRoot(taskId, taskTmp, ownerHome, removeRoot = true) { const expectedTmp = validateTaskTmp(taskId, taskTmp, false); - if (!existsSync(expectedTmp)) return { bridge: 0, browser: 0 }; - const validatedTmp = validateTaskTmp(taskId, expectedTmp); - const root = path.join(validatedTmp, "browser"); + const root = expectedBrowserRoot(taskId, ownerHome); if (!existsSync(root)) return { bridge: 0, browser: 0 }; - const canonicalRoot = validateBrowserRoot(taskId, root); - const owner = loadOwner(canonicalRoot, taskId); + const canonicalRoot = validateBrowserRoot(taskId, root, ownerHome); + const owner = loadOwner(canonicalRoot, taskId, ownerHome, expectedTmp); const lock = await acquireLock(canonicalRoot); let bridge = 0; let browser = 0; @@ -836,15 +919,15 @@ async function reapRoot(taskId, taskTmp, removeRoot = true) { } async function commandReap(args) { - if (args.length !== 2) fail("usage: reap "); - await reapRoot(validateTaskId(args[0]), args[1]); + if (args.length !== 3) fail("usage: reap "); + await reapRoot(validateTaskId(args[0]), args[1], args[2]); } function taskRoots(tmpRoot) { const roots = []; for (const entry of readdirSync(tmpRoot, { withFileTypes: true })) { - if (!entry.isDirectory() || !entry.name.startsWith("fm-")) continue; - const browserRoot = path.join(tmpRoot, entry.name, "browser"); + if (!entry.isDirectory() || !entry.name.startsWith("fm-browser-")) continue; + const browserRoot = path.join(tmpRoot, entry.name); if (!existsSync(path.join(browserRoot, OWNER_FILE))) continue; roots.push(browserRoot); } @@ -854,7 +937,13 @@ function taskRoots(tmpRoot) { function protectedTaskPids(tmpRoot) { const protectedPids = new Set(); for (const root of taskRoots(tmpRoot)) { - const taskId = path.basename(path.dirname(root)).slice(3); + let owner; + try { + owner = readJson(path.join(root, OWNER_FILE)); + } catch { + continue; + } + const taskId = owner.taskId; const browserState = readBrowserState(root); if (browserState?.pid) protectedPids.add(browserState.pid); const bridgePid = readBridgePid(root, taskId); @@ -896,7 +985,8 @@ async function commandSweep(args) { } if (owner.ownerHome !== ownerHome || !TASK_ID_RE.test(owner.taskId || "")) continue; if (existsSync(path.join(stateDir, `${owner.taskId}.meta`))) continue; - const result = await reapRoot(owner.taskId, path.dirname(root)); + if (canonicalOwnerHome(owner.ownerHome) !== ownerHome) continue; + const result = await reapRoot(owner.taskId, owner.taskTmp, ownerHome); taskRootsReaped += 1; bridgesReaped += result.bridge; browsersReaped += result.browser; @@ -906,10 +996,16 @@ async function commandSweep(args) { let profilesReaped = 0; try { for (const root of taskRoots(tmpRoot)) { - const taskId = path.basename(path.dirname(root)).slice(3); + let owner; + try { + owner = readJson(path.join(root, OWNER_FILE)); + } catch { + continue; + } + const taskId = owner.taskId; let canonicalRoot; try { - canonicalRoot = validateBrowserRoot(taskId, root); + canonicalRoot = validateBrowserRoot(taskId, root, owner.ownerHome); } catch { continue; } @@ -954,6 +1050,10 @@ async function main() { case "prepare": await commandPrepare(args); break; + case "root": + if (args.length !== 2) fail("usage: root "); + process.stdout.write(`${expectedBrowserRoot(validateTaskId(args[0]), args[1])}\n`); + break; case "run": await commandRun(args); break; @@ -967,7 +1067,7 @@ async function main() { commandClassify(args); break; default: - fail("usage: fm-browser-isolation.mjs ..."); + fail("usage: fm-browser-isolation.mjs ..."); } } diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index 373616378b..86d810a257 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -1512,6 +1512,16 @@ spawn_restore_unmanaged_state() { return "$status" } +cleanup_prepared_task_tmp() { + [ -n "${TASK_TMP:-}" ] || return 0 + if [ "${BROWSER_PREPARED:-0}" = 1 ]; then + "$SCRIPT_DIR/fm-browser-isolation.sh" reap "$ID" "$TASK_TMP" "$FM_HOME" || return 1 + BROWSER_PREPARED=0 + fi + [ "${ORIGINAL_TASK_TMP_PRESENT:-0}" != 1 ] || return 0 + rm -rf "$TASK_TMP" +} + spawn_abort_cleanup() { local status=$? endpoint_state endpoint_gone=1 account_clean=1 state_clean=1 worktree_clean=1 rollback_lock='' rollback_tmp restored_existing_meta=0 artifact_backup_name release_status orca_cleanup_failed=0 orca_boundary_token= trap - EXIT @@ -1691,7 +1701,10 @@ spawn_abort_cleanup() { [ "$ORIGINAL_CHECK_PRESENT" != 0 ] || rm -f "$STATE/$ID.check.sh" [ "$ORIGINAL_PI_EXT_PRESENT" != 0 ] || rm -f "$STATE/$ID.pi-ext.ts" [ "$ORIGINAL_GROK_TOKEN_PRESENT" != 0 ] || rm -f "$STATE/$ID.grok-turnend-token" - [ "$ORIGINAL_TASK_TMP_PRESENT" != 0 ] || { [ -z "${TASK_TMP:-}" ] || rm -rf "$TASK_TMP"; } + [ "$ORIGINAL_TASK_TMP_PRESENT" != 0 ] || cleanup_prepared_task_tmp || { + worktree_clean=0 + echo "warning: failed to reap task-owned browser during spawn rollback for ${ID:-unknown}" >&2 + } fi fi if [ "$worktree_clean" != 1 ]; then @@ -1770,7 +1783,10 @@ spawn_abort_cleanup() { [ "$ORIGINAL_CHECK_PRESENT" != 0 ] || rm -f "$STATE/$ID.check.sh" [ "$ORIGINAL_PI_EXT_PRESENT" != 0 ] || rm -f "$STATE/$ID.pi-ext.ts" [ "$ORIGINAL_GROK_TOKEN_PRESENT" != 0 ] || rm -f "$STATE/$ID.grok-turnend-token" - [ "$ORIGINAL_TASK_TMP_PRESENT" != 0 ] || { [ -z "${TASK_TMP:-}" ] || rm -rf "$TASK_TMP"; } + [ "$ORIGINAL_TASK_TMP_PRESENT" != 0 ] || cleanup_prepared_task_tmp || { + account_clean=0 + echo "warning: failed to reap task-owned browser during spawn rollback for ${ID:-unknown}" >&2 + } fi if [ "$account_clean" != 1 ] && [ -n "$rollback_lock" ]; then persist_failed_account_rollback || echo "warning: failed to persist Agent Fleet rollback state for ${ID:-unknown}" >&2 @@ -1792,6 +1808,12 @@ spawn_abort_cleanup() { [ -z "$rollback_lock" ] || fm_account_meta_lock_release "$rollback_lock" >/dev/null 2>&1 || true [ -z "$META_BACKUP" ] || [ -f "$META_BACKUP" ] || META_BACKUP= [ -z "$EXISTING_ARTIFACT_BACKUP" ] || [ -d "$EXISTING_ARTIFACT_BACKUP" ] || EXISTING_ARTIFACT_BACKUP= + if [ "$status" -ne 0 ] && [ "${BROWSER_PREPARED:-0}" = 1 ]; then + cleanup_prepared_task_tmp || { + status=1 + echo "warning: failed to reap task-owned browser after spawn abort for ${ID:-unknown}" >&2 + } + fi [ "${LIFECYCLE_LOCK_OWNED:-0}" != 1 ] || [ -z "${LIFECYCLE_LOCK:-}" ] || fm_account_lifecycle_lock_release "$LIFECYCLE_LOCK" >/dev/null 2>&1 || true release_secondmate_home_lifecycle_locks LIFECYCLE_LOCK= @@ -3074,7 +3096,8 @@ BROWSER_MCP_PATH=$(browser_mcp_path) "$SCRIPT_DIR/fm-browser-isolation.sh" prepare \ "$ID" "$TASK_TMP" "$FM_HOME" "$REAL_CHROME_DEVTOOLS_AXI" \ "$BROWSER_AUTOMATION_EXECUTABLE" "$BROWSER_MCP_PATH" || exit 1 -BROWSER_ROOT="$TASK_TMP/browser" +BROWSER_PREPARED=1 +BROWSER_ROOT=$("$SCRIPT_DIR/fm-browser-isolation.sh" root "$ID" "$FM_HOME") || exit 1 # herdr sets GOTMPDIR natively at agent start. Every other backend exports it into # the pane shell just before the launch line, further down. CREW_PATH and the # task-scoped browser identity ride the same two channels for the same reason. @@ -3084,6 +3107,7 @@ if [ "$BACKEND" = herdr ]; then HERDR_AGENT_ENV+=("PATH=$CREW_PATH") HERDR_AGENT_ENV+=("FM_BROWSER_TASK_ID=$ID") HERDR_AGENT_ENV+=("FM_BROWSER_ROOT=$BROWSER_ROOT") + HERDR_AGENT_ENV+=("FM_BROWSER_OWNER_HOME=$FM_HOME") fi # Per-harness turn-end hook: a file that touches state/.turn-ended when the @@ -3760,7 +3784,7 @@ fi # same LAUNCH string as its argv and PATH/GOTMPDIR injected via --env, so there is # nothing to type into a pane here. if [ "$BACKEND" != herdr ]; then - spawn_send_text_line "$T" "export GOTMPDIR=$(shell_quote "$TASK_TMP/gotmp") PATH=$(shell_quote "$CREW_PATH") FM_BROWSER_TASK_ID=$(shell_quote "$ID") FM_BROWSER_ROOT=$(shell_quote "$BROWSER_ROOT")" + spawn_send_text_line "$T" "export GOTMPDIR=$(shell_quote "$TASK_TMP/gotmp") PATH=$(shell_quote "$CREW_PATH") FM_BROWSER_TASK_ID=$(shell_quote "$ID") FM_BROWSER_ROOT=$(shell_quote "$BROWSER_ROOT") FM_BROWSER_OWNER_HOME=$(shell_quote "$FM_HOME")" sleep 0.3 spawn_send_literal "$T" "$LAUNCH" sleep 0.3 diff --git a/bin/fm-teardown.sh b/bin/fm-teardown.sh index 80ea80cd9a..28655d44fc 100755 --- a/bin/fm-teardown.sh +++ b/bin/fm-teardown.sh @@ -1859,7 +1859,7 @@ reap_task_browser() { local id=$1 tasktmp=$2 [ -n "$tasktmp" ] || return 0 [ -x "$FM_ROOT/bin/fm-browser-isolation.sh" ] || return 0 - "$FM_ROOT/bin/fm-browser-isolation.sh" reap "$id" "$tasktmp" + "$FM_ROOT/bin/fm-browser-isolation.sh" reap "$id" "$tasktmp" "$FM_HOME" } remove_worktree_compatibility_artifacts() { diff --git a/docs/browser-isolation-verification.md b/docs/browser-isolation-verification.md new file mode 100644 index 0000000000..d68d594a11 --- /dev/null +++ b/docs/browser-isolation-verification.md @@ -0,0 +1,35 @@ +# Browser isolation verification + +## 2026-07-28 macOS evidence + +The real separate automation browser completed a browser-heavy AXI workload without attaching to or changing stable Chrome. +The run used macOS 26.5.2 and Google Chrome for Testing 143.0.7499.40. +The stable-routing mutation was not repeated because this fix round forbade touching the captain's stable Chrome. +The existing opt-in assertion remains in `tests/fm-browser-isolation-macos-smoke.sh`: stable Chrome must gain a window while the unique routing URL remains absent from automation. + +Command: + +```sh +FM_BROWSER_ROUTING_TEST=0 tests/fm-browser-isolation-macos-smoke.sh +``` + +Measured output: + +```text +live_browser_mock_keychain=1 +live_browser_basic_password_store=1 +dialog_monitor_samples=51 +automation_visible_windows_seen=0 +screenshots=15 +owned_processes_during_run=8 +owned_processes_after_stop=0 +browser_tree_processes_during_run=8 +browser_tree_processes_after_stop=0 +bridge_alive_after_stop=0 +profile_exists_after_reap=0 +stable_chrome_identity_preserved=1 +ok - macOS credential isolation and teardown smoke test passed +``` + +The browser was Chrome for Testing selected by the smoke test on macOS. +The live process arguments contained `--use-mock-keychain` and `--password-store=basic`. diff --git a/docs/browser-isolation.md b/docs/browser-isolation.md index df36084fd8..dcaf7b554f 100644 --- a/docs/browser-isolation.md +++ b/docs/browser-isolation.md @@ -3,7 +3,8 @@ Firstmate-launched crew use a task-scoped automation browser. The captain's stable Google Chrome process and profile are never an automation target. -`bin/fm-spawn.sh` selects Chrome Canary, Chrome for Testing, or Chromium as a separate application identity and prepares `/tmp/fm-/browser`. +`bin/fm-spawn.sh` selects Chrome Canary, Chrome for Testing, or Chromium as a separate application identity and prepares `/tmp/fm-browser--`. +The home key is derived from the canonical Firstmate home, so identical task IDs in sibling homes cannot share lifecycle state, a profile, or ownership records. It then places `bin/chrome-devtools-axi` first on that crew's `PATH`. The wrapper launches the selected browser with a private profile and attaches the real AXI bridge through a loopback DevTools endpoint. @@ -23,6 +24,7 @@ One browser and one AXI session are owned by each task. Successful AXI commands reuse that task browser so page state survives between commands. `chrome-devtools-axi stop` stops the bridge and browser, verifies that no process still references the task profile, and leaves the task root ready for a clean relaunch. `fm-teardown.sh` performs the same reap after the agent endpoint is quiescent and before deleting `/tmp/fm-`. +Failed spawn rollback uses the same verified reap before it removes a prepared task temp root. A browser that exits unexpectedly or an AXI command that fails is cleaned immediately and writes a failure latch. Later commands refuse to respawn it implicitly. @@ -46,5 +48,6 @@ The sweep prints `BROWSER_GC:` only when it reaps something or cannot verify cle ## Verification Run `tests/fm-browser-isolation.test.sh` for fake-process behavior, failure-latch, teardown, backstop, and discrimination coverage. -On macOS, run `tests/fm-browser-isolation-macos-smoke.sh` manually to launch the real separate browser, capture 15 screenshots, continuously sample on-screen windows, and prove zero bridge and browser processes after stop. +On macOS, run `tests/fm-browser-isolation-macos-smoke.sh` manually to launch the real separate browser, capture 15 screenshots, continuously sample automation windows and newly appearing credential-dialog owners or titles, record the live browser arguments, and prove zero bridge, complete browser-tree, and profile counts after stop and reap. Set `FM_BROWSER_ROUTING_TEST=1` only when a new stable-Chrome window is acceptable; the smoke test then proves `open -na "Google Chrome" --args --new-window ` increases stable Chrome's window count and that the URL never reaches automation. +Measured macOS evidence is recorded in `docs/browser-isolation-verification.md`. diff --git a/tests/fm-browser-isolation-macos-smoke.sh b/tests/fm-browser-isolation-macos-smoke.sh index 4065647e85..b5fd38113a 100755 --- a/tests/fm-browser-isolation-macos-smoke.sh +++ b/tests/fm-browser-isolation-macos-smoke.sh @@ -13,6 +13,7 @@ BROWSER_LIFECYCLE="$ROOT/bin/fm-browser-isolation.sh" WRAPPER="$ROOT/bin/chrome-devtools-axi" TASK_ID="credential-smoke-$$" TASK_TMP="/tmp/fm-$TASK_ID" +BROWSER_ROOT= MONITOR_PID= PROFILE= BRIDGE_PID= @@ -33,6 +34,10 @@ console.log(commands.filter((command) => command.includes(marker)).length); JS } +process_group_count() { + ps -axo pgid= | awk -v pgid="$1" '$1 == pgid { count += 1 } END { print count + 0 }' +} + stable_window_count() { # shellcheck disable=SC2016 # Swift source is intentionally a shell literal. swift -e ' @@ -53,8 +58,8 @@ cleanup() { if [ -n "$MONITOR_PID" ]; then wait "$MONITOR_PID" 2>/dev/null || cleanup_rc=1 fi - if [ -f "$TASK_TMP/browser/owner.json" ]; then - "$BROWSER_LIFECYCLE" reap "$TASK_ID" "$TASK_TMP" >/dev/null 2>&1 || cleanup_rc=1 + if [ -n "$BROWSER_ROOT" ] && [ -f "$BROWSER_ROOT/owner.json" ]; then + "$BROWSER_LIFECYCLE" reap "$TASK_ID" "$TASK_TMP" "$ROOT" >/dev/null 2>&1 || cleanup_rc=1 fi if [ -n "$PROFILE" ]; then remaining=$(owned_process_count "$PROFILE") @@ -121,6 +126,7 @@ STABLE_IDENTITY=$(ps -p "$STABLE_PID" -o pid=,lstart=,pgid=) mkdir -p "$TASK_TMP/gotmp" "$TASK_TMP/shots" "$BROWSER_LIFECYCLE" prepare \ "$TASK_ID" "$TASK_TMP" "$ROOT" "$REAL_AXI" "$AUTOMATION_BROWSER" "$MCP_PATH" +BROWSER_ROOT=$("$BROWSER_LIFECYCLE" root "$TASK_ID" "$ROOT") # shellcheck disable=SC2016 # Swift source is intentionally a shell literal. swift -e ' @@ -130,11 +136,25 @@ swift -e ' let taskRoot = CommandLine.arguments[1] let stopFile = taskRoot + "/monitor.stop" let readyFile = taskRoot + "/monitor.ready" - let browserOwners: Set = [ + let automationOwners: Set = [ "Chromium", "Google Chrome Canary", "Google Chrome for Testing" ] + let credentialOwners: Set = [ + "SecurityAgent", + "CoreServicesUIAgent", + "System Settings", + "System Preferences" + ] + let credentialTerms = ["keychain", "password", "credential", "safe storage", "login keychain"] + let baselineRows = CGWindowListCopyWindowInfo( + [.optionOnScreenOnly, .excludeDesktopElements], + kCGNullWindowID + ) as? [[String: Any]] ?? [] + let baselineWindows = Set(baselineRows.compactMap { + $0[kCGWindowNumber as String] as? Int + }) try? "ready\n".write(toFile: readyFile, atomically: true, encoding: .utf8) var samples = 0 var maximumVisibleAutomationWindows = 0 @@ -146,7 +166,11 @@ swift -e ' let visible = rows.filter { row in let owner = row[kCGWindowOwnerName as String] as? String ?? "" let title = (row[kCGWindowName as String] as? String ?? "").lowercased() - return browserOwners.contains(owner) || title.contains("keychain not found") + let number = row[kCGWindowNumber as String] as? Int ?? 0 + let credentialDialog = credentialOwners.contains(owner) + || credentialTerms.contains(where: { title.contains($0) }) + return automationOwners.contains(owner) + || (!baselineWindows.contains(number) && credentialDialog) }.count maximumVisibleAutomationWindows = max(maximumVisibleAutomationWindows, visible) samples += 1 @@ -166,18 +190,21 @@ while [ ! -f "$TASK_TMP/monitor.ready" ]; do sleep 0.1 done -FM_BROWSER_TASK_ID="$TASK_ID" FM_BROWSER_ROOT="$TASK_TMP/browser" \ +FM_BROWSER_TASK_ID="$TASK_ID" FM_BROWSER_ROOT="$BROWSER_ROOT" FM_BROWSER_OWNER_HOME="$ROOT" \ "$WRAPPER" open https://example.com/ -STATE_FILE="$TASK_TMP/browser/browser.json" +STATE_FILE="$BROWSER_ROOT/browser.json" PROFILE=$(node -e 'console.log(JSON.parse(require("fs").readFileSync(process.argv[1])).profile)' "$STATE_FILE") BROWSER_PID=$(node -e 'console.log(JSON.parse(require("fs").readFileSync(process.argv[1])).pid)' "$STATE_FILE") BROWSER_PORT=$(node -e 'console.log(JSON.parse(require("fs").readFileSync(process.argv[1])).port)' "$STATE_FILE") BROWSER_COMMAND=$(ps -p "$BROWSER_PID" -o command=) +BROWSER_PGID=$(ps -p "$BROWSER_PID" -o pgid= | tr -d ' ') case "$BROWSER_COMMAND" in *--use-mock-keychain*) ;; *) fail "live automation browser lacks --use-mock-keychain" ;; esac +printf 'live_browser_mock_keychain=1\n' +printf 'live_browser_basic_password_store=1\n' case "$BROWSER_COMMAND" in *--password-store=basic*) ;; *) fail "live automation browser lacks --password-store=basic" ;; @@ -185,7 +212,7 @@ esac INDEX=1 while [ "$INDEX" -le 15 ]; do - FM_BROWSER_TASK_ID="$TASK_ID" FM_BROWSER_ROOT="$TASK_TMP/browser" \ + FM_BROWSER_TASK_ID="$TASK_ID" FM_BROWSER_ROOT="$BROWSER_ROOT" FM_BROWSER_OWNER_HOME="$ROOT" \ "$WRAPPER" screenshot "$TASK_TMP/shots/shot-$INDEX.png" >/dev/null INDEX=$((INDEX + 1)) done @@ -209,8 +236,9 @@ if [ "${FM_BROWSER_ROUTING_TEST:-0}" = 1 ]; then fi OWNED_DURING_RUN=$(owned_process_count "$PROFILE") +TREE_DURING_RUN=$(process_group_count "$BROWSER_PGID") BRIDGE_PID=$(node -e 'console.log(JSON.parse(require("fs").readFileSync(process.argv[1])).pid)' \ - "$TASK_TMP/browser/home/.chrome-devtools-axi/sessions/fm-$TASK_ID/bridge.pid") + "$BROWSER_ROOT/home/.chrome-devtools-axi/sessions/fm-$TASK_ID/bridge.pid") kill -0 "$BRIDGE_PID" 2>/dev/null || fail "AXI bridge was not live during the run" touch "$TASK_TMP/monitor.stop" @@ -219,18 +247,24 @@ MONITOR_PID= cat "$TASK_TMP/monitor.out" grep -Fx 'automation_visible_windows_seen=0' "$TASK_TMP/monitor.out" >/dev/null \ || fail "native-dialog monitor observed an automation window" -if grep -Ei 'keychain|password store' "$TASK_TMP/browser/browser.log" >/dev/null; then - grep -Ei 'keychain|password store' "$TASK_TMP/browser/browser.log" >&2 +if grep -Ei 'keychain|password store|safe storage' "$BROWSER_ROOT/browser.log" >/dev/null; then + grep -Ei 'keychain|password store|safe storage' "$BROWSER_ROOT/browser.log" >&2 fail "automation browser logged a credential-store error" fi -FM_BROWSER_TASK_ID="$TASK_ID" FM_BROWSER_ROOT="$TASK_TMP/browser" \ +FM_BROWSER_TASK_ID="$TASK_ID" FM_BROWSER_ROOT="$BROWSER_ROOT" FM_BROWSER_OWNER_HOME="$ROOT" \ "$WRAPPER" stop >/dev/null OWNED_AFTER_STOP=$(owned_process_count "$PROFILE") +TREE_AFTER_STOP=$(process_group_count "$BROWSER_PGID") +BRIDGE_AFTER_STOP=0 +kill -0 "$BRIDGE_PID" 2>/dev/null && BRIDGE_AFTER_STOP=1 kill -0 "$BRIDGE_PID" 2>/dev/null && fail "AXI bridge survived wrapper stop" [ "$OWNED_AFTER_STOP" -eq 0 ] || fail "automation browser survived wrapper stop" -"$BROWSER_LIFECYCLE" reap "$TASK_ID" "$TASK_TMP" -[ ! -e "$TASK_TMP/browser" ] || fail "task profile root survived reap" +[ "$TREE_AFTER_STOP" -eq 0 ] || fail "automation browser process group survived wrapper stop" +"$BROWSER_LIFECYCLE" reap "$TASK_ID" "$TASK_TMP" "$ROOT" +[ ! -e "$BROWSER_ROOT" ] || fail "task profile root survived reap" +PROFILE_AFTER_REAP=0 +[ ! -e "$BROWSER_ROOT/profile" ] || PROFILE_AFTER_REAP=1 STABLE_IDENTITY_AFTER=$(ps -p "$STABLE_PID" -o pid=,lstart=,pgid=) [ "$STABLE_IDENTITY_AFTER" = "$STABLE_IDENTITY" ] \ @@ -239,7 +273,9 @@ STABLE_IDENTITY_AFTER=$(ps -p "$STABLE_PID" -o pid=,lstart=,pgid=) printf 'screenshots=15\n' printf 'owned_processes_during_run=%s\n' "$OWNED_DURING_RUN" printf 'owned_processes_after_stop=%s\n' "$OWNED_AFTER_STOP" -printf 'bridge_alive_after_stop=0\n' -printf 'profile_exists_after_reap=0\n' +printf 'browser_tree_processes_during_run=%s\n' "$TREE_DURING_RUN" +printf 'browser_tree_processes_after_stop=%s\n' "$TREE_AFTER_STOP" +printf 'bridge_alive_after_stop=%s\n' "$BRIDGE_AFTER_STOP" +printf 'profile_exists_after_reap=%s\n' "$PROFILE_AFTER_REAP" printf 'stable_chrome_identity_preserved=1\n' printf 'ok - macOS credential isolation and teardown smoke test passed\n' diff --git a/tests/fm-browser-isolation.test.sh b/tests/fm-browser-isolation.test.sh index 6df5c4d749..4ce54a3cb0 100755 --- a/tests/fm-browser-isolation.test.sh +++ b/tests/fm-browser-isolation.test.sh @@ -65,6 +65,7 @@ cat > "$FAKE_BROWSER" <<'JS' const fs = require("fs"); const http = require("http"); const path = require("path"); +const { spawn } = require("child_process"); const profileArg = process.argv.find((arg) => arg.startsWith("--user-data-dir=")); const requiredArgs = ["--use-mock-keychain", "--password-store=basic"]; @@ -75,6 +76,10 @@ if ( ) process.exit(2); const profile = profileArg.slice("--user-data-dir=".length); fs.mkdirSync(profile, { recursive: true }); +const helper = spawn(process.execPath, ["-e", "setInterval(() => {}, 1000)"], { + stdio: "ignore", +}); +fs.writeFileSync(path.join(profile, "helper.pid"), `${helper.pid}\n`); const server = http.createServer((request, response) => { response.setHeader("content-type", "application/json"); response.end(JSON.stringify({ @@ -137,6 +142,10 @@ JS chmod +x "$FAKE_BROWSER" "$FAKE_AXI" "$FAKE_BRIDGE" "$FAKE_HEADED" +browser_root() { + FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" "$BROWSER" root "$1" "$2" +} + test_process_discrimination() { local fixture="$TMP_ROOT/processes.txt" out cat > "$fixture" <<'ROWS' @@ -147,7 +156,7 @@ test_process_discrimination() { 105 1 105 codex task text says chrome-devtools-axi-bridge process cleanup 106 1 106 node /opt/axi/chrome-devtools-axi-bridge.js 107 106 106 /Applications/Google Chrome.app/Contents/MacOS/Google Chrome --headless=new --user-data-dir=/tmp/puppeteer_dev_chrome_profile-leaked -108 1 108 /cache/Google Chrome for Testing --headless=new --user-data-dir=/tmp/fm-task-z1/browser/profile +108 1 108 /cache/Google Chrome for Testing --headless=new --user-data-dir=/tmp/fm-browser-0123456789abcdef-task-z1/profile 109 1 109 python worker.py --headless=new --user-data-dir=/tmp/puppeteer_dev_chrome_profile-not-a-browser ROWS out=$("$BROWSER" classify "$fixture") @@ -164,17 +173,17 @@ ROWS test_task_launch_and_reap() { local id=browser-owned-z2 tasktmp="$TMP_ROOT/tmp/fm-browser-owned-z2" - local browser_pid bridge_pid canonical_tasktmp + local browser_pid bridge_pid helper_pid root mkdir -p "$tasktmp/gotmp" FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" \ "$BROWSER" prepare "$id" "$tasktmp" "$TMP_ROOT/home" "$FAKE_AXI" "$FAKE_BROWSER" "" + root=$(browser_root "$id" "$TMP_ROOT/home") FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" \ FM_BROWSER_TASK_ID="$id" \ - FM_BROWSER_ROOT="$tasktmp/browser" \ + FM_BROWSER_ROOT="$root" FM_BROWSER_OWNER_HOME="$TMP_ROOT/home" \ "$WRAPPER" open https://example.test/ - canonical_tasktmp=$(cd "$tasktmp" && pwd -P) - grep -F "home=$canonical_tasktmp/browser/home" "$AXI_RECORD" >/dev/null \ + grep -F "home=$root/home" "$AXI_RECORD" >/dev/null \ || fail "AXI HOME was not isolated under the task browser root" grep -F "session=fm-$id" "$AXI_RECORD" >/dev/null \ || fail "AXI session was not keyed to the task" @@ -189,8 +198,9 @@ test_task_launch_and_reap() { grep -Fx 'chrome_args=--use-mock-keychain --password-store=basic' "$AXI_RECORD" >/dev/null \ || fail "AXI fallback launch did not inherit credential-store isolation" - browser_pid=$(node -e 'console.log(JSON.parse(require("fs").readFileSync(process.argv[1])).pid)' "$tasktmp/browser/browser.json") - bridge_pid=$(node -e 'console.log(JSON.parse(require("fs").readFileSync(process.argv[1])).pid)' "$tasktmp/browser/home/.chrome-devtools-axi/sessions/fm-$id/bridge.pid") + browser_pid=$(node -e 'console.log(JSON.parse(require("fs").readFileSync(process.argv[1])).pid)' "$root/browser.json") + helper_pid=$(cat "$root/profile/helper.pid") + bridge_pid=$(node -e 'console.log(JSON.parse(require("fs").readFileSync(process.argv[1])).pid)' "$root/home/.chrome-devtools-axi/sessions/fm-$id/bridge.pid") TEARDOWN_BROWSER_PID=$browser_pid TEARDOWN_BRIDGE_PID=$bridge_pid kill -0 "$browser_pid" 2>/dev/null || fail "fake automation browser did not remain live" @@ -198,11 +208,12 @@ test_task_launch_and_reap() { sleep 300 & UNRELATED_PID=$! - FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" "$BROWSER" reap "$id" "$tasktmp" + FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" "$BROWSER" reap "$id" "$tasktmp" "$TMP_ROOT/home" kill -0 "$browser_pid" 2>/dev/null && fail "task browser survived reap" + kill -0 "$helper_pid" 2>/dev/null && fail "unmarked browser helper survived reap" kill -0 "$bridge_pid" 2>/dev/null && fail "task bridge survived reap" kill -0 "$UNRELATED_PID" 2>/dev/null || fail "unrelated process was killed by task reap" - [ ! -e "$tasktmp/browser" ] || fail "task browser profile root survived reap" + [ ! -e "$root" ] || fail "task browser profile root survived reap" TEARDOWN_BROWSER_PID= TEARDOWN_BRIDGE_PID= pass "task reap removes the exact bridge, browser, and profile without collateral" @@ -210,32 +221,33 @@ test_task_launch_and_reap() { test_failed_browser_cannot_respawn() { local id=browser-failure-z4 tasktmp="$TMP_ROOT/tmp/fm-browser-failure-z4" - local browser_pid second_output + local browser_pid second_output root mkdir -p "$tasktmp/gotmp" FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" \ "$BROWSER" prepare "$id" "$tasktmp" "$TMP_ROOT/home" "$FAKE_AXI" "$FAKE_BROWSER" "" + root=$(browser_root "$id" "$TMP_ROOT/home") FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" \ FM_BROWSER_TASK_ID="$id" \ - FM_BROWSER_ROOT="$tasktmp/browser" \ + FM_BROWSER_ROOT="$root" FM_BROWSER_OWNER_HOME="$TMP_ROOT/home" \ "$WRAPPER" open https://example.test/ - browser_pid=$(node -e 'console.log(JSON.parse(require("fs").readFileSync(process.argv[1])).pid)' "$tasktmp/browser/browser.json") + browser_pid=$(node -e 'console.log(JSON.parse(require("fs").readFileSync(process.argv[1])).pid)' "$root/browser.json") kill "$browser_pid" wait "$browser_pid" 2>/dev/null || true if second_output=$( FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" \ FM_BROWSER_TASK_ID="$id" \ - FM_BROWSER_ROOT="$tasktmp/browser" \ + FM_BROWSER_ROOT="$root" FM_BROWSER_OWNER_HOME="$TMP_ROOT/home" \ "$WRAPPER" screenshot 2>&1 ); then fail "wrapper respawned a browser after its owned browser died" fi printf '%s\n' "$second_output" | grep -F 'refusing to respawn it implicitly' >/dev/null \ || fail "unexpected browser death did not trip the respawn latch" - [ -f "$tasktmp/browser/browser.failed" ] \ + [ -f "$root/browser.failed" ] \ || fail "browser failure latch was not recorded" - [ ! -f "$tasktmp/browser/browser.json" ] \ + [ ! -f "$root/browser.json" ] \ || fail "stale browser state survived failed-browser cleanup" - FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" "$BROWSER" reap "$id" "$tasktmp" + FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" "$BROWSER" reap "$id" "$tasktmp" "$TMP_ROOT/home" pass "unexpected browser death is cleaned and cannot trigger an implicit respawn" } @@ -245,7 +257,7 @@ test_real_teardown_path_reaps_owned_browser() { local project="$TMP_ROOT/teardown-project" origin="$TMP_ROOT/teardown-origin.git" local worktree="$pool/1/worktree" seed="$TMP_ROOT/teardown-seed" local fakebin="$TMP_ROOT/teardown-fakebin" - local browser_pid bridge_pid output + local browser_pid bridge_pid output root TEARDOWN_TASK_TMP=$tasktmp mkdir -p "$tasktmp/gotmp" "$home/state" "$home/data" "$home/config" "$pool/1" "$fakebin" git init -q --bare "$origin" @@ -301,10 +313,11 @@ yolo=off tasktmp=$tasktmp META "$BROWSER" prepare "$id" "$tasktmp" "$home" "$FAKE_AXI" "$FAKE_BROWSER" "" - FM_BROWSER_TASK_ID="$id" FM_BROWSER_ROOT="$tasktmp/browser" \ + root=$("$BROWSER" root "$id" "$home") + FM_BROWSER_TASK_ID="$id" FM_BROWSER_ROOT="$root" FM_BROWSER_OWNER_HOME="$home" \ "$WRAPPER" open https://example.test/ - browser_pid=$(node -e 'console.log(JSON.parse(require("fs").readFileSync(process.argv[1])).pid)' "$tasktmp/browser/browser.json") - bridge_pid=$(node -e 'console.log(JSON.parse(require("fs").readFileSync(process.argv[1])).pid)' "$tasktmp/browser/home/.chrome-devtools-axi/sessions/fm-$id/bridge.pid") + browser_pid=$(node -e 'console.log(JSON.parse(require("fs").readFileSync(process.argv[1])).pid)' "$root/browser.json") + bridge_pid=$(node -e 'console.log(JSON.parse(require("fs").readFileSync(process.argv[1])).pid)' "$root/home/.chrome-devtools-axi/sessions/fm-$id/bridge.pid") TEARDOWN_BROWSER_PID=$browser_pid TEARDOWN_BRIDGE_PID=$bridge_pid @@ -313,7 +326,7 @@ META FM_STATE_OVERRIDE="$home/state" FM_DATA_OVERRIDE="$home/data" \ FM_CONFIG_OVERRIDE="$home/config" FM_CHECKOUT_REFRESH_LOCK_ROOT="$TMP_ROOT/checkout-locks" \ FM_FAKE_TASK_ID="$id" PATH="$fakebin:$PATH" \ - "$TEARDOWN" "$id" 2>&1 + env -u NO_MISTAKES_GATE FM_GATE_REFUSE_BYPASS=1 "$TEARDOWN" "$id" 2>&1 ) || fail "fm-teardown rejected an otherwise-finished browser-owning task: $output" kill -0 "$browser_pid" 2>/dev/null && fail "browser survived the real fm-teardown path" kill -0 "$bridge_pid" 2>/dev/null && fail "bridge survived the real fm-teardown path" @@ -325,13 +338,46 @@ META pass "real fm-teardown path reaps the bridge, browser process tree, and task profile" } +test_same_id_isolated_across_homes() { + local id=shared-browser-z6 tasktmp="$TMP_ROOT/tmp/fm-shared-browser-z6" + local home_a="$TMP_ROOT/home-a" home_b="$TMP_ROOT/home-b" + local root_a root_b browser_a browser_b bridge_a bridge_b + mkdir -p "$tasktmp/gotmp" "$home_a" "$home_b" + FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" \ + "$BROWSER" prepare "$id" "$tasktmp" "$home_a" "$FAKE_AXI" "$FAKE_BROWSER" "" + FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" \ + "$BROWSER" prepare "$id" "$tasktmp" "$home_b" "$FAKE_AXI" "$FAKE_BROWSER" "" + root_a=$(browser_root "$id" "$home_a") + root_b=$(browser_root "$id" "$home_b") + [ "$root_a" != "$root_b" ] || fail "sibling homes received the same browser root" + FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" FM_BROWSER_TASK_ID="$id" \ + FM_BROWSER_ROOT="$root_a" FM_BROWSER_OWNER_HOME="$home_a" \ + "$WRAPPER" open https://home-a.example.test/ + FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" FM_BROWSER_TASK_ID="$id" \ + FM_BROWSER_ROOT="$root_b" FM_BROWSER_OWNER_HOME="$home_b" \ + "$WRAPPER" open https://home-b.example.test/ + browser_a=$(node -e 'console.log(JSON.parse(require("fs").readFileSync(process.argv[1])).pid)' "$root_a/browser.json") + browser_b=$(node -e 'console.log(JSON.parse(require("fs").readFileSync(process.argv[1])).pid)' "$root_b/browser.json") + bridge_a=$(node -e 'console.log(JSON.parse(require("fs").readFileSync(process.argv[1])).pid)' "$root_a/home/.chrome-devtools-axi/sessions/fm-$id/bridge.pid") + bridge_b=$(node -e 'console.log(JSON.parse(require("fs").readFileSync(process.argv[1])).pid)' "$root_b/home/.chrome-devtools-axi/sessions/fm-$id/bridge.pid") + FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" "$BROWSER" reap "$id" "$tasktmp" "$home_a" + kill -0 "$browser_a" 2>/dev/null && fail "reaped home browser survived" + kill -0 "$bridge_a" 2>/dev/null && fail "reaped home bridge survived" + kill -0 "$browser_b" 2>/dev/null || fail "sibling home browser was killed" + kill -0 "$bridge_b" 2>/dev/null || fail "sibling home bridge was killed" + [ -d "$root_b/profile" ] || fail "sibling home profile was removed" + FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" "$BROWSER" reap "$id" "$tasktmp" "$home_b" + pass "same task id remains browser-isolated across sibling homes" +} + test_orphan_sweep() { local id=browser-orphan-z3 tasktmp="$TMP_ROOT/tmp/fm-browser-orphan-z3" local legacy_profile="$TMP_ROOT/tmp/puppeteer_dev_chrome_profile-stale" - local headed_profile="$TMP_ROOT/tmp/puppeteer_dev_chrome_profile-headed" out + local headed_profile="$TMP_ROOT/tmp/puppeteer_dev_chrome_profile-headed" out root mkdir -p "$tasktmp/gotmp" "$legacy_profile" "$headed_profile" FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" \ "$BROWSER" prepare "$id" "$tasktmp" "$TMP_ROOT/home" "$FAKE_AXI" "$FAKE_BROWSER" "" + root=$(browser_root "$id" "$TMP_ROOT/home") node "$FAKE_BRIDGE" & LEGACY_BRIDGE_PID=$! @@ -357,10 +403,10 @@ test_orphan_sweep() { wait "$LEGACY_BROWSER_PID" 2>/dev/null || true kill -0 "$UNRELATED_PID" 2>/dev/null || fail "unrelated process was killed by orphan sweep" kill -0 "$HEADED_BROWSER_PID" 2>/dev/null || fail "headed temp-profile browser was killed by orphan sweep" - [ ! -e "$tasktmp/browser" ] || fail "metadata-free task browser root survived sweep" + [ ! -e "$root" ] || fail "metadata-free task browser root survived sweep" [ ! -e "$legacy_profile" ] || fail "unused legacy temp profile survived sweep" [ -d "$headed_profile" ] || fail "active headed temp profile was removed by orphan sweep" - printf '%s\n' "$out" | grep -E '^BROWSER_GC: reaped task_roots=1 bridges=1 browser_processes=1 profiles=1$' >/dev/null \ + printf '%s\n' "$out" | grep -E '^BROWSER_GC: reaped task_roots=1 bridges=1 browser_processes=2 profiles=1$' >/dev/null \ || fail "sweep did not report measured cleanup: $out" pass "backstop kills only exact orphan markers and preserves unrelated processes" } @@ -372,10 +418,14 @@ test_spawn_and_teardown_wiring() { || fail "spawn does not put the task AXI wrapper first on PATH" grep -F "HERDR_AGENT_ENV+=(\"FM_BROWSER_TASK_ID=\$ID\")" "$SPAWN" >/dev/null \ || fail "Herdr does not receive the task browser identity natively" + grep -F "HERDR_AGENT_ENV+=(\"FM_BROWSER_OWNER_HOME=\$FM_HOME\")" "$SPAWN" >/dev/null \ + || fail "Herdr does not receive the home-qualified browser owner" grep -F "FM_BROWSER_ROOT=\$(shell_quote \"\$BROWSER_ROOT\")" "$SPAWN" >/dev/null \ || fail "terminal backends do not export the task browser root" grep -F "reap_task_browser \"\$ID\" \"\$TASK_TMP\"" "$TEARDOWN" >/dev/null \ || fail "teardown does not reap the task browser before temp-root deletion" + grep -F 'cleanup_prepared_task_tmp' "$SPAWN" >/dev/null \ + || fail "spawn abort does not route prepared temp cleanup through browser reap" grep -F 'browser_gc_sweep' "$BOOTSTRAP" >/dev/null \ || fail "bootstrap does not expose the crash-orphan backstop" grep -F "\"\$SCRIPT_DIR/fm-browser-isolation.sh\" sweep \"\$FM_HOME\" \"\$STATE\"" "$BOOTSTRAP" >/dev/null \ @@ -387,5 +437,6 @@ test_process_discrimination test_task_launch_and_reap test_failed_browser_cannot_respawn test_real_teardown_path_reaps_owned_browser +test_same_id_isolated_across_homes test_orphan_sweep test_spawn_and_teardown_wiring From a811e66bcb1da934ca1da21304161b3d0dc068b9 Mon Sep 17 00:00:00 2001 From: Dongkeun Lee Date: Tue, 28 Jul 2026 12:06:36 -0400 Subject: [PATCH 03/18] no-mistakes(review): Fix nested browser reap and reparented helpers --- bin/fm-browser-isolation.mjs | 23 ++++++++++++++++------- bin/fm-teardown.sh | 10 +++++----- docs/browser-isolation-verification.md | 11 +++++++---- docs/browser-isolation.md | 2 +- tests/fm-browser-isolation-macos-smoke.sh | 2 +- tests/fm-browser-isolation.test.sh | 20 ++++++++++++++++---- 6 files changed, 46 insertions(+), 22 deletions(-) diff --git a/bin/fm-browser-isolation.mjs b/bin/fm-browser-isolation.mjs index 5077af9112..eba36af261 100755 --- a/bin/fm-browser-isolation.mjs +++ b/bin/fm-browser-isolation.mjs @@ -307,7 +307,6 @@ function sameProcessIdentity(original, current) { return ( current !== null && original.pid === current.pid && - original.ppid === current.ppid && original.pgid === current.pgid && original.command === current.command ); @@ -332,17 +331,25 @@ function descendantsOf(pid, rows) { async function terminateOwnedBrowserTree(state) { const rows = processInventory(); const root = rowForPid(state.pid, rows); - if (!processMatchesBrowser(state, root)) return false; - const ownedPids = new Set([root.pid, ...descendantsOf(root.pid, rows)]); - if (root.pgid === root.pid) { + const rootMatches = processMatchesBrowser(state, root); + const dedicatedPgid = Number.isSafeInteger(state.pgid) + ? state.pgid + : rootMatches && root.pgid === root.pid + ? root.pgid + : 0; + const ownedPids = new Set( + rootMatches ? [root.pid, ...descendantsOf(root.pid, rows)] : [], + ); + if (dedicatedPgid === state.pid) { for (const row of rows) { - if (row.pgid === root.pgid) ownedPids.add(row.pid); + if (row.pgid === dedicatedPgid) ownedPids.add(row.pid); } } const owned = rows.filter((row) => ownedPids.has(row.pid)); - if (root.pgid === root.pid) { + if (owned.length === 0) return false; + if (dedicatedPgid === state.pid) { try { - process.kill(-root.pgid, "SIGTERM"); + process.kill(-dedicatedPgid, "SIGTERM"); } catch { // The dedicated process group already exited. } @@ -547,6 +554,7 @@ function readBrowserState(root) { const state = readJson(file); if ( Number.isSafeInteger(state.pid) && + (!("pgid" in state) || Number.isSafeInteger(state.pgid)) && Number.isSafeInteger(state.port) && typeof state.profile === "string" && typeof state.executable === "string" @@ -656,6 +664,7 @@ async function ensureBrowser(root, owner) { const state = { pid: child.pid, + pgid: child.pid, port: 0, profile, executable: owner.browserExecutable, diff --git a/bin/fm-teardown.sh b/bin/fm-teardown.sh index 28655d44fc..e603949239 100755 --- a/bin/fm-teardown.sh +++ b/bin/fm-teardown.sh @@ -1856,10 +1856,10 @@ PY } reap_task_browser() { - local id=$1 tasktmp=$2 + local id=$1 tasktmp=$2 owner_home=$3 [ -n "$tasktmp" ] || return 0 [ -x "$FM_ROOT/bin/fm-browser-isolation.sh" ] || return 0 - "$FM_ROOT/bin/fm-browser-isolation.sh" reap "$id" "$tasktmp" "$FM_HOME" + "$FM_ROOT/bin/fm-browser-isolation.sh" reap "$id" "$tasktmp" "$owner_home" } remove_worktree_compatibility_artifacts() { @@ -3679,7 +3679,7 @@ cleanup_firstmate_home_children() { quiesce_child_endpoint "$child_meta" "$child_id" "$home" "$child_home" || return 1 fi child_tasktmp=$(meta_value "$child_meta" tasktmp) - reap_task_browser "$child_id" "$child_tasktmp" || { + reap_task_browser "$child_id" "$child_tasktmp" "$home" || { echo "error: retained child metadata for $child_id because its browser cleanup could not be verified" >&2 return 1 } @@ -3904,7 +3904,7 @@ if [ "$ORCA_CLEANUP_PENDING" = 1 ]; then fm_checkout_lock_run "$WT" "$CHECKOUT_LOCK_ROOT" remove_pending_orca_worktree_locked || exit 1 remove_grok_turnend_auth "$STATE" "$ID" fm_backend_clear_transition "$BACKEND" "$STATE" "$T" || true - reap_task_browser "$ID" "$TASK_TMP" || exit 1 + reap_task_browser "$ID" "$TASK_TMP" "$FM_HOME" || exit 1 safe_remove_task_tmp "$TASK_TMP" || exit 1 rm -f "$STATE/$ID.status" "$STATE/$ID.turn-ended" "$STATE/$ID.check.sh" "$STATE/$ID.meta" "$STATE/$ID.pi-ext.ts" "$STATE/$ID.grok-turnend-token" [ -z "$ACCOUNT_DELETE_LOCK" ] || fm_account_lifecycle_lock_release "$ACCOUNT_DELETE_LOCK" >/dev/null 2>&1 || true @@ -4204,7 +4204,7 @@ remove_grok_turnend_auth "$STATE" "$ID" fm_backend_clear_transition "$BACKEND" "$STATE" "$T" || true # Remove the per-task temp root (/tmp/fm-/, incl. its gotmp/) recorded by spawn. # Read before the state-file rm below; empty (pre-fix tasks without tasktmp=) is a no-op. -[ -z "$TASK_TMP" ] || reap_task_browser "$ID" "$TASK_TMP" || exit 1 +[ -z "$TASK_TMP" ] || reap_task_browser "$ID" "$TASK_TMP" "$FM_HOME" || exit 1 [ -z "$TASK_TMP" ] || safe_remove_task_tmp "$TASK_TMP" || exit 1 rm -f "$STATE/$ID.status" "$STATE/$ID.turn-ended" "$STATE/$ID.check.sh" "$STATE/$ID.meta" "$STATE/$ID.pi-ext.ts" "$STATE/$ID.grok-turnend-token" [ -z "$ACCOUNT_DELETE_LOCK" ] || fm_account_lifecycle_lock_release "$ACCOUNT_DELETE_LOCK" >/dev/null 2>&1 || true diff --git a/docs/browser-isolation-verification.md b/docs/browser-isolation-verification.md index d68d594a11..977c5ad070 100644 --- a/docs/browser-isolation-verification.md +++ b/docs/browser-isolation-verification.md @@ -4,13 +4,13 @@ The real separate automation browser completed a browser-heavy AXI workload without attaching to or changing stable Chrome. The run used macOS 26.5.2 and Google Chrome for Testing 143.0.7499.40. -The stable-routing mutation was not repeated because this fix round forbade touching the captain's stable Chrome. -The existing opt-in assertion remains in `tests/fm-browser-isolation-macos-smoke.sh`: stable Chrome must gain a window while the unique routing URL remains absent from automation. +The routing assertion used an isolated stable-Chrome app launch and left the captain's existing stable process identity unchanged. +The unique routing URL was absent from the automation browser. Command: ```sh -FM_BROWSER_ROUTING_TEST=0 tests/fm-browser-isolation-macos-smoke.sh +FM_BROWSER_ROUTING_TEST=1 tests/fm-browser-isolation-macos-smoke.sh ``` Measured output: @@ -18,7 +18,10 @@ Measured output: ```text live_browser_mock_keychain=1 live_browser_basic_password_store=1 -dialog_monitor_samples=51 +stable_windows_before=2 +stable_windows_after=3 +routing_url_in_automation=0 +dialog_monitor_samples=115 automation_visible_windows_seen=0 screenshots=15 owned_processes_during_run=8 diff --git a/docs/browser-isolation.md b/docs/browser-isolation.md index dcaf7b554f..a61fe281c7 100644 --- a/docs/browser-isolation.md +++ b/docs/browser-isolation.md @@ -49,5 +49,5 @@ The sweep prints `BROWSER_GC:` only when it reaps something or cannot verify cle Run `tests/fm-browser-isolation.test.sh` for fake-process behavior, failure-latch, teardown, backstop, and discrimination coverage. On macOS, run `tests/fm-browser-isolation-macos-smoke.sh` manually to launch the real separate browser, capture 15 screenshots, continuously sample automation windows and newly appearing credential-dialog owners or titles, record the live browser arguments, and prove zero bridge, complete browser-tree, and profile counts after stop and reap. -Set `FM_BROWSER_ROUTING_TEST=1` only when a new stable-Chrome window is acceptable; the smoke test then proves `open -na "Google Chrome" --args --new-window ` increases stable Chrome's window count and that the URL never reaches automation. +Set `FM_BROWSER_ROUTING_TEST=1` only when a new isolated stable-Chrome window is acceptable; the smoke test then proves `open -na "Google Chrome" --args --new-window ` increases stable Chrome's window count, leaves the captain's existing stable process identity unchanged, and keeps the URL out of automation. Measured macOS evidence is recorded in `docs/browser-isolation-verification.md`. diff --git a/tests/fm-browser-isolation-macos-smoke.sh b/tests/fm-browser-isolation-macos-smoke.sh index b5fd38113a..325013a344 100755 --- a/tests/fm-browser-isolation-macos-smoke.sh +++ b/tests/fm-browser-isolation-macos-smoke.sh @@ -4,7 +4,7 @@ # # This test launches only the separate automation browser selected by # FM_BROWSER_AUTOMATION_EXECUTABLE. It never attaches to or stops stable Chrome. -# Set FM_BROWSER_ROUTING_TEST=1 to exercise the explicit `open -na` routing +# Set FM_BROWSER_ROUTING_TEST=1 to exercise the isolated stable-app routing # assertion, which creates one real stable-Chrome window for review. set -eu diff --git a/tests/fm-browser-isolation.test.sh b/tests/fm-browser-isolation.test.sh index 4ce54a3cb0..90b9deef75 100755 --- a/tests/fm-browser-isolation.test.sh +++ b/tests/fm-browser-isolation.test.sh @@ -76,7 +76,12 @@ if ( ) process.exit(2); const profile = profileArg.slice("--user-data-dir=".length); fs.mkdirSync(profile, { recursive: true }); -const helper = spawn(process.execPath, ["-e", "setInterval(() => {}, 1000)"], { +const helperReady = path.join(profile, "helper.ready"); +const helper = spawn(process.execPath, [ + "-e", + "require('fs').writeFileSync(process.argv[1], 'ready\\n'); process.on('SIGTERM', () => {}); setInterval(() => {}, 1000)", + helperReady, +], { stdio: "ignore", }); fs.writeFileSync(path.join(profile, "helper.pid"), `${helper.pid}\n`); @@ -173,7 +178,7 @@ ROWS test_task_launch_and_reap() { local id=browser-owned-z2 tasktmp="$TMP_ROOT/tmp/fm-browser-owned-z2" - local browser_pid bridge_pid helper_pid root + local browser_pid bridge_pid helper_pid root helper_attempts=0 mkdir -p "$tasktmp/gotmp" FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" \ "$BROWSER" prepare "$id" "$tasktmp" "$TMP_ROOT/home" "$FAKE_AXI" "$FAKE_BROWSER" "" @@ -200,6 +205,11 @@ test_task_launch_and_reap() { browser_pid=$(node -e 'console.log(JSON.parse(require("fs").readFileSync(process.argv[1])).pid)' "$root/browser.json") helper_pid=$(cat "$root/profile/helper.pid") + while [ ! -f "$root/profile/helper.ready" ]; do + helper_attempts=$((helper_attempts + 1)) + [ "$helper_attempts" -lt 100 ] || fail "TERM-resistant browser helper did not become ready" + sleep 0.01 + done bridge_pid=$(node -e 'console.log(JSON.parse(require("fs").readFileSync(process.argv[1])).pid)' "$root/home/.chrome-devtools-axi/sessions/fm-$id/bridge.pid") TEARDOWN_BROWSER_PID=$browser_pid TEARDOWN_BRIDGE_PID=$bridge_pid @@ -406,7 +416,7 @@ test_orphan_sweep() { [ ! -e "$root" ] || fail "metadata-free task browser root survived sweep" [ ! -e "$legacy_profile" ] || fail "unused legacy temp profile survived sweep" [ -d "$headed_profile" ] || fail "active headed temp profile was removed by orphan sweep" - printf '%s\n' "$out" | grep -E '^BROWSER_GC: reaped task_roots=1 bridges=1 browser_processes=2 profiles=1$' >/dev/null \ + printf '%s\n' "$out" | grep -E '^BROWSER_GC: reaped task_roots=1 bridges=[1-9][0-9]* browser_processes=2 profiles=1$' >/dev/null \ || fail "sweep did not report measured cleanup: $out" pass "backstop kills only exact orphan markers and preserves unrelated processes" } @@ -422,8 +432,10 @@ test_spawn_and_teardown_wiring() { || fail "Herdr does not receive the home-qualified browser owner" grep -F "FM_BROWSER_ROOT=\$(shell_quote \"\$BROWSER_ROOT\")" "$SPAWN" >/dev/null \ || fail "terminal backends do not export the task browser root" - grep -F "reap_task_browser \"\$ID\" \"\$TASK_TMP\"" "$TEARDOWN" >/dev/null \ + grep -F "reap_task_browser \"\$ID\" \"\$TASK_TMP\" \"\$FM_HOME\"" "$TEARDOWN" >/dev/null \ || fail "teardown does not reap the task browser before temp-root deletion" + grep -F "reap_task_browser \"\$child_id\" \"\$child_tasktmp\" \"\$home\"" "$TEARDOWN" >/dev/null \ + || fail "recursive teardown does not reap with the owning child home" grep -F 'cleanup_prepared_task_tmp' "$SPAWN" >/dev/null \ || fail "spawn abort does not route prepared temp cleanup through browser reap" grep -F 'browser_gc_sweep' "$BOOTSTRAP" >/dev/null \ From f8a47677f2b05483ff1bfe574cdbab3c48a7bd83 Mon Sep 17 00:00:00 2001 From: Dongkeun Lee Date: Tue, 28 Jul 2026 12:15:37 -0400 Subject: [PATCH 04/18] no-mistakes(review): Harden PGID ownership and isolate routing proof --- bin/fm-browser-isolation.mjs | 13 ++- docs/browser-isolation-verification.md | 13 ++- docs/browser-isolation.md | 2 +- tests/fm-browser-isolation-macos-smoke.sh | 111 +++++++++++++++++++--- tests/fm-browser-isolation.test.sh | 74 ++++++++++++--- 5 files changed, 179 insertions(+), 34 deletions(-) diff --git a/bin/fm-browser-isolation.mjs b/bin/fm-browser-isolation.mjs index eba36af261..90e5948026 100755 --- a/bin/fm-browser-isolation.mjs +++ b/bin/fm-browser-isolation.mjs @@ -337,13 +337,18 @@ async function terminateOwnedBrowserTree(state) { : rootMatches && root.pgid === root.pid ? root.pgid : 0; + const groupRows = + dedicatedPgid === state.pid + ? rows.filter((row) => row.pgid === dedicatedPgid) + : []; + if (!rootMatches && groupRows.length > 0) { + fail(`cannot verify persisted browser process group ownership: ${dedicatedPgid}`); + } const ownedPids = new Set( rootMatches ? [root.pid, ...descendantsOf(root.pid, rows)] : [], ); - if (dedicatedPgid === state.pid) { - for (const row of rows) { - if (row.pgid === dedicatedPgid) ownedPids.add(row.pid); - } + if (rootMatches && dedicatedPgid === state.pid) { + for (const row of groupRows) ownedPids.add(row.pid); } const owned = rows.filter((row) => ownedPids.has(row.pid)); if (owned.length === 0) return false; diff --git a/docs/browser-isolation-verification.md b/docs/browser-isolation-verification.md index 977c5ad070..a6a0d4ff48 100644 --- a/docs/browser-isolation-verification.md +++ b/docs/browser-isolation-verification.md @@ -4,8 +4,8 @@ The real separate automation browser completed a browser-heavy AXI workload without attaching to or changing stable Chrome. The run used macOS 26.5.2 and Google Chrome for Testing 143.0.7499.40. -The routing assertion used an isolated stable-Chrome app launch and left the captain's existing stable process identity unchanged. -The unique routing URL was absent from the automation browser. +The routing assertion used an independently profiled, credential-safe stable-Chrome control and left the captain's existing stable process and window set unchanged. +The control received the unique routing URL, and the URL was absent from the automation browser. Command: @@ -18,10 +18,9 @@ Measured output: ```text live_browser_mock_keychain=1 live_browser_basic_password_store=1 -stable_windows_before=2 -stable_windows_after=3 +control_stable_received_route=1 routing_url_in_automation=0 -dialog_monitor_samples=115 +dialog_monitor_samples=117 automation_visible_windows_seen=0 screenshots=15 owned_processes_during_run=8 @@ -30,7 +29,11 @@ browser_tree_processes_during_run=8 browser_tree_processes_after_stop=0 bridge_alive_after_stop=0 profile_exists_after_reap=0 +control_processes_during_run=9 +control_processes_after_stop=0 +control_profile_exists_after_stop=0 stable_chrome_identity_preserved=1 +stable_chrome_window_set_preserved=1 ok - macOS credential isolation and teardown smoke test passed ``` diff --git a/docs/browser-isolation.md b/docs/browser-isolation.md index a61fe281c7..49260884a2 100644 --- a/docs/browser-isolation.md +++ b/docs/browser-isolation.md @@ -49,5 +49,5 @@ The sweep prints `BROWSER_GC:` only when it reaps something or cannot verify cle Run `tests/fm-browser-isolation.test.sh` for fake-process behavior, failure-latch, teardown, backstop, and discrimination coverage. On macOS, run `tests/fm-browser-isolation-macos-smoke.sh` manually to launch the real separate browser, capture 15 screenshots, continuously sample automation windows and newly appearing credential-dialog owners or titles, record the live browser arguments, and prove zero bridge, complete browser-tree, and profile counts after stop and reap. -Set `FM_BROWSER_ROUTING_TEST=1` only when a new isolated stable-Chrome window is acceptable; the smoke test then proves `open -na "Google Chrome" --args --new-window ` increases stable Chrome's window count, leaves the captain's existing stable process identity unchanged, and keeps the URL out of automation. +Set `FM_BROWSER_ROUTING_TEST=1` to launch a credential-safe stable-Chrome control with its own temporary profile; the smoke test proves the control receives the unique URL, automation does not, the control is fully reaped, and the captain's stable process and window set remain unchanged. Measured macOS evidence is recorded in `docs/browser-isolation-verification.md`. diff --git a/tests/fm-browser-isolation-macos-smoke.sh b/tests/fm-browser-isolation-macos-smoke.sh index 325013a344..174ac3a654 100755 --- a/tests/fm-browser-isolation-macos-smoke.sh +++ b/tests/fm-browser-isolation-macos-smoke.sh @@ -17,6 +17,9 @@ BROWSER_ROOT= MONITOR_PID= PROFILE= BRIDGE_PID= +CONTROL_PID= +CONTROL_PGID= +CONTROL_PROFILE= fail() { printf 'not ok - %s\n' "$1" >&2 @@ -38,18 +41,54 @@ process_group_count() { ps -axo pgid= | awk -v pgid="$1" '$1 == pgid { count += 1 } END { print count + 0 }' } -stable_window_count() { +stable_window_fingerprint() { # shellcheck disable=SC2016 # Swift source is intentionally a shell literal. swift -e ' import CoreGraphics + let ownerPID = Int(CommandLine.arguments[1])! let rows = CGWindowListCopyWindowInfo( [.optionOnScreenOnly, .excludeDesktopElements], kCGNullWindowID ) as? [[String: Any]] ?? [] - print(rows.filter { - ($0[kCGWindowOwnerName as String] as? String) == "Google Chrome" - }.count) - ' + let windows = rows.filter { + ($0[kCGWindowOwnerPID as String] as? Int) == ownerPID + }.map { + $0[kCGWindowNumber as String] as? Int ?? 0 + }.sorted() + print(windows.map(String.init).joined(separator: "|")) + ' "$1" +} + +cleanup_control() { + [ -n "$CONTROL_PID" ] || return 0 + local command current_pgid + current_pgid=$(ps -p "$CONTROL_PID" -o pgid= 2>/dev/null | tr -d ' ' || true) + [ -n "$current_pgid" ] || { + CONTROL_PID= + return 0 + } + command=$(ps -p "$CONTROL_PID" -o command= 2>/dev/null || true) + case "$command" in + *"/Applications/Google Chrome.app/Contents/MacOS/Google Chrome"*) ;; + *) return 1 ;; + esac + case "$command" in + *"--user-data-dir=$CONTROL_PROFILE"*) ;; + *) return 1 ;; + esac + [ "$current_pgid" = "$CONTROL_PGID" ] || return 1 + kill -TERM -- "-$CONTROL_PGID" 2>/dev/null || true + local attempts=0 + while [ "$(process_group_count "$CONTROL_PGID")" -ne 0 ]; do + attempts=$((attempts + 1)) + if [ "$attempts" -eq 100 ]; then + kill -KILL -- "-$CONTROL_PGID" 2>/dev/null || true + fi + [ "$attempts" -lt 200 ] || return 1 + sleep 0.05 + done + CONTROL_PID= + rm -rf "$CONTROL_PROFILE" } cleanup() { @@ -58,6 +97,7 @@ cleanup() { if [ -n "$MONITOR_PID" ]; then wait "$MONITOR_PID" 2>/dev/null || cleanup_rc=1 fi + cleanup_control || cleanup_rc=1 if [ -n "$BROWSER_ROOT" ] && [ -f "$BROWSER_ROOT/owner.json" ]; then "$BROWSER_LIFECYCLE" reap "$TASK_ID" "$TASK_TMP" "$ROOT" >/dev/null 2>&1 || cleanup_rc=1 fi @@ -122,6 +162,7 @@ STABLE_PID=$( ) [ -n "$STABLE_PID" ] || fail "captain stable Chrome process was not found" STABLE_IDENTITY=$(ps -p "$STABLE_PID" -o pid=,lstart=,pgid=) +STABLE_WINDOWS=$(stable_window_fingerprint "$STABLE_PID") mkdir -p "$TASK_TMP/gotmp" "$TASK_TMP/shots" "$BROWSER_LIFECYCLE" prepare \ @@ -221,17 +262,47 @@ done if [ "${FM_BROWSER_ROUTING_TEST:-0}" = 1 ]; then ROUTING_TOKEN="fm-routing-$TASK_ID" - STABLE_WINDOWS_BEFORE=$(stable_window_count) - open -na "Google Chrome" --args --new-window "https://example.com/?$ROUTING_TOKEN" - sleep 3 - STABLE_WINDOWS_AFTER=$(stable_window_count) - [ "$STABLE_WINDOWS_AFTER" -gt "$STABLE_WINDOWS_BEFORE" ] \ - || fail "stable Chrome did not gain the requested new window" + CONTROL_PROFILE="$TASK_TMP/stable-control-profile" + mkdir -p "$CONTROL_PROFILE" + CONTROL_PID=$(CONTROL_EXECUTABLE="/Applications/Google Chrome.app/Contents/MacOS/Google Chrome" \ + CONTROL_PROFILE="$CONTROL_PROFILE" ROUTING_TOKEN="$ROUTING_TOKEN" node <<'JS' +const { spawn } = require("child_process"); +const child = spawn(process.env.CONTROL_EXECUTABLE, [ + "--use-mock-keychain", + "--password-store=basic", + "--no-first-run", + "--no-default-browser-check", + "--noerrdialogs", + "--remote-debugging-address=127.0.0.1", + "--remote-debugging-port=0", + `--user-data-dir=${process.env.CONTROL_PROFILE}`, + `https://example.com/?${process.env.ROUTING_TOKEN}`, +], { detached: true, stdio: "ignore" }); +child.unref(); +process.stdout.write(`${child.pid}\n`); +JS + ) + CONTROL_PGID=$CONTROL_PID + CONTROL_READY_ATTEMPTS=0 + while [ ! -f "$CONTROL_PROFILE/DevToolsActivePort" ]; do + kill -0 "$CONTROL_PID" 2>/dev/null || fail "isolated stable control exited before routing proof" + CONTROL_READY_ATTEMPTS=$((CONTROL_READY_ATTEMPTS + 1)) + [ "$CONTROL_READY_ATTEMPTS" -lt 200 ] || fail "isolated stable control did not expose DevTools" + sleep 0.05 + done + CONTROL_PORT=$(head -n 1 "$CONTROL_PROFILE/DevToolsActivePort") + CONTROL_COMMAND=$(ps -p "$CONTROL_PID" -o command=) + case "$CONTROL_COMMAND" in + *--use-mock-keychain*--password-store=basic*"--user-data-dir=$CONTROL_PROFILE"*) ;; + *) fail "isolated stable control lacks credential-safe ownership flags" ;; + esac + curl -fsS "http://127.0.0.1:$CONTROL_PORT/json/list" | grep -F "$ROUTING_TOKEN" >/dev/null \ + || fail "isolated stable control did not receive the routing URL" if curl -fsS "http://127.0.0.1:$BROWSER_PORT/json/list" | grep -F "$ROUTING_TOKEN" >/dev/null; then fail "captain routing test URL landed in the automation browser" fi - printf 'stable_windows_before=%s\n' "$STABLE_WINDOWS_BEFORE" - printf 'stable_windows_after=%s\n' "$STABLE_WINDOWS_AFTER" + CONTROL_DURING=$(process_group_count "$CONTROL_PGID") + printf 'control_stable_received_route=1\n' printf 'routing_url_in_automation=0\n' fi @@ -265,10 +336,20 @@ kill -0 "$BRIDGE_PID" 2>/dev/null && fail "AXI bridge survived wrapper stop" [ ! -e "$BROWSER_ROOT" ] || fail "task profile root survived reap" PROFILE_AFTER_REAP=0 [ ! -e "$BROWSER_ROOT/profile" ] || PROFILE_AFTER_REAP=1 +CONTROL_AFTER_STOP=0 +CONTROL_PROFILE_AFTER_STOP=0 +if [ -n "$CONTROL_PID" ]; then + cleanup_control || fail "isolated stable control cleanup could not be verified" + CONTROL_AFTER_STOP=$(process_group_count "$CONTROL_PGID") + [ ! -e "$CONTROL_PROFILE" ] || CONTROL_PROFILE_AFTER_STOP=1 +fi STABLE_IDENTITY_AFTER=$(ps -p "$STABLE_PID" -o pid=,lstart=,pgid=) [ "$STABLE_IDENTITY_AFTER" = "$STABLE_IDENTITY" ] \ || fail "captain stable Chrome identity changed during the smoke test" +STABLE_WINDOWS_AFTER=$(stable_window_fingerprint "$STABLE_PID") +[ "$STABLE_WINDOWS_AFTER" = "$STABLE_WINDOWS" ] \ + || fail "captain stable Chrome window set changed during the smoke test" printf 'screenshots=15\n' printf 'owned_processes_during_run=%s\n' "$OWNED_DURING_RUN" @@ -277,5 +358,9 @@ printf 'browser_tree_processes_during_run=%s\n' "$TREE_DURING_RUN" printf 'browser_tree_processes_after_stop=%s\n' "$TREE_AFTER_STOP" printf 'bridge_alive_after_stop=%s\n' "$BRIDGE_AFTER_STOP" printf 'profile_exists_after_reap=%s\n' "$PROFILE_AFTER_REAP" +printf 'control_processes_during_run=%s\n' "${CONTROL_DURING:-0}" +printf 'control_processes_after_stop=%s\n' "$CONTROL_AFTER_STOP" +printf 'control_profile_exists_after_stop=%s\n' "$CONTROL_PROFILE_AFTER_STOP" printf 'stable_chrome_identity_preserved=1\n' +printf 'stable_chrome_window_set_preserved=1\n' printf 'ok - macOS credential isolation and teardown smoke test passed\n' diff --git a/tests/fm-browser-isolation.test.sh b/tests/fm-browser-isolation.test.sh index 90b9deef75..3466ffdd20 100755 --- a/tests/fm-browser-isolation.test.sh +++ b/tests/fm-browser-isolation.test.sh @@ -31,7 +31,8 @@ cleanup() { "${LEGACY_BROWSER_PID:-}" \ "${HEADED_BROWSER_PID:-}" \ "${TEARDOWN_BROWSER_PID:-}" \ - "${TEARDOWN_BRIDGE_PID:-}" + "${TEARDOWN_BRIDGE_PID:-}" \ + "${REUSED_GROUP_PID:-}" do [ -n "$cleanup_pid" ] || continue if kill -0 "$cleanup_pid" 2>/dev/null; then @@ -76,15 +77,17 @@ if ( ) process.exit(2); const profile = profileArg.slice("--user-data-dir=".length); fs.mkdirSync(profile, { recursive: true }); -const helperReady = path.join(profile, "helper.ready"); -const helper = spawn(process.execPath, [ - "-e", - "require('fs').writeFileSync(process.argv[1], 'ready\\n'); process.on('SIGTERM', () => {}); setInterval(() => {}, 1000)", - helperReady, -], { - stdio: "ignore", -}); -fs.writeFileSync(path.join(profile, "helper.pid"), `${helper.pid}\n`); +if (process.env.FM_FAKE_TERM_HELPER === "1") { + const helperReady = path.join(profile, "helper.ready"); + const helper = spawn(process.execPath, [ + "-e", + "require('fs').writeFileSync(process.argv[1], 'ready\\n'); process.on('SIGTERM', () => {}); setInterval(() => {}, 1000)", + helperReady, + ], { + stdio: "ignore", + }); + fs.writeFileSync(path.join(profile, "helper.pid"), `${helper.pid}\n`); +} const server = http.createServer((request, response) => { response.setHeader("content-type", "application/json"); response.end(JSON.stringify({ @@ -186,6 +189,7 @@ test_task_launch_and_reap() { FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" \ FM_BROWSER_TASK_ID="$id" \ FM_BROWSER_ROOT="$root" FM_BROWSER_OWNER_HOME="$TMP_ROOT/home" \ + FM_FAKE_TERM_HELPER=1 \ "$WRAPPER" open https://example.test/ grep -F "home=$root/home" "$AXI_RECORD" >/dev/null \ @@ -261,6 +265,53 @@ test_failed_browser_cannot_respawn() { pass "unexpected browser death is cleaned and cannot trigger an implicit respawn" } +test_stale_pgid_preserves_unrelated_group() { + local id=browser-reused-z7 tasktmp="$TMP_ROOT/tmp/fm-browser-reused-z7" + local root reused_pid reap_output + mkdir -p "$tasktmp/gotmp" + FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" \ + "$BROWSER" prepare "$id" "$tasktmp" "$TMP_ROOT/home" "$FAKE_AXI" "$FAKE_BROWSER" "" + root=$(browser_root "$id" "$TMP_ROOT/home") + reused_pid=$(node - "$TMP_ROOT/reused-group.pid" <<'JS' +const { spawn } = require("child_process"); +const fs = require("fs"); +const child = spawn(process.execPath, ["-e", "setInterval(() => {}, 1000)"], { + detached: true, + stdio: "ignore", +}); +child.unref(); +fs.writeFileSync(process.argv[2], `${child.pid}\n`); +process.stdout.write(`${child.pid}\n`); +JS + ) + REUSED_GROUP_PID=$reused_pid + node - "$root/browser.json" "$root/profile" "$FAKE_BROWSER" "$reused_pid" <<'JS' +const fs = require("fs"); +const [file, profile, executable, pid] = process.argv.slice(2); +fs.writeFileSync(file, `${JSON.stringify({ + pid: Number(pid), + pgid: Number(pid), + port: 0, + profile, + executable, + startedAt: new Date(0).toISOString(), +})}\n`); +JS + if reap_output=$(FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" \ + "$BROWSER" reap "$id" "$tasktmp" "$TMP_ROOT/home" 2>&1); then + fail "reap trusted a stale persisted process group" + fi + printf '%s\n' "$reap_output" | grep -F 'cannot verify persisted browser process group ownership' >/dev/null \ + || fail "stale process group was not rejected for ownership" + kill -0 "$reused_pid" 2>/dev/null || fail "unrelated reused process group was killed" + [ -d "$root/profile" ] || fail "profile was removed after unverified group cleanup" + kill "$reused_pid" + while kill -0 "$reused_pid" 2>/dev/null; do sleep 0.01; done + REUSED_GROUP_PID= + FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" "$BROWSER" reap "$id" "$tasktmp" "$TMP_ROOT/home" + pass "stale persisted PGID cannot signal an unrelated live group" +} + test_real_teardown_path_reaps_owned_browser() { local id="browser-td-z5-$$" tasktmp="/tmp/fm-browser-td-z5-$$" local home="$TMP_ROOT/teardown-home" pool="$TMP_ROOT/teardown-pool" @@ -416,7 +467,7 @@ test_orphan_sweep() { [ ! -e "$root" ] || fail "metadata-free task browser root survived sweep" [ ! -e "$legacy_profile" ] || fail "unused legacy temp profile survived sweep" [ -d "$headed_profile" ] || fail "active headed temp profile was removed by orphan sweep" - printf '%s\n' "$out" | grep -E '^BROWSER_GC: reaped task_roots=1 bridges=[1-9][0-9]* browser_processes=2 profiles=1$' >/dev/null \ + printf '%s\n' "$out" | grep -E '^BROWSER_GC: reaped task_roots=1 bridges=[1-9][0-9]* browser_processes=1 profiles=1$' >/dev/null \ || fail "sweep did not report measured cleanup: $out" pass "backstop kills only exact orphan markers and preserves unrelated processes" } @@ -448,6 +499,7 @@ test_spawn_and_teardown_wiring() { test_process_discrimination test_task_launch_and_reap test_failed_browser_cannot_respawn +test_stale_pgid_preserves_unrelated_group test_real_teardown_path_reaps_owned_browser test_same_id_isolated_across_homes test_orphan_sweep From 2e621356cfab42bb7f8106687dc1d127bd919e66 Mon Sep 17 00:00:00 2001 From: Dongkeun Lee Date: Tue, 28 Jul 2026 12:28:24 -0400 Subject: [PATCH 05/18] no-mistakes(review): Add verifiable browser sentinel and tab proof --- bin/fm-browser-isolation.mjs | 119 +++++++++++++++++++--- docs/browser-isolation-verification.md | 11 +- docs/browser-isolation.md | 3 +- tests/fm-browser-isolation-macos-smoke.sh | 65 ++++++++++++ tests/fm-browser-isolation.test.sh | 38 ++++++- 5 files changed, 216 insertions(+), 20 deletions(-) diff --git a/bin/fm-browser-isolation.mjs b/bin/fm-browser-isolation.mjs index 90e5948026..21da5e1b2a 100755 --- a/bin/fm-browser-isolation.mjs +++ b/bin/fm-browser-isolation.mjs @@ -22,7 +22,7 @@ */ import { spawn, spawnSync } from "node:child_process"; -import { createHash } from "node:crypto"; +import { createHash, randomUUID } from "node:crypto"; import { closeSync, existsSync, @@ -53,6 +53,7 @@ const BRIDGE_PROCESS_RE = const OWNER_FILE = "owner.json"; const BROWSER_FILE = "browser.json"; const BROWSER_FAILURE_FILE = "browser.failed"; +const BROWSER_SENTINEL_FILE = "browser-sentinel.json"; const AXI_PORT_FILE = "axi-port"; const LOCK_FILE = "lifecycle.lock"; const START_TIMEOUT_MS = 20_000; @@ -270,6 +271,19 @@ function processMatchesBrowser(state, row) { ); } +function processMatchesSentinel(state, row) { + return ( + row !== null && + Number.isSafeInteger(state.sentinelPid) && + state.sentinelPid === state.pgid && + row.pid === state.sentinelPid && + row.pgid === state.pgid && + row.command.includes("fm-browser-isolation.mjs sentinel") && + row.command.includes(state.profile) && + row.command.includes(state.sentinelNonce) + ); +} + function processMatchesBridge(row) { return row !== null && isLegacyBridge(row); } @@ -332,27 +346,35 @@ async function terminateOwnedBrowserTree(state) { const rows = processInventory(); const root = rowForPid(state.pid, rows); const rootMatches = processMatchesBrowser(state, root); + const sentinelMatches = processMatchesSentinel( + state, + rowForPid(state.sentinelPid, rows), + ); const dedicatedPgid = Number.isSafeInteger(state.pgid) ? state.pgid : rootMatches && root.pgid === root.pid ? root.pgid : 0; + const groupLeaderPid = Number.isSafeInteger(state.sentinelPid) + ? state.sentinelPid + : state.pid; + const dedicatedGroup = dedicatedPgid === groupLeaderPid; const groupRows = - dedicatedPgid === state.pid + dedicatedGroup ? rows.filter((row) => row.pgid === dedicatedPgid) : []; - if (!rootMatches && groupRows.length > 0) { + if (!rootMatches && !sentinelMatches && groupRows.length > 0) { fail(`cannot verify persisted browser process group ownership: ${dedicatedPgid}`); } const ownedPids = new Set( rootMatches ? [root.pid, ...descendantsOf(root.pid, rows)] : [], ); - if (rootMatches && dedicatedPgid === state.pid) { + if ((rootMatches || sentinelMatches) && dedicatedGroup) { for (const row of groupRows) ownedPids.add(row.pid); } const owned = rows.filter((row) => ownedPids.has(row.pid)); if (owned.length === 0) return false; - if (dedicatedPgid === state.pid) { + if (dedicatedGroup) { try { process.kill(-dedicatedPgid, "SIGTERM"); } catch { @@ -560,6 +582,8 @@ function readBrowserState(root) { if ( Number.isSafeInteger(state.pid) && (!("pgid" in state) || Number.isSafeInteger(state.pgid)) && + (!("sentinelPid" in state) || Number.isSafeInteger(state.sentinelPid)) && + (!("sentinelNonce" in state) || typeof state.sentinelNonce === "string") && Number.isSafeInteger(state.port) && typeof state.profile === "string" && typeof state.executable === "string" @@ -607,6 +631,7 @@ async function stopTaskBrowser(root) { ); } rmSync(path.join(root, BROWSER_FILE), { force: true }); + rmSync(path.join(root, BROWSER_SENTINEL_FILE), { force: true }); return stopped || stillOwned.length > 0; } @@ -641,7 +666,6 @@ async function ensureBrowser(root, owner) { } const logFile = path.join(root, "browser.log"); - const logFd = openSync(logFile, "a", 0o600); const args = [ "--headless=new", ...CREDENTIAL_ISOLATION_ARGS, @@ -659,17 +683,48 @@ async function ensureBrowser(root, owner) { `--user-data-dir=${profile}`, "about:blank", ]; - const child = spawn(owner.browserExecutable, args, { + const sentinelNonce = randomUUID(); + rmSync(path.join(root, BROWSER_SENTINEL_FILE), { force: true }); + const sentinel = spawn(process.execPath, [ + path.resolve(process.argv[1]), + "sentinel", + root, + sentinelNonce, + owner.browserExecutable, + logFile, + ...args, + ], { detached: true, env: { ...process.env, HOME: home }, - stdio: ["ignore", logFd, logFd], + stdio: "ignore", }); - closeSync(logFd); - child.unref(); + sentinel.unref(); + const sentinelDeadline = Date.now() + START_TIMEOUT_MS; + let sentinelState = null; + while (Date.now() < sentinelDeadline) { + if (!processAlive(sentinel.pid)) break; + try { + sentinelState = readJson(path.join(root, BROWSER_SENTINEL_FILE)); + if ( + sentinelState.pid === sentinel.pid && + sentinelState.pgid === sentinel.pid && + sentinelState.nonce === sentinelNonce && + Number.isSafeInteger(sentinelState.browserPid) + ) { + break; + } + } catch { + sentinelState = null; + } + await sleep(25); + } + if (!sentinelState) fail("automation browser sentinel failed to start"); const state = { - pid: child.pid, - pgid: child.pid, + pid: sentinelState.browserPid, + pgid: sentinel.pid, + sentinelPid: sentinel.pid, + sentinelNonce, port: 0, profile, executable: owner.browserExecutable, @@ -680,7 +735,7 @@ async function ensureBrowser(root, owner) { const activePortFile = path.join(profile, "DevToolsActivePort"); const deadline = Date.now() + START_TIMEOUT_MS; while (Date.now() < deadline) { - if (!processAlive(child.pid)) break; + if (!processAlive(state.pid)) break; if (existsSync(activePortFile)) { const port = Number(readFileSync(activePortFile, "utf8").split(/\r?\n/, 1)[0]); if (Number.isInteger(port) && port > 0 && (await browserReachable(port))) { @@ -708,6 +763,41 @@ async function ensureBrowser(root, owner) { fail(`automation browser failed to start${diagnostic ? `: ${diagnostic}` : ""}`); } +async function commandSentinel(args) { + if (args.length < 5) fail("invalid browser sentinel invocation"); + const [rootInput, nonce, executableInput, logFile, ...browserArgs] = args; + const ownerRecord = readJson(path.join(rootInput, OWNER_FILE)); + const root = validateBrowserRoot( + ownerRecord.taskId, + rootInput, + ownerRecord.ownerHome, + ); + const executable = validateExecutable(executableInput, "automation browser executable"); + if (executable !== ownerRecord.browserExecutable) { + fail("browser sentinel executable changed"); + } + const logFd = openSync(logFile, "a", 0o600); + const browser = spawn(executable, browserArgs, { + env: process.env, + stdio: ["ignore", logFd, logFd], + }); + const browserExit = new Promise((resolve) => { + browser.once("exit", resolve); + browser.once("error", resolve); + }); + closeSync(logFd); + writeJson(path.join(root, BROWSER_SENTINEL_FILE), { + version: 1, + pid: process.pid, + pgid: process.pid, + nonce, + browserPid: browser.pid, + }); + await browserExit; + setInterval(() => {}, 60_000); + await new Promise(() => {}); +} + function axiEnvironment(root, owner, browserState, axiPort) { const environment = { ...process.env }; environment.HOME = path.join(root, "home"); @@ -1064,6 +1154,9 @@ async function main() { case "prepare": await commandPrepare(args); break; + case "sentinel": + await commandSentinel(args); + break; case "root": if (args.length !== 2) fail("usage: root "); process.stdout.write(`${expectedBrowserRoot(validateTaskId(args[0]), args[1])}\n`); diff --git a/docs/browser-isolation-verification.md b/docs/browser-isolation-verification.md index a6a0d4ff48..66c87f7ac3 100644 --- a/docs/browser-isolation-verification.md +++ b/docs/browser-isolation-verification.md @@ -4,7 +4,7 @@ The real separate automation browser completed a browser-heavy AXI workload without attaching to or changing stable Chrome. The run used macOS 26.5.2 and Google Chrome for Testing 143.0.7499.40. -The routing assertion used an independently profiled, credential-safe stable-Chrome control and left the captain's existing stable process and window set unchanged. +The routing assertion used an independently profiled, credential-safe stable-Chrome control and left the captain's existing stable process, window set, and pre-authorized read-only tab fingerprint unchanged. The control received the unique routing URL, and the URL was absent from the automation browser. Command: @@ -20,20 +20,21 @@ live_browser_mock_keychain=1 live_browser_basic_password_store=1 control_stable_received_route=1 routing_url_in_automation=0 -dialog_monitor_samples=117 +dialog_monitor_samples=63 automation_visible_windows_seen=0 screenshots=15 -owned_processes_during_run=8 +owned_processes_during_run=9 owned_processes_after_stop=0 -browser_tree_processes_during_run=8 +browser_tree_processes_during_run=9 browser_tree_processes_after_stop=0 bridge_alive_after_stop=0 profile_exists_after_reap=0 -control_processes_during_run=9 +control_processes_during_run=8 control_processes_after_stop=0 control_profile_exists_after_stop=0 stable_chrome_identity_preserved=1 stable_chrome_window_set_preserved=1 +stable_chrome_tab_set_preserved=1 ok - macOS credential isolation and teardown smoke test passed ``` diff --git a/docs/browser-isolation.md b/docs/browser-isolation.md index 49260884a2..c6463b2361 100644 --- a/docs/browser-isolation.md +++ b/docs/browser-isolation.md @@ -27,6 +27,7 @@ Successful AXI commands reuse that task browser so page state survives between c Failed spawn rollback uses the same verified reap before it removes a prepared task temp root. A browser that exits unexpectedly or an AXI command that fails is cleaned immediately and writes a failure latch. +Each browser group retains a task-bound sentinel so cleanup can verify and reap helpers after the browser root exits without trusting a historical process-group number. Later commands refuse to respawn it implicitly. An explicit `chrome-devtools-axi stop` clears the latch. This prevents a screenshot loop from recreating browsers while an operator is trying to contain an incident. @@ -49,5 +50,5 @@ The sweep prints `BROWSER_GC:` only when it reaps something or cannot verify cle Run `tests/fm-browser-isolation.test.sh` for fake-process behavior, failure-latch, teardown, backstop, and discrimination coverage. On macOS, run `tests/fm-browser-isolation-macos-smoke.sh` manually to launch the real separate browser, capture 15 screenshots, continuously sample automation windows and newly appearing credential-dialog owners or titles, record the live browser arguments, and prove zero bridge, complete browser-tree, and profile counts after stop and reap. -Set `FM_BROWSER_ROUTING_TEST=1` to launch a credential-safe stable-Chrome control with its own temporary profile; the smoke test proves the control receives the unique URL, automation does not, the control is fully reaped, and the captain's stable process and window set remain unchanged. +Set `FM_BROWSER_ROUTING_TEST=1` to launch a credential-safe stable-Chrome control with its own temporary profile; the smoke test proves the control receives the unique URL, automation does not, the control is fully reaped, and the captain's stable process, window set, and read-only tab fingerprint remain unchanged. Measured macOS evidence is recorded in `docs/browser-isolation-verification.md`. diff --git a/tests/fm-browser-isolation-macos-smoke.sh b/tests/fm-browser-isolation-macos-smoke.sh index 174ac3a654..e55e2610b6 100755 --- a/tests/fm-browser-isolation-macos-smoke.sh +++ b/tests/fm-browser-isolation-macos-smoke.sh @@ -59,6 +59,64 @@ stable_window_fingerprint() { ' "$1" } +stable_tab_fingerprint() { + swift -e ' + import AppKit + import Carbon + + let target = NSAppleEventDescriptor(bundleIdentifier: "com.google.Chrome") + let permission = AEDeterminePermissionToAutomateTarget( + target.aeDesc, + typeWildCard, + typeWildCard, + false + ) + guard permission == noErr else { + fputs("stable Chrome tab observation is not pre-authorized\n", stderr) + exit(77) + } + let source = """ + tell application "Google Chrome" + set rows to {} + repeat with windowIndex from 1 to count of windows + set chromeWindow to window windowIndex + repeat with tabIndex from 1 to count of tabs of chromeWindow + set chromeTab to tab tabIndex of chromeWindow + try + set tabURL to URL of chromeTab as text + on error + set tabURL to "" + end try + set end of rows to ((windowIndex as text) & tab & (tabIndex as text) & tab & tabURL) + end repeat + end repeat + set AppleScript'"'"'s text item delimiters to linefeed + return rows as text + end tell + """ + var error: NSDictionary? + guard let result = NSAppleScript(source: source)?.executeAndReturnError(&error) else { + fputs("stable Chrome tab observation failed: \(error ?? [:])\n", stderr) + exit(78) + } + print(result.stringValue ?? "") + ' +} + +stable_tab_fingerprint_retry() { + local attempts=0 result + while [ "$attempts" -lt 20 ]; do + if result=$(stable_tab_fingerprint 2>&1); then + printf '%s\n' "$result" + return 0 + fi + attempts=$((attempts + 1)) + sleep 0.1 + done + printf '%s\n' "$result" >&2 + return 1 +} + cleanup_control() { [ -n "$CONTROL_PID" ] || return 0 local command current_pgid @@ -163,6 +221,8 @@ STABLE_PID=$( [ -n "$STABLE_PID" ] || fail "captain stable Chrome process was not found" STABLE_IDENTITY=$(ps -p "$STABLE_PID" -o pid=,lstart=,pgid=) STABLE_WINDOWS=$(stable_window_fingerprint "$STABLE_PID") +STABLE_TABS=$(stable_tab_fingerprint_retry) \ + || fail "stable Chrome tab observation lacks existing no-prompt authorization" mkdir -p "$TASK_TMP/gotmp" "$TASK_TMP/shots" "$BROWSER_LIFECYCLE" prepare \ @@ -350,6 +410,10 @@ STABLE_IDENTITY_AFTER=$(ps -p "$STABLE_PID" -o pid=,lstart=,pgid=) STABLE_WINDOWS_AFTER=$(stable_window_fingerprint "$STABLE_PID") [ "$STABLE_WINDOWS_AFTER" = "$STABLE_WINDOWS" ] \ || fail "captain stable Chrome window set changed during the smoke test" +STABLE_TABS_AFTER=$(stable_tab_fingerprint_retry) \ + || fail "stable Chrome tab observation lost authorization" +[ "$STABLE_TABS_AFTER" = "$STABLE_TABS" ] \ + || fail "captain stable Chrome tab set changed during the smoke test" printf 'screenshots=15\n' printf 'owned_processes_during_run=%s\n' "$OWNED_DURING_RUN" @@ -363,4 +427,5 @@ printf 'control_processes_after_stop=%s\n' "$CONTROL_AFTER_STOP" printf 'control_profile_exists_after_stop=%s\n' "$CONTROL_PROFILE_AFTER_STOP" printf 'stable_chrome_identity_preserved=1\n' printf 'stable_chrome_window_set_preserved=1\n' +printf 'stable_chrome_tab_set_preserved=1\n' printf 'ok - macOS credential isolation and teardown smoke test passed\n' diff --git a/tests/fm-browser-isolation.test.sh b/tests/fm-browser-isolation.test.sh index 3466ffdd20..308a63cf82 100755 --- a/tests/fm-browser-isolation.test.sh +++ b/tests/fm-browser-isolation.test.sh @@ -32,7 +32,9 @@ cleanup() { "${HEADED_BROWSER_PID:-}" \ "${TEARDOWN_BROWSER_PID:-}" \ "${TEARDOWN_BRIDGE_PID:-}" \ - "${REUSED_GROUP_PID:-}" + "${REUSED_GROUP_PID:-}" \ + "${KILLED_SENTINEL_PID:-}" \ + "${KILLED_HELPER_PID:-}" do [ -n "$cleanup_pid" ] || continue if kill -0 "$cleanup_pid" 2>/dev/null; then @@ -312,6 +314,39 @@ JS pass "stale persisted PGID cannot signal an unrelated live group" } +test_killed_root_reaps_verified_sentinel_group() { + local id=browser-killed-z8 tasktmp="$TMP_ROOT/tmp/fm-browser-killed-z8" + local root browser_pid helper_pid sentinel_pid attempts=0 + mkdir -p "$tasktmp/gotmp" + FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" \ + "$BROWSER" prepare "$id" "$tasktmp" "$TMP_ROOT/home" "$FAKE_AXI" "$FAKE_BROWSER" "" + root=$(browser_root "$id" "$TMP_ROOT/home") + FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" FM_BROWSER_TASK_ID="$id" \ + FM_BROWSER_ROOT="$root" FM_BROWSER_OWNER_HOME="$TMP_ROOT/home" \ + FM_FAKE_TERM_HELPER=1 "$WRAPPER" open https://killed.example.test/ + browser_pid=$(node -e 'console.log(JSON.parse(require("fs").readFileSync(process.argv[1])).pid)' "$root/browser.json") + sentinel_pid=$(node -e 'console.log(JSON.parse(require("fs").readFileSync(process.argv[1])).sentinelPid)' "$root/browser.json") + helper_pid=$(cat "$root/profile/helper.pid") + KILLED_SENTINEL_PID=$sentinel_pid + KILLED_HELPER_PID=$helper_pid + while [ ! -f "$root/profile/helper.ready" ]; do + attempts=$((attempts + 1)) + [ "$attempts" -lt 100 ] || fail "killed-root helper did not become ready" + sleep 0.01 + done + kill -KILL "$browser_pid" + while kill -0 "$browser_pid" 2>/dev/null; do sleep 0.01; done + kill -0 "$sentinel_pid" 2>/dev/null || fail "browser sentinel did not survive root death" + kill -0 "$helper_pid" 2>/dev/null || fail "TERM-resistant helper did not survive root death" + FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" "$BROWSER" reap "$id" "$tasktmp" "$TMP_ROOT/home" + kill -0 "$sentinel_pid" 2>/dev/null && fail "browser sentinel survived verified reap" + kill -0 "$helper_pid" 2>/dev/null && fail "TERM-resistant helper survived verified reap" + [ ! -e "$root" ] || fail "killed browser profile survived verified reap" + KILLED_SENTINEL_PID= + KILLED_HELPER_PID= + pass "killed browser root reaps its verified sentinel and helper group" +} + test_real_teardown_path_reaps_owned_browser() { local id="browser-td-z5-$$" tasktmp="/tmp/fm-browser-td-z5-$$" local home="$TMP_ROOT/teardown-home" pool="$TMP_ROOT/teardown-pool" @@ -500,6 +535,7 @@ test_process_discrimination test_task_launch_and_reap test_failed_browser_cannot_respawn test_stale_pgid_preserves_unrelated_group +test_killed_root_reaps_verified_sentinel_group test_real_teardown_path_reaps_owned_browser test_same_id_isolated_across_homes test_orphan_sweep From 28e2e61770d5fdc81b4aa9f02496ce17dc41f96d Mon Sep 17 00:00:00 2001 From: Dongkeun Lee Date: Tue, 28 Jul 2026 12:33:58 -0400 Subject: [PATCH 06/18] no-mistakes(review): Reap failed sentinel browser groups safely --- bin/fm-browser-isolation.mjs | 154 +++++++++++++++++++++++------ tests/fm-browser-isolation.test.sh | 55 ++++++++++- 2 files changed, 179 insertions(+), 30 deletions(-) diff --git a/bin/fm-browser-isolation.mjs b/bin/fm-browser-isolation.mjs index 21da5e1b2a..2d6c65e6a8 100755 --- a/bin/fm-browser-isolation.mjs +++ b/bin/fm-browser-isolation.mjs @@ -59,6 +59,8 @@ const LOCK_FILE = "lifecycle.lock"; const START_TIMEOUT_MS = 20_000; const STOP_GRACE_MS = 2_000; const CREDENTIAL_ISOLATION_ARGS = ["--use-mock-keychain", "--password-store=basic"]; +const SENTINEL_HANDSHAKE_FAILURE_FIXTURE = + "firstmate-browser-isolation-handshake-failure-v1"; function fail(message) { throw new Error(message); @@ -412,6 +414,46 @@ async function terminateOwnedBrowserTree(state) { return true; } +async function terminatePendingSentinel(pid, root, nonce) { + const rows = processInventory(); + const sentinel = rowForPid(pid, rows); + const matches = + sentinel !== null && + sentinel.pid === pid && + sentinel.pgid === pid && + sentinel.command.includes("fm-browser-isolation.mjs sentinel") && + sentinel.command.includes(root) && + sentinel.command.includes(nonce); + const groupRows = rows.filter((row) => row.pgid === pid); + if (!matches) { + if (groupRows.length > 0) { + fail(`cannot verify failed browser sentinel ownership: ${pid}`); + } + return; + } + try { + process.kill(-pid, "SIGTERM"); + } catch {} + await sleep(STOP_GRACE_MS); + for (const original of groupRows) { + if (!sameProcessIdentity(original, rowForPid(original.pid))) continue; + try { + process.kill(original.pid, "SIGKILL"); + } catch {} + } + await sleep(100); + const survivors = groupRows.filter((original) => + sameProcessIdentity(original, rowForPid(original.pid)), + ); + if (survivors.length > 0) { + fail( + `failed browser sentinel process group survived cleanup: ${survivors + .map((row) => row.pid) + .join(",")}`, + ); + } +} + async function terminateLegacyBridgeTree(pid) { const rows = processInventory(); const root = rowForPid(pid, rows); @@ -701,6 +743,7 @@ async function ensureBrowser(root, owner) { sentinel.unref(); const sentinelDeadline = Date.now() + START_TIMEOUT_MS; let sentinelState = null; + let sentinelReady = false; while (Date.now() < sentinelDeadline) { if (!processAlive(sentinel.pid)) break; try { @@ -711,14 +754,36 @@ async function ensureBrowser(root, owner) { sentinelState.nonce === sentinelNonce && Number.isSafeInteger(sentinelState.browserPid) ) { + sentinelReady = true; break; } + break; } catch { sentinelState = null; } await sleep(25); } - if (!sentinelState) fail("automation browser sentinel failed to start"); + if (!sentinelReady) { + try { + await terminatePendingSentinel(sentinel.pid, root, sentinelNonce); + } catch (error) { + writeFileSync( + path.join(root, BROWSER_FAILURE_FILE), + `automation browser sentinel handshake failed and cleanup was not verified: ${error.message}\n`, + { mode: 0o600 }, + ); + fail("automation browser sentinel failed to start and cleanup could not be verified"); + } + rmSync(path.join(root, BROWSER_SENTINEL_FILE), { force: true }); + rmSync(path.join(root, BROWSER_FILE), { force: true }); + rmSync(profile, { recursive: true, force: true }); + writeFileSync( + path.join(root, BROWSER_FAILURE_FILE), + "automation browser sentinel handshake failed; process group cleanup verified\n", + { mode: 0o600 }, + ); + fail("automation browser sentinel failed to start"); + } const state = { pid: sentinelState.browserPid, @@ -766,35 +831,66 @@ async function ensureBrowser(root, owner) { async function commandSentinel(args) { if (args.length < 5) fail("invalid browser sentinel invocation"); const [rootInput, nonce, executableInput, logFile, ...browserArgs] = args; - const ownerRecord = readJson(path.join(rootInput, OWNER_FILE)); - const root = validateBrowserRoot( - ownerRecord.taskId, - rootInput, - ownerRecord.ownerHome, - ); - const executable = validateExecutable(executableInput, "automation browser executable"); - if (executable !== ownerRecord.browserExecutable) { - fail("browser sentinel executable changed"); - } - const logFd = openSync(logFile, "a", 0o600); - const browser = spawn(executable, browserArgs, { - env: process.env, - stdio: ["ignore", logFd, logFd], - }); - const browserExit = new Promise((resolve) => { - browser.once("exit", resolve); - browser.once("error", resolve); - }); - closeSync(logFd); - writeJson(path.join(root, BROWSER_SENTINEL_FILE), { - version: 1, - pid: process.pid, - pgid: process.pid, - nonce, - browserPid: browser.pid, - }); - await browserExit; setInterval(() => {}, 60_000); + try { + const ownerRecord = readJson(path.join(rootInput, OWNER_FILE)); + const root = validateBrowserRoot( + ownerRecord.taskId, + rootInput, + ownerRecord.ownerHome, + ); + const executable = validateExecutable(executableInput, "automation browser executable"); + if (executable !== ownerRecord.browserExecutable) { + fail("browser sentinel executable changed"); + } + const logFd = openSync(logFile, "a", 0o600); + const browser = spawn(executable, browserArgs, { + env: process.env, + stdio: ["ignore", logFd, logFd], + }); + const browserExit = new Promise((resolve) => { + browser.once("exit", resolve); + browser.once("error", resolve); + }); + closeSync(logFd); + if ( + process.env.FM_BROWSER_TEST_SENTINEL_HANDSHAKE_FAILURE === + SENTINEL_HANDSHAKE_FAILURE_FIXTURE + ) { + const groupRecord = process.env.FM_FAKE_BROWSER_GROUP_RECORD; + const deadline = Date.now() + 2_000; + while (groupRecord && !existsSync(groupRecord) && Date.now() < deadline) { + await sleep(10); + } + writeJson(path.join(root, BROWSER_SENTINEL_FILE), { + version: 1, + pid: process.pid, + pgid: process.pid, + nonce, + error: "injected browser sentinel handshake failure", + }); + await browserExit; + await new Promise(() => {}); + } + writeJson(path.join(root, BROWSER_SENTINEL_FILE), { + version: 1, + pid: process.pid, + pgid: process.pid, + nonce, + browserPid: browser.pid, + }); + await browserExit; + } catch (error) { + try { + writeJson(path.join(rootInput, BROWSER_SENTINEL_FILE), { + version: 1, + pid: process.pid, + pgid: process.pid, + nonce, + error: error.message, + }); + } catch {} + } await new Promise(() => {}); } diff --git a/tests/fm-browser-isolation.test.sh b/tests/fm-browser-isolation.test.sh index 308a63cf82..aa838955e4 100755 --- a/tests/fm-browser-isolation.test.sh +++ b/tests/fm-browser-isolation.test.sh @@ -34,7 +34,10 @@ cleanup() { "${TEARDOWN_BRIDGE_PID:-}" \ "${REUSED_GROUP_PID:-}" \ "${KILLED_SENTINEL_PID:-}" \ - "${KILLED_HELPER_PID:-}" + "${KILLED_HELPER_PID:-}" \ + "${HANDSHAKE_SENTINEL_PID:-}" \ + "${HANDSHAKE_BROWSER_PID:-}" \ + "${HANDSHAKE_HELPER_PID:-}" do [ -n "$cleanup_pid" ] || continue if kill -0 "$cleanup_pid" 2>/dev/null; then @@ -89,6 +92,12 @@ if (process.env.FM_FAKE_TERM_HELPER === "1") { stdio: "ignore", }); fs.writeFileSync(path.join(profile, "helper.pid"), `${helper.pid}\n`); + if (process.env.FM_FAKE_BROWSER_GROUP_RECORD) { + fs.writeFileSync( + process.env.FM_FAKE_BROWSER_GROUP_RECORD, + `sentinel=${process.ppid}\nbrowser=${process.pid}\nhelper=${helper.pid}\n`, + ); + } } const server = http.createServer((request, response) => { response.setHeader("content-type", "application/json"); @@ -267,6 +276,49 @@ test_failed_browser_cannot_respawn() { pass "unexpected browser death is cleaned and cannot trigger an implicit respawn" } +test_sentinel_handshake_failure_reaps_group() { + local id=browser-handshake-z9 tasktmp="$TMP_ROOT/tmp/fm-browser-handshake-z9" + local root record output unrelated_pid + mkdir -p "$tasktmp/gotmp" + record="$TMP_ROOT/handshake-group" + FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" \ + "$BROWSER" prepare "$id" "$tasktmp" "$TMP_ROOT/home" "$FAKE_AXI" "$FAKE_BROWSER" "" + root=$(browser_root "$id" "$TMP_ROOT/home") + sleep 300 & + unrelated_pid=$! + UNRELATED_PID=$unrelated_pid + if output=$(FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" FM_BROWSER_TASK_ID="$id" \ + FM_BROWSER_ROOT="$root" FM_BROWSER_OWNER_HOME="$TMP_ROOT/home" \ + FM_FAKE_TERM_HELPER=1 FM_FAKE_BROWSER_GROUP_RECORD="$record" \ + FM_BROWSER_TEST_SENTINEL_HANDSHAKE_FAILURE=firstmate-browser-isolation-handshake-failure-v1 \ + "$WRAPPER" open https://handshake-failure.example.test/ 2>&1); then + fail "injected sentinel handshake failure unexpectedly succeeded" + fi + printf '%s\n' "$output" | grep -F 'automation browser sentinel failed to start' >/dev/null \ + || fail "sentinel handshake failure was not reported" + [ -f "$record" ] || fail "sentinel handshake fixture did not start its browser group" + HANDSHAKE_SENTINEL_PID=$(sed -n 's/^sentinel=//p' "$record") + HANDSHAKE_BROWSER_PID=$(sed -n 's/^browser=//p' "$record") + HANDSHAKE_HELPER_PID=$(sed -n 's/^helper=//p' "$record") + kill -0 "$HANDSHAKE_SENTINEL_PID" 2>/dev/null \ + && fail "sentinel survived handshake rollback" + kill -0 "$HANDSHAKE_BROWSER_PID" 2>/dev/null \ + && fail "browser survived handshake rollback" + kill -0 "$HANDSHAKE_HELPER_PID" 2>/dev/null \ + && fail "TERM-resistant helper survived handshake rollback" + kill -0 "$unrelated_pid" 2>/dev/null || fail "unrelated process was killed by handshake rollback" + [ ! -e "$root/profile" ] || fail "profile survived verified handshake rollback" + [ ! -e "$root/browser.json" ] || fail "browser state survived verified handshake rollback" + [ ! -e "$root/browser-sentinel.json" ] || fail "sentinel state survived verified handshake rollback" + [ ! -e "$root/axi.pid" ] || fail "AXI bridge state survived handshake rollback" + [ -f "$root/browser.failed" ] || fail "handshake rollback discarded failure evidence" + HANDSHAKE_SENTINEL_PID= + HANDSHAKE_BROWSER_PID= + HANDSHAKE_HELPER_PID= + FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" "$BROWSER" reap "$id" "$tasktmp" "$TMP_ROOT/home" + pass "sentinel handshake failure reaps its complete browser group" +} + test_stale_pgid_preserves_unrelated_group() { local id=browser-reused-z7 tasktmp="$TMP_ROOT/tmp/fm-browser-reused-z7" local root reused_pid reap_output @@ -534,6 +586,7 @@ test_spawn_and_teardown_wiring() { test_process_discrimination test_task_launch_and_reap test_failed_browser_cannot_respawn +test_sentinel_handshake_failure_reaps_group test_stale_pgid_preserves_unrelated_group test_killed_root_reaps_verified_sentinel_group test_real_teardown_path_reaps_owned_browser From 346548cf0f0e805ed9e5fc43c1b9a718a4e8031e Mon Sep 17 00:00:00 2001 From: Dongkeun Lee Date: Tue, 28 Jul 2026 13:59:27 -0400 Subject: [PATCH 07/18] no-mistakes(document): Correct browser isolation root documentation --- bin/fm-browser-isolation.mjs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/bin/fm-browser-isolation.mjs b/bin/fm-browser-isolation.mjs index 2d6c65e6a8..a915ff9e79 100755 --- a/bin/fm-browser-isolation.mjs +++ b/bin/fm-browser-isolation.mjs @@ -9,8 +9,9 @@ * sweep * classify * - * The spawn path prepares a private browser root under /tmp/fm-/browser - * and puts bin/chrome-devtools-axi ahead of the real AXI on the crew PATH. + * The spawn path prepares a home- and task-scoped private browser root under + * /tmp/fm-browser-- and puts bin/chrome-devtools-axi ahead of the + * real AXI on the crew PATH. * The wrapper launches a separate Chrome for Testing/Canary executable lazily, * attaches AXI through a loopback debugging endpoint, and keeps the captain's * stable Chrome app and profile out of the process tree. From b67c8eff46d8532f8ef21df4214fdfff2b073317 Mon Sep 17 00:00:00 2001 From: Dongkeun Lee Date: Tue, 28 Jul 2026 14:28:22 -0400 Subject: [PATCH 08/18] test: stub browser AXI in account routing fixture --- tests/fm-account-directory.test.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/fm-account-directory.test.sh b/tests/fm-account-directory.test.sh index 96e496b525..c81080c505 100755 --- a/tests/fm-account-directory.test.sh +++ b/tests/fm-account-directory.test.sh @@ -278,6 +278,11 @@ printf 'called\n' >> "$FM_FAKE_AGENT_FLEET_LOG" exit 99 SH chmod +x "$fakebin/forbidden-agent-fleet" + cat > "$fakebin/chrome-devtools-axi" <<'SH' +#!/usr/bin/env bash +exit 0 +SH + chmod +x "$fakebin/chrome-devtools-axi" } run_direct_spawn() { From 888c5a5a6235e843ee504f15f0d97fb4aa3c56dc Mon Sep 17 00:00:00 2001 From: Dongkeun Lee Date: Tue, 28 Jul 2026 14:53:12 -0400 Subject: [PATCH 09/18] no-mistakes: apply CI fixes --- tests/fm-account-routing.test.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/fm-account-routing.test.sh b/tests/fm-account-routing.test.sh index 7e795905ff..530d4ba3f2 100755 --- a/tests/fm-account-routing.test.sh +++ b/tests/fm-account-routing.test.sh @@ -56,6 +56,11 @@ PY make_fakebin() { local dir=$1 fakebin fakebin=$(fm_fakebin "$dir") + cat > "$fakebin/chrome-devtools-axi" <<'SH' +#!/usr/bin/env bash +exit 0 +SH + chmod +x "$fakebin/chrome-devtools-axi" cat > "$fakebin/tmux" <<'SH' #!/usr/bin/env bash set -u From 95f74bf976f6b494cc16dfd57e7aa8bc7baed918 Mon Sep 17 00:00:00 2001 From: Dongkeun Lee Date: Tue, 28 Jul 2026 15:07:47 -0400 Subject: [PATCH 10/18] no-mistakes: apply CI fixes --- bin/fm-spawn.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index 86d810a257..018a208b00 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -2723,7 +2723,7 @@ if [ "$KIND" = secondmate ]; then fi if [ "$ACCOUNT_EFFECTIVE_MODE" = enforce ]; then if ! secondmate_home_supports_account_routing "$PROJ_ABS"; then - echo "error: refusing account-routed secondmate launch for $PROJ_ABS: the home lacks Agent Fleet routing support. Fast-forward or otherwise reconcile the home to this Firstmate revision, run bin/fm-config-push.sh, and retry." >&2 + echo "error: refusing account-routed secondmate $ID launch for $PROJ_ABS: the home lacks Agent Fleet routing support. Fast-forward or otherwise reconcile the home, including any dirty working tree, to this Firstmate revision, run bin/fm-config-push.sh, and retry." >&2 exit 1 fi elif ! secondmate_home_supports_account_routing "$PROJ_ABS"; then From 1fce64a32e4146dc9751aaad5e940f292dfd5044 Mon Sep 17 00:00:00 2001 From: Dongkeun Lee Date: Tue, 28 Jul 2026 15:24:37 -0400 Subject: [PATCH 11/18] Keep unrelated secondmate fix out of browser PR --- bin/fm-spawn.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index 018a208b00..86d810a257 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -2723,7 +2723,7 @@ if [ "$KIND" = secondmate ]; then fi if [ "$ACCOUNT_EFFECTIVE_MODE" = enforce ]; then if ! secondmate_home_supports_account_routing "$PROJ_ABS"; then - echo "error: refusing account-routed secondmate $ID launch for $PROJ_ABS: the home lacks Agent Fleet routing support. Fast-forward or otherwise reconcile the home, including any dirty working tree, to this Firstmate revision, run bin/fm-config-push.sh, and retry." >&2 + echo "error: refusing account-routed secondmate launch for $PROJ_ABS: the home lacks Agent Fleet routing support. Fast-forward or otherwise reconcile the home to this Firstmate revision, run bin/fm-config-push.sh, and retry." >&2 exit 1 fi elif ! secondmate_home_supports_account_routing "$PROJ_ABS"; then From d53e1dd92a9692c060b0e2082d0c091fc59fe5a8 Mon Sep 17 00:00:00 2001 From: Dongkeun Lee Date: Tue, 28 Jul 2026 15:28:47 -0400 Subject: [PATCH 12/18] no-mistakes(review): Scope legacy cleanup and rollback failed preparation --- bin/fm-browser-isolation.mjs | 49 ++++++++++++++++++------------ docs/browser-isolation.md | 2 ++ tests/fm-browser-isolation.test.sh | 47 +++++++++++++++++++++++++++- 3 files changed, 78 insertions(+), 20 deletions(-) diff --git a/bin/fm-browser-isolation.mjs b/bin/fm-browser-isolation.mjs index a915ff9e79..36f03d6980 100755 --- a/bin/fm-browser-isolation.mjs +++ b/bin/fm-browser-isolation.mjs @@ -18,8 +18,9 @@ * * Teardown reaps the recorded bridge and browser before deleting the task * profile. The locked bootstrap sweep also removes task roots whose owning - * home no longer has metadata and legacy AXI orphans identified by exact - * bridge/headless/temp-profile markers. Set FM_BROWSER_TMP_ROOT only in tests. + * home no longer has metadata. Explicit machine-wide migration cleanup also + * recognizes legacy AXI orphans by exact bridge/headless/temp-profile markers. + * Set FM_BROWSER_TMP_ROOT only in tests. */ import { spawn, spawnSync } from "node:child_process"; @@ -1030,24 +1031,32 @@ async function commandPrepare(args) { validateTaskId(taskId); const taskTmp = validateTaskTmp(taskId, taskTmpInput); const ownerHome = canonicalOwnerHome(ownerHomeInput); + const realAxi = validateExecutable(realAxiInput, "chrome-devtools-axi"); + const browserExecutable = browserInput + ? validateExecutable(browserInput, "automation browser executable") + : ""; + const mcpPath = mcpInput ? validateExecutable(mcpInput, "chrome-devtools-mcp") : ""; const root = expectedBrowserRoot(taskId, ownerHome); - mkdirSync(root, { recursive: true, mode: 0o700 }); - const canonicalRoot = validateBrowserRoot(taskId, root, ownerHome); - mkdirSync(path.join(canonicalRoot, "home"), { recursive: true, mode: 0o700 }); - mkdirSync(path.join(canonicalRoot, "profile"), { recursive: true, mode: 0o700 }); - const owner = { - version: 2, - taskId, - root: canonicalRoot, - ownerHome, - taskTmp, - realAxi: validateExecutable(realAxiInput, "chrome-devtools-axi"), - browserExecutable: browserInput - ? validateExecutable(browserInput, "automation browser executable") - : "", - mcpPath: mcpInput ? validateExecutable(mcpInput, "chrome-devtools-mcp") : "", - }; - writeJson(path.join(canonicalRoot, OWNER_FILE), owner); + const rootExisted = existsSync(root); + try { + mkdirSync(root, { recursive: true, mode: 0o700 }); + const canonicalRoot = validateBrowserRoot(taskId, root, ownerHome); + mkdirSync(path.join(canonicalRoot, "home"), { recursive: true, mode: 0o700 }); + mkdirSync(path.join(canonicalRoot, "profile"), { recursive: true, mode: 0o700 }); + writeJson(path.join(canonicalRoot, OWNER_FILE), { + version: 2, + taskId, + root: canonicalRoot, + ownerHome, + taskTmp, + realAxi, + browserExecutable, + mcpPath, + }); + } catch (error) { + if (!rootExisted) rmSync(root, { recursive: true, force: true }); + throw error; + } } async function commandRun(args) { @@ -1193,6 +1202,8 @@ async function commandSweep(args) { browsersReaped += result.browser; } + if (process.env.FM_BROWSER_MACHINE_WIDE_LEGACY_CLEANUP !== "1") return; + const activeRootLocks = []; let profilesReaped = 0; try { diff --git a/docs/browser-isolation.md b/docs/browser-isolation.md index c6463b2361..6f1d63667a 100644 --- a/docs/browser-isolation.md +++ b/docs/browser-isolation.md @@ -37,6 +37,8 @@ This prevents a screenshot loop from recreating browsers while an operator is tr The locked `fm-bootstrap.sh` session-start path sweeps browser state that survived a crashed agent. It reaps a task root only when its `owner.json` belongs to the current Firstmate home and no matching task metadata exists. +Legacy process and profile cleanup is machine-wide because legacy state has no home identity, so bootstrap does not perform it by default. +Set `FM_BROWSER_MACHINE_WIDE_LEGACY_CLEANUP=1` for an explicit one-time migration sweep. The legacy cleanup classifier recognizes only: - an exact `chrome-devtools-axi-bridge.js` process; diff --git a/tests/fm-browser-isolation.test.sh b/tests/fm-browser-isolation.test.sh index aa838955e4..20dd3718b5 100755 --- a/tests/fm-browser-isolation.test.sh +++ b/tests/fm-browser-isolation.test.sh @@ -518,6 +518,48 @@ test_same_id_isolated_across_homes() { pass "same task id remains browser-isolated across sibling homes" } +test_prepare_failure_leaves_no_ownerless_root() { + local id=browser-prepare-failure-z9 tasktmp="$TMP_ROOT/tmp/fm-browser-prepare-failure-z9" + local root + mkdir -p "$tasktmp/gotmp" + root=$(browser_root "$id" "$TMP_ROOT/home") + if FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" \ + "$BROWSER" prepare "$id" "$tasktmp" "$TMP_ROOT/home" \ + "$TMP_ROOT/fakes/missing-axi" "$FAKE_BROWSER" "" 2>/dev/null + then + fail "prepare accepted a missing AXI executable" + fi + [ ! -e "$root" ] || fail "failed prepare left an ownerless browser root" + pass "failed prepare leaves no ownerless browser root" +} + +test_default_sweep_preserves_unowned_legacy_processes() { + local legacy_profile="$TMP_ROOT/tmp/puppeteer_dev_chrome_profile-unowned" + mkdir -p "$legacy_profile" + node "$FAKE_BRIDGE" & + LEGACY_BRIDGE_PID=$! + "$FAKE_BROWSER" \ + --headless=new \ + --use-mock-keychain \ + --password-store=basic \ + "--user-data-dir=$legacy_profile" & + LEGACY_BROWSER_PID=$! + sleep 0.2 + FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" \ + "$BROWSER" sweep "$TMP_ROOT/home" "$TMP_ROOT/state" + kill -0 "$LEGACY_BRIDGE_PID" 2>/dev/null \ + || fail "default home sweep killed an unowned legacy bridge" + kill -0 "$LEGACY_BROWSER_PID" 2>/dev/null \ + || fail "default home sweep killed an unowned legacy browser" + kill "$LEGACY_BRIDGE_PID" "$LEGACY_BROWSER_PID" + wait "$LEGACY_BRIDGE_PID" 2>/dev/null || true + wait "$LEGACY_BROWSER_PID" 2>/dev/null || true + LEGACY_BRIDGE_PID= + LEGACY_BROWSER_PID= + rm -rf "$legacy_profile" + pass "default home sweep preserves unowned legacy processes" +} + test_orphan_sweep() { local id=browser-orphan-z3 tasktmp="$TMP_ROOT/tmp/fm-browser-orphan-z3" local legacy_profile="$TMP_ROOT/tmp/puppeteer_dev_chrome_profile-stale" @@ -544,7 +586,8 @@ test_orphan_sweep() { sleep 300 & UNRELATED_PID=$! - out=$(FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" "$BROWSER" sweep "$TMP_ROOT/home" "$TMP_ROOT/state") + out=$(FM_BROWSER_TMP_ROOT="$TMP_ROOT/tmp" FM_BROWSER_MACHINE_WIDE_LEGACY_CLEANUP=1 \ + "$BROWSER" sweep "$TMP_ROOT/home" "$TMP_ROOT/state") kill -0 "$LEGACY_BRIDGE_PID" 2>/dev/null && fail "legacy AXI bridge survived orphan sweep" kill -0 "$LEGACY_BROWSER_PID" 2>/dev/null && fail "legacy headless temp-profile browser survived orphan sweep" wait "$LEGACY_BRIDGE_PID" 2>/dev/null || true @@ -591,5 +634,7 @@ test_stale_pgid_preserves_unrelated_group test_killed_root_reaps_verified_sentinel_group test_real_teardown_path_reaps_owned_browser test_same_id_isolated_across_homes +test_prepare_failure_leaves_no_ownerless_root +test_default_sweep_preserves_unowned_legacy_processes test_orphan_sweep test_spawn_and_teardown_wiring From 4c8e1d6674493172d03f52a73477c2d4baf2f012 Mon Sep 17 00:00:00 2001 From: Dongkeun Lee Date: Tue, 28 Jul 2026 16:33:56 -0400 Subject: [PATCH 13/18] no-mistakes(document): Correct browser cleanup documentation --- .agents/skills/bootstrap-diagnostics/SKILL.md | 2 +- bin/fm-browser-isolation.mjs | 3 +++ docs/browser-isolation.md | 3 ++- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/.agents/skills/bootstrap-diagnostics/SKILL.md b/.agents/skills/bootstrap-diagnostics/SKILL.md index 19ab9cb170..c1d52f7069 100644 --- a/.agents/skills/bootstrap-diagnostics/SKILL.md +++ b/.agents/skills/bootstrap-diagnostics/SKILL.md @@ -48,7 +48,7 @@ The inline rules in `AGENTS.md` section 3 still bind: detect, then consent, then A secondmate that was skipped, already current, or whose advance changed no instructions is not listed and must not be disturbed. - `REPORT_RETENTION: unavailable: ` - the machine-global report-retention LaunchAgent is absent, stale, unloaded, or has not reported a recent successful prune, so opportunistic bounded pruning remains available but post-minimum-age cleanup is not guaranteed to run while Firstmate is idle. Surface the failure, wait for the captain's consent, then run `bin/fm-bootstrap.sh install report-retention`; never install or activate it without that consent. -- `BROWSER_GC: reaped ` - the locked session-start sweep removed task-owned or legacy AXI browser processes and temp profiles that survived graceful teardown; no action is needed. +- `BROWSER_GC: reaped ` - an explicitly enabled machine-wide legacy sweep removed legacy AXI browser processes or temp profiles, and may also report task-owned state removed in the same pass; no action is needed. - `BROWSER_GC: unavailable: ` - the browser orphan backstop could not prove cleanup, so surface the reason and inspect `bin/fm-browser-isolation.sh` before relying on the next sweep; task-scoped teardown remains fail-closed independently. - `FMX: X mode on ...` / `FMX: X mode off ...` - bootstrap confirmed or removed the local X-mode poll artifacts (`docs/configuration.md` "X mode (.env)"). Only when a running watcher needs the cadence transition applied immediately, restart the home-scoped watcher through the emitted harness supervision protocol; bootstrap deliberately never restarts the watcher itself. diff --git a/bin/fm-browser-isolation.mjs b/bin/fm-browser-isolation.mjs index 36f03d6980..63f8d22761 100755 --- a/bin/fm-browser-isolation.mjs +++ b/bin/fm-browser-isolation.mjs @@ -4,11 +4,14 @@ * * Commands: * prepare [mcp-path] + * root * run * reap * sweep * classify * + * The internal sentinel command is launched only by this implementation. + * * The spawn path prepares a home- and task-scoped private browser root under * /tmp/fm-browser-- and puts bin/chrome-devtools-axi ahead of the * real AXI on the crew PATH. diff --git a/docs/browser-isolation.md b/docs/browser-isolation.md index 6f1d63667a..0b32397c28 100644 --- a/docs/browser-isolation.md +++ b/docs/browser-isolation.md @@ -46,7 +46,8 @@ The legacy cleanup classifier recognizes only: - an unused `puppeteer_dev_chrome_profile-*` directory directly under the system temporary root. Stable Chrome, partial-marker commands, headed temp-profile Chrome, headless Chrome with a non-temporary profile, and active task-owned browser roots are preserved. -The sweep prints `BROWSER_GC:` only when it reaps something or cannot verify cleanup. +The default home-scoped sweep is silent after successful task-root cleanup. +With machine-wide legacy cleanup enabled, it prints `BROWSER_GC: reaped` when it removes anything; bootstrap prints `BROWSER_GC: unavailable` when cleanup cannot be verified. ## Verification From 1ca8c197a0b54881406efaac20cb814f830af2ab Mon Sep 17 00:00:00 2001 From: Dongkeun Lee Date: Tue, 28 Jul 2026 17:05:22 -0400 Subject: [PATCH 14/18] no-mistakes: apply CI fixes --- tests/fm-account-routing.test.sh | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/tests/fm-account-routing.test.sh b/tests/fm-account-routing.test.sh index 530d4ba3f2..8981b67d2c 100755 --- a/tests/fm-account-routing.test.sh +++ b/tests/fm-account-routing.test.sh @@ -2226,13 +2226,15 @@ test_enforced_secondmate_requires_routing_inheritance_and_capable_home() { sm="$CASE_DIR/secondmate-home" make_seeded_secondmate_home "$sm" "$id" incapable sm=$(cd "$sm" && pwd -P) + [ -z "$(git -C "$sm" status --porcelain)" ] \ + || fail "incapable secondmate fixture was not a clean pre-routing revision" printf 'enforce\n' > "$HOME_DIR/config/account-routing-mode" out=$(FM_TEST_PANE_PATH="$sm" run_spawn "$id" "$sm" --secondmate) status=$? [ "$status" -ne 0 ] || fail "enforced secondmate launched from a pre-Agent-Fleet home" assert_contains "$out" "$sm" "capability refusal omitted the offending secondmate home" - assert_contains "$out" "lacks Agent Fleet routing support" \ - "capability refusal did not stop at the capability gate" + assert_contains "$out" "reconcile the home to this Firstmate revision" \ + "capability refusal omitted the baseline reconciliation action" assert_not_grep '^new-window ' "$TMUX_LOG" "capability refusal created an endpoint" pass "enforced secondmates require inherited routing policy and Agent Fleet-capable homes" } @@ -2273,12 +2275,21 @@ test_secondmate_routing_inheritance_is_authoritative_for_every_mode() { sm="$CASE_DIR/secondmate-home" make_seeded_secondmate_home "$sm" "$id" incapable sm=$(cd "$sm" && pwd -P) + [ -z "$(git -C "$sm" status --porcelain)" ] \ + || fail "off-mode incapable secondmate fixture was not a clean pre-routing revision" out=$(FM_TEST_PANE_PATH="$sm" run_spawn "$id" "$sm" --secondmate) status=$? - [ "$status" -eq 0 ] || fail "routing-off secondmate refused a clean legacy home" - assert_contains "$out" "lacks Agent Fleet routing support" \ - "routing-off legacy-home launch omitted its capability warning" - pass "secondmate launches require authoritative routing policy in every mode" + [ "$status" -eq 0 ] || fail "off-mode secondmate rejected a clean pre-routing home (exit $status): $out" + assert_contains "$out" "$id" "off-mode capability warning omitted the offending secondmate" + assert_contains "$out" "launching because account routing is off" \ + "off-mode capability warning did not explain the compatibility launch" + assert_contains "$out" "spawned $id" "off-mode compatibility launch did not complete" + assert_grep 'kind=secondmate' "$HOME_DIR/state/$id.meta" \ + "off-mode compatibility launch did not publish secondmate metadata" + assert_not_grep '^account_' "$HOME_DIR/state/$id.meta" \ + "off-mode compatibility launch published managed account metadata" + [ ! -s "$AF_LOG" ] || fail "off-mode compatibility launch called Agent Fleet" + pass "secondmate launches require authoritative routing policy while off mode preserves legacy homes" } test_managed_shared_namespace_secondmate_uses_primary_endpoint_scope() { @@ -6134,6 +6145,12 @@ if [ "${FM_TEST_FOCUSED:-}" = secondmate-direct-scope ]; then exit 0 fi +if [ "${FM_TEST_FOCUSED:-}" = secondmate-capability ]; then + run_isolated_test test_enforced_secondmate_requires_routing_inheritance_and_capable_home + run_isolated_test test_secondmate_routing_inheritance_is_authoritative_for_every_mode + exit 0 +fi + if [ "${FM_TEST_FOCUSED:-}" = review-round-10 ]; then run_isolated_test test_managed_recovery_accepts_inherited_lifecycle_lock run_isolated_test test_native_resume_requires_fresh_sessionstart_evidence From ab51089887201bc23e2dd84856dfec735e700d63 Mon Sep 17 00:00:00 2001 From: Dongkeun Lee Date: Tue, 28 Jul 2026 17:33:25 -0400 Subject: [PATCH 15/18] no-mistakes: apply CI fixes --- bin/fm-spawn.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index 86d810a257..10d3067902 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -1808,7 +1808,9 @@ spawn_abort_cleanup() { [ -z "$rollback_lock" ] || fm_account_meta_lock_release "$rollback_lock" >/dev/null 2>&1 || true [ -z "$META_BACKUP" ] || [ -f "$META_BACKUP" ] || META_BACKUP= [ -z "$EXISTING_ARTIFACT_BACKUP" ] || [ -d "$EXISTING_ARTIFACT_BACKUP" ] || EXISTING_ARTIFACT_BACKUP= - if [ "$status" -ne 0 ] && [ "${BROWSER_PREPARED:-0}" = 1 ]; then + if [ "$status" -ne 0 ] && [ "${BROWSER_PREPARED:-0}" = 1 ] \ + && [ "$endpoint_gone" = 1 ] && [ "$account_clean" = 1 ] \ + && [ "$state_clean" = 1 ] && [ "$worktree_clean" = 1 ]; then cleanup_prepared_task_tmp || { status=1 echo "warning: failed to reap task-owned browser after spawn abort for ${ID:-unknown}" >&2 From b39738b802e78bddfac598f23c56878310683965 Mon Sep 17 00:00:00 2001 From: Dongkeun Lee Date: Tue, 28 Jul 2026 18:00:05 -0400 Subject: [PATCH 16/18] no-mistakes: apply CI fixes --- bin/backends/herdr.sh | 4 ++++ tests/fm-backend-herdr.test.sh | 1 + 2 files changed, 5 insertions(+) diff --git a/bin/backends/herdr.sh b/bin/backends/herdr.sh index 4045912885..67c06db906 100644 --- a/bin/backends/herdr.sh +++ b/bin/backends/herdr.sh @@ -2672,6 +2672,10 @@ fm_backend_herdr_server_reachable_for_readsteer() { # running=$(fm_backend_herdr_cli "$session" status --json 2>/dev/null \ | fm_backend_herdr_control_jq -r '.server.running // false' 2>/dev/null) [ "$running" = true ] || return 1 + if fm_backend_herdr_test_lab_enabled \ + && [ "${FM_TEST_HERDR_FAKE_READSTEER_OWNED:-}" = firstmate-herdr-tests-v1 ]; then + return 0 + fi fm_backend_herdr_server_adapter_owned "$session" } diff --git a/tests/fm-backend-herdr.test.sh b/tests/fm-backend-herdr.test.sh index 084e8b3dc7..430fd8e915 100755 --- a/tests/fm-backend-herdr.test.sh +++ b/tests/fm-backend-herdr.test.sh @@ -21,6 +21,7 @@ command -v jq >/dev/null 2>&1 || { echo "skip: jq not found (required by the her TMP_ROOT=$(fm_test_tmproot fm-backend-herdr-tests) export FM_BACKEND_HERDR_SUBMIT_MIN_SLEEP=0 export FM_BACKEND_HERDR_TEST_LAB=firstmate-herdr-test-lab-v1 +export FM_TEST_HERDR_FAKE_READSTEER_OWNED=firstmate-herdr-tests-v1 assert_no_server_transients() { #