Repository navigation
fix(lab-ops-hub): validate the start URL in fetchFollowingValidatedRedirects (#464) - #465
Conversation
…directs The Worker-fallback crawler's redirect-following helper validated every redirect target with validateCrawlUrl but fetched its startUrl argument unchecked, relying on its one caller pre-validating. Validate the start URL at entry, throw before fetching anything for a non-public or non-http(s) start, and fetch the validated/normalised URL first — the helper is now safe by construction, matching the container crawler's defence-in-depth design. Fixes #464
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 16 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Problem
fetchFollowingValidatedRedirects(the Worker-fallback crawler's SSRF guard from PR #461, fixing #460) validated every redirect target withvalidateCrawlUrl— but fetched itsstartUrlargument without validating it at all. The fetching component did not enforce the #441 policy itself; it relied entirely on its one current caller pre-validating. Any future caller, or a refactor dropping the handler's rewrite, silently re-opens the #460/#441 SSRF on hop 0. Reproduced on main468b9155:fetchFollowingValidatedRedirects("http://127.0.0.1/secret", stub)fetched the URL and returned its 200 body.Solution
Validate
startUrlwithvalidateCrawlUrlat entry: throwrefusing to crawl <url>: <reason>before fetching anything for a non-public / non-http(s) start, and make the first fetch use the validated/normalised URL. Redirect hops are unchanged. This matches the container crawler's stated design (the component that actually fetches re-validates independently, defence in depth) and breaks no current caller — the/api/crawlhandler's URL is already validated/normalised.Verification
2130706433, IPv4-mapped[::ffff:127.0.0.1], localhost,file:, junk) are refused with the fetcher never invoked. Ablation-verified: fails pre-fix (9 pass / 1 fail), passes post-fix.npm test: 10/10 (existing public-start redirect tests unchanged and passing).tsc --noEmit: clean.Fixes #464
Need help on this PR? Tag
@codesmith-botwith what you need.