Repository: getsops/sops (language: python)

Issue:
--mac-only-encrypted panics on .env files.
Thank you for --mac-only-encrypted  Works great on my .yaml files

However it panics on .env

```shell
$ sops -e --mac-only-encrypted sample.env
panic: interface conversion: interface {} is bool, not string

goroutine 1 [running]:
github.com/getsops/sops/v3/stores/dotenv.(*Store).EmitPlainFile(0xc00058e200?, {0xc000011110?, 0xc0005aae45?, 0xc0005aae45?})
        /home/lance/go/src/github.com/getsops/sops/stores/dotenv/store.go:130 +0x2bf
github.com/getsops/sops/v3/stores/dotenv.(*Store).EmitEncryptedFile(0xc0004e8680?, {{{0xdd1be8f, 0xedce9ddb7, 0x0}, {0x0, 0x0}, {0x0, 0x0}, {0x0, 0x0}, ...}, ...})
        /home/lance/go/src/github.com/getsops/sops/stores/dotenv/store.go:115 +0x3fc
main.encrypt({{0x1231d98, 0xc0004e4690}, {0x12389b0, 0x19bc1a0}, {0x12389b0, 0x19bc1a0}, {0xc000049ad0, 0x24}, {0xc0004daac0, 0x1, ...}, ...})
        /home/lance/go/src/github.com/getsops/sops/cmd/sops/encrypt.go:107 +0x575
main.main.func8(0xc0001898c0)
        /home/lance/go/src/github.com/getsops/sops/cmd/sops/main.go:808 +0xab8
github.com/urfave/cli.HandleAction({0xeb5880?, 0x110f8b0?}, 0xc0004756c0?)
        /home/lance/go/pkg/mod/github.com/urfave/cli@v1.22.14/app.go:524 +0x50
github.com/urfave/cli.(*App).Run(0xc0004756c0, {0xc00003e0a0, 0x5, 0x5})
        /home/lance/go/pkg/mod/github.com/urfave/cli@v1.22.14/app.go:286 +0x766
main.main()
        /home/lance/go/src/github.com/getsops/sops/cmd/sops/main.go:1014 +0x3530
```

I'm running 0cc1d116c47f0311c66f4bb348cf4026c130d275  (main HEAD from Nov 14)
--mac-only-encrypted panics on .env files.
Thank you for --mac-only-encrypted  Works great on my .yaml files

However it panics on .env

```shell
$ sops -e --mac-only-encrypted sample.env
panic: interface conversion: interface {} is bool, not string

goroutine 1 [running]:
github.com/getsops/sops/v3/stores/dotenv.(*Store).EmitPlainFile(0xc00058e200?, {0xc000011110?, 0xc0005aae45?, 0xc0005aae45?})
        /home/lance/go/src/github.com/getsops/sops/stores/dotenv/store.go:130 +0x2bf
github.com/getsops/sops/v3/stores/dotenv.(*Store).EmitEncryptedFile(0xc0004e8680?, {{{0xdd1be8f, 0xedce9ddb7, 0x0}, {0x0, 0x0}, {0x0, 0x0}, {0x0, 0x0}, ...}, ...})
        /home/lance/go/src/github.com/getsops/sops/stores/dotenv/store.go:115 +0x3fc
main.encrypt({{0x1231d98, 0xc0004e4690}, {0x12389b0, 0x19bc1a0}, {0x12389b0, 0x19bc1a0}, {0xc000049ad0, 0x24}, {0xc0004daac0, 0x1, ...}, ...})
        /home/lance/go/src/github.com/getsops/sops/cmd/sops/encrypt.go:107 +0x575
main.main.func8(0xc0001898c0)
        /home/lance/go/src/github.com/getsops/sops/cmd/sops/main.go:808 +0xab8
github.com/urfave/cli.HandleAction({0xeb5880?, 0x110f8b0?}, 0xc0004756c0?)
        /home/lance/go/pkg/mod/github.com/urfave/cli@v1.22.14/app.go:524 +0x50
github.com/urfave/cli.(*App).Run(0xc0004756c0, {0xc00003e0a0, 0x5, 0x5})
        /home/lance/go/pkg/mod/github.com/urfave/cli@v1.22.14/app.go:286 +0x766
main.main()
        /home/lance/go/src/github.com/getsops/sops/cmd/sops/main.go:1014 +0x3530
```

I'm running 0cc1d116c47f0311c66f4bb348cf4026c130d275  (main HEAD from Nov 14)


