Detect Cloudflare challenge on 429#382
Conversation
|
Updates to Preview Branch (fix-waf-detect-cf-429) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThe PR extends Cloudflare WAF detection to recognize HTTP 429 (TooManyRequests) responses as managed challenges when they carry a ChangesCloudflare WAF Detection for 429 Managed Challenges
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Release VersionsApp patch: ChangelogFixed
|
|
🐝 Review App Deployed Homepage: https://hover-pr-382.fly.dev |
Detect Cloudflare challenge on 429
Summary
DetectWAFcf-mitigated check to fire on any non-200 status (was: 403/202 only), so the 429 +Cf-Mitigated: challengeresponse shape Cloudflare uses for Super Bot Fight Mode managed challenges is now classified correctly as a WAF block instead of a generic "Too Many Requests".statusLabelcase for 429 so the recorded reason readscf-mitigated header present on 429rather than the verboseToo Many Requests.cloudflare — cf-mitigated challenge on 429) covering the failing shape captured againsthinu.coandwww.milkcan.com.autoday.Why
Today four jobs against two Shopify storefronts (
hinu.co,www.milkcan.com.au) failed after one task each. The capturedrequest_diagnosticsfor the failed tasks show:That is Cloudflare serving a managed bot challenge, not a true rate limit.
DetectWAFalready has a cf-mitigated branch that would have setdomains.waf_blocked = trueand surfaced a clear error, but it was gated behindisBlockingStatuswhich only matched 403 and 202 — so the verdict silently no-op'd, the executor burnt three retries, and the user saw a misleading "Too Many Requests" failure.The doc comment on
DetectWAFalready described the intent as "cf-mitigated header set on a non-200 response"; this PR aligns the implementation with the documented behaviour.Test plan
go test ./internal/crawler/...— all WAF cases pass including the new 429 case and the existing "must NOT trip on 200" negative case.go vet ./internal/crawler/...gofmt -l— clean.failedafter the first task, not after 3 retriesdomains.waf_blockedflips totruewithwaf_vendor = cloudflareNeed help on this PR? Tag
@codesmithwith what you need.Summary by CodeRabbit
Bug Fixes
Tests
Documentation