Skip to content

feat: add agenix secret add/delete support - #656

Open
Scott McMaster (scottmcmaster) wants to merge 2 commits into
08-11-scott-add-del-sops-secretsfrom
08-12-scott-age-secret-add-del
Open

feat: add agenix secret add/delete support#656
Scott McMaster (scottmcmaster) wants to merge 2 commits into
08-11-scott-add-del-sops-secretsfrom
08-12-scott-age-secret-add-del

Conversation

@scottmcmaster

@scottmcmaster Scott McMaster (scottmcmaster) commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Summary

Implement the agenix secret add/delete analogous to what we did for SOPS. (Not including screenshots since there's no practical difference in the UI.)

Also includes some related DRY refactoring in secrets_management.rs and recipients.rs.

Test Plan

Some new unit tests where possible/appropriate, plus manual e2e testing with my test config repo.

  • No test plan needed

Docs

  • Docs updated (companion PR in darkmatter/nixmac-web: #___)
  • No docs update needed

Scott McMaster (scottmcmaster) commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@scottmcmaster Scott McMaster (scottmcmaster) changed the title scott-age-secret-add-del feat: add agenix secret add/delete support Aug 12, 2026
@scottmcmaster
Scott McMaster (scottmcmaster) marked this pull request as ready for review August 12, 2026 08:01
@darkmatter

darkmatter Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

🎨 Storybook preview

Open Storybook preview

Updated for b76a140


⚠️ Detected UI changes (5)

These stories' HTML snapshots changed. I've added screenshots + links to the changed stories below. Review them carefully then accept the changes to regenerate baselines and include them in this PR:

Flows/Evolve › Playground

Flows/Evolve › Playground

Flows/Evolve › 1. Begin (idle)

Flows/Evolve › 1. Begin (idle)

Flows/Evolve › 2. Evolving (progress)

Flows/Evolve › 2. Evolving (progress)

Flows/Evolve › Evolving With Error Event

Flows/Evolve › Evolving With Error Event

Flows/Evolve › 3. Review (changes generated)

Flows/Evolve › 3. Review (changes generated)


Accept UI changes

  • Click here to accept these changes

Alternatively, you can run bun run test:update-snapshots locally to re-generate the baselines and then push the changes to this PR.

What does this do?

The screenshots above show UI changes detected by the Storybook
snapshot tests run on this PR. Each image is the rendered output of
a Storybook story from the code in this PR branch; the snapshot
test compared it against the committed baseline in
__snapshots__/ and flagged the difference.

Checking the box tells the darkmatter[bot] to regenerate the
baselines from this PR's current code and commit them directly to
this branch. The new baselines become the source of truth for
future runs — only accept after confirming the visual changes are
intentional.

Comparison baseline: the committed __snapshots__/ files on this
PR branch (carried forward from develop). Accept updates them in
place on this branch.

@github-actions

github-actions Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor
Warnings
⚠️

No Linear issue ID found in this PR's title, description, or branch name (expected something like ENG-123). Add one so this work is traceable in Linear, or add #no-linear to the PR description to acknowledge it's intentionally untracked.

Messages
📖 No docs update needed — acknowledged.

📋 PR Overview

Lines changed 1274 (+1102 / -172)
Files 1 added, 13 modified, 0 deleted
Draft / WIP no
Has Test Plan yes
Linear issue no
No Test Plan Needed no
New UI components no
New Storybook stories no
New Rust modules no
New TS source files no
New tests yes (1)
package.json touched no
Cargo.toml touched no
Infra / CI touched no

🔬 Coverage

Report Lines Statements Functions Branches
apps/native/coverage/coverage-summary.json 35.5% 35.1% 30.6% 29.3%

Generated by 🚫 dangerJS against b76a140

@prelint

prelint Bot commented Aug 12, 2026

Copy link
Copy Markdown

Ship with changes Agenix secret add and delete reach parity with SOPS

Product decisions in this change

Agree 1. Agenix delete is allowed without a local decryption key, unlike SOPS delete.

SOPS deletion must decrypt and rewrite a shared YAML file, so the local key is required. Agenix deletion removes a standalone .age file, a rule entry, and a declaration entry. None of those steps need decryption. The asymmetry matches the technical reality of each backend and gives agenix users a capability SOPS users cannot have.

Agree 2. The repository must have no uncommitted changes before any secret add or delete, for both backends.

The uncommitted-changes check makes rollback predictable. When the build check fails after a mutation, the app restores only the files it touched. If unrelated edits were in the working directory, a partial restore could discard user work silently. Requiring a clean state prevents that class of data loss. The tradeoff is that a user with uncommitted work must stash before using the secret UI. That is a reasonable cost for the safety guarantee.

Agree with concerns 3. The runtime path for agenix secrets is hardcoded to `/run/agenix/{name}` in both the preview and the add result.

The agenix module lets users set a custom path via age.secrets.{name}.path. A user who uses a custom path will see the wrong runtime path in the nixmac UI. The displayed path is informational, so the discrepancy does not break anything today. The risk grows if users copy the displayed path into application configuration. Reading the evaluated age.secrets.{name}.path value from the vault at display time would give the correct value. This is correctable later without a user migration.

Disagree 4. The submit button stays enabled when no recipients are registered for the selected backend.

A warning already appears above the recipients list, but the submit button remains active. The backend rejects the operation when the recipients list is empty. Users can fill out the entire form, click submit, wait for encryption to run, and then see the backend error. The invalid guard already disables submit when agenix files are not discoverable. Extending that same check to cover committedRecipients.length === 0 would require one additional condition and would give users immediate feedback before they submit. There is no benefit to surfacing this error only after the user clicks submit.

Agree with concerns 5. The backend selector always starts on SOPS, regardless of which backends the repository has configured.

The vault now exposes agenixRulesFile and agenixDeclarationFile, so the frontend can tell whether agenix is configured. A user with an agenix-only repository still lands on SOPS and must switch before filling in the form. The condition for a smart default is now present in the data. The decision to apply it is deferred. This is low-friction for users with SOPS repos but creates a mismatch for agenix-only users that grows as agenix adoption grows.

Agree with concerns 6. New agenix secrets always land in `secrets/{name}.age`, with no option to place the file elsewhere.

This path is consistent across the UI preview, the backend write, and the Nix rule. Most repos that follow the conventional agenix layout match it. A user who organizes secrets by host or namespace gets files placed in the wrong directory. The risk is low while agenix adoption in nixmac is small and grows as more varied repos are supported.

Agree with concerns 7. Age encryption still runs through `nix shell nixpkgs#age` with no visible progress indicator and no timeout.

This matches the pattern used for other Nix operations in the app. On a cold Nix store the fetch can take ten or more seconds, which a user may interpret as a freeze. No spinner, progress event, or timeout was added in this PR.

Agree with concerns 8. The `SecretsVault` type carries three flat agenix-specific fields at the top level alongside the shared fields.

The flat layout works with two backends. A third backend would force the vault to accumulate more top-level backend-specific fields. A backendMeta map keyed by backend name would scale better. For two backends the current structure is readable and a refactor can wait until a third backend appears.

Agree with concerns 9. When multiple Nix files match `age.secrets`, the error message does not list the conflicting files.

The error says "Found multiple Nix modules containing age.secrets; expected one agenix declaration module". A user who hits this case needs to know which files matched in order to resolve the conflict. Without the list they must search the repository manually. Adding the candidate paths to the error string would make the error actionable at no additional cost.

Agree with concerns 10. Both backend options always appear in the toggle for every repository, even if only one backend is configured.

Showing both options communicates that nixmac supports both backends. For a SOPS-only repository, the agenix panel shows an error on switch. For a repository with neither backend, both panels show empty recipients and errors. The experience is not broken but is mildly confusing. Disabling the agenix option when no agenix files are discoverable would make the toggle reflect repository reality without hiding the feature.

Since the last review

  • Still open: Backend selector always starts on SOPS regardless of repository configuration (add-secret-view.tsx still initializes useState<SecretBackend>("sops") with no check against vault fields)
  • Still open: New agenix secrets always land in secrets/{name}.age with no placement option (secrets_management.rs: encrypted_rel = format!("secrets/{secret_id}.age") remains hardcoded in add_age_secret)
  • Still open: Submit button stays enabled when no recipients are registered for the selected backend (invalid in add-secret-view.tsx does not include a committedRecipients.length === 0 condition)
  • Still open: Age encryption runs through nix shell nixpkgs#age with no visible progress or timeout (age_encrypt_command still passes ["shell", "nixpkgs#age", "-c", "age", "--encrypt"] with no spinner or timeout added)
  • Still open: SecretsVault carries three flat agenix-specific fields at the top level (secrets_management.rs and orpc-bindings.ts both add the three fields flat; no backendMeta wrapper introduced)
  • Still open: Fallback age.secrets scan error does not list the conflicting files (find_secret_declaration_file error string still omits the list of candidate paths)
  • Still open: Both backend options always appear in the toggle regardless of repository configuration (add-secret-view.tsx maps ["sops", "agenix"] unconditionally without checking vault agenix fields)
  • Still open: No team decision recorded on whether to implement a smart default backend in a follow-up PR (No PR comment or code change addresses the default backend selection)
  • Still open: No spinner or timeout during the nix shell nixpkgs#age cold-cache fetch (encrypt_age_secret adds no progress event or timeout around age_encrypt_command)
  • Still open: Multi-match error message does not list the conflicting Nix files (find_secret_declaration_file error string still omits candidate paths)
  • Addressed in code: Dual-backend support is intentional and both backends can be active simultaneously (Recipients are filtered per registration.backend in add-secret-view.tsx and the toggle allows per-session backend selection, confirming intentional dual-backend support)

Open questions

  • For users who set a custom age.secrets.{name}.path in their configuration, will nixmac show the hardcoded /run/agenix/{name} or the actual evaluated runtime path?

  • Has the team decided to implement a smart default backend selection in a follow-up PR, or is the always-SOPS start the accepted long-term behavior?

  • Is there a plan to surface a spinner or timeout when nix shell nixpkgs#age fetches the age binary on a cold Nix store?

Recommendation

Ship with changes
The submit button stays enabled when zero recipients are registered, letting users reach a predictable backend error only after submitting. Adding one condition to the existing invalid guard closes this before merging. All other concerns are acceptable trade-offs or tracked for follow-up.

@prelint prelint Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

The agenix diff preview hardcodes ../../secrets/${slug}.age as the file path, which is only correct when the declaration module lives at exactly modules/darwin/agenix-secrets.nix (two directorie...

apps/native/src/components/widget/secrets/add-secret-view.tsx:24

1 finding(s) posted as inline comments.

Comment thread apps/native/src/components/widget/secrets/add-secret-view.tsx Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements agenix secret add and delete support in nixmac, bringing the agenix backend to feature parity with the existing SOPS flow. Both flows encrypt/remove a per-secret .age file, edit the classic agenix rules file and the age.secrets declaration module, verify with a darwin-rebuild dry build, and commit — with bounded rollback on failure. The frontend gains a backend toggle (sops-nix / agenix) in the add-secret form and backend-aware copy across the apply and delete UIs. The change also includes DRY refactoring: shared helpers ensure_clean_repo, restore_repo_files_on_failure, and verify_dry_build_for_secret_edit; extracted sops_config_path / discover_agenix_rules_path / match_agenix_secret_entries in recipients.rs; and a new remove_attrpath_in_file plus quote-aware split_attrpath_for_match in nix_file_editor.rs.

Changes:

  • Implement add_age_secret / delete_age_secret with agenix rules + declaration editing, age encryption over stdin, dry-build verification, and rollback.
  • Refactor shared secret-edit safety helpers and agenix rule discovery/matching; add quote-aware attrpath splitting so keys like "api.token.age" resolve correctly.
  • Frontend: backend selector in the add-secret form, backend-aware recipient filtering/empty-state, and agenix-aware copy in the apply sheet and delete dialog.

Reviewed changes

Copilot reviewed 8 out of 8 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 Core agenix add/delete logic, shared clean-repo/verify/rollback helpers, agenix rule/declaration writers and tests.
apps/native/src-tauri/src/secrets/recipients.rs Extracts sops_config_path, discover_agenix_rules_path, and match_agenix_secret_entries; adjusts debug logging.
apps/native/src-tauri/src/evolve/nix_file_editor.rs Adds quote-aware split_attrpath_for_match and filesystem remove_attrpath_in_file, with tests.
apps/native/src/components/widget/secrets/add-secret-view.tsx Backend toggle, backend-specific paths/copy, registration-based recipient filtering, empty-state message.
apps/native/src/components/widget/secrets/secret-detail-view.tsx canDelete allows agenix regardless of local decrypt capability; backend-aware delete dialog copy.
apps/native/src/components/widget/secrets/apply-sheet.tsx Derives backend-specific encryption label ("age"/"SOPS"/fallback) for review and applying states.
apps/native/src/components/widget/secrets/types.ts Adds optional backend field to ApplyRequest.
apps/native/src/components/widget/secrets/secrets-management.tsx Generalizes the add-secret failure message to be backend-agnostic.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread apps/native/src-tauri/src/secrets/recipients.rs Outdated
Comment thread apps/native/src-tauri/src/secrets/secrets_management.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants