feat(age): support provider-backed identities#515
Conversation
Allow age providers to resolve an identity from another provider, matching the keychain bootstrap configuration documented in v1.25.1. The identity reference is kept lazy so encryption-only operations do not read the private identity provider. Decryption keeps config context so nested provider-backed age identities can resolve correctly.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR introduces provider-backed secret references, allowing the age encryption provider to obtain its decryption identity from another configured provider. The change adds a new ChangesProvider-backed Secret References and Age Identity Resolution
Sequence DiagramsequenceDiagram
participant User
participant AgeProvider as AgeEncryptionProvider
participant Resolver as resolve_provider_identity()
participant ResolutionContext as ResolutionContext
participant OtherProvider as ReferencedProvider
participant SecretFetch as get_secret() on ReferencedProvider
User->>AgeProvider: request decryption (Age provider with identity ref)
AgeProvider->>Resolver: resolve_provider_identity() (if env var absent)
Resolver->>ResolutionContext: check/track resolution path (cycle guard)
ResolutionContext-->>Resolver: ok / not in cycle
Resolver->>OtherProvider: construct resolved provider with context
OtherProvider->>SecretFetch: fetch referenced secret value
SecretFetch-->>OtherProvider: return secret string
OtherProvider-->>Resolver: secret string
Resolver-->>AgeProvider: identity string
AgeProvider->>AgeProvider: use identity to decrypt
AgeProvider-->>User: decrypted secret
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR implements
Confidence Score: 5/5Safe to merge — the new identity resolution path is correctly isolated from the encryption path and the cycle-detection machinery handles both direct and multi-hop cycles. The AgeIdentityCycleGuard correctly shares state across nested resolution hops via Arc::clone and the RAII drop guard ensures cleanup on both success and error. All call sites that were using the no-context get_provider_from_resolved have been updated. The bats tests exercise the full happy path, the lazy-skip-on-encrypt property, nested two-hop resolution, and mutual cycle detection. No files require special attention. Important Files Changed
Reviews (3): Last reviewed commit: "Merge branch 'main' into fix/age-provide..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/fnox-core/src/providers/age.rs`:
- Around line 56-82: resolve_provider_identity in AgeEncryptionProvider creates
a fresh ResolutionContext and only rejects direct self-reference, so transitive
provider identity cycles (age-A → age-B → age-A) are not detected; to fix,
change resolve_provider_identity to accept and propagate a shared cycle guard
(e.g., pass a &mut ResolutionContext or a &mut HashSet<String> of in-progress
provider names) into the resolve_provider_ref call instead of creating a new
ctx, add an early check that inserts the current provider_name into the guard
before delegating and removes it after, and ensure resolve_provider_ref will
return an error if it finds the provider_name already in the guard (mirroring
resolve_provider_config_with_context behavior); update the test suite (the bats
test for nested provider-backed age identity) to include a new test asserting
that mutual age↔age references produce the expected
ProviderConfigCycle/FnoxError.
🪄 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: b1db9a6d-8d67-468b-bfa0-78874d458ee1
📒 Files selected for processing (10)
crates/fnox-core/build/generate_providers.rscrates/fnox-core/providers/age.tomlcrates/fnox-core/src/providers/age.rscrates/fnox-core/src/providers/mod.rscrates/fnox-core/src/providers/resolver.rscrates/fnox-core/src/providers/secret_ref.rsdocs/providers/age.mddocs/public/schema.jsonsrc/commands/provider/add.rstest/age.bats
Summary
The keychain provider docs added in v1.25.1 recommend storing a single age identity in the OS keychain and using the age provider for bulk secret storage:
With the documented config, fnox currently fails while parsing the age provider:
This implements provider-backed age identities so the documented bootstrap pattern works.
What changed
identity = { provider = "...", value = "..." }support to the age provider config.fnox setdo not require access to the backing identity provider.provider add age, JSON schema, docs, and tests.Test plan
cargo checkcargo buildcargo test -p fnox-core providers::secret_reftest/bats/bin/bats test/age.batscargo fmt --checkSummary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests