fix(gateway): require bearer auth for /v1/admin/* - #100
Merged
Conversation
Public :80/:443 exposed unauthenticated seal and backend registry CRUD. Fail-closed when REQUIRE_OWNER=1; wire token into seal/register scripts.
📝 WalkthroughWalkthroughThe gateway now protects ChangesAdmin bearer authentication
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant Gateway
participant AdminAuth
participant AdminEndpoint
Operator->>Gateway: Send /v1/admin/* with Authorization header
Gateway->>AdminAuth: Validate bearer token
AdminAuth-->>Gateway: Allow or reject request
Gateway->>AdminEndpoint: Forward authenticated request
AdminEndpoint-->>Operator: Return admin response
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Normalize ./ // .. before is_admin_path so public /challenge/*/v1/./admin/* cannot bypass the master-local admin gate after reqwest collapses the path.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
crates/gateway-core/src/admin_auth.rs (2)
47-48: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAccept mixed-case truthy values for
BASE_GATEWAY_REQUIRE_OWNER.The match list covers
1,true,TRUE,yes, andYES. A value such asTrueorYesdisables the fail-closed check. The gateway then starts with open/v1/admin/*routes. Compare case-insensitively.♻️ Proposed case-insensitive parse
let require_owner = std::env::var(REQUIRE_OWNER_ENV) - .is_ok_and(|v| matches!(v.trim(), "1" | "true" | "TRUE" | "yes" | "YES")); + .is_ok_and(|v| { + let v = v.trim(); + v == "1" || v.eq_ignore_ascii_case("true") || v.eq_ignore_ascii_case("yes") + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/gateway-core/src/admin_auth.rs` around lines 47 - 48, Update the require_owner parsing in the environment-variable initialization to compare truthy values case-insensitively, so mixed-case variants such as “True” and “Yes” are accepted alongside “1”. Preserve trimming and the existing fail-closed behavior for all other values.
143-147: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winParse the
Bearerscheme case-insensitively.RFC 7235 defines the authentication scheme token as case-insensitive. The current code accepts only
Bearerandbearer. A client that sendsBEARER <token>receives 401. Match the scheme without case sensitivity.♻️ Proposed scheme parse
fn bearer_from_headers(headers: &axum::http::HeaderMap) -> Option<String> { let raw = headers.get(header::AUTHORIZATION)?.to_str().ok()?; - let token = raw - .strip_prefix("Bearer ") - .or_else(|| raw.strip_prefix("bearer "))?; + let (scheme, token) = raw.split_once(' ')?; + if !scheme.eq_ignore_ascii_case("bearer") { + return None; + } let token = token.trim();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/gateway-core/src/admin_auth.rs` around lines 143 - 147, Update bearer_from_headers to parse the Authorization scheme case-insensitively, accepting BEARER and any other casing while preserving the existing token extraction and None behavior for invalid headers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@deploy/secrets/README.md`:
- Around line 22-26: Update the token-generation commands in the README to
create a mode-0600 temporary file, apply UID 65532 ownership and final mode 0400
before publication, then atomically rename it to gateway_admin_token; avoid
redirecting directly into the runtime secret path.
In `@docs/OPERATOR_SECURITY.md`:
- Line 56: Update the deployment guidance in OPERATOR_SECURITY.md so staging and
production require BASE_GATEWAY_ADMIN_TOKEN_FILE, with the token supplied
through age-encrypted files under deploy/env/ or deploy/secrets/. Restrict
BASE_GATEWAY_ADMIN_TOKEN to local tests only, and preserve the requirement that
an owner-protected gateway cannot expose /v1/admin/* without a configured token.
---
Nitpick comments:
In `@crates/gateway-core/src/admin_auth.rs`:
- Around line 47-48: Update the require_owner parsing in the
environment-variable initialization to compare truthy values case-insensitively,
so mixed-case variants such as “True” and “Yes” are accepted alongside “1”.
Preserve trimming and the existing fail-closed behavior for all other values.
- Around line 143-147: Update bearer_from_headers to parse the Authorization
scheme case-insensitively, accepting BEARER and any other casing while
preserving the existing token extraction and None behavior for invalid headers.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4033b048-afa4-4103-8fdb-2d578f692955
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
AGENTS.mdbins/weights-smoke/src/main.rscrates/gateway-core/Cargo.tomlcrates/gateway-core/src/admin_attest.rscrates/gateway-core/src/admin_auth.rscrates/gateway-core/src/lib.rscrates/gateway/src/lib.rscrates/gateway/src/proxy.rscrates/gateway/tests/admin_auth.rsdeploy/compose/env-prod.ymldeploy/compose/env-staging.ymldeploy/env/gateway.env.exampledeploy/scripts/prod-burn-seal.shdeploy/scripts/prod-real-seal.shdeploy/scripts/register-challenge-backends.shdeploy/secrets/README.mddocs/OPERATOR_SECURITY.md
Comment on lines
+22
to
+26
| ```bash | ||
| # Generate once per environment; never commit the bytes. | ||
| openssl rand -hex 32 > deploy/secrets/gateway_admin_token | ||
| chown 65532:65532 deploy/secrets/gateway_admin_token | ||
| chmod 0400 deploy/secrets/gateway_admin_token |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Create the token with restrictive permissions from the start.
The redirection creates deploy/secrets/gateway_admin_token before chown and chmod. With a permissive umask, another local user can read the token during this window. Generate a mode-0600 temporary file, secure it, and atomically rename it into place.
Proposed fix
-# Generate once per environment; never commit the bytes.
-openssl rand -hex 32 > deploy/secrets/gateway_admin_token
-chown 65532:65532 deploy/secrets/gateway_admin_token
-chmod 0400 deploy/secrets/gateway_admin_token
+# Generate once per environment; never commit the bytes.
+umask 077
+tmp="$(mktemp deploy/secrets/gateway_admin_token.XXXXXX)"
+trap 'rm -f "$tmp"' EXIT
+openssl rand -hex 32 >"$tmp"
+chown 65532:65532 "$tmp"
+chmod 0400 "$tmp"
+mv -f "$tmp" deploy/secrets/gateway_admin_tokenAs per coding guidelines, runtime secret files must use mode 0400 and be owned by UID 65532.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ```bash | |
| # Generate once per environment; never commit the bytes. | |
| openssl rand -hex 32 > deploy/secrets/gateway_admin_token | |
| chown 65532:65532 deploy/secrets/gateway_admin_token | |
| chmod 0400 deploy/secrets/gateway_admin_token | |
| # Generate once per environment; never commit the bytes. | |
| umask 077 | |
| tmp="$(mktemp deploy/secrets/gateway_admin_token.XXXXXX)" | |
| trap 'rm -f "$tmp"' EXIT | |
| openssl rand -hex 32 >"$tmp" | |
| chown 65532:65532 "$tmp" | |
| chmod 0400 "$tmp" | |
| mv -f "$tmp" deploy/secrets/gateway_admin_token |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@deploy/secrets/README.md` around lines 22 - 26, Update the token-generation
commands in the README to create a mode-0600 temporary file, apply UID 65532
ownership and final mode 0400 before publication, then atomically rename it to
gateway_admin_token; avoid redirecting directly into the runtime secret path.
Source: Coding guidelines
| ## 4. Gateway and TLS | ||
|
|
||
| - [ ] Gateway hotkey equals on-chain `SubnetOwnerHotkey` (else process exits 2). | ||
| - [ ] `BASE_GATEWAY_ADMIN_TOKEN_FILE` (or `BASE_GATEWAY_ADMIN_TOKEN`) is set whenever `BASE_GATEWAY_REQUIRE_OWNER=1` — `/v1/admin/*` must not be open on a public listener. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Require the file-based token in deployed environments.
BASE_GATEWAY_ADMIN_TOKEN places the bearer token in the process environment. The deployment guidance requires secrets through age-encrypted files under deploy/env/ or deploy/secrets/. Require BASE_GATEWAY_ADMIN_TOKEN_FILE for staging and production, and limit the direct environment form to local tests.
As per coding guidelines, secrets must use age-encrypted files under deploy/env/ or deploy/secrets/.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/OPERATOR_SECURITY.md` at line 56, Update the deployment guidance in
OPERATOR_SECURITY.md so staging and production require
BASE_GATEWAY_ADMIN_TOKEN_FILE, with the token supplied through age-encrypted
files under deploy/env/ or deploy/secrets/. Restrict BASE_GATEWAY_ADMIN_TOKEN to
local tests only, and preserve the requirement that an owner-protected gateway
cannot expose /v1/admin/* without a configured token.
Source: Coding guidelines
4 tasks
echobt
added a commit
that referenced
this pull request
Aug 8, 2026
After #100, POST /v1/admin/backends requires Authorization: Bearer. remote-deploy's inline reseed omitted it and staging master CI failed 401.
3 tasks
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
1 participant
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
/v1/admin/*(seal, backends, attest-grant) wheneverBASE_GATEWAY_REQUIRE_OWNER=1, so the public listener is not an open admin plane.is_admin_path/is_view_pathso public/challenge/*/v1/./admin/*cannot bypass the master-local admin gate after reqwest collapses.segments.Test plan
cargo test -p gateway --lib proxy::testsGET /challenge/design/v1/./admin/rounds/1/candidates→ 403 (not upstream 401)GET /v1/admin/backends→ 401/403 without bearerSummary by CodeRabbit
New Features
Security
401 Unauthorizedwhen credentials are missing or invalid.Documentation