stores: test for duplicate keys, reserve keyword (yaml only now)

stores/json/store_test.go

@@ -539,6 +539,28 @@ func TestNoIndent(t *testing.T) {
 	out, err := store.EmitPlainFile(tree.Branches)
 	assert.Nil(t, err)
 	assert.Equal(t, expected, string(out))
+
+}
+
+func TestConflictingAttributes(t *testing.T) {
+	// See https://stackoverflow.com/a/23195243
+	// Duplicate keys in json is technically valid, but discouraged.
+	// Implementations may handle them differently. ECMA-262 says
+	//
+	// > In the case where there are duplicate name Strings within an object,
+	// > lexically preceding values for the same key shall be overwritten.
+
+	data := `
+{
+  "hello": "Sops config file", 
+  "hello": "Doubles are ok", 
+  "hello": ["repeatedly"],
+  "hello": 3.14
+}
+`
+	s := new(Store)
+	_, err := s.LoadPlainFile([]byte(data))
+	assert.Nil(t, err)
 }
 
 func TestComments(t *testing.T) {

stores/yaml/store.go

@@ -307,6 +307,13 @@ func (store *Store) LoadEncryptedFile(in []byte) (sops.Tree, error) {
 // sops.Tree runtime object
 func (store *Store) LoadPlainFile(in []byte) (sops.TreeBranches, error) {
 	var branches sops.TreeBranches
+	if len(in) > 0 {
+		// This is needed to make the yaml-decoder check for uniqueness of keys
+		// Can probably be removed when https://github.com/go-yaml/yaml/issues/814 is merged.
+		if err := yaml.NewDecoder(bytes.NewReader(in)).Decode(make(map[string]interface{})); err != nil {
+			return nil, err
+		}
+	}
 	d := yaml.NewDecoder(bytes.NewReader(in))
 	for {
 		var data yaml.Node
@@ -322,6 +329,12 @@ func (store *Store) LoadPlainFile(in []byte) (sops.TreeBranches, error) {
 		if err != nil {
 			return nil, fmt.Errorf("Error unmarshaling input YAML: %s", err)
 		}
+		// Prevent use of reserved keywords
+		for _, item := range branch {
+			if item.Key == stores.SopsMetadataKey {
+				return nil, fmt.Errorf("YAML doc used reserved word '%v'", item.Key)
+			}
+		}
 		branches = append(branches, branch)
 	}
 	return branches, nil

stores/yaml/store_test.go

@@ -62,19 +62,19 @@ key4: *bar
 var ALIASES_BRANCHES = sops.TreeBranches{
 	sops.TreeBranch{
 		sops.TreeItem{
-			Key:   "key1",
+			Key: "key1",
 			Value: []interface{}{
 				"foo",
 			},
 		},
 		sops.TreeItem{
-			Key:   "key2",
+			Key: "key2",
 			Value: []interface{}{
 				"foo",
 			},
 		},
 		sops.TreeItem{
-			Key:   "key3",
+			Key: "key3",
 			Value: sops.TreeBranch{
 				sops.TreeItem{
 					Key:   "foo",
@@ -87,7 +87,7 @@ var ALIASES_BRANCHES = sops.TreeBranches{
 			},
 		},
 		sops.TreeItem{
-			Key:   "key4",
+			Key: "key4",
 			Value: sops.TreeBranch{
 				sops.TreeItem{
 					Key:   "foo",
@@ -237,7 +237,6 @@ prometheus-node-exporter:
     - --collector.filesystem.ignored-fs-types=^(autofs|binfmt_misc|cgroup|configfs|debugfs|devpts|devtmpfs|fusectl|hugetlbfs|mqueue|overlay|proc|procfs|pstore|rpc_pipefs|securityfs|sysfs|tracefs)$
 `)
 
-
 func TestUnmarshalMetadataFromNonSOPSFile(t *testing.T) {
 	data := []byte(`hello: 2`)
 	_, err := (&Store{}).LoadEncryptedFile(data)
@@ -398,3 +397,35 @@ func TestHasSopsTopLevelKey(t *testing.T) {
 	})
 	assert.Equal(t, ok, false)
 }
+
+func TestDuplicateAttributes(t *testing.T) {
+	// Duplicate keys are _not_ valid yaml.
+	//
+	// See https://yaml.org/spec/1.2.2/#mapping
+	// > The content of a mapping node is an unordered set of key/value node pairs,
+	// > with the restriction that each of the keys is unique.
+	//
+	data := `
+hello: Sops config file
+hello: Duplicates are not ok
+rootunique:
+  key2: "value"
+  key2: "foo"
+`
+	s := new(Store)
+	_, err := s.LoadPlainFile([]byte(data))
+	assert.NotNil(t, err)
+	assert.Equal(t, `yaml: unmarshal errors:
+  line 3: mapping key "hello" already defined at line 2`, err.Error())
+}
+
+func TestReservedAttributes(t *testing.T) {
+	data := `
+hello: Sops config file
+sops: The attribute 'sops' must be rejected, otherwise the file cannot be opened later on 
+`
+	s := new(Store)
+	_, err := s.LoadPlainFile([]byte(data))
+	assert.NotNil(t, err)
+	assert.Equal(t, `YAML doc used reserved word 'sops'`, err.Error())
+}