AgentHistory._filter_sensitive_data_from_dict only recursed one level into
lists: a string or dict directly inside a list was filtered, but a list inside
a list (or any deeper container) was returned untouched. An action parameter
shaped like {"input": {"rows": [["token-123"]]}} was therefore written to the
history file with the secret intact.
Replace the hand-rolled one-level walk with a single recursive value
dispatcher that handles str/dict/list/tuple, mirroring how
Registry._replace_sensitive_data walks params on the injection side, so the
redaction and injection paths cover the same container shapes.
Fixes#5623
SecurityWatchdog enforces prohibited_domains as a navigation restriction
alongside allowed_domains. Without prohibited_domains in the evaluate()
guard, a profile using only the deny-list could still load an allowed page
and have the agent fetch() into a blocked domain via JS.
Add prohibited_domains to the restriction check.
The agent's evaluate() action called Runtime.evaluate directly through
CDP. SecurityWatchdog only subscribes to navigation events, so JS
running inside an already-allowed page could fetch() arbitrary internal
URLs, read cookies and localStorage from any allowed origin's context,
and otherwise act as if allowed_domains / block_ip_addresses weren't
configured.
When a profile has allowed_domains or block_ip_addresses set, the
operator has signalled the agent is constrained — exposing an unmediated
JS evaluation primitive contradicts that signal. Refuse evaluate()
outright on such profiles; agents that need JS can run on an unrestricted
profile.
Empty allowed_domains=[] is treated as 'no restriction' elsewhere in the
codebase (e.g. SecurityWatchdog); evaluate() behaves consistently and
does not refuse in that case.
The per-session HMAC auth token (de14b9aa) gates command dispatch, but
asyncio.start_unix_server creates the socket file with the process
umask, leaving it 0o755 by default. On multi-user hosts a co-tenant
can connect() and probe behavior even though the handshake will
ultimately fail.
Set umask to 0o077 around start_unix_server and chmod 0o600 after,
matching the auth-token file's posture.
Addresses third P1 codex review on #4866.
Per RFC 3490 / UTS46, four code points act as label separators in IDNA
processing — `.` (U+002E), `。` (U+3002 IDEOGRAPHIC FULL STOP),
`．` (U+FF0E FULLWIDTH FULL STOP), and `｡` (U+FF61 HALFWIDTH IDEOGRAPHIC
FULL STOP). WHATWG URL parsing folds all four to `.` before resolution,
so `http://127。0。0。1/` and `http://127｡0｡0｡1/` reach 127.0.0.1.
NFKC handles only U+FF0E and partially maps U+FF61 → U+3002, leaving the
IPv4 parser unable to match the dominant ideographic-dot form. Explicitly
replace U+3002 and U+FF61 with `.` after NFKC.
Test `test_idna_dot_separators_blocked` covers all four dot variants
both at `_is_url_allowed` and `_is_ip_address` levels, plus a combined
case with circled digits (`①②⑦。⓪。⓪。①`).
Addresses second P1 codex review on #4866.
WHATWG URL canonicalization maps fullwidth digits (`１２７.０.０.１`),
circled digits (`①②⑦.⓪.⓪.①`), and Unicode-prefixed hex forms
(`０x7f000001`) to ASCII IPv4 literals (`127.0.0.1` and `0x7f000001`)
before resolution. The classifier saw only the original string and
returned False, so `block_ip_addresses=True` was still bypassable by
re-encoding the IP with any equivalent Unicode digit variant.
Add a `unicodedata.normalize('NFKC', ...)` step after percent-decoding
and before parsing. NFKC handles all the bot's example forms and is
stdlib-only (no IDNA dependency). Wrapped in try/except to preserve the
never-throw invariant.
Tests:
- test_unicode_normalized_ipv4_blocked covers fullwidth, fullwidth+ASCII
hex, and circled-digit forms at both `_is_url_allowed` and
`_is_ip_address` boundaries.
- test_idn_domains_not_misclassified_as_ip is a false-positive guard
ensuring legitimate IDN domains (`café.example`, `日本.example`, their
punycode equivalents) remain classified as domains.
Addresses second codex review on #4866 (P1).
Chromium percent-decodes the host component before applying its IPv4
parser, so a URL like `http://%30x7f000001/` (decodes to `0x7f000001`,
i.e. 127.0.0.1) or `http://%31%32%37.0.0.1/` (decodes to `127.0.0.1`)
still reaches the IP address despite `block_ip_addresses=True`. Without
decoding, `_is_ip_address` sees the literal `%`-encoded string and
returns False — bypassing the block.
Call `urllib.parse.unquote` on the host before passing it to both
`ipaddress.ip_address` and `socket.inet_aton`. Wrap in try/except to
preserve the never-throw invariant for the classifier.
Adds two regression tests:
- test_percent_encoded_ipv4_blocked covers mixed (`%30x7f000001`), fully
encoded canonical (`%31%32%37.0.0.1`), and fully encoded decimal
(`%32%31%33%30%37%30%36%34%33%33`) bypass forms.
- test_malformed_percent_encoding_does_not_crash covers lone `%`, `%zz`,
`%2` — `unquote` leaves these as-is and the classifier must not throw.
Addresses codex review on #4866.
`socket.inet_aton` raises `UnicodeEncodeError` (not `OSError`) for
hostnames containing lone surrogates — common in URLs produced by
URL-decoding malformed UTF-8. The earlier patch only caught `OSError`,
so `_is_ip_address('\udcff')` would propagate the exception through
`_is_url_allowed` and crash the navigation security check whenever
`block_ip_addresses=True`.
Restore the original code's defensive `except Exception` posture for
both `ipaddress.ip_address` and `socket.inet_aton`. The classifier
should never throw — it returns True (recognized IP) or False (not a
recognizable IP); downstream domain-allowlist handling then applies.
Regression test covers lone-surrogate hostnames in
`test_malformed_unicode_hostnames_do_not_crash_classifier`.
Addresses codex review on #4865.
For remote (`is_local=False`) sessions, `params.path` is meant to reference
a file on the remote machine. A coincidental basename collision with a
local FileSystem-managed file (e.g. `/tmp/note.md` colliding with a local
managed `note.md`) would silently rewrite the upload to point at the local
file, uploading the wrong file with no indication to the agent.
Gate the FileSystem rewrite on `browser_session.is_local`. On remote
sessions, fall through to the existing pass-through branch that allows
remote-accessible absolute paths.
Add a regression test that exercises the remote-session basename-collision
case and asserts the local FileSystem path never appears in the resolved
upload path.
GHSA-rv9j-wqjp-2fv4 (critical), GHSA-66xh-g88g-2h8j, GHSA-hpr4-fqgr-xhj9.
`DownloadsWatchdog` joined attacker-controlled filenames from CDP
(`Page.downloadWillBegin.suggestedFilename`) and `Content-Disposition`
headers directly into the configured `downloads_path`. Strings like
`../../escape.bin` or `/etc/shadow.bak` would `os.path.join` outside the
downloads directory, writing the fetched bytes (also attacker-controlled
— the response body is the exploit content) to an arbitrary location
with the agent's process privileges.
`download_file_from_url` triggers passively for any
`Content-Disposition: attachment` response, so this is reachable from any
visited site — `allowed_domains` does not mitigate it.
Add two private helpers on DownloadsWatchdog:
- `_sanitize_download_filename(name)`: keep only the basename, normalize
Windows separators, strip null bytes, fall back to `'download'` for
empty / pure-traversal inputs.
- `_is_path_contained(path, dir)`: realpath containment check for the
on-disk sinks.
Wire the sanitizer at every attacker-controlled filename ingress:
- `download_will_begin_handler` (CDP suggestedFilename → cache + events)
- `_handle_cdp_download` (same field, separate path)
- Network-monitor Content-Disposition parser
- `download_file_from_url` (suggested_filename argument)
- `_handle_download` (Playwright `download.suggested_filename`)
Wire the containment check at every on-disk write site:
- `download_file_from_url` write
- `_handle_download` (Playwright save_as path)
- `trigger_pdf_download` write (defense in depth — already basename'd)
GHSA-xrfv-gg9f-wwjp, GHSA-g27c-8gp4-28cv.
`SecurityWatchdog._is_ip_address` only recognized IP strings that
`ipaddress.ip_address()` accepts — i.e. the canonical dotted-quad form
(`127.0.0.1`) and full IPv6. Chromium and the kernel resolver, however,
also accept several non-standard IPv4 representations:
http://2130706433/ → 127.0.0.1 (decimal int)
http://0x7f000001/ → 127.0.0.1 (hex)
http://0177.0.0.1/ → 127.0.0.1 (octal)
http://127.1/ → 127.0.0.1 (short-form)
http://127.0.1/ → 127.0.0.1 (short-form)
`block_ip_addresses=True` was therefore trivially bypassed by re-encoding
the IP in any of these forms.
Fall back to `socket.inet_aton` after `ipaddress.ip_address()` fails — it
accepts the same liberal IPv4 forms the kernel resolver does, so the
classifier matches the browser's behavior.
The existing `test_ipv4_lookalike_domains_allowed` test was codifying the
buggy behavior for `1.2.3` (which IS a short-form IPv4 == 1.2.0.3).
Removed that assertion and added a dedicated `TestNonStandardIPv4Representations`
class covering decimal/hex/octal/short-form blocking, lookalike-domain
non-interference, and the interaction with `allowed_domains`.
GHSA-j9hj-92j8-jv9h.
The `upload_file` action constructed the absolute upload path by joining
`file_system.get_dir()` with the agent-controlled `params.path`. Because
`FileSystem.get_file()` matches by basename (`os.path.basename` first), an
agent-controlled path like `../note.md` would:
1. Pass `get_file()` lookup if a file named `note.md` exists in the FileSystem.
2. Be naively joined to `data_dir`, producing `data_dir/../note.md` — which
resolves to a sibling file outside the FileSystem directory.
3. Be uploaded to the browser as the resolved (escaped) file, surfacing
arbitrary contents from outside `browseruse_agent_data` to whatever file
input was targeted.
Use the FileSystem-owned `file_obj.full_name` for the join. Add a
`os.path.realpath` containment check as defense in depth; if it ever resolves
outside `data_dir`, refuse the upload.
GHSA-vfcm-843v-w6v3.
`retry_with_browser_use_agent` defaulted `allowed_domains` to `[]` when the
client omitted the argument, and then forwarded that value to
`BrowserProfile(allowed_domains=[])`. `SecurityWatchdog` interprets the empty
list as "no allowlist configured — allow every URL", silently disabling any
admin-configured allowlist on the underlying profile.
Default to `None` so admin profile defaults are preserved when the client
omits the argument, and treat an explicit empty list the same as omitting
(falsy override is not applied). Schema default removed and description
updated so MCP clients see the new contract.
This change ensures that sensitive data in action results is filtered before being included in state messages sent to the LLM. This prevents accidental leakage of private information. New tests are added to verify this functionality.
Co-authored-by: mailmertunsal <mailmertunsal@gmail.com>
Mohil-Ahuja