fix(agents): detect Cloudflare challenge pages and provide actionable error message#67717
fix(agents): detect Cloudflare challenge pages and provide actionable error message#67717SARAMALI15792 wants to merge 25 commits into
Conversation
Add new failure kind for Cloudflare browser challenge detection. Related: openclaw#67670
Add regex pattern and helper function to detect Cloudflare browser challenge pages by matching cf-browser-verification, challenges.cloudflare.com, _cf_chl_, 'just a moment', and 'ray id:' patterns. Related: openclaw#67670
Insert Cloudflare challenge detection before generic auth_html_403 path to correctly classify CF browser challenges. Related: openclaw#67670
Return user-facing message explaining TLS fingerprint detection and suggesting reverse proxy workaround with cloudscraper. Related: openclaw#67670
Verify that 403 HTML with cf-browser-verification patterns returns cloudflare_challenge failure kind. Related: openclaw#67670
Ensure non-Cloudflare 403 HTML responses still return auth_html_403 failure kind. Related: openclaw#67670
Verify error message contains TLS fingerprint explanation and reverse proxy workaround guidance. Related: openclaw#67670
Add assertions to check error message contains TLS fingerprint explanation and reverse proxy workaround guidance. Related: openclaw#67670
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f46fbd85f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const SANDBOX_BLOCKED_RE = | ||
| /\bapproval is required\b|\bapproval timed out\b|\bapproval was denied\b|\bblocked by sandbox\b|\bsandbox\b.*\b(?:blocked|denied|forbidden|disabled|not allowed)\b/i; | ||
| const CLOUDFLARE_CHALLENGE_RE = | ||
| /\bcf-browser-verification\b|challenges\.cloudflare\.com|\b_cf_chl_|\bjust a moment\b|\bray id:/i; |
There was a problem hiding this comment.
Require Cloudflare-specific markers for challenge detection
The new CLOUDFLARE_CHALLENGE_RE matches on generic phrases like just a moment by itself, so any non-Cloudflare 403 HTML page containing that text will now be classified as cloudflare_challenge. In that scenario, users get a TLS/proxy workaround message instead of the correct auth/access guidance, which can send debugging in the wrong direction. Consider requiring at least one Cloudflare-unique token (for example cf-browser-verification or challenges.cloudflare.com) or a multi-signal match before returning cloudflare_challenge.
Useful? React with 👍 / 👎.
Greptile SummaryThis PR adds Confidence Score: 5/5Safe to merge — no P0 or P1 issues; all findings are P2 or lower. The Cloudflare challenge detection is correctly gated (status 403 + valid HTML structure + CF-specific regex), backward-compatible, and tested with a regression guard. The unrelated No files require special attention. Reviews (1): Last reviewed commit: "test(agents): verify Cloudflare challeng..." | Re-trigger Greptile |
Add required role, content, api, and model properties to AssistantMessage test fixture to satisfy TypeScript type checking. Related: openclaw#67670
Complete AssistantMessage type requirements for Cloudflare challenge test. Related: openclaw#67670
Use input/output/cacheRead/cacheWrite/totalTokens/cost structure instead of prompt_tokens/completion_tokens/total_tokens to match @mariozechner/pi-ai Usage interface. Related: openclaw#67670
Complete cost object structure with total field required by Usage interface. Related: openclaw#67670
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2bc556ecb7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const SANDBOX_BLOCKED_RE = | ||
| /\bapproval is required\b|\bapproval timed out\b|\bapproval was denied\b|\bblocked by sandbox\b|\bsandbox\b.*\b(?:blocked|denied|forbidden|disabled|not allowed)\b/i; | ||
| const CLOUDFLARE_CHALLENGE_RE = | ||
| /\bcf-browser-verification\b|challenges\.cloudflare\.com|\b_cf_chl_|\bjust a moment\b|\bray id:/i; |
There was a problem hiding this comment.
Require challenge-specific markers before classifying 403 pages
This regex classifies any 403 HTML containing Ray ID: as cloudflare_challenge, but Cloudflare’s non-challenge “Sorry, you have been blocked” pages also include a Ray ID footer, so blocked-IP/WAF cases will get the TLS-fingerprint workaround message even when no browser challenge exists. Fresh evidence: the matcher is a single OR-list, so ray id: alone is sufficient to return cloudflare_challenge. Tightening this to require at least one challenge-specific token (for example cf-browser-verification, _cf_chl_, or challenges.cloudflare.com) would avoid misdirecting users.
Useful? React with 👍 / 👎.
Remove unused type import to fix linting error. Related: openclaw#67670
…cloudflare-challenge-detection
steipete
left a comment
There was a problem hiding this comment.
Codex deep review: not merge-ready yet; the classifier direction is useful, but this adds a new runtime failure kind without updating the existing raw-error suppression paths.
What this is trying to fix:
- Issue #67670 reports
openai-codexbehind a mainland China proxy getting Cloudflare 403 browser-challenge HTML. - Current
mainclassifies 403 HTML asauth_html_403, which gives misleading auth guidance. - #67642 already fixed the non-403 CDN HTML path; this PR handles the 403 challenge variant.
Blocking fix needed:
src/agents/pi-embedded-subscribe.handlers.lifecycle.tssuppressesrawError=...console suffixes forauth_html_403,auth_scope, andauth_refresh.src/agents/pi-embedded-runner/run/failover-observation.tshas the same suppression list.- This PR changes Cloudflare 403 challenge HTML from
auth_html_403tocloudflare_challenge, but does not add the new kind to either suppression list. Result: the same HTML page that was previously suppressed can be appended again asrawError=<html preview...>in lifecycle/failover console output.
Suggested patch:
- Add
cloudflare_challengeto both suppression lists. - Add focused tests for lifecycle/failover observation proving Cloudflare challenge errors keep the friendly message and do not append raw HTML in
consoleMessage.
Other cleanup:
- Drop the unrelated
.gitignore.worktrees/change from this PR. - The dependency/workaround itself is plausible: upstream
VeNoMouS/cloudscraperhas a 3.0.0 release in 2025 and recent repo activity, but I would keep the product copy cautious. Say Cloudflare returned a browser challenge and suggest a trusted local/browser-compatible proxy; avoid making “Node TLS fingerprint was detected” sound more certain than the runtime can prove from an HTML page alone.
Once the suppression/test gap is fixed, this looks like the right continuation of #67642 rather than a duplicate/close candidate.
…cific tokens Remove 'just a moment' and 'ray id:' from CLOUDFLARE_CHALLENGE_RE. Both appear on non-challenge Cloudflare pages (WAF blocks, nginx 403s), causing misclassification and wrong TLS-proxy guidance to users. Only cf-browser-verification, challenges.cloudflare.com, and _cf_chl_ are unique to the browser challenge flow.
…re_challenge message Runtime cannot prove TLS fingerprinting from HTML alone. Replace with factual 'Cloudflare returned a browser challenge' and actionable proxy suggestion without asserting a cause.
…enge errors cloudflare_challenge replaces auth_html_403 for CF challenge pages. Without this, the HTML preview reappears as rawError=<html...> in lifecycle console output.
…nge errors Mirrors the lifecycle fix — same suppression list, same gap.
|
All review feedback addressed — here's what changed: Regex tightened ( Raw-error suppression added to both paths ( Error message copy made cautious ( Unrelated All affected tests pass. Branch is up to date with upstream main. |
|
Codex review: needs real behavior proof before merge. Latest ClawSweeper review: 2026-05-19 22:59 UTC / May 19, 2026, 6:59 PM ET. Workflow note: Future ClawSweeper reviews update this same comment in place. How this review workflow works
Summary Reproducibility: yes. at source level: current main routes status-prefixed 403 HTML provider responses through PR rating Rank-up moves:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. Real behavior proof Risk before merge
Maintainer options:
Next step before merge Security Review findings
Review detailsBest possible solution: Keep the narrow diagnostic classifier, align it with existing challenge-specific markers while preserving WAF false-positive guards, drop unrelated churn, and require redacted after-fix real behavior proof before merge. Do we have a high-confidence way to reproduce the issue? Yes at source level: current main routes status-prefixed 403 HTML provider responses through Is this the best way to solve the issue? No, not yet. A distinct failure kind with cautious browser-compatible proxy copy is the right narrow direction, but the matcher should cover existing challenge-specific markers and the PR still needs real behavior proof. Label justifications:
Full review comments:
Overall correctness: patch is incorrect What I checked:
Likely related people:
Codex review notes: model gpt-5.5, reasoning high; reviewed against ecb6da9289b1. |
|
ClawSweeper PR egg 🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat. Where did the egg go?
|
|
Thanks for the patch. Closing with #67670 because this targets the legacy provider-direct Current supported config is canonical |
Summary
openai-codexprovider accessed via proxy from mainland China. Current error message says "Authentication failed" which is misleading — the issue is TLS fingerprint detection, not auth.cloudflare_challengefailure kind to detect Cloudflare challenge pages (403 withcf-browser-verification,challenges.cloudflare.com, etc.) and return actionable error message explaining TLS fingerprinting + reverse proxy workaround.auth_html_403. No changes to transport layer, provider config, or TLS implementation.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Root Cause (if applicable)
classifyProviderRuntimeFailureKindclassifies all 403 HTML responses asauth_html_403without checking for Cloudflare-specific challenge patterns. Cloudflare challenge pages contain distinctive markers (cf-browser-verification,challenges.cloudflare.com) that distinguish them from genuine auth failures.Regression Test Plan (if applicable)
src/agents/pi-embedded-helpers/provider-error-patterns.test.tscf-browser-verification,challenges.cloudflare.com,_cf_chl_,just a moment,ray id:) returnscloudflare_challengefailure kind, notauth_html_403.User-visible / Behavior Changes
Before: Users seeing Cloudflare 403 challenges get misleading error:
After: Users get actionable error explaining root cause and workaround:
Diagram (if applicable)
Security Impact (required)
NoNoNoNoNoRepro + Verification
Environment
Steps
openai-codexprovider with valid OAuth tokenopenai-codexproviderExpected
Error message explains TLS fingerprint detection and suggests reverse proxy workaround.
Actual
Before fix: "Authentication failed with an HTML 403 response from the provider."
After fix: "Cloudflare blocked the request with a browser challenge (HTTP 403). Node.js TLS fingerprint was detected as non-browser traffic. Set up a local reverse proxy using a browser-compatible HTTP client (e.g. Python cloudscraper) and point the provider's baseUrl to it."
Evidence
Test results:
Human Verification (required)
Verified scenarios:
cf-browser-verification,challenges.cloudflare.com,_cf_chl_,just a moment,ray id:) →cloudflare_challengeclassificationauth_html_403classification (regression guard)Edge cases checked:
auth_html_403cloudflare_challenge(status check first)cf-browser-verificationor other markers fall through to generic classificationWhat I did NOT verify:
openai-codexprovider endpointReview Conversations
Compatibility / Migration
YesNoNoRisks and Mitigations
Risk: False positives — generic HTML pages containing "cloudflare" text might get misclassified as challenges.
cf-browser-verification,challenges.cloudflare.com, etc.), not just keyword "cloudflare". Pattern is case-insensitive and matches partial strings safely.Risk: Cloudflare changes challenge page structure, patterns no longer match.