#3320 · security · agent-browser-safety

The flag that switched off its own guard

--allow-file-access exists to widen one rule: let agent-browser read file:// URLs. It was implemented as an early return ahead of every other check, so it switched off eight of them and turned a blocked request into an explicit approval.

What the early return skipped

Nine checks follow it. The flag reached none of them.

  Check 0  encryption-key leak ──────────────── deny    ran (above the flag)
  guard    is this an agent-browser command?
  pre-1    --allow-file-access ──▶ RETURN allow  ⚠ early exit
             │
             ├─ Check 1  URL blocklist          DENY   skipped
             ├─ Check 2  rate limiting          DENY   skipped
             ├─ Check 3  robots.txt             DENY   skipped
             ├─ Check 4  sensitive actions      warn   skipped
             ├─ Check 5  network route          DENY   skipped
             ├─ Check 6  inspect / cdp-url      warn   skipped
             ├─ Check 7  clipboard read         warn   skipped
             ├─ Check 8  HAR capture stop       warn   skipped
             └─ Check 9  --user-agent spoof     warn   skipped

  four real DENIES suppressed · four warnings suppressed

Measured, not argued

Same URL. The flag is the only difference. Probed 2026-08-09 through the real dispatcher.

commandbeforeafter
open http://169.254.169.254/latest/meta-data/ denydeny
--allow-file-access open http://169.254.169.254/… allowdeny
--allow-file-access inspect file warning onlyDevTools warning kept
--allow-file-access open file:///tmp/report.html allowallow

Row 2 is the defect: a cloud-metadata SSRF URL, denied on its own, approved once the flag is present. Not a missing deny — outputAllowWithContext sets permissionDecision: 'allow', so the guard actively auto-approved it. Row 4 is the guard that the fix did not break.

The fix: deny returns, warnings collect

One invariant replaces the special case.

  DENY checks  ──▶ return immediately          (unchanged)
  warn checks  ──▶ push onto warnings[]         (new)
  terminal      ──▶ allow + joined warnings

  every allow path folds in warnings collected before it,
  so a later warning can no longer swallow an earlier one

The early return's stated reason does not survive reading: it claims it must run first "to avoid a false deny on file:// URLs", but isBlockedUrl() already short-circuits file:// on its own. The false deny it guarded against cannot occur, so the bypass bought nothing it was not already getting.

Tests that can fail

Run against the pre-fix hook before being run against the fixed one.

  5 of 6 new tests FAIL on the pre-fix hook, PASS on the fixed one
  1 of 6 passes on both — deliberately: "still allows the legitimate
  file:// case", the guard proving the flag still does its real job

A test that passes before and after proves nothing about the fix. The five that flip are the evidence; the sixth is there so a future change cannot close the hole by simply denying everything.

Deliberately not in this change

file:// is unblocked by default, independent of the flag: allowFile = process.env.ORCHESTKIT_AGENT_BROWSER_ALLOW_FILE !== '0'. BLOCKED_URL_PATTERNS contains /file:\/\//i, but that line short-circuits it, and the deny message still advertises "Blocked patterns include … file:// URLs". Probed: agent-browser open file:///etc/passwd with no flag returns no decision at all.

That is a second, separate defect — a control that reads as protection and is not. Fixing it flips a default and could break local-file testing workflows, so it is an operator decision, not a rider on a security fix. Filed rather than folded in.

Audit another guard for the same early-return shape.