sops icon indicating copy to clipboard operation
sops copied to clipboard

Add (simple) support for quoted multiline variables in .env files

Open MidasLamb opened this issue 10 months ago • 3 comments

This re-uses almost all previously existing logic, it only changes what constitutes a "line" when parsing the file.

This is not a fully fledged parser (e.g. escaping quotes doesn't work) but it should handle some additional cases now.

Fixes #965

MidasLamb avatar Feb 04 '25 19:02 MidasLamb

Thanks for your contribution!

Please note that this is a breaking change and cannot be merged. There has been an earlier attempt at this (#622) that had to be reverted because of this (#706).

Ref: #1435

felixfontein avatar Feb 04 '25 21:02 felixfontein

Thanks for the quick response @felixfontein !

I quickly went through the issue, and I'd like to point out that this PR does not add a full dotenv parser. On the encryption side, it just looks make sure that it considers an entire key-value pair, not just lines, but how to value is encrypted has not changed. On the decryption side there is maybe a small inconsistency, that shouldn't be an actual issue, where it might turn a "\n" combination into and actual newline. i.e.

FOO="bar\nbaz"

becomes

FOO="bar
baz"

after an encryption->decryption cycle. But semantically, these values are actually the same, so this shouldn't be a breaking change.

This PR is just meant as a small incremental step to allow for multiline strings, without changing anything else really. In my reasoning the changes introduced here just expand the amount of allowed values, without doing any actual internal changes.

The main issue seemed to be with this change: https://github.com/getsops/sops/pull/622/files#diff-c9aa3c7e333fa3bc6fa314822b8da1e449389f6bb14b68608e47d048c4997a08R104, where it adds single quotes unconditionally to values.

If you want some additional tests/proof about this addition not being a breaking change in the way the earlier code was (which seems more like a proper dotenv parser, while this is just a slight expansion upon the currently working code.)

MidasLamb avatar Feb 05 '25 20:02 MidasLamb

While it's definitely true that your implementation comes very close to not breaking backwards compatibility, there are some cases that are broken. One of them you yourself listed. Your claim on "semantically equivalence" is problematic, since there is no well-defined .env file format. There are some assumptions that are true for many of these .env formats, but SOPS' implementation always was somewhat different. And since there is no formal specification to follow, every change is breaking.

Next to the change you mentioned above, is that if someone already encrypted a=foo\nbar" (something not compatible with many .env file format definitions, but with SOPS' definition), then your change will result in this being decrypted to

a=foo
bar"

Right now I don't see a way of adjusting the .env format (and the .ini format) without some versioning/file format identifier. I'll add some more thoughts on that to #1435 later today.

felixfontein avatar Feb 15 '25 15:02 felixfontein