Repository navigation
fix(lab-ops-hub): refuse DEL/C1/NBSP and WHATWG-forbidden hosts in container validator (#510) - #515
Conversation
…ntainer validator (#510) The #507 check stopped at 0x20: DEL (0x7F), the C1 controls (0x80-0x9F), NBSP (0xA0) and the WHATWG forbidden host codepoints < > ^ | still fell through validate_crawl_url as 'a public DNS name', while the Worker's validateCrawlUrl refuses every one. Extend the up-front check to ord <= 0x20 or 0x7F <= ord <= 0xA0 or one of <>^|. Non-ASCII above NBSP (valid IDN hosts) is untouched. New regression test covers each refused class plus IDN/underscore controls; it fails with only container/app.py reverted. Differential probe over the issue's battery (printable ASCII + 0x7F-0xA0 + é + combining char) now shows full TS/Python parity.
|
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 52 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. |
Fixes #510
Problem
PR #508 (#507) added an up-front host check to the container crawler's
validate_crawl_url(pkgs/lab-ops-hub/container/app.py), but it stopped atord(c) <= 0x20— C0 controls and space only. The rest of the WHATWG forbidden-host set still fell through and was certified as "a public DNS name", while the Worker'svalidateCrawlUrlrefuses every one of them:http.client's own disallowed set, so the crawl died deep inside the fetcher instead of at the validator<,>,^,|Reproduced on main
025794c8by driving the real Python validator: all nine forms ALLOW, while Node's WHATWG URL parser (the TS validator's first step) rejects every one.Solution
Extend the existing up-front check in
validate_crawl_urlto refuse a host containing any character withord(c) <= 0x20,0x7F <= ord(c) <= 0xA0, or one of<>^|, with a clear validator error. Nothing else changes: non-ASCII above NBSP (valid IDN hosts, e.g.examéple.com),exam_ple.com, ordinary DNS names, valid IPv6 literals and public targets are still accepted on both sides.Verification
test_refuses_del_c1_nbsp_and_forbidden_host_characterscovers each refused class (DEL, C1 low/NEL/high, NBSP,< > ^ |) plus IDN/underscore controls.npm testinpkgs/lab-ops-hub: TS 19/19 + Python 14/14 pass.container/app.pyreverted, the new test fails (ValueError not raisedfor the DEL host); restored, the suite is green.ruff formatapplied (the repo's treefmt Python formatter); diff is exactly the validator check + the new test.Single-repo: the validator pair lives only in developing-today/code (per #510's cross-repo check).
Note: an extended probe beyond the issue's battery (Unicode space separators, bidi/format controls, U+FFFD) shows a further IDNA-class divergence — filed separately as a follow-up issue rather than expanding this fix.
Need help on this PR? Tag
@codesmith-botwith what you need.