search

getsops / sops

Simple and flexible tool for managing secrets
https://getsops.io/
Mozilla Public License 2.0
16.24k stars 855 forks source link

Preserve comments isolated in documents (and an error decrypting in edge cases) #936

Open patricknelson opened 2 years ago

patricknelson commented 2 years ago

When testing comments at the top of YAML files in #757, I found that comments in YAML streams which are isolated into their own documents are coalesced into the adjacent document (but only if they are at the very top of the YAML stream). Also, I found a possible bug where sops will encrypt a document that it cannot decrypt later on whenever these isolated comments are located anywhere else in the YAML stream.

Coalescing separate documents into a single document:

#@data/values
---
foo: bar

Becomes:

#@data/values
foo: bar

Errors: Each of the following variants of the above YAML stream will encrypt without an error. However, they cannot be decrypted, resulting in the exact same error message:

Could not marshal tree: Error marshaling to yaml: yaml: expected SCALAR, SEQUENCE-START, MAPPING-START, or ALIAS, but got document end

---
#@data/values
---
foo: bar
foo: bar
---
#@data/values
---
baz: qux
foo: bar
---
#@data/values

Note: While these examples utilize some code from ytt which handles comments semantically, it's not expected for sops to support ytt syntax per se. Rather, it's just a real-world example of the usefulness of having comments in this positions and isolated in this way, particularly since sops can be used to encrypt values that would be used in a templating engine like ytt (for annotations in a document containing sensitive data that are then interpreted by a parser).

felixfontein commented 2 years ago

This are actually two problems:

  1. sops currently cannot distinguish between empty documents and empty top-level maps. So the empty document will probably decode as {} (with the comment ahead) if the invalid merging bug is fixed. (See #908, #907.)
  2. The invalid merging bug comes from a bug in yaml.v3, for which I have a fix bug that fix is still waiting for a review (https://github.com/go-yaml/yaml/pull/690).
patricknelson commented 2 years ago

Since this is a little esoteric for me, can you interpret (or simplify) this for me a bit? I'm inferring that you're saying that a single comment in a document is "empty" so are you suggesting that there's no path to being able to ever encrypt/decrypt this symmetrically and cleanly?

#@data/values
---
foo: bar