Skip to content

Commit 627bb97

Browse files
committed
stores: test for duplicate keys, reseve keyword (yaml only now)
stores/json: use assert stores/yaml: fix failing test (empty data) stores/yaml: use assert in tests unfix error handling and ignore error Signed-off-by: Martin Holst Swende <[email protected]>
1 parent b7da2fc commit 627bb97

File tree

3 files changed

+65
-0
lines changed

3 files changed

+65
-0
lines changed

stores/json/store_test.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -394,3 +394,24 @@ func TestEmitValueString(t *testing.T) {
394394
assert.Nil(t, err)
395395
assert.Equal(t, []byte("\"hello\""), bytes)
396396
}
397+
398+
func TestConflictingAttributes(t *testing.T) {
399+
// See https://stackoverflow.com/a/23195243
400+
// Duplicate keys in json is technically valid, but discouraged.
401+
// Implementations may handle them differently. ECMA-262 says
402+
//
403+
// > In the case where there are duplicate name Strings within an object,
404+
// > lexically preceding values for the same key shall be overwritten.
405+
406+
data := `
407+
{
408+
"hello": "Sops config file",
409+
"hello": "Doubles are ok",
410+
"hello": ["repeatedly"],
411+
"hello": 3.14
412+
}
413+
`
414+
s := new(Store)
415+
_, err := s.LoadPlainFile([]byte(data))
416+
assert.Nil(t, err)
417+
}

stores/yaml/store.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,13 @@ func (store *Store) LoadEncryptedFile(in []byte) (sops.Tree, error) {
294294
// sops.Tree runtime object
295295
func (store *Store) LoadPlainFile(in []byte) (sops.TreeBranches, error) {
296296
var branches sops.TreeBranches
297+
if len(in) > 0 {
298+
// This is needed to make the yaml-decoder check for uniqueness of keys
299+
// Can probably be removed when https://github.com/go-yaml/yaml/issues/814 is merged.
300+
if err := yaml.NewDecoder(bytes.NewReader(in)).Decode(make(map[string]interface{})); err != nil {
301+
return nil, err
302+
}
303+
}
297304
d := yaml.NewDecoder(bytes.NewReader(in))
298305
for {
299306
var data yaml.Node
@@ -309,6 +316,12 @@ func (store *Store) LoadPlainFile(in []byte) (sops.TreeBranches, error) {
309316
if err != nil {
310317
return nil, fmt.Errorf("Error unmarshaling input YAML: %s", err)
311318
}
319+
// Prevent use of reserved keywords
320+
for _, item := range branch {
321+
if item.Key == "sops" {
322+
return nil, fmt.Errorf("YAML doc used reserved word '%v'", item.Key)
323+
}
324+
}
312325
branches = append(branches, branch)
313326
}
314327
return branches, nil

stores/yaml/store_test.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,3 +281,34 @@ func TestComment7(t *testing.T) {
281281
assert.Equal(t, string(COMMENT_7_OUT), string(bytes))
282282
assert.Equal(t, COMMENT_7_OUT, bytes)
283283
}
284+
func TestDuplicateAttributes(t *testing.T) {
285+
// Duplicate keys are _not_ valid yaml.
286+
//
287+
// See https://yaml.org/spec/1.2.2/#mapping
288+
// > The content of a mapping node is an unordered set of key/value node pairs,
289+
// > with the restriction that each of the keys is unique.
290+
//
291+
data := `
292+
hello: Sops config file
293+
hello: Duplicates are not ok
294+
rootunique:
295+
key2: "value"
296+
key2: "foo"
297+
`
298+
s := new(Store)
299+
_, err := s.LoadPlainFile([]byte(data))
300+
assert.NotNil(t, err)
301+
assert.Equal(t, `yaml: unmarshal errors:
302+
line 3: mapping key "hello" already defined at line 2`, err.Error())
303+
}
304+
305+
func TestReservedAttributes(t *testing.T) {
306+
data := `
307+
hello: Sops config file
308+
sops: The attribute 'sops' must be rejected, otherwise the file cannot be opened later on
309+
`
310+
s := new(Store)
311+
_, err := s.LoadPlainFile([]byte(data))
312+
assert.NotNil(t, err)
313+
assert.Equal(t, `YAML doc used reserved word 'sops'`, err.Error())
314+
}

0 commit comments

Comments
 (0)