feat: edit existing secrets in-place for sops-nix and agenix

[Scott McMaster (scottmcmaster)]

Summary

Hook up the edit functionality for secrets for both SOPS and agenix.

ATTENTION

1. Please note the pecularity specific to SOPS where we need to deduce the repo path from the nix store path for the secret, and if you have any better ideas, please let me know.
2. There is a subtle difference in how recipients are handled which you should double-check, this is reflected in the UI:
   a. For age, the updated value keeps the same recipients.
   b. For SOPS, we use what's currently in the repo's .sops.yaml.

Also did some refactoring of the YAML-related utility methods, and enhanced some of the unit tests.

Screenshots (note that I hid all of the extra controls from the "add" flow since they were distracting and not directly relevant to the "edit" flow but per prelint comments recipients are included):

Test Plan

New unit tests in Rust and TypeScript, manually tested the SOPS and age paths in the UI.

• No test plan needed

Docs

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

[Scott McMaster (scottmcmaster)]

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

• feat: edit existing secrets in-place for sops-nix and agenix #658 👈 (View in Graphite)
• feat: add agenix secret add/delete support #656
• feat: add SOPS secret add/delete with darwin-rebuild verification and commit #655
• feat: resolve agenix age.identityPaths and "classic" rules for read-only secrets view and decryption #653
• main

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

[github-actions]

	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	1251 (+1059 / -192)
Files	3 added, 12 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	yes (1)
New TS source files	no
New tests	yes (2)
package.json touched	no
Cargo.toml touched	no
Infra / CI touched	no

🔬 Coverage

Report	Lines	Statements	Functions	Branches
apps/native/coverage/coverage-summary.json	34.9%	34.4%	29.9%	30.1%

Generated by 🚫 dangerJS against fc4f15f

[darkmatter]

🎨 Storybook preview

Open Storybook preview

Updated for fc4f15f

---

🧭 Story changes

Compared to 08-12-scott-age-secret-add-del (snapshot diff at story level):

✏️ Changed stories (8)

• Widget/Secrets/SecretsManagement › Add Recipient
• Widget/Secrets/SecretsManagement › Add Secret
• Widget/Secrets/SecretsManagement › Keys And Recipients
• Widget/Secrets/SecretsManagement › Rotate And Rekey
• Widget/Secrets/SecretsManagement › Secret Detail
• Widget/Secrets/SecretsManagement › Secret Detail No Access
• Widget/Secrets/SecretsManagement › Vault
• Widget/Secrets/SecretsManagement › Without Prompt Bar

💡 Update snapshots ↗ to regenerate baselines and open a PR.

---

⚠️ 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 › 1. Begin (idle)

Flows/Evolve › 2. Evolving (progress)

Flows/Evolve › Evolving With Error Event

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.

[prelint]

Agenix edit gated; SOPS pre-flight and scope warning still absent

Product decisions in this change

1. The agenix edit button is now hidden when recipient metadata cannot be resolved or when it resolves to an empty set, with a distinct message for each case.

Two failure cases are handled separately. The unresolved-metadata message names the security reason: age cannot preserve unknown recipients. The empty-resolved-set message names the functional blocker. Both cases prevent a user from entering a value that will fail on submission. Tests cover both cases explicitly.

2. Recipients are displayed read-only in the edit form, with different explanatory text for agenix (exact file recipients preserved) and SOPS (current .sops.yaml recipients applied).

A user replacing a compromised secret now sees who can decrypt the new value before committing. The UI correctly distinguishes the two models. An unregistered agenix recipient is shown by its raw public key with an "Unclassified recipient" label, which is transparent. This directly addresses the prior gap about the edit flow giving no access confirmation.

3. The SOPS edit button remains always visible with no pre-flight check for whether the source file can be located.

The agenix path now has a pre-flight gate. The SOPS path has no equivalent. A SOPS user whose file cannot be located because it was re-encrypted after the last Nix evaluation sees the edit button and enters a new value. After submission, a backend error appears. That error still references the Nix store path and provides no recovery step. The asymmetry is now more visible because agenix is explicitly gated.

Option	What users see	What it costs	Effort to change later
Always visible (current)	Uniform SOPS UI	Late failure after value entry, confusing error	Low
Pre-flight locatability check	Early warning before value entry	Requires a backend query at render time	Low

4. Editing a key in a shared SOPS file re-encrypts every key in that file under the current .sops.yaml recipients, with no UI note about the full-file scope.

