docs(rfc): explicit OURIOS_S3_* credential env keys (RFC 0019 §3.4 + RFC0019.8) - #306
Conversation
…RFC0019.8) RFC 0019 §3.4 currently states "No Ourios-specific credential config is introduced" and relies entirely on the AWS credential chain (the AWS_*-named env vars object_store's AmazonS3Builder::from_env() reads). Ourios is S3-compatible, not AWS-specific, so the credential env surface should be S3-named too. Amend §3.4 to a two-layer, explicit-over-chain model: Ourios reads OURIOS_S3_ACCESS_KEY_ID / OURIOS_S3_SECRET_ACCESS_KEY / OURIOS_S3_SESSION_TOKEN and applies them to the builder; the from_env() chain is retained as the fallback (it is the only path for AWS IRSA, whose AWS_ROLE_ARN / AWS_WEB_IDENTITY_TOKEN_FILE the EKS webhook injects and which have no Ourios-named equivalent). Secret hygiene (§3.4 / RFC0019.6) is widened, not relaxed: Ourios now *reads* credential material (it read none before), so it owns redaction of the OURIOS_S3_* secret keys across config errors, StoreError, logs, and metric attributes. New scenario RFC0019.8 gates both the explicit-credential authentication path and the widened redaction; it is greened by the implementation PR. Spec-only (no code). mdbook build verified. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 28 minutes and 42 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?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 credits. 🚦 How do rate 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 see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a "§9 Amendment (2026-06-28)" to RFC0019, defining optional RFC0019 §9 Amendment
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 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
This PR amends RFC 0019’s storage-backend selection spec to allow S3 credentials to be provided via Ourios-named OURIOS_S3_* environment variables (while retaining the standard AWS credential chain as a fallback), and updates the RFC’s acceptance criteria/testing strategy accordingly.
Changes:
- Specify a two-layer S3 credential resolution model: explicit
OURIOS_S3_*credentials preferred, credential-chain fallback retained (e.g., for IRSA). - Extend the RFC’s config surface documentation to include
OURIOS_S3_ACCESS_KEY_ID,OURIOS_S3_SECRET_ACCESS_KEY, andOURIOS_S3_SESSION_TOKEN. - Add a new scenario (RFC0019.8) covering explicit credentials and widened secret-redaction expectations.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…-cred fail-fast Two review points on the spec: 1. status consistency (docs/rfcs/README.md: green = all §5 criteria pass) — the amendment added RFC0019.8 to §5 while the RFC stays green. Resolved by keeping the green body intact and moving the entire change into a dated §9 amendment (specified, pending impl); the implementation PR promotes RFC0019.8 into §5 and folds the env vars/design into §3.1/§3.4, keeping green honest. 2. partial/invalid credential combinations were unspecified. §9.3 now states a fail-fast rule: access key id and secret are a pair (set one → both required); a session token without that pair fails too; the error names only the key, never a value (§3.4/RFC0019.6). Otherwise fall back to the credential chain. mdbook build verified. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/rfcs/0019-storage-backend-selection.md (1)
326-332: 🧹 Nitpick | 🔵 TrivialClarify "all unset" to avoid ambiguity with the optional session token.
The phrase "When the explicit keys are all unset" could be read as requiring all three
OURIOS_S3_*variables to be unset before falling back to the credential chain. SinceOURIOS_S3_SESSION_TOKENis optional and meaningless without the static pair, consider rephrasing to "When the explicit static credential pair is unset" or similar to make clear only the access key / secret key pair gates the fallback.🤖 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/rfcs/0019-storage-backend-selection.md` around lines 326 - 332, The fallback wording is ambiguous because “all unset” could imply the optional session token must also be absent before using the standard credential chain. Update the RFC text in the storage backend selection section to say that the fallback applies when the explicit static credential pair is unset, and make clear that OURIOS_S3_SESSION_TOKEN is optional and does not by itself gate AmazonS3Builder::from_env() fallback behavior.
🤖 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.
Nitpick comments:
In `@docs/rfcs/0019-storage-backend-selection.md`:
- Around line 326-332: The fallback wording is ambiguous because “all unset”
could imply the optional session token must also be absent before using the
standard credential chain. Update the RFC text in the storage backend selection
section to say that the fallback applies when the explicit static credential
pair is unset, and make clear that OURIOS_S3_SESSION_TOKEN is optional and does
not by itself gate AmazonS3Builder::from_env() fallback behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 63508733-3a09-45ee-a6a1-d99d9f576b60
📒 Files selected for processing (1)
docs/rfcs/0019-storage-backend-selection.md
…qualify from_env - §3.4 note: name the OURIOS_S3_ACCESS_KEY_ID/SECRET_ACCESS_KEY/SESSION_TOKEN credential keys explicitly, distinct from the non-secret OURIOS_S3_* addressing keys (the bare prefix was ambiguous). - §9.2 table: OURIOS_S3_SESSION_TOKEN default now states it is valid only with the static key pair (§9.3), not independently supplied. - §9.5 RFC0019.8: fully-qualify AmazonS3Builder::from_env(). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…een status Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…_* shorthand Copilot flagged every OURIOS_S3_* in §9 as ambiguous with the non-secret addressing keys. Define the term "S3 credential keys" once in §9.2 and use it in §9.4/§9.5/§9.6 instead of the prefix shorthand. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs(rfc): accept RFC 0019 — storage-backend selection (maintainer sign-off) Promote RFC 0019 green -> accepted (terminal). All eight §5 criteria pass (green since #301, amended #306/#307): backend selection + credential scrub unit tests, and the localstack S3 integration covering WAL-stays-local, ingest→query on S3, the compactor manifest swap, and cross-tenant isolation. No validated stage applies — it's server wiring, not a thesis gate — so acceptance follows green directly (RFC 0001/0008 precedent). §9 records the sign-off. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X467Druw4cQeEPVp52DTG3 * docs(rfc): sync §6 stage line with accepted status (RFC 0019) Copilot review on #455: §6 still said "the RFC is `green`" after the status flipped to `accepted` — internal inconsistency. Now "accepted (§9)". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X467Druw4cQeEPVp52DTG3 --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
What
Amends RFC 0019 §3.4 so S3 credentials can be supplied via S3-named env
vars (
OURIOS_S3_ACCESS_KEY_ID/OURIOS_S3_SECRET_ACCESS_KEY/OURIOS_S3_SESSION_TOKEN) instead of only the AWS-SDK-named credential chain.Ourios is S3-compatible (AWS S3 and MinIO/R2/Hetzner/Ceph/…), so its own
credential surface should not read as AWS-specific.
Spec-only — no code. The implementation lands in a follow-up PR; the
S3-native Helm chart (#304) then adopts the S3-named Secret keys and merges.
Design (two-layer, explicit-over-chain)
OURIOS_S3_*keys are read and applied to theAmazonS3Builder(with_access_key_id/with_secret_access_key/with_token). These authenticate AWS S3 and every S3-compatible storeidentically.
AmazonS3Builder::from_env()resolves the usual way (staticAWS_*, sharedprofile, IRSA, instance metadata). Retained deliberately: AWS IRSA
injects its own
AWS_ROLE_ARN/AWS_WEB_IDENTITY_TOKEN_FILE(EKS webhook),which have no Ourios-named equivalent — on EKS the chain is the source.
Invariant touched — §3.4 secret hygiene (RFC0019.6)
Widened, not relaxed. Ourios now reads credential material (it read none
before), so it owns redaction: an
OURIOS_S3_*secret value is never echoed ina config error,
StoreError, log line, or metric attribute. Themissing-required error still names only the key (
OURIOS_S3_BUCKET). Newscenario RFC0019.8 gates both the explicit-credential authentication path
(localstack,
OURIOS_S3_*only, noAWS_*) and the widened redaction(extends the
rfc0019_6_*unit test).Verification
mdbook build— clean.🤖 Generated with Claude Code
Summary by CodeRabbit