Repository: getsops/sops (language: python)

Issue:
SOPS displacing comments and causing YAML-breaking invalid syntax error and data loss
Hello everyone; I found a bug in the SOPS encryption, which you can replicate yourself.

Unencrypted file `test.yaml`:
```yaml
this:
  is:
    an: example
  # comment
```

I use age encryption, so I set up the SOPS_AGE_RECIPIENTS and SOPS_AGE_KEY_FILE variables, and then run `sops --encrypt --encrypted-regex an test.yaml`, obtaining this:

```yaml
this:
    is:
        an: ENC[AES256_GCM,data:7ew99FdoYQ==,iv:62k8jSLjg2l7YyorqOf9bobpgT7m+1qeEK66JpyObTE=,tag:caRiamKAO9TqZHj2qfKAgA==,type:str]
sops:
# comment
    kms: []
    gcp_kms: []
    azure_kv: []
    hc_vault: []
    age:
        - recipient: age107p580kfl3mlr2x7jqfhattzv0a3kd88ugjq2p5tsjz59jzdmdfsj2qwm2
          enc: |
            -----BEGIN AGE ENCRYPTED FILE-----
            YWdlLWVuY3J5cHRpb24ub3JnL3YxCi0+IFgyNTUxOSBZa3VuTFBucnV4QldrZW0z
            am9jTkw1ejZIUjBUVHpRTjRONUNTYnRXWFVRCncwdk5adnpDZktnK3JwVWU0R1o4
            UGlIOHpSdUpTQjBhWnMwcXAyZlhmTFEKLS0tIFk2bitXOHN2T2VIbHZzVnVlTVpn
            VGNUSkdFWENLSEFQdGh4OW1TMk1HZHMKvMCQo5SXkMk9HQ4Bv7n1UFj+IZDi0z8K
            1LJKCovXp7d+zceCwbVM9zfbleF7AvFuyWSbw7o11WpE4vtaD8fDWg==
            -----END AGE ENCRYPTED FILE-----
    lastmodified: "2022-05-31T14:38:08Z"
    mac: ENC[AES256_GCM,data:BNPYx43KNPdggGC/+qgHQfoMj+ctnrwjOpt+pOzoWcQCcS3IP1h0FtD/UcSc8mLTxS2+6gBBiFU8JIdnr8XwueIeGbcx0zEk8LZdspv0DuVQoC5AbiU4qnvCFRY7MIdmSKacG7zXVN4nQ2kW/Ip7jqhBAqZJl5cbUBcaQOgfGCo=,iv:aLQ+lhmgxzyrQphjknd1O1KHC6BVG1E0hM9tCfFs+J4=,tag:sHhaEqLU+/VRDRjTajAnwQ==,type:str]
    pgp: []
    encrypted_regex: an
    version: 3.7.3
```

As you can see, the `# comment` line is moved inside the sops metadata; whenever that file is decrypted, the comment will be gone (hence the data loss I mention in the title).

The reason why this is important is because my team is using some comment "tags" to mark code blocks, and the last tag disappears after encrypting-and-decrypting a file.

I did some extra testing to try to understand when this was happening, and I gathered some conclusions: it only happens if the indentation is **at least** the one in my example file (so at least 3) and the if the comment is "less indented" than the last line.

Sorry if this is a known issue and I missed it! :)


Discussion / hints:
Adding to this issue, it can break a YAML file, by producing an invalid syntax.

