Re: [PATCH v4 0/6] config: allow specifying config entries via env

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

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/
2. https://lore.kernel.org/git/20201117023454.GA34754@coredump.intra.peff.net/
3. https://lore.kernel.org/git/20201118015907.GD650959@coredump.intra.peff.net/
4. https://lore.kernel.org/git/87k0upflk4.fsf@evledraar.gmail.com/