Skip to content

fix(browser): apply private-page guard to browser_cdp frame_id routing - #57464

Closed
srojk34 wants to merge 1 commit into
NousResearch:mainfrom
srojk34:fix/browser-cdp-frame-id-private-guard
Closed

fix(browser): apply private-page guard to browser_cdp frame_id routing#57464
srojk34 wants to merge 1 commit into
NousResearch:mainfrom
srojk34:fix/browser-cdp-frame-id-private-guard

Conversation

@srojk34

@srojk34 srojk34 commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Summary

browser_cdp's frame_id (OOPIF) routing path returns early via _browser_cdp_via_supervisor(...) before _browser_cdp_private_guard(...) ever runs. The stateless path (no frame_id) a few lines below correctly calls the guard before dispatching. This means a cloud browser that has navigated to a private/internal URL can still have its content read via browser_cdp(method="Runtime.evaluate", ..., frame_id="..."), even though the identical call without frame_id is correctly blocked.

This is the same private-page/SSRF boundary that browser_snapshot, browser_console, browser_eval, and the stateless browser_cdp path already enforce (see the guard series in _browser_cdp_private_guard's own docstring: "raw CDP calls ... must not become the sibling bypass for the guarded browser tools").

Fix

Call the same _browser_cdp_private_guard(...) used by the stateless path before dispatching to _browser_cdp_via_supervisor(...), so both routing modes share one boundary. No other behavior changes.

Test plan

  • Added test_frame_id_route_blocked_when_current_page_is_private — confirms a frame_id call is blocked (and the supervisor is never reached) when the current page is private, mirroring the existing stateless-path test.
  • Added test_frame_id_route_allowed_when_page_is_not_private — sanity check that ordinary frame_id routing still works when the page isn't private.
  • uv run --frozen --extra dev python -m pytest tests/tools/test_browser_cdp_tool.py -x -q — 25 passed.
  • uv run --frozen --extra dev python -m pytest tests/tools/ -k browser -q — 514 passed, 23 skipped (unrelated), no regressions.
  • ruff check on both changed files — clean.

browser_cdp's frame_id (OOPIF) path returned early via
_browser_cdp_via_supervisor before _browser_cdp_private_guard ever ran,
unlike the stateless path a few lines below. A model that navigated a
cloud browser to a private/internal URL could still read page content
by passing frame_id, bypassing the same SSRF/private-page boundary
already enforced on Runtime.evaluate, Page.navigate, and other raw CDP
calls.

Apply the same guard call used by the stateless path before dispatching
to the supervisor, so both routing modes share one boundary.
@alt-glitch alt-glitch added type/security Security vulnerability or hardening tool/browser Browser automation (CDP, Playwright) sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data P1 High — major feature broken, no workaround labels Jul 3, 2026
@egilewski

Copy link
Copy Markdown
Contributor

suggesting changes

Security evidence:

  • trust boundary: browser_cdp is the raw CDP escape hatch for browser state, and frame_id routing dispatches directly into an OOPIF supervisor session, so page-content CDP methods must enforce the private/internal-address boundary for the selected frame, not only for the top-level page.
  • source/sink/invariant: the source is browser_cdp(..., frame_id=...) plus the supervisor's frame_info.url/origin for that selected frame; the sink is _browser_cdp_via_supervisor() dispatching to the live child session; the invariant is that private/internal target-frame content is blocked before supervisor dispatch while non-private frame routing still works.
  • current-main reproduction: importing tools.browser_cdp_tool from current GitHub main 551e5af, a private-page Runtime.evaluate with frame_id returned success and called the supervisor once.
  • PR-head validation: PR head 1702baa blocks that top-level private-page case, but a run-root probe importing tools.browser_cdp_tool.py from the PR worktree set the top page to https://example.com, set the selected child frame_info.url to http://169.254.169.254/latest/meta-data/, and browser_cdp(method="Runtime.evaluate", frame_id="private-frame") still dispatched to child-sid and returned success.
  • remaining blocker: CodeRabbit reported the targeted OOPIF origin gap, and source inspection confirms the new guard runs before frame_info lookup and only calls _current_page_private_url(task_id), while _browser_cdp_via_supervisor() later reads frame_info.url and origin but does not check either before dispatch.
  • reviewer validation: the submitted tests cover the top-level private-page frame route and the ordinary non-private frame route, and the recorded git merge-tree, git diff --check, and focused tests/tools/test_browser_cdp_tool.py run passed, but there is no regression covering a private/internal OOPIF under a public top page.

Please make the frame_id path validate the selected supervisor frame's URL or origin before dispatching page-content CDP methods, while preserving the existing Page.navigate, Runtime.evaluate expression, allowed-method, and top-level private-page checks. Add regression coverage for a public top page with a private/internal OOPIF target so this bypass cannot recur.

Signed: GPT-5.5-xhigh in Codex

@teknium1

teknium1 commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Merged via PR #57660 — your commit was cherry-picked onto current main with your authorship preserved in git log (rebase-merge). Thanks for the fix!

@teknium1 teknium1 closed this Jul 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P1 High — major feature broken, no workaround 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