fix: return Ok(None) when project not found in project_data_with#33
Merged
Conversation
`project_data_with_impl` mapped a registry 404 (project not found) to `Err(RegistryError::Response(...))` instead of `Ok(None)`. Callers that classify registry errors as transient outages then fail open, letting any well-formed but unregistered project id through. Surface a missing project as `Ok(None)`, matching `project_data_with_limits_impl`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
geekbrother
pushed a commit
that referenced
this pull request
Jun 18, 2026
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
added a commit
that referenced
this pull request
Jun 19, 2026
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: root <root@v2202606366752468913.goodsrv.de> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
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
project_data_with_implmapped a registry 404 (project not found) toErr(RegistryError::Response("Project not found"))instead ofOk(None).This is inconsistent with
project_data_with_limits_impl, which correctlyreturns
Ok(None)when the base project lookup misses.Why this matters
Callers can't tell "project doesn't exist" apart from "registry had a transient
problem" — both arrive as
Err. Services that fail open on transient registryerrors therefore let any well-formed-but-unregistered project ID through.
Concretely in blockchain-api:
project_data_internaltreats any non-ConfigRegistryErroras a transientoutage → opens its circuit breaker → returns
RegistryTemporarilyUnavailable.validate_project_access[_and_quota]treatsRegistryTemporarilyUnavailableas "skip the check" (fail open).
Net effect: a 32-hex project ID that isn't registered (e.g.
00000000000000000000000000000000) bypasses project access + quota validationentirely. Only malformed IDs (wrong length / non-hex) get rejected, since those
are caught earlier by
is_valid_project_id.Fix
Return
Ok(None)when the base project-data lookup returnsNone(registry404), matching
project_data_with_limits_impl. Callers can then treat it as"not found" and reject, instead of failing open.
Test
Adds
project_data_with_returns_none_when_project_not_found: mocks a 404 on theproject-data endpoint and asserts
project_data_with(...)returnsOk(None).