If .sops.yaml changed since the file was last encrypted, editing any one key silently changes access for all other keys in the file. The UI explains which recipients apply to the new value but does not say that sibling keys are also re-encrypted under those same recipients. If a recipient was added to .sops.yaml since the last edit, all other keys gain that new recipient in this commit. A one-line note in the recipient section would close this gap with no backend changes.

5. When SOPS source-file matching fails, the error message still contains the Nix store path and provides no recovery guidance.

The most common cause of this failure is that the SOPS file was re-encrypted in the repository after the last Nix evaluation. The fix is a rebuild. Neither fact appears in the error text. A user who hits this error has no obvious path forward without external help. A short rewrite naming the likely cause and the next step would resolve this.

6. The dry build before committing an edited secret has no documented scope at the point where it is called.

Editing only the encrypted content does not change any Nix declarations. A dry build cannot detect a bad encryption or a corrupted file. If the intent is to confirm the Nix configuration evaluates cleanly before committing, the check provides that. If the intent is to validate the encrypted output, it does not. A brief note where this check is called would prevent future contributors from misreading its purpose.

7. The edit flow is a named navigation state alongside "detail", "add", "browse", and "rotate", so the back button in the edit form returns the user to the detail view.

A user who cancels an edit returns to the detail view, which matches where they came from. This is consistent with the add flow, where back returns to browse. Making edit a distinct state also keeps the correct secret selected when the user navigates back.

8. The edit flow reuses the add-secret form with conditional hiding of add-only sections rather than a dedicated edit component.

This approach is acceptable for a first implementation. The add and edit flows now diverge on name field, backend selector, runtime path section, encrypt-target hint, and submission logic. Any change to recipient display or form validation in the add path must be evaluated for the edit path. A dedicated edit component becomes the right choice when a third branch appears or when the conditional count grows further.

Since the last review

• Still open: SOPS edit button always visible; no pre-flight check for source-file locatability (secret-detail-view.tsx: the new agenixEditUnavailable guard checks backend === "agenix" only; the SOPS edit button remains unconditional)
• Still open: Editing one SOPS key re-encrypts all keys in the shared file with no UI warning about the scope (No new UI copy about sibling key re-encryption added in add-secret-view.tsx; recipient text mentions .sops.yaml recipients but not full-file scope)
• Still open: SOPS source-file mismatch error references the Nix store path and provides no recovery guidance (match_repo_file_by_name_and_contents error strings still include evaluated_file.display() (the store path) and name no rebuild step)
• Still open: Dry build check before committing has no documented scope (No explanatory comment added near the verify_dry_build_for_secret_edit call sites in secrets_management.rs)
• Still open: Concurrent remote commit to the same SOPS shared file could be silently overwritten (No fetch or pull step added; ensure_clean_repo checks only local uncommitted changes)
• Still open: No plan stated for a SOPS pre-flight backend query to gate the edit button the way agenix now is (SOPS edit button in secret-detail-view.tsx has no pre-flight guard; only agenix received the agenixEditUnavailable gate)

Open questions

• Is a pre-flight check planned for SOPS source-file locatability, so the SOPS edit button can be gated the same way the agenix button now is?

• Can a concurrent change to another key in the same SOPS shared file be silently overwritten if another contributor pushes between form-open and re-encrypt?

• Is the dry build meant to catch unrelated configuration drift before committing, or to validate the encrypted output itself?

Recommendation

Ship with changes
The two gaps from the previous "ship with changes" call are resolved: recipients are now visible in the edit form, and the agenix edit button is gated on recipient resolution. The remaining concerns were all "agree_with_concerns" in the original review and were not listed as blockers. Two one-line fixes are worth making before merge: the SOPS error message should name the likely cause (re-encryption since last build) and the next step (run a rebuild), and the recipient section should note that all keys in the shared SOPS file are re-encrypted, not only the one being edited.

[Copilot]

Pull request overview

This PR wires up the previously stubbed "edit secret value" flow for both the sops-nix and agenix backends. On the backend it adds an edit_secret command that decrypts the existing encrypted file, replaces the requested value in place (preserving the declaration, file location, backend, and recipients), re-encrypts, runs a dry darwin-rebuild check, and commits — with best-effort rollback on failure. On the frontend it reuses AddSecretView in an "edit" mode that hides the add-only controls, and replaces the "not implemented" edit button with a real one. It also extracts the YAML helpers (replace_yaml_path, remove_yaml_path, validate_yaml_syntax) into a new shared yaml_utils module.

