-
-
Notifications
You must be signed in to change notification settings - Fork 897
fix(sandbox_web): guard Playwright fallback redirects #723
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,7 +25,7 @@ | |
| import subprocess | ||
| import tempfile | ||
| import time | ||
| from typing import Optional | ||
| from typing import Callable, Optional | ||
|
|
||
| from .fetch_chain import Attempt | ||
| from .validators import Verdict, validate | ||
|
|
@@ -134,6 +134,7 @@ def run_playwright_fallback( | |
| timeout: int = 90, | ||
| profile_dir: Optional[str] = None, | ||
| force_executor: Optional[str] = None, | ||
| scope_check: Optional[Callable[[str], bool]] = None, | ||
| ) -> tuple[Attempt, str]: | ||
| """Invoke the appropriate Playwright executor. | ||
|
|
||
|
|
@@ -195,6 +196,7 @@ def run_playwright_fallback( | |
| # template runs without xvfb; set INSANE_HEADLESS=0 (with xvfb present) | ||
| # to run headful, which evades headless-detecting WAFs (Akamai/DataDome). | ||
| "headless": os.environ.get("INSANE_HEADLESS", "1") not in ("0", "false", "no"), | ||
| "allowPrivate": False, | ||
| } | ||
| if choice == "playwright_mobile_chrome": | ||
| args["device"] = "iPhone 13 Pro" | ||
|
|
@@ -213,13 +215,34 @@ def run_playwright_fallback( | |
| # Fall back to treating raw stdout as HTML for forward/backward compat. | ||
| html, final_url, status, cookies, user_agent, automation = _parse_envelope(stdout, url) | ||
|
|
||
| from . import safety | ||
|
|
||
| final_url = final_url or url | ||
| ok, reason = safety.classify_url(final_url, allow_private=safety.allow_private_default()) | ||
| if not ok: | ||
| att.status = status | ||
| att.body_size = len(html) | ||
| att.verdict = Verdict.BLOCKED.value | ||
| att.reasons = [f"ssrf_blocked:{reason}"] | ||
| att.error = f"ssrf_blocked:{reason}" | ||
| att.url = final_url | ||
| return att, "" | ||
| if scope_check is not None and not scope_check(final_url): | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This RoE check runs only after Useful? React with 👍 / 👎. |
||
| att.status = status | ||
| att.body_size = len(html) | ||
| att.verdict = Verdict.BLOCKED.value | ||
| att.reasons = ["roe_out_of_scope"] | ||
| att.error = "ROE_REFUSED: final URL not in engagement scope" | ||
| att.url = final_url | ||
| return att, "" | ||
|
|
||
| resp = _FakeResp(html, status=status, final_url=final_url) | ||
| vr = validate(resp, success_selectors=success_selectors) | ||
| att.status = status | ||
| att.body_size = len(html) | ||
| att.verdict = vr.verdict.value | ||
| att.reasons = list(vr.reasons) + ([f"automation:{automation}"] if automation else []) | ||
| att.url = final_url or url | ||
| att.url = final_url | ||
|
|
||
| # Cookie bridge: a browser that cleared a JS challenge yields exactly the | ||
| # cookies + UA a plain HTTP client needs. Seed the curl_cffi pool so | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -676,6 +676,12 @@ def _jitter(): | |
|
|
||
| # If a terminal-nonsuccess (404/auth/429) stopped us, browser won't help. | ||
| skip_browser = stop_reason in _TERMINAL_NONSUCCESS_VALUES | ||
| # Curl transport already refused a private/internal redirect hop. Do not | ||
| # re-try the same attacker-controlled URL with the browser tier, which would | ||
| # otherwise follow that redirect outside the curl SSRF guard. | ||
| if any((a.error or "").startswith("ssrf_redirect_blocked:") for a in trace): | ||
| skip_browser = True | ||
| stop_reason = stop_reason or "ssrf_redirect_blocked" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When any curl attempt records Useful? React with 👍 / 👎. |
||
|
|
||
| # -------- Phase 3: Playwright fallback ---------------------------------- | ||
| if enable_playwright and not skip_browser: | ||
|
|
@@ -697,6 +703,7 @@ def _jitter(): | |
| device_class=device_class, | ||
| force_executor=fb_name, | ||
| timeout=timeout if timeout and timeout > 30 else 90, | ||
| scope_check=scope_check, | ||
| ) | ||
| trace.append(pw_attempt) | ||
| browser_used += 1 | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In environments that intentionally set
INSANE_ALLOW_PRIVATE=1, the curl transport and the Python final-URL check usesafety.allow_private_default(), but the browser template is always invoked withallowPrivate: false. That makes the new route guard abort loopback/RFC1918 requests before the Python-side opt-in can permit them, regressing local/private engagements that require Playwright fallback.Useful? React with 👍 / 👎.