feat: credential registry for admin secrets (env as bootstrap only) - #55
Merged
Conversation
Load ADMIN_TOKEN / ADMIN_TOKENS through a process-local CredentialRegistry (get_credential) seeded from optional WAF_IDS_CREDENTIALS_PATH JSON (preferred) or env bootstrap. Expose credentials_source and admin_auth_configured on /healthz without leaking secret values. Document ops/threat-model path.
📝 WalkthroughWalkthrough관리자 자격 증명을 시작 시 Changes관리자 자격 증명 레지스트리 흐름
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant run_from_env
participant CredentialRegistry
participant AppConfig
participant AppState
participant HealthStatus
run_from_env->>CredentialRegistry: 관리자 토큰과 RBAC 토큰 부트스트랩
CredentialRegistry-->>run_from_env: 토큰 조회와 출처 반환
run_from_env->>AppConfig: 관리자 인증 설정
run_from_env->>AppState: credentials_source 설정
AppState->>HealthStatus: 출처와 admin_auth_configured 계산
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
seonghobae
enabled auto-merge (squash)
July 31, 2026 13:28
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
src/lib.rs (1)
1775-1832: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
run_from_env의 자격 증명 배선은 정확합니다. 파일 기반 경로에 대한 통합 테스트를 추가하는 것을 권장합니다.
run_from_env는CredentialRegistry::bootstrap_secrets를 호출하고, 그 결과로AppConfig.admin_token,admin_tokens,with_credentials_source를 올바르게 채웁니다. 다만 기존run_from_env테스트 중WAF_IDS_CREDENTIALS_PATH를 실제로 설정해 파일 기반 자격 증명이 끝까지 반영되는지 확인하는 테스트가 없습니다.credentials.rs의 단위 테스트는bootstrap_secrets만 검증합니다.health_reports_runtime_configuration테스트는with_credentials_source를 직접 호출합니다. 두 부분을run_from_env를 통해 연결하는 종단 테스트를 추가하면 배선 회귀를 조기에 잡을 수 있습니다.원한다면 이 테스트를 작성해 드릴까요?
Also applies to: 1864-1864
🤖 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 `@src/lib.rs` around lines 1775 - 1832, Add an end-to-end test covering run_from_env with WAF_IDS_CREDENTIALS_PATH pointing to a temporary credentials file. Verify the file-backed admin token and admin tokens are applied through AppConfig and the built application, and that the runtime configuration reports the credentials source from the registry. Keep the test focused on the full run_from_env wiring rather than only testing CredentialRegistry::bootstrap_secrets.
🤖 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 `@AGENTS.md`:
- Line 33: Remove the unauthorized “Remaining deviation” exception in the
AGENTS.md configuration guidance by either migrating the operational settings
currently read via std::env::var() in the configuration-loading flow to
CredentialRegistry/KV access, or explicitly documenting a tracking issue for
adopting a durable store and completing that migration.
In `@docs/runbooks/operations.md`:
- Around line 41-42: Update the health reporting description in the operations
runbook to clarify that credentials_source=file means at least one credential
was loaded from a file, even when other missing values were supplemented from
the environment; it does not mean all credentials came from the file. Preserve
the existing file/env/none values and admin_auth_configured description.
- Around line 26-38: Update the credentials-file guidance around
WAF_IDS_CREDENTIALS_PATH to require service-account ownership, chmod 600
permissions, exclusion from repository commits and backups, and external
secret-manager usage in production. Reference the existing
docs/security/threat-model.md hardening requirement and retain the current
example commands.
- Around line 15-19: Update the “Admin secrets (credential registry)” section to
accurately describe the startup data path: CredentialRegistry values are copied
into AppConfig and AppState during bootstrap, and runtime authentication uses
those stored values rather than querying the registry per request. Explicitly
state that changes to the credentials file or bootstrap environment take effect
only after restarting the process.
In `@docs/security/threat-model.md`:
- Line 31: Update the threat-model table entry’s health disclosure wording to
state that health exposes only the two non-sensitive fields, credentials_source
and admin_auth_configured, while preserving that no secret values are disclosed.
In `@src/credentials.rs`:
- Around line 59-65: Update has_admin_auth so the CRED_ADMIN_TOKEN check trims
its value before testing for emptiness, matching the existing CRED_ADMIN_TOKENS
validation and treating whitespace-only tokens as unset.
- Around line 38-44: Replace the derived Debug implementation on
CredentialRegistry with a manual implementation that never formats the values
map or its secret contents. Preserve Debug output for non-sensitive metadata
such as source, and keep the existing Clone and Default derives unchanged.
- Around line 136-149: Update json_value_as_nonempty_string so only non-empty
JSON string values return Some(text); return None for numbers, booleans, arrays,
objects, null, and empty strings, removing the fallback conversion through
to_string().
---
Nitpick comments:
In `@src/lib.rs`:
- Around line 1775-1832: Add an end-to-end test covering run_from_env with
WAF_IDS_CREDENTIALS_PATH pointing to a temporary credentials file. Verify the
file-backed admin token and admin tokens are applied through AppConfig and the
built application, and that the runtime configuration reports the credentials
source from the registry. Keep the test focused on the full run_from_env wiring
rather than only testing CredentialRegistry::bootstrap_secrets.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: dd7af366-3f01-4103-8414-2b7aa9e16f3e
📒 Files selected for processing (5)
AGENTS.mddocs/runbooks/operations.mddocs/security/threat-model.mdsrc/credentials.rssrc/lib.rs
seonghobae
enabled auto-merge (squash)
August 2, 2026 10:32
seonghobae
enabled auto-merge (squash)
August 2, 2026 10:32
seonghobae
added a commit
that referenced
this pull request
Aug 2, 2026
This was referenced Aug 2, 2026
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.
Why
Buyers (and org governance) expect secrets not to be the long-term runtime source of truth via raw
std::env::var. Admin tokens were still read directly from env inrun_from_env.What changed
CredentialRegistrywithget_credentialfor secret-bearing keys (admin_token,admin_tokens).WAF_IDS_CREDENTIALS_PATHJSON per-key, then env fill for missing keys.run_from_envseeds auth from the registry; operational config remains env for now./healthzreportscredentials_source(file|env|none) andadmin_auth_configuredwithout secret values.Buyer impact
Operators can ship lab/prod packaging with a credentials file instead of baking tokens only into the process environment, and health evidence proves auth was configured without leaking tokens.
Test plan
cargo fmt --checkcargo test --locked --workspacecargo clippy --locked --workspace --all-targets -- -D warningsSummary by CodeRabbit
새 기능
문서