Skip to content

security(browser): send_to_browser_vm and playwright_service bypass the SSRF/DNS-rebind guard the other browser paths enforce #13204

Closed

Description

@mrveiss

Part of #12645 / #12756. Found while auditing the browser stacks for #12651.

The SSRF guard is not applied uniformly across the browser execution paths

AutoBot reaches the web through four browser/fetch entry points. They do not share a URL guard, and the two that an agent can steer are the weakest.

path guard
content_reach/backends/browser.py ensure_public_url + ensure_robots_allowed (shared DNS-resolving guard)
research_browser_manager.py DNS-rebind re-check after navigation (#13018)
api/browser_mcp.py → send_to_browser_vm() none
services/playwright_service.py none

Detail

1. send_to_browser_vm() performs no URL validation.

api/browser_mcp.py does define is_url_allowed(), but it is called from exactly one place — the POST /mcp/navigate HTTP endpoint. The internal helper that actually drives the browser worker takes action + params and posts them straight through:

async def send_to_browser_vm(action, params, session_id=DEFAULT_BROWSER_SESSION_ID):
    payload = {"action": action, "params": params, "session_id": ...}
    async with await http_client.post(f"{BROWSER_VM_URL}/automation", json=payload, ...)

That helper is on the agent path: chat_workflow/tool_handler.py::_web_search_via_browser_vm calls it with {"url": search_url}, and it is the final fallback of the web-search tool. A navigate to an internal address or a link-local metadata endpoint is not blocked here.

This matters more than a normal SSRF surface because the content the agent is reading is itself untrusted — the injection-to-navigation path is exactly what #12757 (IDPI / trust-boundary wrapping) is about.

2. is_url_allowed() is a regex allowlist, not a resolving guard.

Where it is applied it matches on the URL string only, so it cannot see where a hostname resolves — the DNS-rebinding class that #13018 fixed for the research browser. It also allowlists localhost, the loopback address and a hardcoded internal VM range, and still carries a placeholder entry (.*\.example\.com, commented "Example - add real domains").

Note it does not allowlist the search domain that _web_search_via_browser_vm navigates to — further evidence that path never passes through it.

3. services/playwright_service.py validates nothing.

capture_screenshot(url=...) and search_web(...) post caller-supplied URLs to the container with no check.

Why this belongs with #12651

#12651 is converging these stacks behind one canonical browser interface. The guard should be enforced in that interface, once, rather than re-added to three call sites that have already drifted apart — which is the whole argument for the convergence. Fixing it per-stack now would create a fourth thing to keep in sync.

Suggested sequencing:

  1. Land the canonical interface (tech-debt(backend): unify two Playwright browser stacks behind one interface #12651) with ensure_public_url (DNS-resolving) enforced on every navigate/fetch primitive it exposes.
  2. Migrate send_to_browser_vm and playwright_service onto it.
  3. Retire is_url_allowed's regex allowlist in favour of the shared guard, keeping any genuinely needed internal-host exceptions as an explicit, documented allowlist rather than a regex over the raw URL.

If #12651 slips, item 1 should still be done standalone for send_to_browser_vm, since that is the agent-reachable one.

Verification done

  • is_url_allowed call sites: 1 (/mcp/navigate only).
  • url_safety / ensure_public_url references: research_browser_manager.py 2, content_reach/backends/browser.py 3, services/playwright_service.py 0, api/browser_mcp.py 0.
  • _web_search_via_browser_vm → send_to_browser_vm("navigate", {"url": ...}) confirmed in chat_workflow/tool_handler.py.

No exploit was attempted; this is a static read of the call graph.

Activity

  1. mrveiss commented on Aug 2, 2026

    @mrveiss
    OwnerAuthor

    Enumerated the call sites, as this issue and #13236 both said had to happen before the guard is enforced. The result de-risks it substantially.

    Only one action carries a URL

    send_to_browser_vm(action, params, session_id) is called with many actions. Counting what params actually contains across every call site in api/browser_mcp.py:

    6  "selector"        click / fill / hover / wait_for_selector / get_text ...
    1  "script"          evaluate
    1  "value"           fill
    1  "index" / "select_index" / "hover_index"
    1  "timeout"
    1  "screenshot"
    1  "navigate"        <- the only one carrying a URL
    

    So the guard surface is navigate, not the whole helper. That is much narrower than "every call through an unvalidated transport".

    Where a URL actually reaches it

    call site action URL source guarded today
    api/browser_mcp.py:708 navigate request.url (HTTP endpoint) ✅ is_url_allowed (regex)
    chat_workflow/tool_handler.py:2885 navigate DuckDuckGo search URL ❌ none
    chat_workflow/tool_handler.py:2210 tool_name (dynamic) agent-supplied params ❌ none
    chat_workflow/tool_handler.py:2697 evaluate fixed script, no URL n/a

    tool_handler.py:2210 is the one that matters. It forwards a dynamic tool_name with agent-supplied params — so when the agent calls the browser navigate tool, the URL comes from the model and reaches the worker with no validation at all. That is the agent-reachable path, and it is the reason this is filed as security rather than tidiness.

    The question you actually have to answer: does anything legitimately navigate internally?

    The existing allowlist permits internal targets, which is what made me flag that enforcing the guard might break real usage:

    ALLOWED_URL_PATTERNS = [
        r"^https?://localhost",
        r"^https?://127\.0\.0\.1",
        r"^https?://<internal-vm-range>",     # "AutoBot VMs"
        r"^https?://.*\.example\.com$",       # "Example - add real domains"
        r"^https?://github\.com", ...
    ]

    On the evidence, nothing in production uses those entries. Searching every non-test caller for a navigation to localhost / 127.0.0.1 returns nothing. The internal entries look like leftovers, sitting beside an admitted placeholder (*.example.com, commented "add real domains").

    Two further signals that the allowlist is not load-bearing:

    • it does not include the search domain tool_handler.py:2885 navigates to, yet that path works — because it never passes through is_url_allowed;
    • is_url_allowed guards exactly one call site out of the whole module.

    What I would change, given the above

    1. Enforce ensure_public_url (DNS-resolving) on navigate — via the canonical interface for tool_handler, which is ADR-009 step 4.
    2. Replace is_url_allowed's regex with the same guard on /mcp/navigate, keeping an explicit, documented internal-host exception list — which on this evidence can start empty.
    3. Leave the non-URL actions alone; they carry no URL to validate.

    The residual risk is not in production code but in the HTTP endpoint: a human or the frontend could be calling /mcp/navigate against an internal address for diagnostics. That is not answerable from the codebase, and it is the one thing worth your confirmation before step 4 lands.

    Relates to #13236 (step 4 is what closes this), #12645.

  2. github-actions commented on Aug 2, 2026

    @github-actions
    Contributor

    PR #13310 (merged to Dev_new_gui) references this issue with a close keyword.

    fix(security): validate URLs at the browser transport, exempting the admin surface (#13204)

    If this issue is fully resolved, close it manually. If work remains, no action is needed.

  3. added this to the Backlog milestone on Sep 12, 2026
  4. mrveiss commented on Sep 13, 2026

    @mrveiss
    OwnerAuthor

    Post-merge closure audit: verified on main at 0abe0d4. This issue has no checkbox ACs, so this comment maps each stated problem to the fix.

    Stated problem Fix on main
    1. send_to_browser_vm() performs no URL validation Fixed earlier in a7afad8 (#13310), which is an ancestor of main. api/browser_mcp.py:613 _reject_non_public_url calls is_public_url_async at :624, and send_to_browser_vm (:632) calls it at :680.
    2. is_url_allowed() is a regex allowlist, not a resolving guard services/browser_url_guard.py:198 is_url_allowed now decides through classify_url (:157), which calls the DNS-resolving is_public_url_async (:179). A non-public address is admitted only for loopback or a fleet-registry host. The module docstring records that #13236 step 5 removed the regex allowlist.
    3. services/playwright_service.py validates nothing PR #16544, merge commit 0abe0d4: capture_screenshot (:308) checks is_public_url_async(url) at :325 and returns "URL is not a public address" (:327) before any I/O. The test is in services/playwright_service_test.py. search_web (:132) takes a query rather than a URL, so it has nothing to validate.

    The rest of the ADR-009 work, migrating the remaining callers onto the canonical interface, is tracked in #13236 (open).

    The issue stays closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions