feat: exec-env/-file support multiple args

[hraban]

Don’t pass through sh; let the user do that if they want.

Fixes #1469

[felixfontein]

Please note that this is a breaking change and thus has no chance of being merged as-is.

[hraban]

Yeah I tried to somehow detect if the user passed -- after exec-env to only do it then, but I couldn't find a way to get that information out of the argv parser sops uses. Open to other ideas because this is a pretty fundamental feature that becomes particularly relevant when you want to nest multiple sops exec-env calls to allow a hierarchy of envvars (particularly if you have to do so from a bare POSIX shell rather than bash -__- ).

[felixfontein]

Yeah I tried to somehow detect if the user passed -- after exec-env to only do it then, but I couldn't find a way to get that information out of the argv parser sops uses.

Even then it would be a breaking change, since you can already use -- now (it's handled by the argument parser).

What would be possible is to add a new parameter (probably a boolean flag) which allows to control the behavior, like --no-shell, that explicitly enables the new behavior.

[hraban]

One could argue that was never explicitly documented in the first place so it would only be breaking Hyrum's law / xkcd 1172 , but I'm happy either way :)

It would probably also be nice not to have {} expanded in filenames when that no-shell arg is passed. What do you think?

[hraban]

(for the record I was just being glib, I actually think your suggestion of a separate flag is better UX)

[felixfontein]

Even if -- wasn't explicitly documented, it's a very common pattern for CLI parsers to that you kind of have to assume that users correctly guessed that they can use it without it being explicitly documented :)

It would probably also be nice not to have {} expanded in filenames when that no-shell arg is passed. What do you think?

Why? Or do you mean to only expand it when the whole argument is "{}", and not when {} appears as part of an argument?

This can be very annoying for programs where you have to provide paths as part of an argument. One common example is Docker's -v option, where you could use something like -v {}:/file:ro to mount the decrypted file as /file into the container. If {} isn't replaced here, you cannot use exec-file to call docker and pass the file into the container via mounts.

[hraban]

Why? Or do you mean to only expand it when the whole argument is "{}", and not when {} appears as part of an argument?

This can be very annoying for programs where you have to provide paths as part of an argument. One common example is Docker's -v option, where you could use something like -v {}:/file:ro to mount the decrypted file as /file into the container. If {} isn't replaced here, you cannot use exec-file to call docker and pass the file into the container via mounts.

Because it makes sops exec unsafe as an actual "transparent" exec. That's the conventional point of --: when the rest of the argument aren't vetted and potentially attacker controlled. This lets you do things like ls -- * without fearing someone inserts a file -la. Or in this case {}.

The name exec in particular I'm sure come from the exec[v][e] syscall, and from bash's exec statement, and tools like aws-vault exec .... Sops exec-env is a perfect tool to place in a transparent wrapper / trampoline script which has the job of "injecting" some safe envvars, but otherwise continuing to exec argv without any changes. If a user then calls, say, find on that, suddenly {} will be interpreted by sops.

It's a very useful feature to be sure, but if there is no way to actually disable it, it makes it very hard to use sops as a generic command wrapper.

(edit: s/exec-env/exec-file/ but you get the idea)

[felixfontein]

sops exec-env does not (and never did) interpret {}. {} is only used by sops exec-file. Not replacing {} by the filename pretty much wrecks sops exec-file, since you don't know where the file has been written to.

[hraban]

Yes, I meant to type exec-file. Forcing string substitution like that reduces sops' flexibility as a transparent trampoline script and would require wrapping of potential downstream code, e.g. anything that takes arguments which might include arbitrary file names. I personally don't have enough steam to see this ticket through without buy-in from upstream so I'm going to stick to my fork and leave this here for anyone else who is facing the same issues and is interested in picking up the work. Thank you for your time 👍