To reproduce, use this starting file `test.yaml`:
```yaml
this:
  is:
    an: example
  # comment
 
someArray: []
```
Again, run `sops --encrypt --encrypted-regex an test.yaml`. This will produce:
```yaml
this:
    is:
        an: ENC[AES256_GCM,data:BVubel9Etw==,iv:eSU4vD0Gl+ArqJYXz601lTYCcpuXGfdMj5ViU2jGOaE=,tag:Du0JCAnjc+pGYBRBqpIg+w==,type:str]
someArray:
# comment
[]

sops:
    kms: []
    gcp_kms: []
    azure_kv: []
    hc_vault: []
    age:
        - recipient: age1atpv5v2yrfuxxpe2gpddfq2n5g7vck89eg8fjqytrjrxy7plqfcs7l7788
          enc: |
            -----BEGIN AGE ENCRYPTED FILE-----
            YWdlLWVuY3J5cHRpb24ub3JnL3YxCi0+IFgyNTUxOSBrUDRtTjRCaktvbnhFQVpq
            ZGRyVm9rL2YrOUNYdnExOXVlQWhiczhZOVNvClFBVTZyWjh4ck53aE55cC9pZThv
            NExTS3BpZFdpWG9tMFZOeUZ3dFNtYncKLS0tIFRBTmYwNzd3dXZGN3pOYlBraS9y
            aklTbVpZNm54MjNVaXQ5bTRCQURkWU0KkoNrtSRxjGBjCGjAR8HRZqRH8tAZ432B
            HP7spoDOqtsRUuZPhBqQpvV+mP50z9ztNqrw5f8MDIirXsiw1qvYyg==
            -----END AGE ENCRYPTED FILE-----
    lastmodified: "2022-05-31T16:21:18Z"
    mac: ENC[AES256_GCM,data:BBYePXfdlh6EwLdvft7OncjUWpE2WrLdhtaKNONPZymdv7q23QEGJJxG3ktE02PcdS2aDsflwM3wSkPq5PPQSQXVZbtIMutKCdrKZGerEPUSF6KXy00CqiLn12kbO31JIisw5K+JvmhQ0vxopPAn0o4w0hABvy9Pt/ZKFt8pjkU=,iv:n+hzjCnrxQL986BmrTbToS3dWiWKPK8uRxdN+Dqc8xA=,tag:CGKCtco5LiZyhAbInkBaAQ==,type:str]
    pgp: []
    encrypted_regex: an
    version: 3.7.3
```

As you can see, `# comment` got in the middle of the empty array definition. This breaks the YAML syntax; the file can't even be decrypted anymore!
Thanks for this report! I can confirm that this happens with the latest version.
Interestingly I cannot reproduce this when removing the empty line after the comment... Then in both cases it works fine. But if there is an empty line after the comment, the error happens.

Fix patch:
diff --git a/stores/yaml/store.go b/stores/yaml/store.go
index da9b946b2..0a95d7c6a 100644
--- a/stores/yaml/store.go
+++ b/stores/yaml/store.go
@@ -233,7 +233,7 @@ func (store *Store) appendTreeBranch(branch sops.TreeBranch, mapping *yaml.Node)
 		if beginning {
 			comments = store.addCommentsHead(mapping, comments)
 		} else {
-			comments = store.addCommentsFoot(mapping.Content[len(mapping.Content) - 1], comments)
+			comments = store.addCommentsFoot(mapping.Content[len(mapping.Content) - 2], comments)
 		}
 	}
 }


Test patch:
diff --git a/stores/yaml/store_test.go b/stores/yaml/store_test.go
index bf7c972f5..245a43104 100644
--- a/stores/yaml/store_test.go
+++ b/stores/yaml/store_test.go
@@ -111,6 +111,53 @@ var COMMENT_6_BRANCHES = sops.TreeBranches{
 	},
 }
 
+// The following is a regression test for https://github.com/mozilla/sops/issues/1068
+var COMMENT_7_IN = []byte(`a:
+    b:
+        c: d
+    # comment
+
+e:
+    - f
+`)
+
+var COMMENT_7_BRANCHES = sops.TreeBranches{
+	sops.TreeBranch{
+		sops.TreeItem{
+			Key:   "a",
+			Value: sops.TreeBranch{
+				sops.TreeItem{
+					Key:   "b",
+					Value: sops.TreeBranch{
+						sops.TreeItem{
+							Key:   "c",
+							Value: "d",
+						},
+					},
+				},
+				sops.TreeItem{
+					Key:   sops.Comment{" comment"},
+					Value: nil,
+				},
+			},
+		},
+		sops.TreeItem{
+			Key:   "e",
+			Value: []interface{}{
+				"f",
+			},
+		},
+	},
+}
+
+var COMMENT_7_OUT = []byte(`a:
+    b:
+        c: d
+    # comment
+e:
+    - f
+`)
+
 func TestUnmarshalMetadataFromNonSOPSFile(t *testing.T) {
 	data := []byte(`hello: 2`)
 	_, err := (&Store{}).LoadEncryptedFile(data)
@@ -224,3 +271,13 @@ func TestEmitValue(t *testing.T) {
 	assert.Equal(t, string(PLAIN_0), string(bytes))
 	assert.Equal(t, PLAIN_0, bytes)
 }
+
+func TestComment7(t *testing.T) {
+	branches, err := (&Store{}).LoadPlainFile(COMMENT_7_IN)
+	assert.Nil(t, err)
+	assert.Equal(t, COMMENT_7_BRANCHES, branches)
+	bytes, err := (&Store{}).EmitPlainFile(branches)
+	assert.Nil(t, err)
+	assert.Equal(t, string(COMMENT_7_OUT), string(bytes))
+	assert.Equal(t, COMMENT_7_OUT, bytes)
+}