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@xxxxxxxxxxxxxxxxxxx/ 2. https://lore.kernel.org/git/20201117023454.GA34754@xxxxxxxxxxxxxxxxxxxxxxx/ 3. https://lore.kernel.org/git/20201118015907.GD650959@xxxxxxxxxxxxxxxxxxxxxxx/ 4. https://lore.kernel.org/git/87k0upflk4.fsf@xxxxxxxxxxxxxxxxxxx/
