security: vet every browser navigation, not just the model-supplied URL (SSRF) - #564
Open
coderdailyone wants to merge 1 commit into
Open
Conversation
browser_open_url checked the initial URL with check_url once, then handed navigation to Playwright, which follows redirects, runs page JS, and re-resolves DNS itself. redirect_refusal (OPE-124) re-checks only the single goto's final URL after the fact; a later navigation — a 30x hop, a meta-refresh, location=… — to 127.0.0.1 / 169.254.169.254 was never re-vetted before browser_read_page / browser_screenshot lifted its content into the agent context. browser_read_page in particular applies no page.url check at all. Fix: install a context-level Playwright route interceptor that runs the same address guard (check_url) on every NAVIGATION request and aborts the blocked ones before the request goes out. This covers redirects, JS- and meta-refresh-driven navigation, and narrows the DNS-rebinding window (the handler sees the concrete request). Subresource fetches are not gated (they can't read internal content back, and vetting each would resolve DNS dozens of times per page) — the read-back path is closed; a resolver-pinning proxy remains the larger design for blind SSRF. Fixes andrewyng#524 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CLxsdFGXjdztTRNHjNgPXP
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #524.
browser_open_urlchecks the initial URL withcheck_urlonce, then hands navigation to Playwright — which follows redirects, runs page JS, and re-resolves DNS itself.redirect_refusal(OPE-124) re-checks only the singlegoto's final URL after the fact; a later navigation — a 30x hop, a meta-refresh,location = …— to127.0.0.1/169.254.169.254is never re-vetted beforebrowser_read_page/browser_screenshotlift its content into the agent context.browser_read_pageapplies nopage.urlcheck at all, so the JS-navigation path is fully readable.Change
_guard_route) that runs the same address guard (check_url, asweb_fetchuses) on every navigation request androute.abort("blockedbyclient")s the blocked ones before the request goes out. This covers redirects, JS- and meta-refresh-driven navigation, and narrows the DNS-rebinding TOCTOU (the handler sees the concrete request about to fire). Fails closed if the guard errors or the request shape is unexpected.redirect_refusalstays as complementary defense in depth (nice error messaging for thegotocase).Tests
New
tests/test_browser_url_guard.pyexercises_guard_routedirectly (no Playwright needed) with fake Route/Request objects, reusing the address-guard suite's DNS-monkeypatch pattern:is_navigation_request()raises.All pass; existing
test_browser_root_scopingandtest_url_address_guardsuites stay green (64 total).🤖 Generated with Claude Code
https://claude.ai/code/session_01CLxsdFGXjdztTRNHjNgPXP