diff --git a/cmd/sops/edit.go b/cmd/sops/edit.go index bfc3a37b94..74e954995e 100644 --- a/cmd/sops/edit.go +++ b/cmd/sops/edit.go @@ -268,8 +268,10 @@ func runEditorUntilOk(opts runEditorUntilOkOpts) error { continue } // Replace the whole tree, because otherwise newBranches would - // contain the SOPS metadata - opts.Tree = &t + // contain the SOPS metadata. The caller's tree has to be updated + // in place, since the caller encrypts and emits that tree. + t.FilePath = opts.Tree.FilePath + *opts.Tree = t } else { if userErr, _ := validateFileForEncryption(opts.OutputStore, newBranches); userErr != nil { log.WithField( @@ -279,8 +281,8 @@ func runEditorUntilOk(opts runEditorUntilOkOpts) error { waitForKeyPress() continue } + opts.Tree.Branches = newBranches } - opts.Tree.Branches = newBranches needVersionUpdated, err := version.AIsNewerThanB(version.Version, opts.Tree.Metadata.Version) if err != nil { return common.NewExitError(fmt.Sprintf("Failed to compare document version %q with program version %q: %v", opts.Tree.Metadata.Version, version.Version, err), codes.FailedToCompareVersions) diff --git a/cmd/sops/edit_test.go b/cmd/sops/edit_test.go new file mode 100644 index 0000000000..1b5dc8c9d4 --- /dev/null +++ b/cmd/sops/edit_test.go @@ -0,0 +1,50 @@ +package main + +import ( + "os" + "path/filepath" + "runtime" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/getsops/sops/v3" + "github.com/getsops/sops/v3/aes" +) + +func TestEditKeepsChanges(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("the test editor is a shell script") + } + for _, showMasterKeys := range []bool{false, true} { + name := "plain" + if showMasterKeys { + name = "show-master-keys" + } + t.Run(name, func(t *testing.T) { + isolateAgeEnvironment(t) + key := newTestAgeKey(t) + path := encryptTestFile(t, "secret: old\n", []sops.KeyGroup{{key}}) + + editor := filepath.Join(t.TempDir(), "editor.sh") + script := "#!/bin/sh\nsed 's/secret: old/secret: new/' \"$1\" > \"$1.tmp\" && mv \"$1.tmp\" \"$1\"\n" + require.NoError(t, os.WriteFile(editor, []byte(script), 0700)) + t.Setenv("SOPS_EDITOR", editor) + + store := testYAMLStore() + output, err := edit(editOpts{ + Cipher: aes.NewCipher(), + InputStore: store, + OutputStore: store, + InputPath: path, + KeyServices: testKeyServices(), + ShowMasterKeys: showMasterKeys, + }) + require.NoError(t, err) + require.NoError(t, os.WriteFile(path, output, 0600)) + + assert.Equal(t, "secret: new\n", decryptTestFile(t, path)) + }) + } +} diff --git a/cmd/sops/helpers_test.go b/cmd/sops/helpers_test.go new file mode 100644 index 0000000000..ceb9cc77c6 --- /dev/null +++ b/cmd/sops/helpers_test.go @@ -0,0 +1,102 @@ +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" + + fage "filippo.io/age" + "github.com/stretchr/testify/require" + + "github.com/getsops/sops/v3" + "github.com/getsops/sops/v3/aes" + "github.com/getsops/sops/v3/age" + "github.com/getsops/sops/v3/cmd/sops/common" + "github.com/getsops/sops/v3/cmd/sops/formats" + "github.com/getsops/sops/v3/config" + "github.com/getsops/sops/v3/keyservice" +) + +// isolateAgeEnvironment makes sure that the age key source only sees the +// identities passed through SOPS_AGE_KEY by newTestAgeKey, and not keys of +// the user running the tests (like ~/.ssh/id_ed25519). +func isolateAgeEnvironment(t *testing.T) { + t.Helper() + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config")) + for _, name := range []string{ + age.SopsAgeKeyEnv, + age.SopsAgeKeyFileEnv, + age.SopsAgeKeyCmdEnv, + age.SopsAgeSshPrivateKeyFileEnv, + age.SopsAgeSshPrivateKeyCmdEnv, + } { + // t.Setenv registers the cleanup that restores the original value. + t.Setenv(name, "") + require.NoError(t, os.Unsetenv(name)) + } +} + +// newTestAgeKey generates a throwaway age identity, makes it available for +// decryption through SOPS_AGE_KEY, and returns the matching master key. +func newTestAgeKey(t *testing.T) *age.MasterKey { + t.Helper() + identity, err := fage.GenerateX25519Identity() + require.NoError(t, err) + identities := []string{identity.String()} + if existing, ok := os.LookupEnv(age.SopsAgeKeyEnv); ok && existing != "" { + identities = append([]string{existing}, identities...) + } + t.Setenv(age.SopsAgeKeyEnv, strings.Join(identities, "\n")) + key, err := age.MasterKeyFromRecipient(identity.Recipient().String()) + require.NoError(t, err) + return key +} + +func testYAMLStore() common.Store { + return common.StoreForFormat(formats.Yaml, config.NewStoresConfig()) +} + +func testKeyServices() []keyservice.KeyServiceClient { + return []keyservice.KeyServiceClient{keyservice.NewLocalClient()} +} + +// encryptTestFile writes plaintext YAML to a temporary file, encrypts it in +// place with the given key groups, and returns the file's path. +func encryptTestFile(t *testing.T, plaintext string, groups []sops.KeyGroup) string { + t.Helper() + path := filepath.Join(t.TempDir(), "secret.yaml") + require.NoError(t, os.WriteFile(path, []byte(plaintext), 0600)) + store := testYAMLStore() + encrypted, err := encrypt(encryptOpts{ + Cipher: aes.NewCipher(), + InputStore: store, + OutputStore: store, + InputPath: path, + KeyServices: testKeyServices(), + encryptConfig: encryptConfig{ + KeyGroups: groups, + UnencryptedSuffix: sops.DefaultUnencryptedSuffix, + }, + }) + require.NoError(t, err) + require.NoError(t, os.WriteFile(path, encrypted, 0600)) + return path +} + +// decryptTestFile decrypts the file at path and returns the plaintext YAML. +func decryptTestFile(t *testing.T, path string) string { + t.Helper() + store := testYAMLStore() + plaintext, err := decrypt(decryptOpts{ + Cipher: aes.NewCipher(), + InputStore: store, + OutputStore: store, + InputPath: path, + KeyServices: testKeyServices(), + }) + require.NoError(t, err) + return string(plaintext) +}