--mac-only-encrypted panics on .env files. about sops HOT 15 CLOSED

I wrote a test duplicating the bug dotenv/store_test.go. And I quickly coded up a type checker in dotenv/store.go. 🤔 But it feels wrong. It feels like the TreeItem key of sops_mac_only_encrypted should be a string by the time it reaches store.go and not a bool.

from sops.

I have a branch with a fix here: lancerushing@934c2cd

It is a quick fix for the cast issues in dotenv.store.go for Metadata.MACOnlyEncrypted being a bool.

🤔 Perhaps it would be better to make Metadata.MACOnlyEncrypted a string(edit) instead. (??) Maybe not..

I do like idea of having more expressive types on Metadata. However, the marshal code for dotenv (and ini?) will need to handle any non-string types.

from sops.

CC @mitar

from sops.

@felixfontein Thanks for CCing me.

@lancerushing Thanks for this report and suggested fix. I think you are right. Or we fix support for ini types or we change Metadata.MACOnlyEncrypted into a string? (I think you are suggesting that, it seems like a sentence ends early there.)

@felixfontein What do you think? My personal take would be to fix parsing of ini metadata like @lancerushing did above.

from sops.

I think I agree that fixing the parsing is the best approach here, and add support for new types and their fields when needed. (Having generic code for this would be nice, but seems a bit like overengineering right now.)

from sops.

Maybe we should write a dotenv and ini marshaler for Metadata, and have annotations on the Metadata struct like we have for json and yaml. (???)

Edit. Some projects with dotenv marshalers.
koanf: https://github.com/knadh/koanf/blob/master/parsers/dotenv/dotenv.go
godotenv: https://github.com/joho/godotenv/blob/main/parser.go#L20, we could do something similar

from sops.

@lancerushing Are you are interested in writing that? Otherwise we might go with the simple fix for now and then if metadata grows into too much spaghetti code we could rewrite it?

from sops.

Yeah I can take a stab at it tonight. Let's see how far I can get it done. I'll do for ini files too.

If it doesn't work out, we could run the ugly version.

from sops.

I also noticed that dotenv doesn't do the "complex" data types of Metadata.KeyGroups. I could attempt to add support for that.

We have to decide on an format that accommodate arrays in the dotenv. Maybe an "index" suffix? sops_keygoup_0...n
or we could json enoode the key groups? sops_key_groups=[{json here}, {}, ...] 🤔 🤔

from sops.

After looking through the code, both ini and dotenv have the panic for the bool Metadata.MACOnlyEncrypted.

Both use stores.Unflatten(). Also Flatten and Unflatten are only used for metadata. 🤔 I have a couple ways I can add a fix. I'll do both, and see which one you prefer.

Option A) Quick fix: I'll and a func FixTypesForMetaData(in map[string]interface{}) map[string]interface{} , and add it right after stores.Unflatten(m) in dotenv/store.go and ini/store.go. Least amount of code, and it encapsulates the change neatly.

Option B) I'll change the signature of Flatten to func Flatten(m store.Metadata) map[string]interface{} and Unflatten to func Unflatten(in map[string]interface{}) (Metadata, error) . This will eliminate the duplicate code in the ini and dotenv stores.

from sops.

Option A - PR is here: #1355

Give me an hour (or so) and I'll have Option B ready

from sops.

Option B - PR is here: #1356

The casting is the same as Option A. In additional I consolidated the json marshalling and \n handling into stores/flatten.go. Plus wrote additional tests.

from sops.

Future: we probably should add some dotenv and ini tests into functional-tests. For now after I ran make install I did a manual test in the shell.

export SOPS_AGE_RECIPIENTS="age1lzd99uklcjnc0e7d860axevet2cz99ce9pq6tzuzd05l5nr28ams36nvun"

echo "key = value" > example.env
echo "key = value" > example.ini

sops --encrypt --mac-only-encrypted example.env > example.encrypted.env
sops --encrypt --mac-only-encrypted example.ini > example.encrypted.ini

sops --decrypt example.encrypted.env > example.decrypted.env
sops --decrypt example.encrypted.ini > example.decrypted.ini

diff example.ini example.decrypted.ini
diff example.env example.decrypted.env

from sops.

Awesome work, thanks!

from sops.

I'm glad to help. Let me know if you need anything else in the PRs.

from sops.