From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Scott Moser <scott.moser@chainguard.dev>, git@vger.kernel.org
Subject: Re: Can dependency on /bin/sh be removed?
Date: Tue, 16 Jul 2024 13:02:54 -0700 [thread overview]
Message-ID: <xmqqo76x6r69.fsf@gitster.g> (raw)
In-Reply-To: <20240716192307.GA12536@coredump.intra.peff.net> (Jeff King's message of "Tue, 16 Jul 2024 15:23:07 -0400")
Jeff King <peff@peff.net> writes:
> [credential]
> helper = cache --socket=/path/to/socket --timeout=123
>
> Arguably we could have gotten away with word-splitting ourselves,
> sticking the result in child_process.args, and avoided the shell. But
> the use of the shell is documented in gitcredentials(7):
>
> helper
> The name of an external credential helper, and any associated
> options. If the helper name is not an absolute path, then the string
> git credential- is prepended. The resulting string is executed by
> the shell (so, for example, setting this to foo --option=bar will
> execute git credential-foo --option=bar via the shell. See the
> manual of specific helpers for examples of their use.
>
> So users may be depending on that to do "--socket=$HOME/.foo", or even
> more exotic shell constructs.
>
> Again, it's possible that we could detect that no shell metacharacters
> are in play and do the word-splitting ourselves. But at that point I
> think it should go into run-command's prepare_shell_cmd(). That is, I I
> think it could take space out of the list of metachars that force us to
> invoke the shell, and do the word-splitting there. But not having
> thought very hard about it, there are probably corner cases where that
> optimization is detectable by the user (presumably unusual IFS, but
> maybe more?).
Well, I strongly object to an approach for us to "parse" anything.
But even then it would be sensible to formulate:
argv[0] = sh
argv[1] = -c
argv[2] = git-credential-cache --socket=/path/ --timeout=123 "$@"
argv[3] = -
argv[4] = NULL
and if there is an argument say "get", extend it to
argv[0] = sh
argv[1] = -c
argv[2] = git-credential-cache --socket=/path/ --timeout=123 "$@"
argv[3] = -
argv[4] = get
argv[5] = NULL
before passing the array to execv(), no?
And with the metacharacter optimization to drop .use_shell we
already have, a single-token /bin/myhelper case would then become
argv[0] = /bin/myhelper
argv[1] = get
argv[2] = NULL
naturally.
next prev parent reply other threads:[~2024-07-16 20:03 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-15 18:41 Can dependency on /bin/sh be removed? Scott Moser
2024-07-15 20:18 ` Junio C Hamano
2024-07-15 21:46 ` brian m. carlson
2024-07-15 23:52 ` Jeff King
2024-07-16 15:23 ` Scott Moser
2024-07-16 16:32 ` Junio C Hamano
2024-07-16 19:23 ` Jeff King
2024-07-16 20:02 ` Junio C Hamano [this message]
2024-07-17 5:52 ` Jeff King
2024-07-16 21:30 ` Andreas Schwab
2024-07-16 21:40 ` Paul Smith
2024-07-17 5:53 ` Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqo76x6r69.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=peff@peff.net \
--cc=scott.moser@chainguard.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.