feat: resolve agenix age.identityPaths and "classic" rules for read-only secrets view and decryption - #653
Conversation
Scott McMaster (scottmcmaster)
changed the title
age.identityPaths and "classic" rules for read-only secrets view and decryption
🎨 Storybook previewUpdated for df62437 🧭 Story changesCompared to ✏️ Changed stories (1)
|
![prelint[bot]](https://avatars.githubusercontent.com/in/2822344?s=60&v=4){kind=small}
There was a problem hiding this comment.
Warning
apps/native/src-tauri/src/secrets/recipients.rs [behavior_change]: load_agenix_rules returns Err when a conventional-location secrets.nix or secrets/secrets.nix exists but fails Nix evaluation — for example when the file is a NixOS module (a function value rather than an attrset), causing builtins.mapAttrs in the --apply expression to throw a type error. That error propagates through load_recipients → load_secrets_vault via ?, breaking the entire Secrets Management view for users who have SOPS secrets and an identically-named file used for other purposes. The explicit RULES override path deliberately fails hard (documented in the function comment), but the conventional-path discovery (secrets.nix, secrets/secrets.nix) should degrade to Ok(AgenixRules::default()) on evaluation failure rather than fatally failing the vault load.
There was a problem hiding this comment.
Pull request overview
This PR completes the read-only agenix support in the Secrets Management view. Previously the app could list agenix secrets but could not resolve their recipients or decrypt them (several TODO(agenix-read) placeholders). It now projects cfg.age.identityPaths as local decryption identities, evaluates the "classic" agenix rules file (honoring $RULES, then secrets.nix/secrets/secrets.nix) to build a per-secret recipient inventory, and adds an agenix decrypt path that shells out to age --decrypt with the configured identities. The reveal RPC is now backend-qualified end-to-end, and the detail view adapts its layout for agenix (no SOPS-key box, "age" wording).
Changes:
- Backend: split
decrypt_secretintodecrypt_sops_secret/decrypt_agenix_secret, addage_decrypt_command, evaluate agenix rules into aRecipientInventory+ repository recipient registrations, and materialize agenix SSH/native identities. - Identity projection: expose
agenixIdentityPathsand per-host-keyusedByAgenix; threadbackendandsecret_entriesthroughdecrypt_secret,load_recipients, andapply_recipients_to_secrets_with_identities. - Frontend:
DecryptSecretInputgainsbackend; detail view conditionally renders the SOPS-key box and swaps SOPS/age wording; snapshot updated.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
apps/native/src-tauri/src/secrets/secrets_management.rs |
Backend-qualified decrypt split; new age_decrypt_command; wires agenix inventory into vault load |
apps/native/src-tauri/src/secrets/recipients.rs |
Evaluates classic agenix rules, builds recipient inventory/registrations, materializes agenix identities |
apps/native/src-tauri/src/secrets/identities.rs |
Projects age.identityPaths and usedByAgenix; new struct fields + tests |
apps/native/src-tauri/src/orpc/secrets.rs |
Adds backend to DecryptSecretInput and passes it through |
apps/native/src/ipc/orpc-bindings.ts |
Generated type update for DecryptSecretInput.backend |
apps/native/src/components/widget/secrets/secret-detail-view.tsx |
Sends backend; adapts grid/wording for agenix vs sops |
apps/native/src/components/widget/secrets/__snapshots__/secrets-management.stories.tsx.snap |
Updated Secret Detail snapshot for agenix layout |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
📋 PR Overview
🔬 Coverage
|
|
| Option | What users see | What it costs | Effort to change later |
|---|---|---|---|
| Current (silent filter) | age error with no context when the filtered key was correct | User cannot self-diagnose | Low: add filtered file names to the error message |
| Report skipped files in the error | Error names the skipped files | Slightly longer error message | Paths are already in scope at the error build point |
7. Agenix secrets are decrypted using a version of age drawn from nixpkgs rather than the version pinned in the user's own flake.
This is prior concern 3. The behavior is unchanged. The age format has been stable since v1.0, so practical risk is very low. The decision is consistent with how SOPS decryption works and avoids requiring flake input resolution at decrypt time.
8. When a rule entry's path cannot be matched directly, the system falls back to matching by filename alone, with no user-visible indication that the match was approximate.
This is prior concern 4. The basename fallback is still silent. A user with two agenix secrets that share a filename sees both show Unknown recipients, and the only record of the collision is a log warning the user cannot see. A user who sees a correctly resolved recipient list cannot tell whether the match came from an exact path or a basename guess.
9. After a user successfully decrypts an agenix secret, the capability indicator does not update to reflect that the reveal proved they hold a working key.
This is prior concern 5. A successful decrypt is direct proof that the user holds the correct key. The indicator staying at Unknown while the plaintext is visible on screen is a factual contradiction. The snapshot-at-load model is an accepted constraint for now. The concern is more visible for agenix than for SOPS because rules file resolution fails for a wider share of agenix configurations.
10. When the rules file cannot be located, the secrets view shows Unknown capability with no user-visible explanation of why recipients could not be resolved.
This is prior concern 6. The capability unknown text now correctly says "age" rather than "SOPS", but it still does not tell the user why the rules file was not found. A user whose rules file is at a non-standard path sees the same Unknown indicator as a user who has no rules file at all. Adding one conditional line — for example, "No rules file found at secrets.nix or secrets/secrets.nix; set the RULES variable to specify a custom path" — would let most affected users self-diagnose. The previous review rated the effort to add this as very low.
11. Users running agenix-rekey or other tools that do not use a classic rules file see the same permanent Unknown capability as users who have a resolvable rules file that failed to load.
This is prior concern 7. The UI text says "revealing will ask age to try the identities available to this process", which is accurate but sets no expectation that recipients will remain permanently unresolved. A user who installs nixmac alongside agenix-rekey cannot tell whether Unknown means their setup is expected and working, or whether nixmac failed to read their configuration.
Since the last review
- Still open: SSH key classification by .pub sibling: a native age key with an unrelated .pub sibling in the same directory is misclassified as SSH. (The .pub-sibling check in the materialize_recipients agenix section is unchanged; the fallback path still pushes a DecryptionIdentity with empty public_keys when ssh-to-age conversion fails.)
- Still open: Unreadable identity files are silently removed before decryption with no user-visible feedback. (readable_agenix_identity_paths in secrets_management.rs filters without logging or surfacing which paths were dropped.)
- Still open: Agenix secrets are decrypted using a nixpkgs age binary rather than the version pinned in the user's flake. (age_decrypt_command still invokes nix shell nixpkgs#age; no change from the previous version.)
- Still open: Basename fallback matching produces recipient lists in the UI with no indication the match was approximate. (build_agenix_rules still falls through to basename matching when evaluated exact matches are empty; the only record is a debug-level log.)
- Still open: The capability indicator does not update after a successful reveal proves the user holds a working key. (secret-detail-view.tsx contains no post-decrypt capability update; the capability value is set at load time only.)
- Still open: Unknown capability shows with no user-visible explanation of why the rules file could not be found. (The capability unknown text in secret-detail-view.tsx now correctly says "age" rather than "SOPS" but adds no rules-file-not-found explanation.)
- Still open: Users running agenix-rekey or similar tools see the same permanent Unknown capability as users whose rules file failed to load. (No distinct capability state or UI text was added for agenix-rekey users; Unknown remains the only unresolved state.)
- Still open: Are non-standard rules file paths common enough that silent Unknown is a day-one friction point for a significant share of users? (No usage data, user research, or PR comment addresses this question.)
- Still open: Is there a product plan to update the capability indicator after a successful reveal? (No PR comment or code change addresses a post-reveal capability update.)
- Still open: Should users running agenix-rekey see a distinct capability state rather than the same Unknown shown when resolution fails? (No new capability state or UI distinction was introduced for non-classic-rules agenix users.)
- Still open: Should the UI name the skipped identity files when a decrypt attempt fails after silent filtering? (readable_agenix_identity_paths discards paths silently; the error path in decrypt_agenix_secret does not list filtered files.)
Open questions
-
Do most nixmac agenix users store their rules file at secrets.nix or secrets/secrets.nix, or is a non-standard path common enough that the silent Unknown state is a day-one friction point for a significant share of users?
-
Is there a product plan to update the capability indicator after a successful reveal, or is the snapshot-at-load model accepted long-term for both SOPS and agenix?
-
Should users running agenix-rekey see a distinct capability state (such as 'Externally managed') rather than the same Unknown shown when resolution fails for other reasons?
-
When one or more identity files are silently removed before a decrypt attempt, should the error message name the skipped files? This matters most when the decrypt then fails and the user believes a valid identity is configured.
Recommendation
Ship with changes
The core feature is functionally correct and the critical reviewer concern (auto-discovered rules file crashing the entire Secrets view on Nix evaluation failure) was addressed in code. Prior concern 6, a one-line explanation when no rules file is found, was rated very low effort in the previous review and remains unimplemented despite being the most direct way to help users with non-standard paths self-diagnose. Prior concern 11, the absence of any feedback about silently filtered identity files, is more consequential now that decryption is live: a user whose only valid identity was filtered will see an age error with no pointer to the cause. These two additions are the minimum changes before shipping.
045ed63 to
feddd2d
Compare
I addressed this comment, see |
There was a problem hiding this comment.
Caution
load_agenix_rules uses raw config_dir.join(path) for a relative $RULES value, and build_agenix_rules uses rules_dir.join(secret_path) / config_dir.join(secret_path) for relative secret-fil...
apps/native/src-tauri/src/secrets/recipients.rs:530
1 finding(s) posted as inline comments.
f45afc8 to
6500519
Compare
0523e39 to
0dc059a
Compare
|
Prelint reached the review limit for this pull request (5 reviews of each kind). New pushes do not start a review.
|
darkmatteragent
left a comment
darkmatteragent
left a comment
There was a problem hiding this comment.
Review — REQUEST CHANGES
0dc059afb54d · 4 findings
Implements agenix read-only support end to end — identity projection, classic-rules evaluation, backend-qualified reveal, UI — but requesting changes because the reveal path contradicts the capability the vault reports.
age1…(ssh-to-age) rules show Available, yet reveal hands the raw SSH private key toage -i, which cannot unwrap that stanza.availablestats the identity file while reveal requires opening it — unreadable root-owned host keys still show Available, then reveal errors.- Minor: suffix-based 'basename' matching can leak the wrong rule's recipients, and a broken explicit $RULES fails SOPS-only reveals.
Findings
Caution
blocker · correctness — Agenix reveal passes raw SSH keys to age, but capability logic claims ssh-to-age age1… recipients are decryptable
apps/native/src-tauri/src/secrets/secrets_management.rs:140
age_decrypt_command (gutter 140, command.args(["--identity", identity_path])) passes each cfg.age.identityPaths entry as-is. An OpenSSH private key given to age --identity is parsed as an ssh::Identity and can only unwrap ssh-ed25519/ssh-rsa stanzas — it cannot unwrap the X25519 stanza produced when a rules file lists the host's recipient as the ssh-to-age-converted age1… key (the
Warning
major · correctness — available uses is_file() while reveal requires File::open, so unreadable private keys are reported Available then dropped at reveal
apps/native/src-tauri/src/secrets/recipients.rs:1089
The new agenix block sets available = path.is_file() (gutter 1089), and host keys newly admitted by the used_by_sops || used_by_agenix filter (gutter 887, availability at 902) do the same. But the consumer this patch adds, readable_agenix_identity_paths (secrets_management.rs:78, std::fs::File::open(identity_path).is_ok()), filters identities by readability before decrypt. The predicates d
Important
minor · correctness — Basename fallback is a string-suffix match, so my-password.age inherits the recipients of rule password.age
apps/native/src-tauri/src/secrets/recipients.rs:738
The fallback at gutter 738 filters with entry.file.ends_with(basename) — Rust's str::ends_with, a raw string-suffix test, though the comment (725-728) promises a 'unique string-basename match'. Concrete input: rule keyed password.age and an evaluated declaration at /nix/store/<hash>-my-password.age (which ends with the string password.age) — the wrong rule's encrypted_for is attributed
Important
minor · correctness — SOPS secret reveal now fails when an explicit $RULES agenix file is invalid
apps/native/src-tauri/src/secrets/secrets_management.rs:107
decrypt_sops_secret calls load_recipients(host_attr, config_dir, &[])? (gutter 107). This patch made load_recipients evaluate the classic agenix rules file and propagate errors for an explicitly configured $RULES (recipients.rs:540-569, 638-653). Deterministic path: set $RULES to a file with a nix error → evaluate_agenix_rules (601-620) fails → handle_agenix_rules_result passes the e
Warning major · correctness — Raw SSH recipients from agenix rules don't resolve to the local identity in the secret detail view Line 731 ( reviewed: 0dc059a — 2026-09-01T03:10Z verdict: request_changes findings: 4
Run details
|
||||||||||||||||||||||||||||
0dc059a to
0766016
Compare
… readability checks and fallback mechanisms and $RULES rules
darkmatteragent
left a comment
darkmatteragent
left a comment
There was a problem hiding this comment.
Review — REQUEST CHANGES
6db3da238818 · 1 finding
Feature looks correct end-to-end on the Rust side, but one integration gap blocks merge. - The new backend-qualified reveal RPC is fed by a UI that still selects secrets by id alone, so same-named SOPS/agenix declarations route to the SOPS entry and reveal the wrong secret's plaintext. - Previously this collision failed loudly ("ambiguous across backends"); the patch removes that guard without completing the addressing fix the removed TODO described. - All other consumer traces (inventory keys, merge/dedupe, age fallback, bindings, snapshot, tests) check out.
Findings
Caution
blocker · correctness — Reveal RPC is backend-qualified but selection still keys on id alone; same-named SOPS/agenix secrets reveal the wrong plaintext
apps/native/src/components/widget/secrets/secret-detail-view.tsx:72
The patch sends secretId: secret.id, / backend: secret.backend, (diff lines 70-73) and the Rust decrypt_secret correctly dispatches per backend — but nothing upstream guarantees secret is the entry the user clicked. vault-view.tsx:180 calls onOpenSecret(secret.id) (signature (secretId: string) => void, vault-view.tsx:23) with key={secret.id} (vault-view.tsx:178), `secrets-managemen
darkmatteragent
left a comment
darkmatteragent
left a comment
There was a problem hiding this comment.
Review — COMMENT
df62437d0e93 · 1 finding
Backend wiring is solid — backend-qualified reveal, rules evaluation, and inventory keying all trace cleanly to their consumers — but one consumer gap undercuts the PR's own alias invariant. - Agenix rules written with raw SSH keys (the classic form) produce detail-view rows that no longer resolve to the local identity recipient, losing its label and 'local identity' chip.
Findings
Warning
major · correctness — Raw SSH recipients from agenix rules don't resolve to the local identity in the secret detail view
apps/native/src-tauri/src/secrets/recipients.rs:731
Line 731 (RecipientIdentity::Age(value) | RecipientIdentity::Ssh(value) => value.clone()) canonicalizes an agenix rule's publicKeys, so a rule written with a raw SSH key keeps the ssh-ed25519 AAAA… form. That raw form flows into the per-secret inventory (encrypted_for, lines 720-724) and becomes entry.publicRecipients (recipients.rs:210-217). When that raw key belongs to a configured loc
| ); | ||
| for identity in &encrypted_for { | ||
| let canonical = match identity { | ||
| RecipientIdentity::Age(value) | RecipientIdentity::Ssh(value) => value.clone(), |
There was a problem hiding this comment.
[major] Raw SSH recipients from agenix rules don't resolve to the local identity in the secret detail view
Line 731 (RecipientIdentity::Age(value) | RecipientIdentity::Ssh(value) => value.clone()) canonicalizes an agenix rule's publicKeys, so a rule written with a raw SSH key keeps the ssh-ed25519 AAAA… form. That raw form flows into the per-secret inventory (encrypted_for, lines 720-724) and becomes entry.publicRecipients (recipients.rs:210-217). When that raw key belongs to a configured local identity, merge_config_recipients (recipients.rs:903-918) matches it via identities_for_recipient_with_aliases and folds the registration into the local recipient — whose displayed publicKey is the ssh-to-age age1… form; the raw SSH alias lives only in DecryptionIdentity.public_keys (lines 1199-1209). The consumer, secret-detail-view.tsx:162-165, resolves each publicRecipients entry with an exact candidate.publicKey === publicKey find, which finds nothing for the raw SSH string: the row falls back to the raw key as its label, renders the unknown-kind icon, and drops the recipientHasLocalIdentity 'local identity' chip — even though recipientIds did match th…
Summary
This removes the previous TODO's about retrieving agenix secrets in the read-only Secrets Management view, including the decryption support.
For purposes here, recipients need to be identifiable from one of three locations:
If we can't identify any from these locations, the secrets remain visible but their public recipients and local capability will show as "Unknown" in the UI.
The rest of the functionality in this PR is pretty self-explanatory, I believe.
Screenshots:
Test Plan
New unit tests where appropriate, plus manual testing with my test config repo.
Docs