diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ec19a47..45e628c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -136,6 +136,15 @@ ### Fixed +- **`ankra cluster encrypt manifest --set` can rotate a value that is + already sealed.** Setting a new value on a sealed key (for example a + registry robot's `.dockerconfigjson`) failed with `encryption failed: + Failed to encrypt content`, so the only way to rotate a secret was to + decrypt it into Git first or create a new manifest. The CLI now unseals + the manifest in memory, applies the `--set` edits, and seals the result + again, keeping every key the previous seal covered sealed. The plaintext + never reaches Git. + - **`ankra application ship` no longer deploys a branch you did not ask for.** It reused any application already registered for the repository, so `ship . --branch main` on a repository whose application tracked diff --git a/cmd/cluster_encrypt.go b/cmd/cluster_encrypt.go index 56ef639c..a67bab5b 100644 --- a/cmd/cluster_encrypt.go +++ b/cmd/cluster_encrypt.go @@ -1160,11 +1160,18 @@ func runEncryptManifestCluster(cmd *cobra.Command, manifestName string, leafKeys return fmt.Errorf("fetch current manifest content: %w", err) } + var previouslySealedKeys []string if len(encryptSetEntries) > 0 { assignments, assignErr := collectSetAssignments(encryptSetEntries, nil, nil) if assignErr != nil { return assignErr } + unsealedB64, sealedKeys, unsealErr := unsealManifestForSet(encoded) + if unsealErr != nil { + return unsealErr + } + encoded = unsealedB64 + previouslySealedKeys = sealedKeys mutatedB64, setErr := applyManifestSet(encoded, assignments, "", "") if setErr != nil { return setErr @@ -1190,6 +1197,11 @@ func runEncryptManifestCluster(cmd *cobra.Command, manifestName string, leafKeys } leafKeys = plaintextKeys } + stillPresentSealedKeys, presenceErr := keysPresentInManifest(decoded, previouslySealedKeys) + if presenceErr != nil { + return presenceErr + } + leafKeys = unionEncryptedPaths(leafKeys, stillPresentSealedKeys) _, _ = fmt.Fprintf(cmd.OutOrStdout(), "Encrypting %s in manifest %q (stack %q)...\n", describeEncryptKeys(leafKeys), manifestName, stack.Name) pathsToSeal := pathsToSealWithDeclared(cmd.OutOrStdout(), leafKeys, manifest.EncryptedPaths) @@ -1239,6 +1251,79 @@ func runEncryptManifestCluster(cmd *cobra.Command, manifestName string, leafKeys return nil } +// unsealManifestForSet decrypts an already sealed manifest in memory so --set +// can replace a sealed value. Editing the sealed document directly leaves the +// old sops metadata and MAC over a value that is now plaintext, which the +// platform cannot decrypt to re-seal ("Failed to encrypt content"). The keys +// the previous seal covered are returned so the caller seals them again; the +// plaintext only travels to the platform's encrypt endpoint, never to Git. +// A manifest with no sops metadata is returned unchanged. +func unsealManifestForSet(manifestBase64 string) (unsealedBase64 string, sealedKeys []string, err error) { + rawManifest, err := base64.StdEncoding.DecodeString(manifestBase64) + if err != nil { + return "", nil, fmt.Errorf("base64-decode manifest content: %w", err) + } + sealedKeys, isSopsDocument, err := deriveSopsEncryptedPaths(rawManifest) + if err != nil { + return "", nil, err + } + if !isSopsDocument { + return manifestBase64, nil, nil + } + decrypted, err := apiClient.DecryptYAML(string(rawManifest)) + if err != nil { + return "", nil, fmt.Errorf("decrypt sealed manifest before applying --set: %w", err) + } + return base64.StdEncoding.EncodeToString([]byte(decrypted)), sealedKeys, nil +} + +// keysPresentInManifest keeps the keys that still name a scalar value in the +// manifest, so a key the previous seal covered but a --set edit removed is not +// handed to encrypt as a path to seal. +func keysPresentInManifest(manifestYAML []byte, keys []string) ([]string, error) { + if len(keys) == 0 { + return nil, nil + } + presentKeys := map[string]bool{} + decoder := yaml.NewDecoder(bytes.NewReader(manifestYAML)) + for { + var document yaml.Node + decodeErr := decoder.Decode(&document) + if errors.Is(decodeErr, io.EOF) { + break + } + if decodeErr != nil { + return nil, fmt.Errorf("parse manifest YAML: %w", decodeErr) + } + collectScalarKeyNames(&document, presentKeys) + } + kept := make([]string, 0, len(keys)) + for _, key := range keys { + if presentKeys[key] { + kept = append(kept, key) + } + } + return kept, nil +} + +func collectScalarKeyNames(node *yaml.Node, presentKeys map[string]bool) { + switch node.Kind { + case yaml.DocumentNode, yaml.SequenceNode: + for _, childNode := range node.Content { + collectScalarKeyNames(childNode, presentKeys) + } + case yaml.MappingNode: + for entryIndex := 0; entryIndex+1 < len(node.Content); entryIndex += 2 { + valueNode := node.Content[entryIndex+1] + if valueNode.Kind == yaml.ScalarNode { + presentKeys[node.Content[entryIndex].Value] = true + continue + } + collectScalarKeyNames(valueNode, presentKeys) + } + } +} + func runEncryptAddonCluster(cmd *cobra.Command, addonName string, leafKeys []string) error { // The command ends in a partial-stack PATCH, which the backend serves // synchronously (DB transaction + a full GitOps commit/push when the diff --git a/cmd/cluster_encrypt_set_sealed_test.go b/cmd/cluster_encrypt_set_sealed_test.go new file mode 100644 index 00000000..03b13586 --- /dev/null +++ b/cmd/cluster_encrypt_set_sealed_test.go @@ -0,0 +1,155 @@ +package cmd + +import ( + "bytes" + "encoding/base64" + "errors" + "strings" + "testing" +) + +// A --set on a manifest whose value is already sealed edited the sealed +// document in place, leaving the old sops metadata and MAC over a value that +// was now plaintext; the platform could not decrypt it to re-seal and every +// rotation failed with "Failed to encrypt content". The CLI now unseals the +// manifest in memory first and seals the edited plaintext afresh. + +func runEncryptManifestOnSealedSecret(t *testing.T, mock *upgradeMock, extraArgs ...string) (string, error) { + t.Helper() + setMockClient(t, mock) + resetUpgradeCommandFlags(t) + + cmd := rootCmd + out := new(bytes.Buffer) + cmd.SetOut(out) + cmd.SetErr(out) + arguments := append([]string{"cluster", "encrypt", "manifest", "demo-namespace", "--cluster", fakeClusterUUID}, extraArgs...) + cmd.SetArgs(arguments) + executeError := cmd.Execute() + return out.String(), executeError +} + +func TestRunEncryptManifest_SetRotatesAnAlreadySealedValue(t *testing.T) { + mock := &upgradeMock{ + iac: sampleIaCYAMLForCmd, + manifestB64: base64.StdEncoding.EncodeToString([]byte(encryptedSecretManifestYAML)), + decryptResult: plainSecretManifestYAML, + encryptResult: encryptedSecretManifestYAML, + } + output, executeError := runEncryptManifestOnSealedSecret(t, mock, + "--key", "password", + "--set", "data.password=cm90YXRlZA==", + ) + if executeError != nil { + t.Fatalf("execute failed: %v\noutput: %s", executeError, output) + } + + if len(mock.encryptCalls) != 1 { + t.Fatalf("expected exactly one EncryptYAML call, got %d", len(mock.encryptCalls)) + } + sentContent := mock.encryptCalls[0].YamlContent + if strings.Contains(sentContent, "sops:") || strings.Contains(sentContent, "ENC[") { + t.Errorf("the content sent to encrypt still carries the old seal:\n%s", sentContent) + } + if !strings.Contains(sentContent, "password: cm90YXRlZA==") { + t.Errorf("the content sent to encrypt does not carry the new value:\n%s", sentContent) + } + if len(mock.capturedRequests) != 1 { + t.Fatalf("expected one PATCH, got %d", len(mock.capturedRequests)) + } +} + +func TestRunEncryptManifest_SetOnASealedManifestKeepsEveryPreviouslySealedKeySealed(t *testing.T) { + mock := &upgradeMock{ + iac: sampleIaCYAMLForCmd, + manifestB64: base64.StdEncoding.EncodeToString([]byte(encryptedSecretManifestYAML)), + decryptResult: plainSecretManifestYAML, + encryptResult: bothKeysEncryptedSecretManifestYAML, + } + output, executeError := runEncryptManifestOnSealedSecret(t, mock, + "--key", "username", + "--set", "data.username=cm9vdA==", + ) + if executeError != nil { + t.Fatalf("execute failed: %v\noutput: %s", executeError, output) + } + + callPaths := mock.encryptCalls[0].EncryptedPaths + if !containsEncryptedPath(callPaths, "username") || !containsEncryptedPath(callPaths, "password") { + t.Errorf("encrypted paths = %v, want the requested key and the previously sealed password", callPaths) + } + manifest := mock.capturedRequests[0].Body.Spec.Stacks[0].Manifests[0] + if !containsEncryptedPath(manifest.EncryptedPaths, "password") { + t.Errorf("manifest.encrypted_paths = %v, want password recorded", manifest.EncryptedPaths) + } +} + +func TestRunEncryptManifest_SetOnAPlaintextManifestDoesNotDecrypt(t *testing.T) { + mock := &upgradeMock{ + iac: sampleIaCYAMLForCmd, + manifestB64: base64.StdEncoding.EncodeToString([]byte(plainSecretManifestYAML)), + decryptErr: errors.New("decrypt must not be called"), + encryptResult: encryptedSecretManifestYAML, + } + output, executeError := runEncryptManifestOnSealedSecret(t, mock, + "--key", "password", + "--set", "data.password=bmV3", + ) + if executeError != nil { + t.Fatalf("execute failed: %v\noutput: %s", executeError, output) + } + if paths := mock.encryptCalls[0].EncryptedPaths; len(paths) != 1 || paths[0] != "password" { + t.Errorf("encrypted paths = %v, want [password]", paths) + } +} + +func TestRunEncryptManifest_SetOnASealedManifestReportsADecryptFailure(t *testing.T) { + mock := &upgradeMock{ + iac: sampleIaCYAMLForCmd, + manifestB64: base64.StdEncoding.EncodeToString([]byte(encryptedSecretManifestYAML)), + decryptErr: errors.New("key mismatch"), + } + output, executeError := runEncryptManifestOnSealedSecret(t, mock, + "--key", "password", + "--set", "data.password=bmV3", + ) + if executeError == nil { + t.Fatalf("expected an error, got none\noutput: %s", output) + } + if !strings.Contains(executeError.Error(), "decrypt sealed manifest before applying --set") { + t.Errorf("error = %v, want it to name the decrypt step", executeError) + } + if len(mock.encryptCalls) != 0 || len(mock.capturedRequests) != 0 { + t.Errorf("nothing should be encrypted or pushed after a failed decrypt") + } +} + +const sealedSecretWithDroppedPasswordYAML = `apiVersion: v1 +kind: Secret +metadata: + name: my-secret + namespace: web +type: Opaque +data: + username: YWRtaW4= +` + +func TestRunEncryptManifest_SetDoesNotSealAPreviouslySealedKeyTheEditRemoved(t *testing.T) { + mock := &upgradeMock{ + iac: sampleIaCYAMLForCmd, + manifestB64: base64.StdEncoding.EncodeToString([]byte(encryptedSecretManifestYAML)), + decryptResult: sealedSecretWithDroppedPasswordYAML, + encryptResult: bothKeysEncryptedSecretManifestYAML, + } + output, executeError := runEncryptManifestOnSealedSecret(t, mock, + "--key", "username", + "--set", "data.username=cm9vdA==", + ) + if executeError != nil { + t.Fatalf("execute failed: %v\noutput: %s", executeError, output) + } + callPaths := mock.encryptCalls[0].EncryptedPaths + if containsEncryptedPath(callPaths, "password") { + t.Errorf("encrypted paths = %v, want password left out: the edited manifest no longer has it", callPaths) + } +}