fix(cluster): let encrypt manifest --set rotate an already-sealed value

[CodeStaple]

Summary

ankra cluster encrypt manifest <name> --set 'data.K=<new>' failed with encryption failed: Failed to encrypt content whenever K was already sealed, so a SOPS secret could not be rotated in place.

Cause: --set edited the sealed document directly. The result carried the old sops metadata and MAC over a value that was now plaintext. The platform's encrypt endpoint decrypts an already-sealed document to merge paths, the MAC no longer verified, and the request failed.

Fix: when --set targets a manifest with sops metadata, the CLI decrypts it in memory through the platform's decrypt endpoint, applies the edits to the plaintext, and seals the result afresh. Every key the previous seal covered is carried into the encrypted paths, so --key other does not unseal a key it did not name. A plaintext manifest is never decrypted. The plaintext only goes to the platform's encrypt endpoint and never reaches Git.

Tests

cmd/cluster_encrypt_set_sealed_test.go:

• rotating a sealed value sends unsealed content carrying the new value to encrypt
• --key username --set ... on a manifest with password sealed keeps password sealed and recorded
• --set on a plaintext manifest does not call decrypt
• a decrypt failure stops before anything is encrypted or pushed

Three of the four fail without the fix. The full go test ./... and golangci-lint pass (pre-commit hook).

Bead: ankra-0b9mio

🤖 Generated with Claude Code

[ankra-platform]

Ankra AI review

Verdict: comments.

The fix correctly unseals a sops-sealed manifest in memory before applying --set edits and re-seals the union of requested and previously sealed keys, with good test coverage including the decrypt-failure and dropped-key cases. The main concern is that keysPresentInManifest matches previously sealed keys by bare key name anywhere in any YAML document, which can keep (and re-seal) a key the edit actually removed if the same name appears elsewhere. CI is still running, so build status is unknown.

Findings

• [medium] Presence check matches key names globally, not the edited path (cmd/cluster_encrypt.go:1309-1319)
  collectScalarKeyNames records every scalar mapping key by bare name across all documents and nesting levels, so keysPresentInManifest keeps a previously sealed key if that name appears anywhere in the manifest - e.g. a password field in an unrelated ConfigMap document or a nested config section - even when the --set edit removed the actual sealed entry under data. The key would then be handed to encrypt as a path to seal and recorded in encrypted_paths although the intended value is gone (and a same-named but different value may get sealed instead). Consider tracking the full path of sealed keys (or at least scoping the presence check to the same parent path the seal covered) rather than matching on the leaf name alone.

• [low] Null-valued keys count as present and will be re-sealed (cmd/cluster_encrypt.go:1318)
  A mapping entry whose value is an explicit null (password: with no value) is a ScalarNode, so it is treated as present and kept in the seal list. Sealing a null path is likely to fail or produce a confusing encrypted_paths entry; consider skipping scalar nodes with the null tag.

• [low] Confirm decoded reflects the post-set manifest content (cmd/cluster_encrypt.go:1200)
  keysPresentInManifest is called on decoded, but the diff does not show where decoded is assigned relative to the encoded = unsealedB64 / applyManifestSet mutations above. If decoded is derived from the pre-set or pre-unseal content, the presence filter would run against the wrong document (e.g. still-sealed ENC[...] scalars, which would make every previously sealed key look present). Please verify decoded is decoded from the final mutated base64.

---

Reviewed commit b7b0740. This review is read-only and advisory.

[ankra-platform]

[medium] Re-sealing a key that --set just removed may fail or seal a stale path

previouslySealedKeys is derived from the old sops metadata before the --set edits are applied, then unconditionally unioned into leafKeys. If a --set assignment deletes or renames a key that the previous seal covered, the encrypt request will ask the platform to seal a path that no longer exists in the edited document. I cannot see how pathsToSealWithDeclared or the encrypt endpoint handles a missing path (error vs. silent skip), so please confirm a removed sealed key does not turn the rotation into a new failure mode, or filter previouslySealedKeys against the post-edit document.

_{Ankra AI review. Read-only and advisory.}

[ankra-platform]

[low] Behavior of deriveSopsEncryptedPaths and unionEncryptedPaths not visible in diff

The correctness of the fix rests on deriveSopsEncryptedPaths returning the exact set of paths the previous seal covered (and reliably distinguishing sops documents from plaintext) and on unionEncryptedPaths deduplicating/normalizing paths consistently with how leafKeys are built elsewhere. Neither helper is shown in this diff, so I cannot verify edge cases such as nested paths, duplicate entries, or a sops document whose metadata lists paths differently from the --key flag format.

_{Ankra AI review. Read-only and advisory.}

[CodeStaple]

Addressed the review:

• [medium] re-sealing a key --set removed: fixed in b7b0740. Previously sealed keys are now filtered against the edited manifest (keysPresentInManifest), so a key an edit dropped is not sent as a path to seal. Covered by TestRunEncryptManifest_SetDoesNotSealAPreviouslySealedKeyTheEditRemoved.
• [low] helper behaviour not in the diff: deriveSopsEncryptedPaths (already used by file mode) reports a document as SOPS only when it has a top-level sops mapping, and returns the bare key names whose values are ENC[...], skipping the metadata subtree. unionEncryptedPaths dedupes by leaf key, so data.password and password count as one entry. Both are exercised by the new tests, including the case where --key username keeps an already sealed password sealed.

[ankra-platform]

[medium] Presence check matches key names globally, not the edited path

collectScalarKeyNames records every scalar mapping key by bare name across all documents and nesting levels, so keysPresentInManifest keeps a previously sealed key if that name appears anywhere in the manifest - e.g. a password field in an unrelated ConfigMap document or a nested config section - even when the --set edit removed the actual sealed entry under data. The key would then be handed to encrypt as a path to seal and recorded in encrypted_paths although the intended value is gone (and a same-named but different value may get sealed instead). Consider tracking the full path of sealed keys (or at least scoping the presence check to the same parent path the seal covered) rather than matching on the leaf name alone.

_{Ankra AI review. Read-only and advisory.}

[ankra-platform]

[low] Null-valued keys count as present and will be re-sealed

A mapping entry whose value is an explicit null (password: with no value) is a ScalarNode, so it is treated as present and kept in the seal list. Sealing a null path is likely to fail or produce a confusing encrypted_paths entry; consider skipping scalar nodes with the null tag.

_{Ankra AI review. Read-only and advisory.}

[ankra-platform]

[low] Confirm decoded reflects the post-set manifest content

keysPresentInManifest is called on decoded, but the diff does not show where decoded is assigned relative to the encoded = unsealedB64 / applyManifestSet mutations above. If decoded is derived from the pre-set or pre-unseal content, the presence filter would run against the wrong document (e.g. still-sealed ENC[...] scalars, which would make every previously sealed key look present). Please verify decoded is decoded from the final mutated base64.

_{Ankra AI review. Read-only and advisory.}