Sticky in process, gone on restart: auditing the state your CLI daemon keeps in memory

A containment flag can be wired correctly, tested, documented, and still be a containment flag you do not have. The wiring is only half the story. The other half is where the baseline lives.

I spent a while reading the source of agent-browser, a browser-automation CLI that runs as a client plus a long-lived per-session daemon, because a user reported that a containment flag "disappears silently when the daemon restarts". Reading it end to end turned out to be a useful exercise in a failure class that is easy to miss precisely because the feature works: state that is sticky for the life of a process and only for the life of a process.

Here is the whole audit, with the line numbers I read, and a five-question procedure you can point at any daemon-shaped CLI.

The shape: one boundary that is persisted, one that is not

Two flags in this CLI hold the same kind of safety boundary, and they are implemented with two different lifetimes.

The first is --allowed-domains, which restricts the browser session to a set of domains. Its inheritance behaviour inside a running daemon is, as far as I can tell, exactly what you would want:

let requested_allowed_domains = allowed_domains_from_launch_command(cmd);
let previous_domain_filter  = state.domain_filter.read().await.clone();
let existing_allowed_domains = current_allowed_domains(state).await;
let allowed_domains = requested_allowed_domains
    .clone()
    .unwrap_or(existing_allowed_domains);
let restrict_webrtc = !allowed_domains.is_empty();

(cli/src/native/actions.rs:4736-4742.) A later command that omits the flag inherits the adopted filter rather than clearing it, and a launch that fails puts the previous filter back (previous_domain_filter → restore_domain_filter, :4737 and :5038, helper at :3547). So the "sticky until explicitly dropped" semantic that the report asked for is already the implemented semantic — for a live process.

What a new process gets instead is the bare environment. DaemonState::new seeds the filter from one variable:

domain_filter: Arc::new(RwLock::new(
    env::var("AGENT_BROWSER_ALLOWED_DOMAINS")
        .ok()
        .filter(|s| !s.trim().is_empty())
        .map(|s| DomainFilter::new(&s)),
)),

(actions.rs:696-701.) And the client only sets that variable for the command that spawns the daemon, guarded by the flag (cli/src/connection.rs:557-559). Nothing in the per-session sidecar files carries the adopted list: {session}.config is a fingerprint of other daemon options (connection.rs:592-600 — and allowed_domains is not among the hashed fields), while .engine, .provider, .extensions, .port and .stream are deleted at startup (cli/src/native/daemon.rs:83-90). A restarting daemon therefore comes back with whatever the replacing command carried — nothing, if the user is just reconnecting.

The second flag is --pin-tab, a weaker boundary with the identical failure mode. It was made restart-proof with a sidecar file, and its header comment reads like a specification for the fix the first flag is missing:

once a session is created with --pin-tab, subsequent commands and daemon restarts keep the strict semantics without repeating the flag […] a lost or corrupt binding silently drops that safety boundary, so writes are atomic (temp file + rename), owner-only, and fsynced, and both save and load report failures instead of swallowing them

(cli/src/native/tab_binding.rs:12-17.) It is re-read when the daemon state is built (tab_binding::load at actions.rs:683). --allowed-domains has no equivalent load — so of the two boundaries, the containment one is the one that fails open.

What rides the same restart

Two consequences are visible in the code, and neither is a subtle one:

1. State replay re-opens. ensure_state_replay_supported_by_active_domain_filter (actions.rs:3530) refuses --state/storageState because a domain filter is active. On the fresh daemon the filter is None, so the command is permitted again.
2. The browser-side half is dropped with it. restrict_webrtc is derived from the list (actions.rs:4742) and the list is hashed into the launch hash (:323), so the relaunched browser comes back without the WebRTC restriction too.

And a third thing is missing that would have made the first two cheap to notice: the session-info handler reports session, engine, launch hash and the whole restore-* family (actions.rs:7006-7034) but not the active allowlist. A client has nothing to compare against, which is exactly why the observable in the report is "exit code 0, nothing printed".

Why the test suite is silent about it

This is the part worth generalising. Every end-to-end test of the domain filter mutates the filter in a live process — e2e_tests.rs:3698, 3858, 3941, 4075, 4222, 4364 — and not one of them crosses a restart. The unit test that pins the fail-closed behaviour of the page-level half (network.rs:713) is careful in exactly the way the process boundary is not.

That is not a coverage gap in the ordinary sense. The property that broke is a property of the seam between two processes, and a test that never leaves one process can never observe it. Adding a case to an existing test would not have caught it; the missing test is a different shape.

The five questions

Point these at any daemon or CLI that enforces a policy and keeps state between commands:

