Re: Can dependency on /bin/sh be removed?

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 15, 2024, 20:18 UTC
Message-ID: <xmqq8qy21k9f.fsf@gitster.g>
In-Reply-To: <CADaTQqDZ_6wORXOFc2CE90aizgHJ116NDHZhNeY4Nx7NH8DHJw@mail.gmail.com>

Scott Moser <scott.moser@chainguard.dev> writes:

Show 6 quoted lines

>   [credential-helper]
>   helper = /bin/myhelper
>
> In that case, the shell is only being used to tokenize 'myhelper get'.
>
> Is there a solution that I'm missing here?

Isn't the above example faulty?  Shouldn't it be more like

	[credential]
		helper = /bin/myhelper

> Would upstream be open to some modifier on the helper value that would
> indicate "do not pass to shell" ? Like a '@' to indicate "direct
> invoke" rather than letting shell handle?

Absolutely not.

My first gut reaction regarding what needs to happen was that somebody needs to teach credential.c:run_credential_helper() the same trick as run-command.c::prepare_shell_cmd().

In the latter codepath, run_command() calls start_command() which in turn calls prepare_cmd(). The prepare_cmd() then dispatches between prepare_shell_cmd() and strvec_pushv(), but even when .use_shell is set and prepare_shell_cmd() is called, prepare_shell_cmd() knows that it can bypass the shell altogether if the command is simple enough (and /bin/myhelper is indeed simple enough). And when that simplification is taken, shell is not involved at all to run that simple command.

Even though the code path starting from start_command() is what run_credential_helper() does use, what is run is NOT a simple command "/bin/myhelper". It will receive arguments, like

	/bin/myhelper erase
	/bin/myhelper get
	/bin/myhelper store

etc., because the caller appends these operation verbs to the value of the configuration variable. And as you found out, to tokenize them into two, we need shell.

We may be able to teach credential.c:credential_do() not to paste the operation verb to the command line so early. Instead you could teach the function to send the command line and operation verb separately down to run_credential_helper() though. That way, we might be able to avoid the shell in this particular case. That is, if we can

 * Have start_command() -> prepare_cmd() -> prepare_shell_cmd()
   codepath to take the usual route _without_ the operation verb
   tucked to the command line, we would get cmd->args.v[] that does
   not rely on the shell;

 * Then before the prepared command is executed, if we can somehow
   _append_ to cmd->args.v[] the operation verb (after all, that
   wants to become the argv[1] to the spawned command) before
   start_command() exec's it

then we are done.

Having said that, I do not think you can avoid /bin/sh if your goal is "minimal image *to run git*", as there are many things we run, starting from the editor and the pager and end-user hooks. The credential helper is probably the least of your problems. What's a minimum /bin/dash image cost these days?