Proposed implementation for `::merge` about modus HOT 5 CLOSED

Relevant crate: https://github.com/sfackler/shell-escape

from modus.

I am still a bit concerned about the use of shell-escaping, because such tools do not provide any correctness guarantees. As a result, (1) the language semantics is not well-defined (we cannot guarantee that an arbitrary command will be executed the way it is typed), (2) we open opportunities for shell injection attacks.

I suggest to think more about what we can do to avoid these problems. For example, if there is a way to set envorinment variables, we can use cp "$SOURCE" "$DESTINATION" (although this will pollute the environment of the spawned shell process). We can consider also reading the arguments from temporary files, as passing them to using command substitution, e.g. cp "`cat /tmp/source.txt`" "`cat /tmp/destination.txt`".

from modus.

I thought about this as well, and I don't like relying on escaping either.

About the first approach - the only problem I can see is that there is a limit to the combined size of environment variables, although it is quite large (128 KB on Linux), but still nevertheless could cause problem if the user has a long "run" script, or if the parent image already contains tons of env vars.

I like the second approach a bit more (we are already using temporary mounts to implement copy so no problem just adding some temporary text files as well). Also polluting the file system feels a bit "better" than polluting env vars.

As to shell injection attacks - all these scripts are run inside the container it's building, and if you want to inject some shell commands you could already just use run normally.

from modus.

@maowtm, ok, your plan sounds good.

Regarding shell injection, imagine the following situation: a user of Modus wrote a Modusfile that accepts a file path as an argument of an image predicate. The user exposes this argument through a web service, accepting untrusted inputs under the assumption that file paths inside their container are all safe. If we use the shell escaping approach, this will make the service vulnerable to shell injection.

from modus.

(shell escape initially)

from modus.