Re: [PATCH] alias: detect loops in mixed execution mode

> On Thu, Oct 18, 2018 at 10:57:39PM +0000, Ævar Arnfjörð Bjarmason wrote:
>
>> Add detection for aliasing loops in cases where one of the aliases
>> re-invokes git as a shell command. This catches cases like:
>>
>>     [alias]
>>     foo = !git bar
>>     bar = !git foo
>>
>> Before this change running "git {foo,bar}" would create a
>> forkbomb. Now using the aliasing loop detection and call history
>> reporting added in 82f71d9a5a ("alias: show the call history when an
>> alias is looping", 2018-09-16) and c6d75bc17a ("alias: add support for
>> aliases of an alias", 2018-09-16) we'll instead report:
>>
>>     fatal: alias loop detected: expansion of 'foo' does not terminate:
>>       foo <==
>>       bar ==>
>
> The regular alias expansion can generally assume that there's no
> conditional recursion going on, because it's expanding everything
> itself. But when we involve multiple processes, things get trickier.
>
> For instance, I could do this:
>
>   [alias]
>   countdown = "!f() { echo \"$@\"; test \"$1\" -gt 0 && git countdown $(($1-1)); }; f"
>
> which works now, but not with your patch.
>
> Now obviously that's a silly toy example, but are there real cases which
> might trigger this? Some plausible ones I can think of:
>
>   - an alias which handles some special cases, then chains to itself for
>     the simpler one (or to another alias or script, which ends up
>     chaining back to the original)
>
>   - an alias that runs a git command, which then spawns a hook or other
>     user-controlled script, which incidentally uses that same alias
>
> I'd guess this sort of thing is pretty rare. But I wonder if we're
> crossing the line of trying to assume too much about what the user's
> arbitrary code does.
>
> A simple depth counter can limit the fork bomb, and with a high enough
> depth would be unlikely to trigger a false positive. It could also
> protect non-aliases more reasonably, too (e.g., if you have a 1000-deep
> git process hierarchy, there's a good chance you've found an infinite
> loop in git itself).

>> +static void init_cmd_history(struct strbuf *env, struct string_list *cmd_list)
>> +{
>> +	const char *old = getenv(COMMAND_HISTORY_ENVIRONMENT);
>> +	struct strbuf **cmd_history, **ptr;
>> +
>> +	if (!old || !*old)
>> +		return;
>> +
>> +	strbuf_addstr(env, old);
>> +	strbuf_rtrim(env);
>> +
>> +	cmd_history = strbuf_split_buf(old, strlen(old), ' ', 0);
>> +	for (ptr = cmd_history; *ptr; ptr++) {
>> +		strbuf_rtrim(*ptr);
>> +		string_list_append(cmd_list, (*ptr)->buf);
>> +	}
>> +	strbuf_list_free(cmd_history);
>
> Maybe string_list_split() would be a little simpler?