Skip to content

fix(browser): block private OOPIF CDP frame reads - #63582

Open
necoweb3 wants to merge 1 commit into
NousResearch:mainfrom
necoweb3:fix/browser-cdp-frame-url-guard
Open

fix(browser): block private OOPIF CDP frame reads#63582
necoweb3 wants to merge 1 commit into
NousResearch:mainfrom
necoweb3:fix/browser-cdp-frame-url-guard

Conversation

@necoweb3

Copy link
Copy Markdown
Contributor

Summary

This closes a residual private-network bypass in browser_cdp frame_id routing.

browser_cdp already blocks raw CDP reads when the top-level page is private/internal, and #57660 added that guard to the frame_id route. However, the frame_id path still only checked the top-level page before resolving the selected OOPIF frame. If the top page was public but the selected child frame had a private/internal URL, browser_cdp(method="Runtime.evaluate", frame_id=...) still dispatched into that child frame's CDP session.

Why

frame_id routing is a live CDP escape hatch into a specific out-of-process iframe. The private-network invariant needs to apply to the selected frame, not only to the top page.

Without this, a public page containing or reaching a private OOPIF could expose private frame content through raw CDP methods even though the guarded browser tools block the same private-page content class.

Changes

  • Add a selected-frame private URL/origin guard inside _browser_cdp_via_supervisor().
  • Reuse the existing browser private URL policy (_is_always_blocked_url / _is_safe_url).
  • Preserve allowed navigation/inspection methods from the existing CDP private-page allowlist.
  • Add regression coverage for a public top page with a private child OOPIF frame.

Testing

ruff check --no-cache tools/browser_cdp_tool.py tests/tools/test_browser_cdp_tool.py

@alt-glitch alt-glitch added type/security Security vulnerability or hardening P2 Medium — degraded but workaround exists tool/browser Browser automation (CDP, Playwright) sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Jul 13, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for closing the selected-OOPIF gap; current main does route the selected frame session after only a top-page check (tools/browser_cdp_tool.py:433-446).

Problems

  • tools/browser_cdp_tool.py:183 adds a frame URL/origin guard without applying _eval_ssrf_guard_active(task_id). The existing policy deliberately skips this boundary for local backends, local sidecars, and allow_private_urls (tools/browser_tool.py:3347-3360). As written, a local CDP session with an internal child OOPIF would now be rejected even though the existing top-page guard permits it.

Suggested changes

  • Pass task_id to the new helper and return early when _eval_ssrf_guard_active(task_id) is false.
  • Add a regression test for the inactive-gate case; the new test currently exercises only the active guarded path.

This is an automated hermes-sweeper review.

Comment thread tools/browser_cdp_tool.py
return None


def _browser_cdp_frame_private_guard(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This selected-frame guard needs the same _eval_ssrf_guard_active(task_id) gate as _browser_cdp_private_guard; otherwise local sidecars and allow_private_urls sessions will start rejecting private OOPIFs despite the policy in browser_tool.py:3347-3360 explicitly exempting them.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 16, 2026
@egilewski

Copy link
Copy Markdown
Contributor

suggesting changes

CI is currently failing on this PR head in Python tests / Run tests slice 1/8. The comparable current main status is not failing for those check(s), so deeper review should wait for the PR-specific required-check failure to be fixed.

Please fix or rerun the failing check, then push a new head or ask for re-review.

Signed: GPT-5.6-terra-low in Codex

@egilewski

Copy link
Copy Markdown
Contributor

suggesting changes

[P1] Revalidate the selected frame at dispatch time

_browser_cdp_via_supervisor() validates URL/origin from a copied snapshot(), then keeps that copy's session_id until _cdp() runs on the supervisor loop. _on_frame_navigated() can replace the live frame URL/origin while preserving the same child session, so a public OOPIF can navigate to a private address after this check and still receive a page-content CDP command. A deterministic interleaving on this head passed a public snapshot, changed the live frame to http://169.254.169.254/latest/meta-data/ before the scheduled coroutine ran, and Runtime.evaluate returned success through that same session. The head also dispatches when an attached OOPIF has a session but empty URL/origin metadata. Please re-resolve and validate the frame/session on the supervisor loop immediately before _cdp(), failing closed for missing, transitioning, metadata-less, or private state.

Security evidence:

  • trust boundary: Page-controlled OOPIF navigation crosses the cloud browser's private-network boundary into raw CDP page-content access.
  • source/sink/invariant: The source is copied frame URL/origin plus a reusable child session, the sink is supervisor._cdp() for non-allowlisted methods, and the selected frame must still be known-public at dispatch.
  • current-main reproduction: Current main dispatches stable-private, metadata-less, and public-snapshot/private-live-frame cases, establishing the pre-fix baseline.
  • PR-head or patch-replay validation: The current-main replay blocks a stable private child, but still dispatches the metadata-less child and the navigation interleaving to the private live URL.
  • positive/negative cases: A stable public child dispatches and a stable private child is denied; the metadata-less and public-to-private cases are the failing negatives.
  • residual bypass search: Traced snapshot copying, raw-frame lookup, navigation replacement with preserved cdp_session_id, loop scheduling, and the final _cdp() sink; no live frame revalidation occurs after scheduling.
  • reviewer validation: The invocation-level probe loaded the replayed module from the managed checkout, and git diff --cached --check passed.

Not checked:

  • Pytest validation
  • Live network-backed OOPIF timing
  • Full test suite
  • CodeRabbit review

Signed: GPT-5.6-sol-xhigh in Codex

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

Labels

P2 Medium — degraded but workaround exists sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data tool/browser Browser automation (CDP, Playwright) type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants