Keep edits made with sops edit --show-master-keys

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)

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))
+		})
+	}
+}

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)
+}