Merge pull request #1289 from felixfontein/binary-data-error

Improve handling of errors when binary store handles bad data

cmd/sops/decrypt.go

@@ -1,14 +1,19 @@
 package main
 
 import (
+	"errors"
 	"fmt"
 
 	"github.com/getsops/sops/v3"
 	"github.com/getsops/sops/v3/cmd/sops/codes"
 	"github.com/getsops/sops/v3/cmd/sops/common"
 	"github.com/getsops/sops/v3/keyservice"
+	"github.com/getsops/sops/v3/stores/json"
 )
 
+const notBinaryHint = ("This is likely not an encrypted binary file?" +
+	" If not, use --output-type to select the correct output type.")
+
 type decryptOpts struct {
 	Cipher      sops.Cipher
 	InputStore  sops.Store
@@ -45,6 +50,9 @@ func decrypt(opts decryptOpts) (decryptedFile []byte, err error) {
 		return extract(tree, opts.Extract, opts.OutputStore)
 	}
 	decryptedFile, err = opts.OutputStore.EmitPlainFile(tree.Branches)
+	if errors.Is(err, json.BinaryStoreEmitPlainError) {
+		err = fmt.Errorf("%s\n\n%s", err.Error(), notBinaryHint)
+	}
 	if err != nil {
 		return nil, common.NewExitError(fmt.Sprintf("Error dumping file: %s", err), codes.ErrorDumpingTree)
 	}
@@ -59,6 +67,9 @@ func extract(tree *sops.Tree, path []interface{}, outputStore sops.Store) (outpu
 	if newBranch, ok := v.(sops.TreeBranch); ok {
 		tree.Branches[0] = newBranch
 		decrypted, err := outputStore.EmitPlainFile(tree.Branches)
+		if errors.Is(err, json.BinaryStoreEmitPlainError) {
+			err = fmt.Errorf("%s\n\n%s", err.Error(), notBinaryHint)
+		}
 		if err != nil {
 			return nil, common.NewExitError(fmt.Sprintf("Error dumping file: %s", err), codes.ErrorDumpingTree)
 		}

stores/json/store.go

@@ -3,6 +3,7 @@ package json //import "github.com/getsops/sops/v3/stores/json"
 import (
 	"bytes"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 
@@ -42,15 +43,24 @@ func (store BinaryStore) EmitEncryptedFile(in sops.Tree) ([]byte, error) {
 	return store.store.EmitEncryptedFile(in)
 }
 
+var BinaryStoreEmitPlainError = errors.New("error emitting binary store")
+
 // EmitPlainFile produces plaintext json file's bytes from its corresponding sops.TreeBranches object
 func (store BinaryStore) EmitPlainFile(in sops.TreeBranches) ([]byte, error) {
+	if len(in) != 1 {
+		return nil, fmt.Errorf("%w: there must be exactly one tree branch", BinaryStoreEmitPlainError)
+	}
 	// JSON stores a single object per file
 	for _, item := range in[0] {
 		if item.Key == "data" {
-			return []byte(item.Value.(string)), nil
+			if value, ok := item.Value.(string); ok {
+				return []byte(value), nil
+			} else {
+				return nil, fmt.Errorf("%w: 'data' key in tree does not have a string value", BinaryStoreEmitPlainError)
+			}
 		}
 	}
-	return nil, fmt.Errorf("No binary data found in tree")
+	return nil, fmt.Errorf("%w: no binary data found in tree", BinaryStoreEmitPlainError)
 }
 
 // EmitValue extracts a value from a generic interface{} object representing a structured set

stores/json/store_test.go

@@ -3,8 +3,8 @@ package json
 import (
 	"testing"
 
-	"github.com/stretchr/testify/assert"
 	"github.com/getsops/sops/v3"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestDecodeJSON(t *testing.T) {
@@ -320,6 +320,75 @@ func TestLoadJSONFormattedBinaryFile(t *testing.T) {
 	assert.Equal(t, "data", branches[0][0].Key)
 }
 
+func TestEmitBinaryFile(t *testing.T) {
+	store := BinaryStore{}
+	data, err := store.EmitPlainFile(sops.TreeBranches{
+		sops.TreeBranch{
+			sops.TreeItem{
+				Key:   "data",
+				Value: "foo",
+			},
+		},
+	})
+	assert.Nil(t, err)
+	assert.Equal(t, []byte("foo"), data)
+}
+
+func TestEmitBinaryFileWrongBranches(t *testing.T) {
+	store := BinaryStore{}
+	data, err := store.EmitPlainFile(sops.TreeBranches{
+		sops.TreeBranch{
+			sops.TreeItem{
+				Key:   "data",
+				Value: "bar",
+			},
+		},
+		sops.TreeBranch{
+			sops.TreeItem{
+				Key:   "data",
+				Value: "bar",
+			},
+		},
+	})
+	assert.Nil(t, data)
+	assert.Contains(t, err.Error(), "there must be exactly one tree branch")
+
+	data, err = store.EmitPlainFile(sops.TreeBranches{})
+	assert.Nil(t, data)
+	assert.Contains(t, err.Error(), "there must be exactly one tree branch")
+}
+
+func TestEmitBinaryFileNoData(t *testing.T) {
+	store := BinaryStore{}
+	data, err := store.EmitPlainFile(sops.TreeBranches{
+		sops.TreeBranch{
+			sops.TreeItem{
+				Key:   "foo",
+				Value: "bar",
+			},
+		},
+	})
+	assert.Nil(t, data)
+	assert.Contains(t, err.Error(), "no binary data found in tree")
+}
+
+func TestEmitBinaryFileWrongDataType(t *testing.T) {
+	store := BinaryStore{}
+	data, err := store.EmitPlainFile(sops.TreeBranches{
+		sops.TreeBranch{
+			sops.TreeItem{
+				Key: "data",
+				Value: sops.TreeItem{
+					Key:   "foo",
+					Value: "bar",
+				},
+			},
+		},
+	})
+	assert.Nil(t, data)
+	assert.Contains(t, err.Error(), "'data' key in tree does not have a string value")
+}
+
 func TestEmitValueString(t *testing.T) {
 	bytes, err := (&Store{}).EmitValue("hello")
 	assert.Nil(t, err)