Skip to content

fix: return Ok(None) when project not found in project_data_with#33

Merged
geekbrother merged 1 commit into
mainfrom
fix/project_data_not_found
Jun 18, 2026
Merged

fix: return Ok(None) when project not found in project_data_with#33
geekbrother merged 1 commit into
mainfrom
fix/project_data_not_found

Conversation

@geekbrother

@geekbrother geekbrother commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Summary

project_data_with_impl mapped a registry 404 (project not found) to
Err(RegistryError::Response("Project not found")) instead of Ok(None).
This is inconsistent with project_data_with_limits_impl, which correctly
returns 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 registry
errors therefore let any well-formed-but-unregistered project ID through.

Concretely in blockchain-api:

  • project_data_internal treats any non-Config RegistryError as a transient
    outage → opens its circuit breaker → returns RegistryTemporarilyUnavailable.
  • validate_project_access[_and_quota] treats RegistryTemporarilyUnavailable
    as "skip the check" (fail open).

Net effect: a 32-hex project ID that isn't registered (e.g.
00000000000000000000000000000000) bypasses project access + quota validation
entirely. 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 returns None (registry
404), 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 the
project-data endpoint and asserts project_data_with(...) returns Ok(None).

`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 geekbrother self-assigned this Jun 18, 2026
@geekbrother geekbrother marked this pull request as ready for review June 18, 2026 08:22
@geekbrother geekbrother requested a review from xDarksome as a code owner June 18, 2026 08:22
@geekbrother geekbrother merged commit 7fafad6 into main Jun 18, 2026
9 of 11 checks passed
@geekbrother geekbrother deleted the fix/project_data_not_found branch June 18, 2026 08:23
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant