On Wed, Dec 09 2020, Patrick Steinhardt wrote:

this is the fourth version of my patch series which aims to implement a
way to pass config entries via the environment while avoiding any
requirements to perform shell quoting on the user's side.

Given that the What's Cooking report notes that my third version is
about to be dropped dropped because the `--config-env` way of doing
things is preferred, I've now adopted that approach. I've taken the
patch which Peff posted originally (with one change strchr->strrchr) and
added documentation and tests to it.

This patch series still includes my old proposal as it would actually be
a better fit for our usecase at GitLab I have in mind, which is to put
all configuration which applies to all git commands into the commands
instead of using a config file for this. I have structured the series in
such a way though that those patches come last -- so if you continue to
think this approach shouldn't make it in, please feel free to drop
patches 3-6.

To add even more to your headaches (sorry!) I hadn't really fully looked
at that --config-env proposal.

As noted in my per-patch reply in [1] it will still expose the key part
of the key=value, and in at least one place (url.<base>.insteadOf) the
key is where we'll pass the user/password on the command-line still with
that.

I'd much prefer either your 6/6 over --config-env for that reason & that
--config-env makes it impossible to pass a key with "=" in. For "-c" I
don't think that's much of an issue, but e.g. with
"url.<base>.insteadOf" needing to take arbitrary passwords + us
implicitly/explicitly advertising this as a "here's how you can pass the
password" feature not being able to have "=" is more painful.

I mildly prefer Jeff's suggestion of just getting GIT_CONFIG_PARAMETERS
to the point where we could document it [2][3] to both of those, but
that's mostly an asthetic concern of dealing with N values. It won't
matter for the security aspect (but I think you (but haven't tested)
that you still can't pass a "=", but your 6/6 does allow that).

I still can't quite shake the bad spidey-sense feeling that any of these
are bad in some way we haven't thought of, just from the perspective
that no other tool I can think of that accepts a password has this
mechanism for passing in a user/password or other sensitive data.

E.g. openssh explicitly has refused to add anything of the sort (a
--password parameter, but maybe they didn't consider
--password=ENV_VAR). E.g. curl has a mode where you can have a password
on the command-line, but they then make you use -netrc-file to grab it
from a file. From searching around I see concerns about shell histories
being part of the security model, maybe that's why it's not a common
pattern.

So I still wonder if some version of what I tried with /dev/fd/321 in
[4] would be best, i.e. something that combines transitory+no extra
command invocation+not adding things to shell history. We support that
pattern in general, just not in fetch.c/remote.c for no particular good
reason AFAICT.

I do that that whatever we go for this series would be much better if
the commit messages / added docs explained why we're doing particular
things, and to users why they'd use one method but not the other.

E.g. IIRC this whole series is because it's a hassle to invoke
core.askpass in some stateful program where you'd like to just provide a
transitory password. I think some brief cross-linking or explanation
somewhere of these various ways to pass sensitive values around would be
relly helpful.

1. https://lore.kernel.org/git/871rfzxctq.fsf@evledraar.gmail.com/ (local)
2. https://lore.kernel.org/git/20201117023454.GA34754@coredump.intra.peff.net/ (local)
3. https://lore.kernel.org/git/20201118015907.GD650959@coredump.intra.peff.net/ (local)
4. https://lore.kernel.org/git/87k0upflk4.fsf@evledraar.gmail.com/ (local)