Discussion / hints:
I wrote a test duplicating the bug `dotenv/store_test.go`.  And I quickly coded up a type checker in `dotenv/store.go`.  :thinking:  But it *feels* wrong.   It feels like the `TreeItem` key of `sops_mac_only_encrypted` should be a string by the time it reaches `store.go` and not a bool.


I wrote a test duplicating the bug `dotenv/store_test.go`.  And I quickly coded up a type checker in `dotenv/store.go`.  :thinking:  But it *feels* wrong.   It feels like the `TreeItem` key of `sops_mac_only_encrypted` should be a string by the time it reaches `store.go` and not a bool.



Fix patch:
diff --git a/stores/dotenv/store.go b/stores/dotenv/store.go
index db895fcd9..58d7c6d53 100644
--- a/stores/dotenv/store.go
+++ b/stores/dotenv/store.go
@@ -2,7 +2,6 @@ package dotenv //import "github.com/getsops/sops/v3/stores/dotenv"
 
 import (
 	"bytes"
-	"encoding/json"
 	"fmt"
 	"sort"
 	"strings"
@@ -49,7 +48,9 @@ func (store *Store) LoadEncryptedFile(in []byte) (sops.Tree, error) {
 		}
 	}
 
-	metadata, err := mapToMetadata(mdMap)
+	stores.DecodeNewLines(mdMap)
+	stores.DecodeNonStrings(mdMap)
+	metadata, err := stores.UnflattenMetadata(mdMap)
 	if err != nil {
 		return sops.Tree{}, err
 	}
