feat(server): rfc 0026 green a — the token store (RFC0026.1) - #390
Conversation
auth.tokens config section (Option-al: absent = open mode, present-but-
empty = startup error), ${env:...}-only token values with parse-time
inline-literal rejection and Debug redaction, the validated TokenStore
(constant-time authenticate via subtle, TenantSet::All | Listed) both
enforcement slices will consume, and the registry-backed
ourios.server.auth.open_mode startup warning.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 49 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR implements RFC 0026 bearer-token authentication with tenant binding for ourios-server. It adds a ChangesRFC 0026 Bearer-Token Authentication
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Main as main.rs
participant FileConfig
participant Auth as auth::build_token_store
participant Store as TokenStore
participant Client
Main->>FileConfig: parse config file (auth section)
Main->>Auth: build_token_store(auth section)
Auth-->>Main: Option<TokenStore>
Main->>Main: warn if auth is None and roles enabled
Client->>Store: authenticate(presented token)
Store->>Store: constant-time compare against tokens
Store-->>Client: Option<&ResolvedToken>
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Pull request overview
Implements RFC 0026 green slice (a) by introducing a configuration-backed bearer-token store (auth.tokens) for the server, including secret-hygiene rules (env-only tokens), validation into a resolved TokenStore, and an “open mode” startup warning/event when network roles are enabled without auth configured.
Changes:
- Add
auth.tokensschema support with per-leaf${env:…}substitution and inline-literal token rejection + redactedDebug. - Introduce
ourios_server::authwithbuild_token_store,TokenStore::authenticate(constant-time per-candidate compare), andTenantSet. - Wire the resolved auth store into
ourios-serverstartup, emitting the new registry-backedourios.server.auth.open_modewarning when applicable, plus add scenario tests.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| semconv/registry/events.yaml | Registers new ourios.server.auth.open_mode event in the semconv registry. |
| crates/ourios-server/tests/rfc0026_auth.rs | Adds spawned-binary tests for empty-token-list startup error and open-mode warning behavior. |
| crates/ourios-server/src/main.rs | Wires auth into ServerConfig, runs shared validation path, and emits open-mode warning at startup. |
| crates/ourios-server/src/lib.rs | Exposes new auth module from the ourios-server library. |
| crates/ourios-server/src/config/file.rs | Adds auth schema, ${env:…} substitution for auth leaves, inline-token literal check, and tests. |
| crates/ourios-server/src/auth.rs | New resolved token-store implementation + validation + unit tests. |
| crates/ourios-server/Cargo.toml | Adds subtle dependency for constant-time comparisons. |
| crates/ourios-semconv/src/lib.rs | Regenerates semconv constants to include EVENT_OURIOS_SERVER_AUTH_OPEN_MODE. |
| Cargo.lock | Locks in subtle as a direct dependency of ourios-server. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
An undrained stderr pipe could fill and block the spawned server before its stdout readiness line; and a literal token: "" now fails the parse-time reference rule (a bearer token has no unset-with-fallback reading, unlike an S3 credential) instead of the misleading resolved-to-empty message, with an absent key getting its own token-is-required error. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/ourios-server/src/auth.rs (1)
87-100: 🩺 Stability & Availability | 🔵 TrivialRemember Prometheus metrics/tracing once
authenticateis wired to a hot path.This lookup isn't called from a request path in this slice (enforcement is deferred per the PR description), so no gap exists yet. As per coding guidelines,
**/crates/ourios-{ingester,querier,server}/**/*.rsshould "Use Prometheus metrics for every subsystem, emit structured logs via Ourios on hot paths, and trace every RPC" once this becomes reachable from the listener enforcement path.🤖 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/ourios-server/src/auth.rs` around lines 87 - 100, The TokenStore::authenticate lookup is currently fine in isolation, but once it is connected to the request listener hot path it should be instrumented per our subsystem guidelines. Update the authenticate call path to emit Prometheus metrics and structured Ourios logs, and add tracing around the request/RPC boundary where this method becomes reachable so the hot path is observable without changing the constant-time token comparison logic.Source: Coding guidelines
🤖 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 `@crates/ourios-server/tests/rfc0026_auth.rs`:
- Around line 41-50: The test command setup around timeout and Command::new
currently leaves the spawned ourios-server process running if the future is
dropped on timeout. Update the Command used for output() to enable
kill_on_drop(true) before launching it so the child process is terminated
automatically if the 15s timeout ever elapses; keep the change localized to the
RFC0026 auth test helper that builds the command.
---
Nitpick comments:
In `@crates/ourios-server/src/auth.rs`:
- Around line 87-100: The TokenStore::authenticate lookup is currently fine in
isolation, but once it is connected to the request listener hot path it should
be instrumented per our subsystem guidelines. Update the authenticate call path
to emit Prometheus metrics and structured Ourios logs, and add tracing around
the request/RPC boundary where this method becomes reachable so the hot path is
observable without changing the constant-time token comparison logic.
🪄 Autofix (Beta)
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: bb3575b7-98ff-47ba-9214-0c2fab63cb48
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
crates/ourios-semconv/src/lib.rscrates/ourios-server/Cargo.tomlcrates/ourios-server/src/auth.rscrates/ourios-server/src/config/file.rscrates/ourios-server/src/lib.rscrates/ourios-server/src/main.rscrates/ourios-server/tests/rfc0026_auth.rssemconv/registry/events.yaml
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
What
RFC 0026 green slice (a) — the token store (Scenario RFC0026.1):
config::file): a top-levelauth.tokenssection —Option<AuthSection>so an absent section (open mode) stays distinguishable from a present-but-empty one (startup error). Entries carryname/token/tenants, get per-leaf${env:…}substitution like every other scalar, and the token value follows the S3-credential secret rules: inline literals are rejected at parse time (FileConfigError::InlineToken, naming the entry index, never the value) andDebugredacts.ourios_server::auth, new lib module):build_token_storevalidates the section into aTokenStore— non-empty by construction, unique names (the audit/metric label), unique token values (one value bound to two tenant sets is ambiguous), and aTenantSetenum (All|Listed) so wildcard-vs-list is unrepresentable as a mixed state.TokenStore::authenticatecompares in constant time viasubtle(already in the tree transitively through rustls). Error text names entries by name/index only.src/main.rs):ServerConfig.auth, mapped from the file through the same single validation path as every section; the env-only path always resolves open (tokens ride the${env:…}indirection by design). When no auth is configured and a network role is enabled, startup emits the registry-backedourios.server.auth.open_modewarning naming the exposure.ourios.server.auth.open_mode(weaver-generated constant).Enforcement on the listeners is deliberately not in this slice — it lands with green (b) ingest and (c) query, which consume
TokenStore/TenantSetas built here.Scenario mapping
RFC0026.1goes green across four homes (the RFC 0020 §5 placement pattern):config::fileunit tests,authunit tests,src/main.rsrfc0026_1_auth_section_maps_onto_the_token_store,tests/rfc0026_auth.rsagainst the spawned binary..4/.5/.6stubs remain#[ignore]d for the next slices.Invariants / hazards
.6, asserted when enforcement lands).${env:…}indirection; every error/Debug/log surface in this slice names entries, never values — covered by tests on each surface.ourios.server.auth.open_modegoes throughsemconv/registry+ weaver generate (the semconv CI gate).Checks run locally
cargo fmt --all --check,cargo clippy --all-targets --all-features -- -D warnings(touched crates),cargo test -p ourios-server --all-features.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests