fix: require validated API key on GET /usage (GHSA-5rhx-xgvv-h78h)#183
Draft
dobby-coder[bot] wants to merge 1 commit into
Draft
fix: require validated API key on GET /usage (GHSA-5rhx-xgvv-h78h)#183dobby-coder[bot] wants to merge 1 commit into
dobby-coder[bot] wants to merge 1 commit into
Conversation
The `/usage` route used the `ApiKey` guard, whose `FromRequest` always succeeds (yielding the anonymous default tier when no key is present). The handler's default-tier branch then looked up usage by the caller-supplied `email` query parameter with no ownership check, so any unauthenticated caller could probe usage for an arbitrary address. Introduce a `ValidatedApiKey` request guard that fails the request (401 on NoCredentials/Rejected, 503 on PkgUnreachable) so the "authenticated" intent is enforced by the type system. Switch `/usage` to that guard and key the lookup on the validated tenant; the `email` query param is now optional and only echoed back. Also make the `cryptify_token` comparisons constant-time via `subtle::ConstantTimeEq` (new `cryptify_tokens_match` helper), mirroring `recovery_tokens_match`, in `check_cryptify_token`, `upload_chunk`, and `classify_chunk_request`. Adds a regression test asserting unauthenticated `/usage` returns 401, and updates the OpenAPI description. Co-Authored-By: Claude Opus 4.8 <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.
Addresses the access-control issue tracked in the draft security advisory GHSA-5rhx-xgvv-h78h (see #182). Specifics of the vulnerability are intentionally kept in the private advisory.
Changes
Enforce authentication on
GET /usageby construction. Adds aValidatedApiKeyrequest guard whoseFromRequestfails the request rather than degrading to the anonymous default tier:NoCredentials/Rejected→401 UnauthorizedPkgUnreachable→503 Service Unavailable(the key cannot be confirmed)This contrasts with the existing
ApiKeyguard, which always returnsOutcome::Success./usagenow carriesValidatedApiKey, so the "authenticated" intent is enforced by the type system. Usage is keyed to the validated tenant; theemailquery parameter is now optional and only echoed back in the response — it no longer drives the lookup.Constant-time
cryptify_tokencomparison. Newcryptify_tokens_matchhelper usingsubtle::ConstantTimeEq, mirroringrecovery_tokens_match. Applied incheck_cryptify_token(finalize) and the chunk-upload token checks (upload_chunk,classify_chunk_request).Regression test
usage_rejects_unauthenticated_request: an unauthenticatedGET /usagenow returns401.OpenAPI + CHANGELOG updated to reflect the auth requirement.
Behavioural note
GET /usagenow requires a validAuthorization: Bearer PG-…API key. Callers on the default (no-API-key) tier that previously used this endpoint will receive401.Verification
cargo fmt --all -- --check✅cargo clippy --all-targets -- -D warnings✅cargo test --all-targets✅ (129 passed)Kept as draft pending review.