Skip to content

security(browser): enforce cloud-metadata floor on all backends; CDP is non-local - #52349

Merged
teknium1 merged 1 commit into
NousResearch:mainfrom
claudlos:security/browser-metadata-ssrf-floor
Jul 1, 2026
Merged

security(browser): enforce cloud-metadata floor on all backends; CDP is non-local#52349
teknium1 merged 1 commit into
NousResearch:mainfrom
claudlos:security/browser-metadata-ssrf-floor

Conversation

@claudlos

@claudlos claudlos commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

browser_navigate's always-blocked cloud-metadata floor (169.254.169.254,
metadata.google.internal, ECS/Azure/GCP IMDS) was gated on
not _is_local_backend(), contradicting both the adjacent comment and the
is_always_blocked_url docstring ("denied regardless of backend"). A default
local headless Chromium on a cloud VM — or an off-host CDP browser — could
navigate to IMDS and read instance credentials into the model context. Make the
floor unconditional on the initial-nav and post-redirect paths.

Also: _is_local_backend() ignored a CDP override while _is_local_mode() honors
it, so an off-host CDP browser was treated as "local" and skipped the broader
private/internal SSRF check too. Treat a CDP override as non-local.

Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com

Infographic

Cloud-metadata floor now unconditional — browser SSRF hardening

@alt-glitch alt-glitch added type/security Security vulnerability or hardening tool/browser Browser automation (CDP, Playwright) P1 High — major feature broken, no workaround sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Jun 25, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary

Verdict: Approved

This security PR makes the cloud metadata endpoint block unconditional across all browser backends. Previously, the check was gated on not _is_local_backend(), but a local Chromium on a cloud VM still reaches the host IMDS (169.254.169.254). The CDP override case is also correctly treated as non-local.

Key observations

  • _is_local_backend() now returns False when a CDP override is active (browser may be off-host)
  • browser_navigate blocks metadata URLs unconditionally (not just non-local backends)
  • Post-navigation redirect check also made unconditional
  • Comments updated to explain the rationale

Looks Good

  • Correctly identifies that a local Chromium on a cloud VM still has host IMDS access
  • CDP override detection prevents treating off-host browsers as trusted
  • Both pre-nav and post-nav checks are now consistent

Reviewed by Hermes Agent (cron)

egilewski

This comment was marked as duplicate.

@egilewski egilewski 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.

requesting changes

The CDP override path is still not consistently treated as non-local. The new _get_cdp_override() check runs after the _is_camofox_mode() short-circuit, but _get_cdp_override() also reads persistent browser.cdp_url from config while Camofox mode only suppresses itself for the BROWSER_CDP_URL environment override. On PR head I reproduced this with CAMOFOX_URL set, no BROWSER_CDP_URL, and a synthetic persistent CDP override: _is_local_mode() returned False, _is_local_backend() returned True, and browser_navigate("http://10.0.0.5/internal") reached the backend instead of the private/internal SSRF gate. That leaves the broader private/internal protection bypassed for a configured CDP override whenever Camofox is also configured, so the "CDP is non-local" part of the fix is incomplete.

Security evidence:

  • trust boundary: model-driven browser_navigate input crosses into browser backend network access.
  • source/sink/invariant: source is an untrusted URL; sink is backend navigation; invariant is CDP override must be treated as non-local for private/internal SSRF checks.
  • current-main reproduction: direct probes on current GitHub main e62afaca6259278ef08d23cb178abf477597f986 reproduced the original local IMDS bypass: local initial and redirect IMDS navigations returned success, and CDP override made _is_local_mode() false while _is_local_backend() stayed true.
  • PR-head or patch-replay validation: PR head 2320d6b1c1a47876038fc3f579ec1bf45e5214dc fixed local initial and redirect IMDS blocking and made normal CDP override non-local, but with CAMOFOX_URL plus persistent CDP override it still returned _is_local_backend() == True and delegated a private URL.
  • positive/negative cases: tests/tools/test_browser_ssrf_local.py passed on PR head, and the existing hybrid-sidecar IMDS tests pass on current main too, so they do not cover this residual persistent-CDP/Camofox case.
  • residual bypass search: checked the Camofox priority path and found the bypass at tools/browser_tool.py:636, before the new _get_cdp_override() branch at line 646.

Signed: GPT-5.5-xhigh in Codex

Comment thread tools/browser_tool.py
# model-driven navigate could reach internal/metadata services reachable
# from the CDP host but not the terminal. _is_local_mode() already honors
# this distinction; keep the two helpers in agreement.
if _get_cdp_override():

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 CDP override check still runs too late. _is_camofox_mode() returns above it when CAMOFOX_URL is set, but Camofox only disables itself for the BROWSER_CDP_URL env override while _get_cdp_override() also reads persistent browser.cdp_url. With CAMOFOX_URL plus config CDP, PR head still reports _is_local_mode() == False and _is_local_backend() == True, so a private URL skips the non-local SSRF gate instead of being blocked. The CDP override check needs to win before the Camofox local-backend return.

@claudlos
claudlos force-pushed the security/browser-metadata-ssrf-floor branch 2 times, most recently from cdd25ae to e9e471e Compare June 25, 2026 18:15

@egilewski egilewski 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.

requesting changes

The metadata-endpoint floor is fixed, and the earlier private-URL SSRF path is blocked now, but the CDP override precedence is still only half-applied. CodeRabbit reported this, and I reproduced it: with CAMOFOX_URL plus a persistent browser.cdp_url override, _is_local_backend() is now false, but browser_navigate("https://example.com") still takes the later _is_camofox_mode() branch and delegates to Camofox instead of the configured CDP browser. That contradicts _get_cdp_override()'s contract that browser.cdp_url skips the local launcher and connects directly to the supplied CDP endpoint.

Security evidence:

  • trust boundary: model-supplied browser URLs crossing from Hermes into local, Camofox, CDP, and cloud browser backends.
  • source/sink/invariant: browser_navigate() must never expose cloud metadata endpoints, and CDP override backends must not skip private/internal SSRF checks as trusted local backends.
  • current-main reproduction: browser_ssrf_probe.py on c6575df92781a5b6859845b39ee59d7f07a8cf31 allowed local IMDS initial navigation and local redirect-to-IMDS, and kept _is_local_backend() true for the CAMOFOX_URL plus persistent CDP override case.
  • PR-head validation: the same probe on e9e471eccd9a802064e2ae2aa945e9cfeed9dadc blocked local IMDS initial navigation, blocked the Camofox plus persistent CDP private URL before delegation, and blanked/blocked redirect-to-IMDS, but a follow-up public-URL probe still delegated through Camofox when only the persistent CDP override was set.
  • positive/negative cases: the existing focused suite tests/tools/test_browser_ssrf_local.py tests/tools/test_url_safety.py passed on the PR head (150 passed), covering ordinary private-sidecar behavior and always-blocked metadata URLs.
  • residual bypass search: source review confirmed _get_cdp_override() now runs before _is_camofox_mode() in _is_local_backend(), and both initial-navigation and post-redirect metadata checks are no longer gated on local-backend status; the remaining gap is the later Camofox dispatch in browser_navigate(), while tools/browser_camofox.py::is_camofox_mode() suppresses only the BROWSER_CDP_URL env override.
  • reviewer validation: CodeRabbit completed after a rate-limit wait and reported this same major finding; I verified it with source review and a focused probe.

Signed: GPT-5.5-xhigh in Codex

@claudlos
claudlos force-pushed the security/browser-metadata-ssrf-floor branch from e9e471e to f585901 Compare June 25, 2026 22:18

@egilewski egilewski 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.

looks mergeable

The new head fixes the remaining CDP/Camofox precedence gap I requested changes for earlier. With CAMOFOX_URL set and only persistent browser.cdp_url configured, Camofox mode now yields to the CDP override, the backend is treated as non-local for SSRF checks, and browser_navigate("https://example.com") no longer dispatches through camofox_navigate.

Security evidence:

  • trust boundary: model-supplied browser URLs crossing from Hermes into local, Camofox, CDP, and cloud browser backends.
  • source/sink/invariant: cloud metadata endpoints must be blocked for every backend, and CDP override browsers must not be treated as trusted local browsers or silently replaced by Camofox dispatch.
  • current-main reproduction: prior review reproduced the old main behavior allowing local IMDS navigation and preserving local-backend treatment for a Camofox plus config-CDP override path.
  • PR-head validation: on f585901c268548e0d4b9f34b4f0f61800939d210, the focused CDP/Camofox probe returns is_camofox_mode() == False, _is_local_backend() == False, and public navigation uses the normal CDP-backed command path rather than Camofox.
  • positive/negative cases: tests/tools/test_browser_cdp_override.py tests/tools/test_browser_ssrf_local.py tests/tools/test_url_safety.py passed (158 passed), including the new config-CDP Camofox suppression case and existing metadata/private URL safety cases.
  • residual bypass search: source review confirmed the always-blocked metadata floor remains unconditional on initial navigation and redirects, _is_local_backend() checks _get_cdp_override() before Camofox, and Camofox mode now suppresses both env and config CDP overrides.
  • reviewer validation: CodeRabbit completed with one minor non-blocking config-type hygiene note; no security blocker was found.

Signed: GPT-5.5-xhigh in Codex

@claudlos
claudlos force-pushed the security/browser-metadata-ssrf-floor branch 2 times, most recently from 7c4701b to 35e22f9 Compare June 27, 2026 17:33
@claudlos
claudlos force-pushed the security/browser-metadata-ssrf-floor branch from 35e22f9 to 0d668d5 Compare June 28, 2026 14:04

@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 the focused browser hardening patch. The premise holds on current main: browser_navigate still gates the IMDS always-blocked floor on _is_local_backend() at tools/browser_tool.py:2626, and _is_local_backend() still lacks the CDP override check that _is_local_mode() has (tools/browser_tool.py:739-762).

Problems

  • Minor comment drift: the added comment in tools/browser_tool.py:675-678 says _is_camofox_mode() only suppresses itself for BROWSER_CDP_URL, but this PR also updates tools/browser_camofox.py:90-93 to suppress Camofox for persistent browser.cdp_url. The logic looks right; the explanatory comment should match it.

Suggested changes

  • Reword that comment to say the CDP check remains before the Camofox short-circuit to keep _is_local_backend() aligned with _is_local_mode(), without the now-stale env-only claim.

The substantive fix looks sound: PR head makes the initial metadata floor unconditional (tools/browser_tool.py:2523), makes redirect-to-metadata unconditional (tools/browser_tool.py:2594-2598), and adds a config-CDP/Camofox regression test (tests/tools/test_browser_cdp_override.py:120-143). This is an automated hermes-sweeper review.

Comment thread tools/browser_tool.py Outdated
# off-host). Don't treat it as a trusted local backend — otherwise a
# model-driven navigate could reach internal/metadata services reachable
# from the CDP host but not the terminal. This MUST be checked before the
# camofox short-circuit below: _is_camofox_mode() only suppresses itself for

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 explanation is now stale: this PR also changes is_camofox_mode() to suppress Camofox for config browser.cdp_url, not only BROWSER_CDP_URL. The logic is fine, but the comment should be reworded so it does not contradict the paired browser_camofox.py change.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jun 29, 2026
@claudlos
claudlos force-pushed the security/browser-metadata-ssrf-floor branch from 0d668d5 to 6ce4b53 Compare June 29, 2026 13:41
@claudlos

Copy link
Copy Markdown
Contributor Author

Thanks @teknium1 — reworded the comment in tools/browser_tool.py to drop the stale "env-var only" claim. It now says the CDP check stays before the Camofox short-circuit and notes the override is honored from either BROWSER_CDP_URL or the persistent browser.cdp_url config (both now suppress camofox in browser_camofox.py), keeping _is_local_backend() aligned with _is_local_mode(). Also rebased onto current main.

@egilewski

Copy link
Copy Markdown
Contributor

looks mergeable

The current head addresses the follow-up comment drift and preserves the substantive browser hardening: with CAMOFOX_URL set and only persistent browser.cdp_url configured, Camofox now yields to the CDP override, the backend is non-local for SSRF checks, cloud-metadata URLs are blocked before navigation for every backend, private/internal navigation is stopped by the non-local gate, public navigation uses the normal CDP-backed path, and the focused browser SSRF/CDP tests, py_compile, merge-tree, diff check, and CodeRabbit validation completed without new blockers.

Security evidence: model-supplied browser URLs cross from Hermes into local, Camofox, CDP, and cloud browser backends; the invariant is that cloud metadata endpoints must be blocked for every backend and CDP override browsers must not be treated as trusted local browsers or silently replaced by Camofox dispatch. On f1345290edb87a5da7b28288dc39c46b0be79313, the direct probe with CAMOFOX_URL plus config-only browser.cdp_url still reported Camofox/local backend behavior and delegated IMDS/private/public navigation through Camofox; on 6ce4b5380dd3e98ce05b82ee2efa2beb927ef3ef, the same probe returned non-Camofox/non-local behavior, blocked initial IMDS via the always-blocked floor, blocked private/internal navigation through the non-local SSRF gate, and allowed public navigation through the normal browser command path.

Signed: GPT-5.5-xhigh in Codex

@claudlos
claudlos force-pushed the security/browser-metadata-ssrf-floor branch from 6ce4b53 to b36611d Compare June 30, 2026 14:09
@egilewski

Copy link
Copy Markdown
Contributor

looks mergeable

The force-pushed head is a clean rebase of the same browser-hardening patch I previously validated: its stable patch-id matches the prior clean head, and it still keeps the CDP override ahead of Camofox, treats CDP as non-local for SSRF checks, and enforces the cloud-metadata floor for every backend. I re-ran the current-head checks: merge-tree, diff check, focused browser CDP override tests, py_compile for the changed files, and CodeRabbit all completed without findings.

Security evidence: model-supplied browser URLs cross from Hermes into local, Camofox, CDP, and cloud browser backends; the invariant is that cloud metadata endpoints must be blocked for every backend and CDP override browsers must not be treated as trusted local browsers or silently replaced by Camofox dispatch. The current head b36611dfa083c87a9d846808d9faf8b219878e4f preserves the earlier fix by making Camofox yield to both BROWSER_CDP_URL and persistent browser.cdp_url, making _is_local_backend() return non-local for any CDP override before the Camofox short-circuit, and blocking metadata URLs unconditionally on initial navigation and redirects.

Signed: GPT-5.5-xhigh in Codex

…is non-local

browser_navigate's always-blocked cloud-metadata floor (169.254.169.254,
metadata.google.internal, ECS/Azure/GCP IMDS) was gated on
`not _is_local_backend()`, contradicting both the adjacent comment and the
is_always_blocked_url docstring ("denied regardless of backend"). A default
local headless Chromium on a cloud VM — or an off-host CDP browser — could
navigate to IMDS and read instance credentials into the model context. Make the
floor unconditional on the initial-nav and post-redirect paths.

Also: _is_local_backend() ignored a CDP override while _is_local_mode() honors
it, so an off-host CDP browser was treated as "local" and skipped the broader
private/internal SSRF check too. Treat a CDP override as non-local.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@claudlos
claudlos force-pushed the security/browser-metadata-ssrf-floor branch from b36611d to e565013 Compare July 1, 2026 11:34
@teknium1
teknium1 merged commit 0a75616 into NousResearch:main Jul 1, 2026
31 checks passed
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:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform 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.

5 participants