Re: [PATCH v2] git submodule foreach: Skip eval for more than one argument

> ‘eval "$@"’ created an extra layer of shell interpretation, which was
> probably not expected by a user who passed multiple arguments to git
> submodule foreach:
>
> $ git grep "'"
> [searches for single quotes]
> $ git submodule foreach git grep "'"
> Entering '[submodule]'
> /usr/lib/git-core/git-submodule: 1: eval: Syntax error: Unterminated quoted string
> Stopping at '[submodule]'; script returned non-zero status.
>
> To fix this, if the user passed more than one argument, just execute
> "$@" directly instead of passing it to eval.
>
> Signed-off-by: Anders Kaseorg <andersk@mit.edu>

> On Fri, 27 Sep 2013, Johan Herland wrote:
>> 2. If we are unlucky there might be existing users that work around the
>> existing behavior by adding an extra level of quoting (i.e. doing the
>> equivalent of git submodule foreach git grep "\'" in your example
>> above). Will their workaround break as a result of your change? Is that
>> acceptable?
>
> Anyone adding an extra level of quoting ought to realize that they should
> be passing a single argument to submodule foreach, so that the reason for
> the extra quoting is clear:
>   git submodule foreach "git grep \'"
> will not break.  If someone is actually doing
>   git submodule foreach git grep "\'"
> then this will change in behavior.  I think this change is important.
>
> (One could even imagine someone feeding untrusted input to
>   git submodule foreach git grep "$variable"
> which, without my patch, results in a nonobvious shell code injection
> vulnerability.)
>
> I considered an alternative fix where the first argument is always
> shell-evaulated and any others are not (i.e. cmd=$1 && shift && eval
> "$cmd \"\$@\""), which is potentially more useful in case the command
> needs to use $path.  But that may be too confusing, and this way has some
> precedent (e.g. perl’s system()).