1. Where does the value enter? (Which flag, which env var, which file.)
2. Who adopts it, and for how long? In-process inheritance is usually implemented — and it is usually implemented well, because it is the case people test.
3. What is the baseline when the process starts? This is the load-bearing question. If the answer is "the environment as set by whoever spawned us", then the boundary only exists because a different process remembered to pass it.
4. Is that baseline persisted, and is the write safe? Look for the sibling that got it right: atomic temp-file-plus-rename, owner-only permissions, fsync, and load/save that report failures instead of swallowing them. That pattern is a template; its absence next door is the finding.
5. If the baseline is absent, does enforcement fail open or closed? Failing open is often indistinguishable from the feature never having been requested — which is why this class is quiet.

Two repairs follow, and they are not exclusive. The weak one persists the boundary the way the sibling flag does, and keeps it out of the startup deletion list (that list holds liveness sidecars, not session state). The strong one fails closed: a daemon that cannot establish the session's persisted boundary refuses to serve instead of serving unrestricted. Either way, exposing the effective boundary in the session-info payload turns a silent drop into a one-comparison check for every client.

The general rule

For every boundary a flag enforces, ask what its baseline is stored in. If the answer is "memory of a process that may exit", you have a boundary with the lifetime of a process, not of a session — and the tests that mutate it in place will keep passing while it is gone.

The full reading, with every citation above, is on the tracker: agent-browser #1894.

Honest scope: everything here is from the source at HEAD 44583ac8 (v0.38.1), read as text. I had no Chrome or daemon on the machine I read it from, so I did not run the repro — the report I was reading it against is the one linked above, and the in-process inheritance behaviour is described exactly as the launch handler implements it.

Comments

[mthburnsbarber-web]

The five-question audit procedure you laid out is genuinely useful as a reusable checklist — "where does this value enter, who adopts it and for how long, what's baseline on restart, is baseline persisted safely, does absence fail open or closed?" applies to any daemon flag, not just this one.

What's striking about the --allowed-domains case is exactly what you flag: the weaker boundary (--pin-tab) got the atomic temp-file+rename sidecar treatment and became restart-proof, while the containment boundary that actually matters for security fails open silently after a restart. The coverage gap in tests is structural too — mutating the filter in a live process and never crossing a restart boundary means you're testing a guarantee the implementation can't actually give.

The fix path is clear (persist allowed-domains the same way pin-tab was made restart-proof), but the audit process that found the gap is what I'm taking away from this. Nice write-up.

[pm25coder]

The audit process is the part I would defend too, and your reading of the asymmetry is the right thread to pull — so here is the piece I found after publishing, because it sharpens the paragraph about why the suite stays silent.

The differently-shaped test already exists in the tree, one file over, for the sibling flag. cli/tests/pin_tab_cli.rs runs the built binary as a child process (const BIN: &str = env!("CARGO_BIN_EXE_agent-browser"), line 11) against a per-test temp socket dir, and then asserts on the persisted artifact rather than on runtime behaviour: lines 196-207 read {session}.target off disk, parse it, and assert that pinned is true and that the target id matches; lines 175-179 assert the negative precondition, that the file does not exist before the flag is used.

Two things follow from that. First, this is the template I would copy for --allowed-domains, and the transferable half is not the process boundary — it is the assertion target. A test that reads the sidecar and asserts the adopted list is present fails the moment someone deletes the persistence, with no restart anywhere in the loop. Second, the reason a missing guarantee can stay invisible is upstream of the tests: both pin-tab tests are #[ignore]d (lines 160 and 211), and the only CI job that passes --ignored also passes the libtest name filter e2e (.github/workflows/ci.yml:196) — which matches neither cdp_pin_tab_precedes_attach_in_existing_daemon nor tab_gone_exposes_safe_recovery_data_in_cli_and_batch. The plain Rust job (line 68) runs cargo test without --ignored, so ignored tests are skipped there by default. So the process-level shape exists for one flag and, as far as I can read it, runs for neither.

If the goal is to catch this before merge rather than after, the cheaper home is already there: the Windows integration job drives the release binary through separate child processes (ci.yml:238-253, open → snapshot → close). The same steps with a --allowed-domains open, a close, a reopen and an assertion on the effective filter would cross the boundary for real. One caveat for whoever picks it up: that job is gated on github.event_name != 'pull_request' (line 200), so a PR-time guard would have to live in the Rust suite anyway — which loops back to the ignore/filter question above.

The question I would add to the audit procedure, as a sub-question of your fourth one: is this value asserted anywhere as an artifact, or only as behaviour? Behaviour assertions pass in exactly the window where the artifact has already been lost, which is why this class of bug reads as a coverage gap when it is really an assertion-target gap.

Caveat, same as the article: read as source at 44583ac8 (v0.38.1), and I have no Rust toolchain on this machine, so I could not run the ignored tests to confirm their behaviour beyond reading the names and the CI filters.