@@ -101,10 +102,14 @@ func (store *Store) LoadPlainFile(in []byte) (sops.TreeBranches, error) {
 // runtime object
 func (store *Store) EmitEncryptedFile(in sops.Tree) ([]byte, error) {
 	metadata := stores.MetadataFromInternal(in.Metadata)
-	mdItems, err := metadataToMap(metadata)
+	mdItems, err := stores.FlattenMetadata(metadata)
 	if err != nil {
 		return nil, err
 	}
+
+	stores.EncodeNonStrings(mdItems)
+	stores.EncodeNewLines(mdItems)
+
 	var keys []string
 	for k := range mdItems {
 		keys = append(keys, k)
@@ -158,41 +163,6 @@ func (store *Store) EmitExample() []byte {
 	return bytes
 }
 
-func metadataToMap(md stores.Metadata) (map[string]interface{}, error) {
-	var mdMap map[string]interface{}
-	inrec, err := json.Marshal(md)
-	if err != nil {
-		return nil, err
-	}
-	err = json.Unmarshal(inrec, &mdMap)
-	if err != nil {
-		return nil, err
-	}
-	flat := stores.Flatten(mdMap)
-	for k, v := range flat {
-		if s, ok := v.(string); ok {
-			flat[k] = strings.Replace(s, "\n", "\\n", -1)
-		}
-	}
-	return flat, nil
-}
-
-func mapToMetadata(m map[string]interface{}) (stores.Metadata, error) {
-	for k, v := range m {
-		if s, ok := v.(string); ok {
-			m[k] = strings.Replace(s, "\\n", "\n", -1)
-		}
-	}
-	m = stores.Unflatten(m)
-	var md stores.Metadata
-	inrec, err := json.Marshal(m)
-	if err != nil {
-		return md, err
-	}
-	err = json.Unmarshal(inrec, &md)
-	return md, err
-}
-
 func isComplexValue(v interface{}) bool {
 	switch v.(type) {
 	case []interface{}:
diff --git a/stores/flatten.go b/stores/flatten.go
index 00d603302..c894e6e29 100644
--- a/stores/flatten.go
+++ b/stores/flatten.go
@@ -1,6 +1,7 @@
 package stores
 
 import (
+	"encoding/json"
 	"fmt"
 	"strconv"
 	"strings"
@@ -60,6 +61,22 @@ func Flatten(in map[string]interface{}) map[string]interface{} {
 	return newMap
 }
 
+// FlattenMetadata flattens a Metadata struct into a flat map.
+func FlattenMetadata(md Metadata) (map[string]interface{}, error) {
+	var mdMap map[string]interface{}
+	jsonBytes, err := json.Marshal(md)
+	if err != nil {
+		return nil, err
+	}
+	err = json.Unmarshal(jsonBytes, &mdMap)
+	if err != nil {
+		return nil, err
+	}
+
+	flat := Flatten(mdMap)
+	return flat, nil
+}
+
 type token interface{}
 
 type mapToken struct {
@@ -185,3 +202,57 @@ func Unflatten(in map[string]interface{}) map[string]interface{} {
 	}
 	return newMap
 }
+
+// UnflattenMetadata unflattens a map flattened by FlattenMetadata into Metadata
+func UnflattenMetadata(in map[string]interface{}) (Metadata, error) {
+	m := Unflatten(in)
+	var md Metadata
+	jsonBytes, err := json.Marshal(m)
+	if err != nil {
+		return md, err
+	}
+	err = json.Unmarshal(jsonBytes, &md)
+	return md, err
+}
+
+// DecodeNewLines replaces \\n with \n for all string values in the map.
+// Used by config stores that do not handle multi-line values (ini, dotenv).
+func DecodeNewLines(m map[string]interface{}) {
+	for k, v := range m {
+		if s, ok := v.(string); ok {
+			m[k] = strings.Replace(s, "\\n", "\n", -1)
+		}
+	}
+}
+
+// EncodeNewLines replaces \n with \\n for all string values in the map.
+// Used by config stores that do not handle multi-line values (ini, dotenv).
+func EncodeNewLines(m map[string]interface{}) {
+	for k, v := range m {
+		if s, ok := v.(string); ok {
+			m[k] = strings.Replace(s, "\n", "\\n", -1)
+		}
+	}
+}
+
+// DecodeNonStrings will look for known metadata keys that are not strings and decode to the appropriate type
+func DecodeNonStrings(m map[string]interface{}) {
+	if v, ok := m["mac_only_encrypted"]; ok {
+		m["mac_only_encrypted"] = false
+		if v == "true" {
+			m["mac_only_encrypted"] = true
+		}
+	}
+}
+
+// EncodeNonStrings will look for known metadata keys that are not strings and will encode it to strings
+func EncodeNonStrings(m map[string]interface{}) {
+	if v, found := m["mac_only_encrypted"]; found {
+		if vBool, ok := v.(bool); ok {
+			m["mac_only_encrypted"] = "false"
+			if vBool {
+				m["mac_only_encrypted"] = "true"
+			}
+		}
+	}
+}
diff --git a/stores/ini/store.go b/stores/ini/store.go
index d703d87cf..12bd17ce1 100644
--- a/stores/ini/store.go
+++ b/stores/ini/store.go
@@ -183,19 +183,13 @@ func (store *Store) LoadEncryptedFile(in []byte) (sops.Tree, error) {
 }
 
 func (store *Store) iniSectionToMetadata(sopsSection *ini.Section) (stores.Metadata, error) {
-
 	metadataHash := make(map[string]interface{})
 	for k, v := range sopsSection.KeysHash() {
-		metadataHash[k] = strings.Replace(v, "\\n", "\n", -1)
-	}
-	m := stores.Unflatten(metadataHash)
-	var md stores.Metadata
-	inrec, err := json.Marshal(m)
-	if err != nil {
-		return md, err
+		metadataHash[k] = v
 	}
-	err = json.Unmarshal(inrec, &md)
-	return md, err
+	stores.DecodeNewLines(metadataHash)
+	stores.DecodeNonStrings(metadataHash)
+	return stores.UnflattenMetadata(metadataHash)
 }
 
 // LoadPlainFile loads a plaintext INI file's bytes onto a sops.TreeBranches runtime object
@@ -229,24 +223,13 @@ func (store *Store) EmitEncryptedFile(in sops.Tree) ([]byte, error) {
 }
 
 func (store *Store) encodeMetadataToIniBranch(md stores.Metadata) (sops.TreeBranch, error) {
-	var mdMap map[string]interface{}
-	inrec, err := json.Marshal(md)
-	if err != nil {
-		return nil, err
-	}
-	err = json.Unmarshal(inrec, &mdMap)
-	if err != nil {
-		return nil, err
-	}
-	flat := stores.Flatten(mdMap)
-	for k, v := range flat {
-		if s, ok := v.(string); ok {
-			flat[k] = strings.Replace(s, "\n", "\\n", -1)
-		}
-	}
+	flat, err := stores.FlattenMetadata(md)
 	if err != nil {
 		return nil, err
 	}
+	stores.EncodeNonStrings(flat)
+	stores.EncodeNewLines(flat)
+
 	branch := sops.TreeBranch{}
 	for key, value := range flat {
 		if value == nil {


Test patch:
diff --git a/stores/flatten_test.go b/stores/flatten_test.go
index b59a10928..4d11dd1ad 100644
--- a/stores/flatten_test.go
+++ b/stores/flatten_test.go
@@ -139,3 +139,108 @@ func TestTokenizeNested(t *testing.T) {
 	tokenized := tokenize(input)
 	assert.Equal(t, expected, tokenized)
 }
+
+func TestFlattenMetadata(t *testing.T) {
+	tests := []struct {
+		input Metadata
+		want  map[string]interface{}
+	}{
+		{Metadata{MACOnlyEncrypted: false}, map[string]interface{}{"mac_only_encrypted": nil}},
+		{Metadata{MACOnlyEncrypted: true}, map[string]interface{}{"mac_only_encrypted": true}},
+		{Metadata{MessageAuthenticationCode: "line1\nline2"}, map[string]interface{}{"mac": "line1\nline2"}},
+		{Metadata{MessageAuthenticationCode: "line1\n\n\nline2\n\nline3"}, map[string]interface{}{"mac": "line1\n\n\nline2\n\nline3"}},
+	}
+
+	for _, tt := range tests {
+		got, err := FlattenMetadata(tt.input)
+		assert.NoError(t, err)
+		for k, v := range tt.want {
+			assert.Equal(t, v, got[k])
+		}
+	}
+}
+
+func TestFlattenMetadataToUnflattenMetadata(t *testing.T) {
+	tests := []struct {
+		input Metadata
+	}{
+		{Metadata{MACOnlyEncrypted: true}},
+		{Metadata{MACOnlyEncrypted: false}},
+		{Metadata{ShamirThreshold: 3}},
+		{Metadata{MessageAuthenticationCode: "line1\nline2"}},
+		{Metadata{MessageAuthenticationCode: "line1\n\n\nline2\n\nline3"}},
+	}
+
+	for _, tt := range tests {
+		flat, err := FlattenMetadata(tt.input)
+		assert.NoError(t, err)
+		md, err := UnflattenMetadata(flat)
+		assert.NoError(t, err)
+		assert.Equal(t, tt.input, md)
+	}
+}
+
+func TestDecodeNewLines(t *testing.T) {
+	tests := []struct {
+		input map[string]interface{}
+		want  map[string]interface{}
+	}{
+		{map[string]interface{}{"mac": "line1\\nline2"}, map[string]interface{}{"mac": "line1\nline2"}},
+		{map[string]interface{}{"mac": "line1\\n\\n\\nline2\\n\\nline3"}, map[string]interface{}{"mac": "line1\n\n\nline2\n\nline3"}},
+	}
+
+	for _, tt := range tests {
+		DecodeNewLines(tt.input)
+		for k, v := range tt.want {
+			assert.Equal(t, v, tt.input[k])
+		}
+	}
+}
+
+func TestEncodeNewLines(t *testing.T) {
+	tests := []struct {
+		input map[string]interface{}
+		want  map[string]interface{}
+	}{
+		{map[string]interface{}{"mac": "line1\nline2"}, map[string]interface{}{"mac": "line1\\nline2"}},
+		{map[string]interface{}{"mac": "line1\n\n\nline2\n\nline3"}, map[string]interface{}{"mac": "line1\\n\\n\\nline2\\n\\nline3"}},
+	}
+
+	for _, tt := range tests {
+		EncodeNewLines(tt.input)
+		for k, v := range tt.want {
+			assert.Equal(t, v, tt.input[k])
+		}
+	}
+}
+
+func TestDecodeNonStrings(t *testing.T) {
+	tests := []struct {
+		input map[string]interface{}
+		want  map[string]interface{}
+	}{
+		{map[string]interface{}{"mac_only_encrypted": "false"}, map[string]interface{}{"mac_only_encrypted": false}},
+		{map[string]interface{}{"mac_only_encrypted": "true"}, map[string]interface{}{"mac_only_encrypted": true}},
+		{map[string]interface{}{"mac_only_encrypted": "something-else"}, map[string]interface{}{"mac_only_encrypted": false}},
+	}
+
+	for _, tt := range tests {
+		DecodeNonStrings(tt.input)
+		assert.Equal(t, tt.want, tt.input)
+	}
+}
+
+func TestEncodeNonStrings(t *testing.T) {
+	tests := []struct {
+		input map[string]interface{}
+		want  map[string]interface{}
+	}{
+		{map[string]interface{}{"mac_only_encrypted": false}, map[string]interface{}{"mac_only_encrypted": "false"}},
+		{map[string]interface{}{"mac_only_encrypted": true}, map[string]interface{}{"mac_only_encrypted": "true"}},
+	}
+
+	for _, tt := range tests {
+		EncodeNonStrings(tt.input)
+		assert.Equal(t, tt.want, tt.input)
+	}
+}