fix(registry): harden against fail-open authorization bugs#34
Merged
Conversation
Follow-up hardening to #33, which fixed `project_data_with_impl` mapping a 404 (project not found) to `Err` instead of `Ok(None)` — a fail-open bug, since callers treat registry errors as transient outages and let the request through. This addresses the same bug class across the rest of the crate. - registry/client.rs: the #33 fix only covered the base project fetch. The `limits` and `features` sub-fetches in the same function still mapped a 404 to `RegistryError::Response`, reintroducing the identical fail-open risk for plan/rate-limit and feature enforcement. Surface them as `Ok(None)`, matching `project_data_with_limits_impl`. - registry/error.rs + parse_http_response: split the catch-all `Response` error into `Forbidden` (403), `RateLimited` (429) and `ServerError` (5xx), so callers can fail closed on terminal denials instead of classifying every non-2xx as a retryable outage. - project/types/origin.rs: anchor the origin parser regex so malformed input is rejected rather than silently truncated to an unintended host; reject empty hostname labels so a `*.example.com` wildcard cannot match `.example.com`; compare scheme and hostname labels case-insensitively. Adds regression tests for each: limits/features 404 -> Ok(None), 403/5xx error mapping, empty-label and trailing-garbage rejection, and case-insensitive matching. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
geekbrother
force-pushed
the
fix/registry-fail-open-hardening
branch
from
e0489de to
ac982b1
Compare
chris13524
approved these changes
Jun 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up security hardening to #33. That PR fixed
project_data_with_implmapping a registry 404 (project not found) toErr(RegistryError::Response(...))instead ofOk(None)— a fail-open bug, because callers classify registry errors as transient outages and let the request through, authorizing any well-formed but unregistered project id.This PR audits the rest of the crate for the same bug class and hardens the adjacent origin-parsing logic.
Findings & fixes
1. HIGH — same fail-open bug still live in the patched function (
registry/client.rs)#33 only fixed the base project fetch. The
limitsandfeaturessub-fetches in the same function still did.ok_or_else(|| RegistryError::Response(...))?, so a 404 on either endpoint produced the exact transient-outage-looking error #33 warned about. A caller usingproject_data_with(req.include_limits())to enforce plan/rate limits would fail open. Now returnsOk(None), matchingproject_data_with_limits_impl.2. MEDIUM — origin parser regex was unanchored (
origin.rs)The hostname group stopped at the first
/or:and silently ignored the rest. Now fully anchored (optional trailing path still accepted and ignored); empty hostname labels are rejected so a*.example.comwildcard cannot match.example.com.3. LOW — case-sensitive scheme/host comparison (
origin.rs)Hostnames and schemes are case-insensitive per RFC; comparison now uses
eq_ignore_ascii_case.4. LOW — error granularity invited caller fail-open (
registry/error.rs)Everything except 2xx/401/404 collapsed into one
Responsevariant, so callers couldn't tell a terminal 403 from a 500 outage. Split intoForbidden(403),RateLimited(429),ServerError(5xx) so callers can fail closed on terminal denials.Breaking changes
RegistryErrorgainsForbidden,RateLimited,ServerErrorvariants (affects exhaustive matches).Testing
cargo test(30 passing, +6 new),cargo clippy --all-targetsclean,cargo fmt --checkclean. New regression tests cover: limits/features 404 →Ok(None), 403/5xx mapping, empty-label & trailing-garbage rejection, and case-insensitive matching.Prevention
A CI grep gate for
ok_or_else(|| RegistryError::inclient.rs, plus the per-dependency 404 tests added here, would catch this class going forward. Recommend documenting theOk(None)= deny /Err= transient contract on theRegistryClienttrait.🤖 Generated with Claude Code