Merge pull request #1311 from felixfontein/errors

Handle errors
getsops · Oct 3, 2023 · 0fd3c72 · 0fd3c72
2 parents ee1513a + dd59dc1
commit 0fd3c72
Show file tree

Hide file tree

Showing 6 changed files with 80 additions and 5 deletions.
diff --git a/cmd/sops/common/common.go b/cmd/sops/common/common.go
@@ -89,6 +89,9 @@ func DecryptTree(opts DecryptTreeOpts) (dataKey []byte, err error) {
 	}
 	fileMac, err := opts.Cipher.Decrypt(opts.Tree.Metadata.MessageAuthenticationCode, dataKey, opts.Tree.Metadata.LastModified.Format(time.RFC3339))
 	if !opts.IgnoreMac {
+		if err != nil {
+			return nil, NewExitError(fmt.Sprintf("Cannot decrypt MAC: %s", err), codes.MacMismatch)
+		}
 		if fileMac != computedMac {
 			// If the file has an empty MAC, display "no MAC" instead of not displaying anything
 			if fileMac == "" {
@@ -318,10 +321,10 @@ func FixAWSKMSEncryptionContextBug(opts GenericDecryptOpts, tree *sops.Tree) (*s
 	}
 
 	file, err := os.Create(opts.InputPath)
-	defer file.Close()
 	if err != nil {
 		return nil, NewExitError(fmt.Sprintf("Could not open file for writing: %s", err), codes.CouldNotWriteOutputFile)
 	}
+	defer file.Close()
 	_, err = file.Write(encryptedFile)
 	if err != nil {
 		return nil, err

diff --git a/cmd/sops/main.go b/cmd/sops/main.go
@@ -1234,6 +1234,11 @@ func extractSetArguments(set string) (path []interface{}, valueToInsert interfac
 	fullPath := strings.TrimRight(pathValuePair[0], " ")
 	jsonValue := pathValuePair[1]
 	valueToInsert, err = jsonValueToTreeInsertableValue(jsonValue)
+	if err != nil {
+		// All errors returned by jsonValueToTreeInsertableValue are created by common.NewExitError(),
+		// so we can simply pass them on
+		return nil, nil, err
+	}
 
 	path, err = parseTreePath(fullPath)
 	if err != nil {

diff --git a/kms/keysource_test.go b/kms/keysource_test.go
@@ -446,6 +446,7 @@ func TestMasterKey_createKMSConfig(t *testing.T) {
 				assert.NoError(t, err)
 
 				creds, err := cfg.Credentials.Retrieve(context.TODO())
+				assert.Nil(t, err)
 				assert.Equal(t, "id", creds.AccessKeyID)
 				assert.Equal(t, "secret", creds.SecretAccessKey)
 				assert.Equal(t, "token", creds.SessionToken)

diff --git a/pgp/keysource_test.go b/pgp/keysource_test.go
@@ -332,6 +332,7 @@ func TestMasterKey_Decrypt(t *testing.T) {
 		fingerprint,
 		"--no-encrypt-to",
 	}, bytes.NewReader(data))
+	assert.Nil(t, err)
 	assert.NoErrorf(t, gnuPGHome.ImportFile(mockPrivateKey), stderr.String())
 
 	encryptedData := stdout.String()
@@ -414,6 +415,7 @@ func TestMasterKey_decryptWithOpenPGP(t *testing.T) {
 			fingerprint,
 			"--no-encrypt-to",
 		}, bytes.NewReader(data))
+		assert.Nil(t, err)
 		assert.NoErrorf(t, gnuPGHome.ImportFile(mockPrivateKey), stderr.String())
 
 		encryptedData := stdout.String()
@@ -462,6 +464,7 @@ func TestMasterKey_decryptWithGnuPG(t *testing.T) {
 			fingerprint,
 			"--no-encrypt-to",
 		}, bytes.NewReader(data))
+		assert.Nil(t, err)
 		assert.NoErrorf(t, gnuPGHome.ImportFile(mockPrivateKey), stderr.String())
 
 		encryptedData := stdout.String()

diff --git a/stores/yaml/store.go b/stores/yaml/store.go
@@ -131,6 +131,10 @@ func (store Store) appendYamlNodeToTreeBranch(node *yaml.Node, branch sops.TreeB
 		return nil, fmt.Errorf("YAML documents that are values are not supported")
 	case yaml.AliasNode:
 		branch, err = store.appendYamlNodeToTreeBranch(node.Alias, branch, false)
+		if err != nil {
+			// This should never happen since node.Alias was already successfully decoded before
+			return nil, err
+		}
 	}
 	if !commentsWereHandled {
 		branch = store.appendCommentToMap(node.FootComment, branch)
@@ -204,9 +208,9 @@ func (store *Store) appendSequence(in []interface{}, sequence *yaml.Node) {
 	}
 	if len(comments) > 0 {
 		if beginning {
-			comments = store.addCommentsHead(sequence, comments)
+			store.addCommentsHead(sequence, comments)
 		} else {
-			comments = store.addCommentsFoot(sequence.Content[len(sequence.Content)-1], comments)
+			store.addCommentsFoot(sequence.Content[len(sequence.Content)-1], comments)
 		}
 	}
 }
@@ -231,9 +235,9 @@ func (store *Store) appendTreeBranch(branch sops.TreeBranch, mapping *yaml.Node)
 	}
 	if len(comments) > 0 {
 		if beginning {
-			comments = store.addCommentsHead(mapping, comments)
+			store.addCommentsHead(mapping, comments)
 		} else {
-			comments = store.addCommentsFoot(mapping.Content[len(mapping.Content)-2], comments)
+			store.addCommentsFoot(mapping.Content[len(mapping.Content)-2], comments)
 		}
 	}
 }

diff --git a/stores/yaml/store_test.go b/stores/yaml/store_test.go
@@ -48,6 +48,59 @@ var BRANCHES = sops.TreeBranches{
 	},
 }
 
+var ALIASES = []byte(`---
+key1: &foo
+  - foo
+key2: *foo
+key3: &bar
+  foo: bar
+  baz: bam
+key4: *bar
+`)
+
+var ALIASES_BRANCHES = sops.TreeBranches{
+	sops.TreeBranch{
+		sops.TreeItem{
+			Key:   "key1",
+			Value: []interface{}{
+				"foo",
+			},
+		},
+		sops.TreeItem{
+			Key:   "key2",
+			Value: []interface{}{
+				"foo",
+			},
+		},
+		sops.TreeItem{
+			Key:   "key3",
+			Value: sops.TreeBranch{
+				sops.TreeItem{
+					Key:   "foo",
+					Value: "bar",
+				},
+				sops.TreeItem{
+					Key:   "baz",
+					Value: "bam",
+				},
+			},
+		},
+		sops.TreeItem{
+			Key:   "key4",
+			Value: sops.TreeBranch{
+				sops.TreeItem{
+					Key:   "foo",
+					Value: "bar",
+				},
+				sops.TreeItem{
+					Key:   "baz",
+					Value: "bam",
+				},
+			},
+		},
+	},
+}
+
 var COMMENT_1 = []byte(`# test
 a:
     b: null
@@ -170,6 +223,12 @@ func TestLoadPlainFile(t *testing.T) {
 	assert.Equal(t, BRANCHES, branches)
 }
 
+func TestLoadAliasesPlainFile(t *testing.T) {
+	branches, err := (&Store{}).LoadPlainFile(ALIASES)
+	assert.Nil(t, err)
+	assert.Equal(t, ALIASES_BRANCHES, branches)
+}
+
 func TestComment1(t *testing.T) {
 	// First iteration: load and store
 	branches, err := (&Store{}).LoadPlainFile(COMMENT_1)