Changes:

• New Rust edit_secret backend (SOPS + agenix), including conservative recovery of a SOPS repo source file from its evaluated /nix/store path, plus the editSecret orpc route and EditSecretResult shared type.
• Frontend edit flow: AddSecretView gains an optional secret prop / buildEditRequest, SecretDetailView exposes onEdit, and SecretsManagement routes the new edit view to editSecret.
• Refactor: YAML utilities moved to yaml_utils.rs with expanded unit tests.

Reviewed changes

Copilot reviewed 12 out of 12 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	Adds edit_secret/edit_sops_secret/edit_age_secret, SOPS store-path source recovery, and tests
apps/native/src-tauri/src/yaml_utils.rs	New module hosting YAML path replace/remove/validate helpers + tests
apps/native/src-tauri/src/evolve/file_ops.rs	Moves validate_yaml_syntax (and its tests) into yaml_utils
apps/native/src-tauri/src/main.rs	Declares the new yaml_utils top-level module
apps/native/src-tauri/src/orpc/secrets.rs	Adds editSecret input type, handler, and route
apps/native/src-tauri/src/shared_types/secrets_management.rs	Adds EditSecretResult
apps/native/src/ipc/orpc-bindings.ts	Generated EditSecretInput/EditSecretResult + editSecret client
apps/native/src/components/widget/secrets/add-secret-view.tsx	Edit mode support and buildEditRequest
apps/native/src/components/widget/secrets/add-secret-view.test.ts	Tests for buildEditRequest
apps/native/src/components/widget/secrets/secret-detail-view.tsx	Replaces stub edit button with real onEdit; removes canEdit
apps/native/src/components/widget/secrets/secrets-management.tsx	Routes edit view and calls editSecret
apps/native/src/components/widget/secrets/types.ts	Adds edit view kind and edit apply origin

---

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

[Scott McMaster (scottmcmaster)]

Secret edit flow: in-place value replacement for SOPS and agenix

Product decisions in this change

1. Editing a secret value is a first-class operation with its own endpoint and commit, separate from adding a new secret.

Separating edit from add is correct. The two operations have different preconditions. Add creates a declaration and an encrypted file. Edit replaces only the encrypted content. Combining them would force the add path to handle cases it was not designed for, such as preserving existing recipients. A dedicated commit message also makes the history legible.

2. The "Edit value" button is always visible on the secret detail view, regardless of whether the edit can succeed for that secret.

The always-visible button keeps the UI uniform, but a user with an agenix secret that has unresolved recipients starts the edit flow and sees a failure only at the build-and-commit step. A user who has typed a new credential value and submitted the form receives a backend error. The error provides no guidance on how to fix the underlying problem.

Option What it gives users What it costs Effort to change later
Always visible (current) Uniform UI Late failure with no guidance for unresolved recipients Low
Conditionally visible Early prevention and a clear message Requires checking recipient resolution state at render time Low
The current approach works if unresolved recipients are rare. It becomes harmful if users regularly encounter this state with no explanation.

3. The edit UI reuses the add-secret form with the name, backend selector, runtime path, and recipient sections hidden when in edit mode.

The shared component avoids duplicating code for a first implementation. But it now serves two divergent models: configure a new secret versus replace an existing value. Mode branches already appear in several places inside the component. Every future change to the add flow must consider whether it also applies to the edit flow. The current approach is acceptable. A dedicated edit component becomes the right choice when a third branch appears.

4. The review screen shows a symbolic placeholder diff representing the replaced value, rather than the actual encrypted bytes.

Encrypted file diffs are ciphertext and carry no useful information for a human reviewer. The placeholder honestly describes what changed without pretending the user can verify the raw bytes. This is consistent with how secrets management tools generally handle the review problem.

5. Editing a secret preserves the exact recipients already recorded on the encrypted file, and does not fall back to the currently registered recipient list.

This is the correct security model. A fallback to the current recipient list could silently add or remove identities that were not on the original file. For agenix, each file can have a distinct recipient set, so the current-registry fallback would be a silent security change. Refusing the edit when recipients cannot be resolved is also correct. A conservative failure is safer than a guess.

6. The edit flow does not show users which recipients can decrypt the new value.

Recipients do not change during an edit, so displaying them is not strictly required. But a user who is replacing a compromised secret wants confirmation that the new value is encrypted to the right identities and that no extra recipients have gained access. The review screen currently offers no security-relevant information beyond the file path and a symbolic placeholder.

Option What it gives users What it costs Effort to change later
No recipient display (current) Simpler edit form No confirmation of access scope Low
Read-only recipient display Explicit confirmation of who can decrypt the new value Minor added UI Low
Recipient data is already available in SecretEntry. The list component already exists in the add flow. A read-only display requires no backend changes.

7. The app locates the editable SOPS file by matching its filename and encrypted contents when the Nix configuration points to an internal store location rather than the repository directly.

When Nix copies a SOPS file into the store, it severs the link to the repository source. The PR recovers it by matching the store artifact name and bytes against files in the repository. Refusing on zero or multiple matches is the correct safety boundary. No wrong guess is ever made.

The fragility is in the content match. If the repository SOPS file has been re-encrypted since the last Nix evaluation, its bytes no longer match the store artifact. Recipient rotation is one common cause of this state. The edit then fails with a message saying the source cannot be found, even though the file clearly exists. The user has no obvious path forward other than running a rebuild to refresh the store artifact first. The PR author acknowledges this limitation. A follow-up should surface a clearer error message that explains the mismatch and tells the user what to do.

8. A dry build check runs before the edited encrypted file is committed.

Editing only the encrypted content does not change any Nix declarations. A dry build cannot detect a bad SOPS encryption or a corrupted agenix file. If the intent is to confirm the Nix configuration still evaluates cleanly after any recent unrelated changes, the check provides that assurance. If the intent is to validate the edited secret itself, it does not. The check is harmless but its rationale should be documented so future contributors understand what it verifies and what it does not.

9. An agenix edit fails explicitly with an error when the existing recipient set cannot be resolved, rather than proceeding with the currently registered recipients.

The agenix model allows per-file recipient lists that differ from the global registry. Re-encrypting with the current registry would be a silent security change. Refusing and surfacing a clear error is the correct behavior. Tests cover both the unresolved and empty-recipient cases.

Open questions

• What happens when a user tries to edit a SOPS secret whose file was re-encrypted in the repository after the last Nix evaluation? The content-matching recovery will fail with a confusing message. Is there a planned path forward, or must the user run a rebuild first?
• Should the edit button be disabled with a tooltip when an agenix secret has unresolved recipients, rather than failing at commit time after the user has already entered a new value?
• Is there a plan to show recipient information as read-only during the edit flow, so users can confirm who gains access to the new value before committing?
• The dry build before committing cannot validate the encrypted output itself. Is the intent to catch unrelated configuration drift, or is a separate validation of the encrypted file planned?
• For a SOPS secret in a shared YAML file with multiple keys: if another key in the same file was changed in the repository after the last Nix evaluation, does the edit operate on the current repository version or on the Nix store version? Could a concurrent change by another contributor be silently overwritten?

Recommendation

Ship with changes The security decisions are correct: recipients are preserved exactly, refusals are conservative, and the SOPS source recovery is honest about ambiguity. Two gaps are worth addressing before merge. The edit review screen shows no recipient information, removing the one confirmation a user replacing a compromised secret would most want. The always-visible edit button also produces a late failure for agenix secrets with unresolved recipients, with no early guidance to the user.

I addressed the two gaps:

1. Bring back recipients view in the Edit flow.
2. Hide the Edit button proactively when age public recipients can't be resolved on the backend.

[gitar-bot]

⚠️ Edge Case: SOPS edit offered when decryption is unavailable

The "Edit value" button is shown whenever !agenixEditUnavailable, which is only ever false for agenix secrets. For a SOPS secret it is always rendered, even when capability === "unavailable". But edit_sops_secret must decrypt_sops_file before re-encrypting, so a SOPS secret with no local identity produces a guaranteed decrypt failure after the user submits. This is the same condition that already gates Delete (canDelete = backend === "agenix" || capability !== "unavailable"), so editing should be gated identically. Compute a canEdit that also requires SOPS decryptability and use it for the button.

Gate the SOPS edit button on decryption capability, mirroring canDelete.:

  const agenixEditUnavailable =
    secret.backend === "agenix" &&
    (!secret.publicRecipientsResolved || secret.publicRecipients.length === 0);
  // SOPS edit decrypts and rewrites the shared YAML, so it needs a local key
  // just like deletion does.
  const canEdit =
    secret.backend === "agenix" ? !agenixEditUnavailable : capability !== "unavailable";
...
        {canEdit && (
          <Button variant="outline" size="sm" onClick={onEdit}>

• Apply fix

_{Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎}

[gitar-bot]

Code Review ⚠️ Changes requested 0 resolved / 1 findings

Adds edit functionality for secrets in SOPS and agenix, with YAML utility refactoring and enhanced unit tests. The Edit button is shown for SOPS secrets even when decryption is unavailable, causing guaranteed failures—it should be gated by the same condition as Delete (canEdit = backend === "agenix" || capability !== "unavailable") to prevent users from attempting edits that will decrypt-fail server-side.

⚠️ Edge Case: SOPS edit offered when decryption is unavailable

📄 apps/native/src/components/widget/secrets/secret-detail-view.tsx:69-74 📄 apps/native/src/components/widget/secrets/secret-detail-view.tsx:280-285 📄 apps/native/src-tauri/src/secrets/secrets_management.rs:190-204

The "Edit value" button is shown whenever !agenixEditUnavailable, which is only ever false for agenix secrets. For a SOPS secret it is always rendered, even when capability === "unavailable". But edit_sops_secret must decrypt_sops_file before re-encrypting, so a SOPS secret with no local identity produces a guaranteed decrypt failure after the user submits. This is the same condition that already gates Delete (canDelete = backend === "agenix" || capability !== "unavailable"), so editing should be gated identically. Compute a canEdit that also requires SOPS decryptability and use it for the button.

Gate the SOPS edit button on decryption capability, mirroring canDelete.

  const agenixEditUnavailable =
    secret.backend === "agenix" &&
    (!secret.publicRecipientsResolved || secret.publicRecipients.length === 0);
  // SOPS edit decrypts and rewrites the shared YAML, so it needs a local key
  // just like deletion does.
  const canEdit =
    secret.backend === "agenix" ? !agenixEditUnavailable : capability !== "unavailable";
...
        {canEdit && (
          <Button variant="outline" size="sm" onClick={onEdit}>

🤖 Prompt for agents

Code Review: Adds edit functionality for secrets in SOPS and agenix, with YAML utility refactoring and enhanced unit tests. The Edit button is shown for SOPS secrets even when decryption is unavailable, causing guaranteed failures—it should be gated by the same condition as Delete (`canEdit = backend === "agenix" || capability !== "unavailable"`) to prevent users from attempting edits that will decrypt-fail server-side.

1. ⚠️ Edge Case: SOPS edit offered when decryption is unavailable
   Files: apps/native/src/components/widget/secrets/secret-detail-view.tsx:69-74, apps/native/src/components/widget/secrets/secret-detail-view.tsx:280-285, apps/native/src-tauri/src/secrets/secrets_management.rs:190-204

   The "Edit value" button is shown whenever `!agenixEditUnavailable`, which is only ever false for agenix secrets. For a SOPS secret it is always rendered, even when `capability === "unavailable"`. But `edit_sops_secret` must `decrypt_sops_file` before re-encrypting, so a SOPS secret with no local identity produces a guaranteed decrypt failure after the user submits. This is the same condition that already gates Delete (`canDelete = backend === "agenix" || capability !== "unavailable"`), so editing should be gated identically. Compute a `canEdit` that also requires SOPS decryptability and use it for the button.

   Fix (Gate the SOPS edit button on decryption capability, mirroring canDelete.):
     const agenixEditUnavailable =
       secret.backend === "agenix" &&
       (!secret.publicRecipientsResolved || secret.publicRecipients.length === 0);
     // SOPS edit decrypts and rewrites the shared YAML, so it needs a local key
     // just like deletion does.
     const canEdit =
       secret.backend === "agenix" ? !agenixEditUnavailable : capability !== "unavailable";
   ...
           {canEdit && (
             <Button variant="outline" size="sm" onClick={onEdit}>

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[cooper (czxtm)]

darkmatteragent please review this PR

[darkmatteragent]

Review — REQUEST CHANGES

65adf7e0ccf5 · 3 findings

Request changes: the new edit flow reuses the add-flow slug validator on the existing secret's ID, so editing any secret whose nix attrname contains an underscore — the common nix convention — always fails with a misleading naming error.

• The delete flows skip that validator, so delete succeeds where edit fails, confirming the reuse is an oversight, not policy.
• Secondary: the relative-path branch of resolve_sops_source_file returns a canonicalized path its caller strips against a non-canonical base, breaking edits when the config dir contains a symlink; and edit_secret's doc overclaims recipient preservation for SOPS.

Findings

Caution

blocker · correctness — Edit rejects existing secrets whose IDs are not lowercase hyphen slugs
apps/native/src-tauri/src/secrets/secrets_management.rs:140

Both edit paths gate on the add-flow validator: line 140 (validate_new_secret(secret_id, value)?; in edit_age_secret) and line 196 (edit_sops_secret). validate_new_secret requires IDs to contain only a-z, 0-9, and '-'. But in the edit flow the ID is not chosen by nixmac — it is the evaluated nix attrname, taken verbatim by load_sops_secrets/load_agenix_secrets (secret(id, id, ...)). Nix

Important

minor · correctness — resolve_sops_source_file returns a canonicalized path only in the relative branch
apps/native/src-tauri/src/secrets/secrets_management.rs:258

Line 258 returns resolve_existing_path_in_dir(base, evaluated_file), which canonicalizes (file_ops.rs returns full_path_canonical), while the sibling branches deliberately preserve the non-canonical base: line 267 return Ok(base.join(relative)), and the store-match branch returns raw walkdir paths under base. The caller, edit_sops_secret line 210, feeds the result to `repo_relative_path_st

Tip

nit · documentation — edit_secret doc claims recipients never change, but the SOPS path re-encrypts with current .sops.yaml rules
apps/native/src-tauri/src/secrets/secrets_management.rs:118

Lines 117–118 document "Replace an existing secret's value without changing its declaration, location, backend, or recipients." That holds for agenix (agenix_edit_recipients, line 179, refuses to change the recipient set), but edit_sops_secret re-encrypts through encrypt_sops_yaml (line 215), which applies the repository's current .sops.yaml creation rules — so SOPS recipients can be added

[darkmatteragent]

[blocker] Edit rejects existing secrets whose IDs are not lowercase hyphen slugs

Both edit paths gate on the add-flow validator: line 140 (validate_new_secret(secret_id, value)?; in edit_age_secret) and line 196 (edit_sops_secret). validate_new_secret requires IDs to contain only a-z, 0-9, and '-'. But in the edit flow the ID is not chosen by nixmac — it is the evaluated nix attrname, taken verbatim by load_sops_secrets/load_agenix_secrets (secret(id, id, ...)). Nix attrnames conventionally contain underscores (the repo's own fixtures and snapshots use wifi_password, cachix_signing_key), and the delete flows (delete_sops_secret/delete_age_secret) do NOT call this validator, so deletion works for such secrets while edit always fails with "Secret names must be lowercase slugs containing only a-z, 0-9, and '-'" — an error about a field the edit UI does not even show (add-secret-view.tsx renders the name input only when !editing). The charset check exists to constrain IDs nixmac creates in the add flow; applying it to pre-existing declarations breaks the new feature for a large class of real secrets. Fix: for edits, validate only that the value is non-empty (e.g. …

[darkmatteragent]

[nit] edit_secret doc claims recipients never change, but the SOPS path re-encrypts with current .sops.yaml rules

Lines 117–118 document "Replace an existing secret's value without changing its declaration, location, backend, or recipients." That holds for agenix (agenix_edit_recipients, line 179, refuses to change the recipient set), but edit_sops_secret re-encrypts through encrypt_sops_yaml (line 215), which applies the repository's current .sops.yaml creation rules — so SOPS recipients can be added or dropped by an edit. The PR description and the SOPS edit UI ("The updated value uses the recipients registered in the repository's .sops.yaml") state this is intentional, so the behavior is fine; only the doc comment overpromises a security-relevant property. Reword it to scope the recipient guarantee to agenix and state the SOPS behavior.

[darkmatteragent]

Reviewer · darkmatter agent

VERDICT REQUEST CHANGES

Request changes: the new edit flow reuses the add-flow slug validator on the existing secret's ID, so editing any secret whose nix attrname contains an underscore — the common nix convention — always fails with a misleading naming error. - SOPS edits also assume YAML unconditionally while sops-nix supports json/ini/dotenv/binary formats and `key = ""` whole-file secrets; non-YAML sources are silently re-encrypted in the wrong format and committed, a break the dry-build gate cannot catch. - The delete flows skip the slug validator, confirming its reuse in edit is an oversight, not policy.

Caution

blocker · correctness — Edit rejects existing secrets whose IDs are not lowercase hyphen slugs
apps/native/src-tauri/src/secrets/secrets_management.rs:140

Both edit paths gate on the add-flow validator: line 140 (validate_new_secret(secret_id, value)?; in edit_age_secret) and line 196 (edit_sops_secret). validate_new_secret requires IDs to contain only a-z, 0-9, and '-'. But in the edit flow the ID is not chosen by nixmac — it is the evaluated nix attrname, taken verbatim by load_sops_secrets/load_agenix_secrets (secret(id, id, ...)). Nix

Warning

major · correctness — SOPS edit assumes YAML format for every sops-nix secret, corrupting json/ini/dotenv sources
apps/native/src-tauri/src/secrets/secrets_management.rs:215

edit_sops_secret unconditionally runs the YAML pipeline: line 211 decrypt_sops_file (hardcodes --output-type yaml), line 212 replace_sops_value (serde_yaml parse/serialize), and line 215 encrypt_sops_yaml (hardcodes --input-type yaml --output-type yaml). But the producer, load_sops_secrets, evaluates only file = toString secret.sopsFile; key = secret.key and never reads format, whi

Tip

nit · documentation — edit_secret doc claims recipients never change, but the SOPS path re-encrypts with current .sops.yaml rules
apps/native/src-tauri/src/secrets/secrets_management.rs:118

Lines 117–118 document "Replace an existing secret's value without changing its declaration, location, backend, or recipients." That holds for agenix (agenix_edit_recipients, line 179, refuses to change the recipient set), but edit_sops_secret re-encrypts through encrypt_sops_yaml (line 215), which applies the repository's current .sops.yaml creation rules — so SOPS recipients can be added

reviewed: 65adf7e — 2026-09-01T08:06Z verdict: request_changes findings: 3

Reviewer darkmatter agent

Run details

Metadata	Value
Agent	darkmatter/agent/reviewer
Session	pr:darkmatter/nixmac#658
Kind	pr
Runtime	platform-runtime
Configured model	litellm/glm-5.2-fp8
Target	darkmatter/nixmac#658
Revision	65adf7e0ccf597f9270ac9e7e6f6fdfa6c49e1e4
Verdict	request_changes
Reviewed at	2026-09-01T08:06Z
Logs	Open Braintrust session

[darkmatteragent]

Review — REQUEST CHANGES

65adf7e0ccf5 · 3 findings

Request changes: the new edit flow reuses the add-flow slug validator on the existing secret's ID, so editing any secret whose nix attrname contains an underscore — the common nix convention — always fails with a misleading naming error.

• SOPS edits also assume YAML unconditionally while sops-nix supports json/ini/dotenv/binary formats and key = "" whole-file secrets; non-YAML sources are silently re-encrypted in the wrong format and committed, a break the dry-build gate cannot catch.
• The delete flows skip the slug validator, confirming its reuse in edit is an oversight, not policy.

Findings

Caution

blocker · correctness — Edit rejects existing secrets whose IDs are not lowercase hyphen slugs
apps/native/src-tauri/src/secrets/secrets_management.rs:140

Both edit paths gate on the add-flow validator: line 140 (validate_new_secret(secret_id, value)?; in edit_age_secret) and line 196 (edit_sops_secret). validate_new_secret requires IDs to contain only a-z, 0-9, and '-'. But in the edit flow the ID is not chosen by nixmac — it is the evaluated nix attrname, taken verbatim by load_sops_secrets/load_agenix_secrets (secret(id, id, ...)). Nix

Warning

major · correctness — SOPS edit assumes YAML format for every sops-nix secret, corrupting json/ini/dotenv sources
apps/native/src-tauri/src/secrets/secrets_management.rs:215

edit_sops_secret unconditionally runs the YAML pipeline: line 211 decrypt_sops_file (hardcodes --output-type yaml), line 212 replace_sops_value (serde_yaml parse/serialize), and line 215 encrypt_sops_yaml (hardcodes --input-type yaml --output-type yaml). But the producer, load_sops_secrets, evaluates only file = toString secret.sopsFile; key = secret.key and never reads format, whi

Tip

nit · documentation — edit_secret doc claims recipients never change, but the SOPS path re-encrypts with current .sops.yaml rules
apps/native/src-tauri/src/secrets/secrets_management.rs:118

Lines 117–118 document "Replace an existing secret's value without changing its declaration, location, backend, or recipients." That holds for agenix (agenix_edit_recipients, line 179, refuses to change the recipient set), but edit_sops_secret re-encrypts through encrypt_sops_yaml (line 215), which applies the repository's current .sops.yaml creation rules — so SOPS recipients can be added

[darkmatteragent]

[blocker] Edit rejects existing secrets whose IDs are not lowercase hyphen slugs

Both edit paths gate on the add-flow validator: line 140 (validate_new_secret(secret_id, value)?; in edit_age_secret) and line 196 (edit_sops_secret). validate_new_secret requires IDs to contain only a-z, 0-9, and '-'. But in the edit flow the ID is not chosen by nixmac — it is the evaluated nix attrname, taken verbatim by load_sops_secrets/load_agenix_secrets (secret(id, id, ...)). Nix attrnames conventionally contain underscores (the repo's own fixtures and snapshots use wifi_password, cachix_signing_key), and the delete flows (delete_sops_secret/delete_age_secret) do NOT call this validator, so deletion works for such secrets while edit always fails with "Secret names must be lowercase slugs containing only a-z, 0-9, and '-'" — an error about a field the edit UI does not even show (add-secret-view.tsx renders the name input only when !editing). The charset check exists to constrain IDs nixmac creates in the add flow; applying it to pre-existing declarations breaks the new feature for a large class of real secrets. Fix: for edits, validate only that the value is non-empty (e.g. …

[darkmatteragent]

[nit] edit_secret doc claims recipients never change, but the SOPS path re-encrypts with current .sops.yaml rules

Lines 117–118 document "Replace an existing secret's value without changing its declaration, location, backend, or recipients." That holds for agenix (agenix_edit_recipients, line 179, refuses to change the recipient set), but edit_sops_secret re-encrypts through encrypt_sops_yaml (line 215), which applies the repository's current .sops.yaml creation rules — so SOPS recipients can be added or dropped by an edit. The PR description and the SOPS edit UI ("The updated value uses the recipients registered in the repository's .sops.yaml") state this is intentional, so the behavior is fine; only the doc comment overpromises a security-relevant property. Reword it to scope the recipient guarantee to agenix and state the SOPS behavior.

[prelint]

Caution

Both edit_age_secret and edit_sops_secret call validate_new_secret(secret_id, value)?, which rejects any ID not matching [a-z0-9-]+.

^{apps/native/src-tauri/src/secrets/secrets_management.rs:140}

Caution

edit_sops_secret unconditionally runs the YAML pipeline: decrypt_sops_file (hardcoded --output-type yaml), replace_sops_value (serde_yaml parse/serialize), and encrypt_sops_yaml (hardcoded `...

^{apps/native/src-tauri/src/secrets/secrets_management.rs:215}

Warning

The edit_secret doc comment states "without changing its declaration, location, backend, or recipients," but edit_sops_secret re-encrypts through encrypt_sops_yaml which applies the repository's...

^{apps/native/src-tauri/src/secrets/secrets_management.rs:118}

3 finding(s) posted as inline comments.

[prelint]

Caution

Both edit_age_secret and edit_sops_secret call validate_new_secret(secret_id, value)?, which rejects any ID not matching [a-z0-9-]+. In the edit flow secret_id is the evaluated nix attrname loaded verbatim from the host configuration — underscores are idiomatic in Nix attribute names — so any secret declared as my_secret or api_key fails with a misleading naming-format error. The delete flows do not call this validator, confirming its use here is an oversight rather than policy.

[prelint]

Caution

edit_sops_secret unconditionally runs the YAML pipeline: decrypt_sops_file (hardcoded --output-type yaml), replace_sops_value (serde_yaml parse/serialize), and encrypt_sops_yaml (hardcoded --input-type yaml --output-type yaml). sops-nix supports json, ini, dotenv, and binary formats; editing a non-YAML secret silently re-encrypts it in YAML format. The dry-build gate cannot detect this corruption because Nix evaluates only the Nix declarations, not the encrypted file format.

[prelint]

Warning

The edit_secret doc comment states "without changing its declaration, location, backend, or recipients," but edit_sops_secret re-encrypts through encrypt_sops_yaml which applies the repository's current .sops.yaml creation rules — so SOPS recipients can change. The PR description itself acknowledges this ("For SOPS, we use what's currently in the repo's .sops.yaml"), making the function-level contract claim actively misleading to future contributors reasoning about the security guarantee.

New findings on updated code.