From: Jeff King <email@example.com> To: Patrick Steinhardt <firstname.lastname@example.org> Cc: "Junio C Hamano" <email@example.com>, "Ævar Arnfjörð Bjarmason" <firstname.lastname@example.org>, email@example.com Subject: Re: [PATCH 2/2] config: allow specifying config entries via envvar pairs Date: Tue, 17 Nov 2020 02:01:08 -0500 [thread overview] Message-ID: <20201117070108.GB41136@coredump.intra.peff.net> (raw) In-Reply-To: <X7NvrmisOs4g036W@ncase> On Tue, Nov 17, 2020 at 07:37:34AM +0100, Patrick Steinhardt wrote: > > then I'd feel comfortable making it a public-facing feature. And for > > most cases it would be pretty pleasant to use (and for the unpleasant > > ones, I'm not sure that a little quoting is any worse than the paired > > environment variables found here). > > I tend to disagree there. As long as you control keys and values > yourself it's not too hard, that's true. But as soon as you start > processing untrusted keys or values, then it's getting a lot harder. > > E.g. suppose you create a fetch mirror for a user, where the source is > protected by a password. We don't want to write the password into the > gitconfig of the mirror repository. Passing it via `-C` will show up in > ps(1). Using GIT_CONFIG_PARAMETERS requires you to quote the value, > which contains arbitrary data, and if you fail to do that correctly you > now have an avenue for arbitrary config injection. > > That scenario is roughly why I came up with the _KEY/_VALUE schema. It > requires no quoting, is trivial to set up (at least for its target > audience, which is mostly scripts or programs) and wouldn't show up in > ps(1). True. Shell-quoting is pretty easy, but it's still a thing that the caller has to do (and get right). Within Git we have nice routines for that, but if you're calling from another program, you probably don't. > > Yeah, scripts can currently assume that: > > > > unset $(git rev-parse --local-env-vars) > > > > will drop any config from the environment. In some cases, having > > rev-parse enumerate the GIT_CONFIG_KEY_* variables that are set would be > > sufficient. But that implies that rev-parse is seeing the same > > environment we're planning to clear. As it is now, a script is free to > > use rev-parse to generate that list, hold on to it, and then use it > > later. > > Good point. Adjusting it would be trivial, though: unset all consecutive > GIT_CONFIG_KEY_$n keys and potentially also GIT_CONFIG_VALUE_$n until we > hit a gap. The parser would stop on the first gap anyway. That doesn't help this case, which currently works: # remember the list so we don't have to invoke rev-parse in # each iteration of the loop vars=$(git rev-parse --local-env-vars) for repo in $repos; do # start with a clean slate unset $vars export GIT_DIR=$repo git do-some-thing # oops, these won't get cleared in the next go-round because # they weren't set when we called rev-parse export GIT_CONFIG_KEY_1=foo.bar export GIT_CONFIG_VALUE_1=whatever git do-another-thing done Now I have no idea if anybody is doing something along those lines, and it's a bit convoluted (I'd probably use a subshell). And obviously nobody is doing this _exact_ thing yet, because the key/value variables don't exist yet. So maybe it would all work out (the caller of this script could have set git vars, but in that case we'd pick them up in the rev-parse call). So I dunno. It's probably unlikely to bite somebody, but it is a departure from the current design. -Peff
next prev parent reply other threads:[~2020-11-17 7:03 UTC|newest] Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-11-13 12:16 [PATCH 0/2] " Patrick Steinhardt 2020-11-13 12:16 ` [PATCH 1/2] config: extract function to parse config pairs Patrick Steinhardt 2020-11-13 12:16 ` [PATCH 2/2] config: allow specifying config entries via envvar pairs Patrick Steinhardt 2020-11-13 13:04 ` Ævar Arnfjörð Bjarmason 2020-11-16 19:39 ` Junio C Hamano 2020-11-17 2:34 ` Jeff King 2020-11-17 6:37 ` Patrick Steinhardt 2020-11-17 7:01 ` Jeff King [this message] 2020-11-17 14:22 ` Ævar Arnfjörð Bjarmason 2020-11-17 23:57 ` Jeff King 2020-11-18 13:44 ` Ævar Arnfjörð Bjarmason 2020-11-18 0:50 ` brian m. carlson 2020-11-18 1:59 ` Jeff King 2020-11-18 2:25 ` brian m. carlson 2020-11-18 7:04 ` Patrick Steinhardt 2020-11-19 2:11 ` brian m. carlson 2020-11-19 6:37 ` Patrick Steinhardt 2020-11-18 5:44 ` Junio C Hamano 2020-11-17 6:28 ` Patrick Steinhardt 2020-11-17 7:06 ` Junio C Hamano 2020-11-18 13:49 ` Ævar Arnfjörð Bjarmason 2020-11-18 13:56 ` Patrick Steinhardt 2020-11-18 16:01 ` Junio C Hamano 2020-11-17 14:03 ` Ævar Arnfjörð Bjarmason 2020-11-13 16:37 ` Philip Oakley 2020-11-17 6:40 ` Patrick Steinhardt 2020-11-13 13:11 ` [PATCH 0/2] " Ævar Arnfjörð Bjarmason
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 List information: http://vger.kernel.org/majordomo-info.html * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20201117070108.GB41136@coredump.intra.peff.net \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --subject='Re: [PATCH 2/2] config: allow specifying config entries via envvar pairs' \ /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
Code repositories for project(s) associated with this inbox: https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).