{"thread":{"id":"54638","subject":"[PATCH 0/2] config: allow specifying config entries via envvar pairs","startedAt":"2020-11-13T12:16:22Z","lastAt":"2020-11-19T06:37:44Z","messageCount":27,"participants":["Patrick Steinhardt","Ævar Arnfjörð Bjarmason","Philip Oakley","Junio C Hamano","Jeff King","brian m. carlson"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"409883","messageId":"cover.1605269465.git.ps@pks.im","threadId":"54638","inReplyTo":null,"subject":"[PATCH 0/2] config: allow specifying config entries via envvar pairs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2020-11-13T12:16:13Z","receivedAt":"2020-11-13T12:16:22Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis patch series adds a way to specify config entries via separate\nenvvars `GIT_CONFIG_KEY_$n` and `GIT_CONFIG_VALUE_$n`. There's two main\nmotivations:\n\n    1. `GIT_CONFIG_PARAMETERS` is undocumented and requires parsing of\n       the key-value pairs. This requires the user to properly escape\n       all potentially harmful characters, which may be hard if the\n       value is controlled by a third party.\n\n    2. `git -c key=val` is not really suited to contain sensitive\n       information, as command line arguments trivially show up in e.g.\n       ps(1).\n\nThis new way of passing envvars tries to fix both of those shortcomings.\n\nPatrick\n\nPatrick Steinhardt (2):\n  config: extract function to parse config pairs\n  config: allow specifying config entries via envvar pairs\n\n Documentation/git-config.txt |  6 ++++\n config.c                     | 65 ++++++++++++++++++++++++++----------\n t/t1300-config.sh            | 23 +++++++++++++\n 3 files changed, 76 insertions(+), 18 deletions(-)\n\n-- \n2.29.2\n\n"},{"id":"409884","messageId":"6ced4b2ddd21828a21e935942e2c3da4deb34705.1605269465.git.ps@pks.im","threadId":"54638","inReplyTo":"cover.1605269465.git.ps@pks.im","subject":"[PATCH 1/2] config: extract function to parse config pairs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2020-11-13T12:16:18Z","receivedAt":"2020-11-13T12:16:23Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The function `git_config_parse_parameter` is responsible for parsing a\n`foo.bar=baz`-formatted configuration key, sanitizing the key and then\nprocessing it via the given callback function. Given that we're about to\nadd a second user which is going to process keys in such which already\nhas keys and values separated, this commit extracts a function\n`config_parse_pair` which only does the sanitization and processing\npart.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n config.c | 24 +++++++++++++++++-------\n 1 file changed, 17 insertions(+), 7 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex 2bdff4457b..3281b1374e 100644\n--- a/config.c\n+++ b/config.c\n@@ -437,11 +437,26 @@ int git_config_key_is_valid(const char *key)\n \treturn !git_config_parse_key_1(key, NULL, NULL, 1);\n }\n \n+static int config_parse_pair(const char *key, const char *value,\n+\t\t\t  config_fn_t fn, void *data)\n+{\n+\tchar *canonical_name;\n+\tint ret;\n+\n+\tif (!strlen(key))\n+\t\treturn error(_(\"empty config key\"));\n+\tif (git_config_parse_key(key, &canonical_name, NULL))\n+\t\treturn -1;\n+\n+\tret = (fn(canonical_name, value, data) < 0) ? -1 : 0;\n+\tfree(canonical_name);\n+\treturn ret;\n+}\n+\n int git_config_parse_parameter(const char *text,\n \t\t\t       config_fn_t fn, void *data)\n {\n \tconst char *value;\n-\tchar *canonical_name;\n \tstruct strbuf **pair;\n \tint ret;\n \n@@ -462,12 +477,7 @@ int git_config_parse_parameter(const char *text,\n \t\treturn error(_(\"bogus config parameter: %s\"), text);\n \t}\n \n-\tif (git_config_parse_key(pair[0]->buf, &canonical_name, NULL)) {\n-\t\tret = -1;\n-\t} else {\n-\t\tret = (fn(canonical_name, value, data) < 0) ? -1 : 0;\n-\t\tfree(canonical_name);\n-\t}\n+\tret = config_parse_pair(pair[0]->buf, value, fn, data);\n \tstrbuf_list_free(pair);\n \treturn ret;\n }\n-- \n2.29.2\n\n"},{"id":"409885","messageId":"44e8dd50c6ea7cbcc5e4fc35c9b9057c0a52038c.1605269465.git.ps@pks.im","threadId":"54638","inReplyTo":"cover.1605269465.git.ps@pks.im","subject":"[PATCH 2/2] config: allow specifying config entries via envvar pairs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2020-11-13T12:16:22Z","receivedAt":"2020-11-13T12:16:39Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"While not document, it is currently possible to specify config entries\nvia the environment by passing `GIT_CONFIG_PARAMETERS`. This variable is\nexpected to hold one or multiple \"section.key=value\" entries separated\nby space.\n\nNext to being undocumented, this way of passing config entries has a\nmajor downside: the config keys need to be parsed. As such, it is left\nto the user to escape any potentially harmful characters in the value,\nwhich is quite hard to do if values are controlled by a third party.\n\nThis commit thus adds a new way of adding config entries via the\nenvironment which doesn't require splitting of keys and values. The user\ncan specify an config entry's key via `GIT_CONFIG_KEY_$n` and a value\nvia `GIT_CONFIG_VALUE_$n`, where `n` is any number starting with 1. It\nis possible to add multiple entries via consecutively numbered envvars\n`GIT_CONFIG_KEY_1`, `GIT_CONFIG_KEY_2`, etc, where each of the keys may\nhave a matching value. When no matching value exists, it's assumed to be\nthe empty value.\n\nWhile the same can be achieved with `git -c <name>=<value>`, one may\nwish to not do so for potentially sensitive information. E.g. if one\nwants to set `http.extraHeader` to contain an authentication token,\ndoing so via `-c` would trivially leak those credentials via e.g. ps(1),\nwhich typically also shows command arguments.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Documentation/git-config.txt |  6 ++++++\n config.c                     | 41 ++++++++++++++++++++++++++----------\n t/t1300-config.sh            | 23 ++++++++++++++++++++\n 3 files changed, 59 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\nindex 7573160f21..83fbac3705 100644\n--- a/Documentation/git-config.txt\n+++ b/Documentation/git-config.txt\n@@ -335,6 +335,12 @@ GIT_CONFIG_NOSYSTEM::\n \tWhether to skip reading settings from the system-wide\n \t$(prefix)/etc/gitconfig file. See linkgit:git[1] for details.\n \n+GIT_CONFIG_KEY_1,GIT_CONFIG_VALUE_1::\n+\tEach pair of GIT_CONFIG_KEY_/GIT_CONFIG_VALUE_ is added to the process's\n+\truntime configuration. It is possible to add multiple entries by adding\n+\tconsecutively numbered pairs, starting at 1. If the value corresponding\n+\tto a key is not set, it is treated as if it was empty.\n+\n See also <<FILES>>.\n \n \ndiff --git a/config.c b/config.c\nindex 3281b1374e..ab40479df2 100644\n--- a/config.c\n+++ b/config.c\n@@ -485,37 +485,56 @@ int git_config_parse_parameter(const char *text,\n int git_config_from_parameters(config_fn_t fn, void *data)\n {\n \tconst char *env = getenv(CONFIG_DATA_ENVIRONMENT);\n+\tstruct strbuf envvar = STRBUF_INIT;\n \tint ret = 0;\n-\tchar *envw;\n+\tchar *envw = NULL;\n \tconst char **argv = NULL;\n \tint nr = 0, alloc = 0;\n \tint i;\n \tstruct config_source source;\n \n-\tif (!env)\n-\t\treturn 0;\n-\n \tmemset(&source, 0, sizeof(source));\n \tsource.prev = cf;\n \tsource.origin_type = CONFIG_ORIGIN_CMDLINE;\n \tcf = &source;\n \n-\t/* sq_dequote will write over it */\n-\tenvw = xstrdup(env);\n+\tif (env) {\n+\t\t/* sq_dequote will write over it */\n+\t\tenvw = xstrdup(env);\n \n-\tif (sq_dequote_to_argv(envw, &argv, &nr, &alloc) < 0) {\n-\t\tret = error(_(\"bogus format in %s\"), CONFIG_DATA_ENVIRONMENT);\n-\t\tgoto out;\n+\t\tif (sq_dequote_to_argv(envw, &argv, &nr, &alloc) < 0) {\n+\t\t\tret = error(_(\"bogus format in %s\"), CONFIG_DATA_ENVIRONMENT);\n+\t\t\tgoto out;\n+\t\t}\n+\n+\t\tfor (i = 0; i < nr; i++) {\n+\t\t\tif (git_config_parse_parameter(argv[i], fn, data) < 0) {\n+\t\t\t\tret = -1;\n+\t\t\t\tgoto out;\n+\t\t\t}\n+\t\t}\n \t}\n \n-\tfor (i = 0; i < nr; i++) {\n-\t\tif (git_config_parse_parameter(argv[i], fn, data) < 0) {\n+\tfor (i = 1; i; i++) {\n+\t\tconst char *key, *value;\n+\n+\t\tstrbuf_addf(&envvar, \"GIT_CONFIG_KEY_%d\", i);\n+\t\tif ((key = getenv(envvar.buf)) == NULL)\n+\t\t\tbreak;\n+\t\tstrbuf_reset(&envvar);\n+\n+\t\tstrbuf_addf(&envvar, \"GIT_CONFIG_VALUE_%d\", i);\n+\t\tvalue = getenv(envvar.buf);\n+\t\tstrbuf_reset(&envvar);\n+\n+\t\tif (config_parse_pair(key, value, fn, data) < 0) {\n \t\t\tret = -1;\n \t\t\tgoto out;\n \t\t}\n \t}\n \n out:\n+\tstrbuf_release(&envvar);\n \tfree(argv);\n \tfree(envw);\n \tcf = source.prev;\ndiff --git a/t/t1300-config.sh b/t/t1300-config.sh\nindex 825d9a184f..2ae9533aa8 100755\n--- a/t/t1300-config.sh\n+++ b/t/t1300-config.sh\n@@ -1316,6 +1316,29 @@ test_expect_success 'detect bogus GIT_CONFIG_PARAMETERS' '\n \t\tgit config --get-regexp \"env.*\"\n '\n \n+test_expect_success 'git config handles environment config pairs' '\n+\tGIT_CONFIG_KEY_1=\"pair.one\" GIT_CONFIG_VALUE_1=\"foo\" \\\n+\t\tGIT_CONFIG_KEY_2=\"pair.two\" GIT_CONFIG_VALUE_2=\"bar\" \\\n+\t\tGIT_CONFIG_KEY_4=\"pair.four\" GIT_CONFIG_VALUE_4=\"not-parsed\" \\\n+\t\tgit config --get-regexp \"pair.*\" >actual &&\n+\tcat >expect <<-EOF &&\n+\tpair.one foo\n+\tpair.two bar\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git config copes with missing config pair value' '\n+\tGIT_CONFIG_KEY_1=\"pair.one\" git config --get-regexp \"pair.*\" >actual &&\n+\techo pair.one >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git config fails with invalid config pair key' '\n+\ttest_must_fail env GIT_CONFIG_KEY_1= git config --list &&\n+\ttest_must_fail env GIT_CONFIG_KEY_1=missing-section git config --list\n+'\n+\n test_expect_success 'git config --edit works' '\n \tgit config -f tmp test.value no &&\n \techo test.value=yes >expect &&\n-- \n2.29.2\n\n"},{"id":"409887","messageId":"87mtzlflw7.fsf@evledraar.gmail.com","threadId":"54638","inReplyTo":"44e8dd50c6ea7cbcc5e4fc35c9b9057c0a52038c.1605269465.git.ps@pks.im","subject":"Re: [PATCH 2/2] config: allow specifying config entries via envvar pairs","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2020-11-13T13:04:40Z","receivedAt":"2020-11-13T13:04:45Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Nov 13 2020, Patrick Steinhardt wrote:\n\n> While not document, it is currently possible to specify config entries\n\n\"While not documented...\"\n\n> +\t\tstrbuf_addf(&envvar, \"GIT_CONFIG_KEY_%d\", i);\n> +\t\tif ((key = getenv(envvar.buf)) == NULL)\n> +\t\t\tbreak;\n\nThe convention in git.git is to avoid explicit NULL checks. So maybe\nsomething like this, which also avoids the assignment inside an \"if\"\n\n    key = getenv(envvar.buf);\n    if (!key)\n        break;\n\n> +test_expect_success 'git config handles environment config pairs' '\n> +\tGIT_CONFIG_KEY_1=\"pair.one\" GIT_CONFIG_VALUE_1=\"foo\" \\\n> +\t\tGIT_CONFIG_KEY_2=\"pair.two\" GIT_CONFIG_VALUE_2=\"bar\" \\\n> +\t\tGIT_CONFIG_KEY_4=\"pair.four\" GIT_CONFIG_VALUE_4=\"not-parsed\" \\\n> +\t\tgit config --get-regexp \"pair.*\" >actual &&\n> +\tcat >expect <<-EOF &&\n> +\tpair.one foo\n> +\tpair.two bar\n> +\tEOF\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'git config copes with missing config pair value' '\n> +\tGIT_CONFIG_KEY_1=\"pair.one\" git config --get-regexp \"pair.*\" >actual &&\n> +\techo pair.one >expect &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'git config fails with invalid config pair key' '\n> +\ttest_must_fail env GIT_CONFIG_KEY_1= git config --list &&\n> +\ttest_must_fail env GIT_CONFIG_KEY_1=missing-section git config --list\n> +'\n> +\n>  test_expect_success 'git config --edit works' '\n>  \tgit config -f tmp test.value no &&\n>  \techo test.value=yes >expect &&\n\nI think we should have a bit more complete tests of what happens if you\nclobber existing config keys, and testing that this is set last after\nsystem/global/local config.\n"},{"id":"409888","messageId":"87k0upflk4.fsf@evledraar.gmail.com","threadId":"54638","inReplyTo":"cover.1605269465.git.ps@pks.im","subject":"Re: [PATCH 0/2] config: allow specifying config entries via envvar pairs","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2020-11-13T13:11:55Z","receivedAt":"2020-11-13T13:12:12Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Nov 13 2020, Patrick Steinhardt wrote:\n\n> this patch series adds a way to specify config entries via separate\n> envvars `GIT_CONFIG_KEY_$n` and `GIT_CONFIG_VALUE_$n`. There's two main\n> motivations:\n>\n>     1. `GIT_CONFIG_PARAMETERS` is undocumented and requires parsing of\n>        the key-value pairs. This requires the user to properly escape\n>        all potentially harmful characters, which may be hard if the\n>        value is controlled by a third party.\n>\n>     2. `git -c key=val` is not really suited to contain sensitive\n>        information, as command line arguments trivially show up in e.g.\n>        ps(1).\n\nFWIW we had an off-list discussion about this where the desire was to\nhave the equivalent of a transitory password in a config file without\nthe bad pattern of putting it in an on-disk config file. The advertised\nsolution we have now is core.askpass, but a user might for some reason\nnot want the hassle of an external program.\n\nI noted that you can do that with some clever hacks that aren't\nexplicitly documented:\n\n1) Use the insteadOf config to on-the-fly rewrite a password-less https\n   URL to have a user/password:\n\n    git -c url.https://user:password@.insteadOf=https:// push\n\n   But that has the downside of showing the password in \"ps\" as Patrick\n   notes. That's OS dependant, but is the default on e.g. Linux, as\n   opposed to envars. See \"hidepid\" in the \"procfs\" manpage.\n\n2) Doing the same via an env var, but via GIT_CONFIG_PARAMETERS:\n\n    GIT_CONFIG_PARAMETERS=\"'url.https://user:password@.insteadOf=https://'\" git push\n\n3) This doesn't work, but I wish it did. First put:\n\n    [include]\n    path = /dev/fd/321\n\n   In your .git/config. Then:\n\n    (echo \"[url \\\"https://user:password\\\"]\" && echo \"insteadOf = https://\") | { git remote get-url origin; } 321<&0\n\n   The reason it doesn't work is because the \"git remote\" config\n   machinery, unlike the general machinery, explicitly doesn't handle\n   includes. I didn't poke at that for long, but I expect that's just an\n   omission. It wants to not read remote.origin.url from ~/.gitconfig or\n   whatever, but I don't see why we wouldn't follow includes in\n   .git/config.\n"},{"id":"409912","messageId":"32a5617e-49b8-c6e9-4b1e-09e77bcae8fa@iee.email","threadId":"54638","inReplyTo":"44e8dd50c6ea7cbcc5e4fc35c9b9057c0a52038c.1605269465.git.ps@pks.im","subject":"Re: [PATCH 2/2] config: allow specifying config entries via envvar pairs","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2020-11-13T16:37:33Z","receivedAt":"2020-11-13T16:37:36Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 13/11/2020 12:16, Patrick Steinhardt wrote:\n> This commit thus adds a new way of adding config entries via the\n> environment which doesn't require splitting of keys and values. The user\n> can specify an config entry's key via `GIT_CONFIG_KEY_$n` and a value\n> via `GIT_CONFIG_VALUE_$n`, where `n` is any number starting with 1. It\n> is possible to add multiple entries via consecutively numbered envvars\n> `GIT_CONFIG_KEY_1`, `GIT_CONFIG_KEY_2`, etc, where each of the keys may\n> have a matching value. \n\n> When no matching value exists, it's assumed to be\n> the empty value.\nIs this a good choice of default in the face of potential mistyping when\nentering commands, or cut&paste editing of scripts. It's easy to see\ncases of mismatched KEY_2 VALUE_1 entries.\n\nWouldn't it be better to warn about un-matched key/value pairs?\n\n\n> +GIT_CONFIG_KEY_1,GIT_CONFIG_VALUE_1::\n\nShouldn't the man page entry indicate that it's '<n>'  ?\n\nPhilip\n"},{"id":"410037","messageId":"xmqqy2j1851k.fsf@gitster.c.googlers.com","threadId":"54638","inReplyTo":"87mtzlflw7.fsf@evledraar.gmail.com","subject":"Re: [PATCH 2/2] config: allow specifying config entries via envvar pairs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-16T19:39:35Z","receivedAt":"2020-11-16T19:39:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> On Fri, Nov 13 2020, Patrick Steinhardt wrote:\n>\n>> While not document, it is currently possible to specify config entries\n>\n> \"While not documented...\"\n>\n>> +\t\tstrbuf_addf(&envvar, \"GIT_CONFIG_KEY_%d\", i);\n>> +\t\tif ((key = getenv(envvar.buf)) == NULL)\n>> +\t\t\tbreak;\n>\n> The convention in git.git is to avoid explicit NULL checks. So maybe\n> something like this, which also avoids the assignment inside an \"if\"\n>\n>     key = getenv(envvar.buf);\n>     if (!key)\n>         break;\n\nAll good suggestions, but...\n\n\"While not documented\" yes, for sure, but we do not document it for\na good reason---it is a pure implementation detail between Git\nprocess that runs another one as its internal implementation detail.\n\nI especially do not think we want to read from unbounded number of\nGIT_CONFIG_KEY_<N> variables like this patch does.  How would a\nscript cleanse its environment to protect itself from stray such\nenvironment variable pair?  Count up from 1 to forever?  Run \"env\"\nand grep for \"GIT_CONFIG_KEY_[0-9]*=\" (the answer is No.  What if\nsome environment variables have newline in its values?)\n\n"},{"id":"410064","messageId":"20201117023454.GA34754@coredump.intra.peff.net","threadId":"54638","inReplyTo":"xmqqy2j1851k.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 2/2] config: allow specifying config entries via envvar pairs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-11-17T02:34:54Z","receivedAt":"2020-11-17T02:35:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 16, 2020 at 11:39:35AM -0800, Junio C Hamano wrote:\n\n> >> While not document, it is currently possible to specify config entries\n> >> [in GIT_CONFIG_PARAMETERS]\n> [...]\n>\n> \"While not documented\" yes, for sure, but we do not document it for\n> a good reason---it is a pure implementation detail between Git\n> process that runs another one as its internal implementation detail.\n\nI have actually been quite tempted to document and promise that it will\ncontinue to work. Because it really is useful in some instances. The\nthing that has held me back is that the documentation would reveal how\nunforgiving the parser is. ;)\n\nIt insists that key/value pairs are shell-quoted as a whole. I think if\nwe made it accept a some reasonable inputs:\n\n  - do not require quoting for values that do not need it\n\n  - allow any amount of shell-style single-quoting (whole parameters,\n    just values, etc).\n\n  - do not bother allowing other quoting, like double-quotes with\n    backslashes. However, document backslash and double-quote as\n    meta-characters that must not appear outside of single-quotes, to\n    allow for future expansion.\n\nthen I'd feel comfortable making it a public-facing feature. And for\nmost cases it would be pretty pleasant to use (and for the unpleasant\nones, I'm not sure that a little quoting is any worse than the paired\nenvironment variables found here).\n\n> I especially do not think we want to read from unbounded number of\n> GIT_CONFIG_KEY_<N> variables like this patch does.  How would a\n> script cleanse its environment to protect itself from stray such\n> environment variable pair?  Count up from 1 to forever?  Run \"env\"\n> and grep for \"GIT_CONFIG_KEY_[0-9]*=\" (the answer is No.  What if\n> some environment variables have newline in its values?)\n\nYeah, scripts can currently assume that:\n\n  unset $(git rev-parse --local-env-vars)\n\nwill drop any config from the environment. In some cases, having\nrev-parse enumerate the GIT_CONFIG_KEY_* variables that are set would be\nsufficient. But that implies that rev-parse is seeing the same\nenvironment we're planning to clear. As it is now, a script is free to\nuse rev-parse to generate that list, hold on to it, and then use it\nlater.\n\n-Peff\n"},{"id":"410069","messageId":"X7NtovvfE7IjWzie@ncase","threadId":"54638","inReplyTo":"xmqqy2j1851k.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 2/2] config: allow specifying config entries via envvar pairs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2020-11-17T06:28:50Z","receivedAt":"2020-11-17T06:29:20Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Nov 16, 2020 at 11:39:35AM -0800, Junio C Hamano wrote:\n> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n> \n> > On Fri, Nov 13 2020, Patrick Steinhardt wrote:\n> >\n> >> While not document, it is currently possible to specify config entries\n> >\n> > \"While not documented...\"\n> >\n> >> +\t\tstrbuf_addf(&envvar, \"GIT_CONFIG_KEY_%d\", i);\n> >> +\t\tif ((key = getenv(envvar.buf)) == NULL)\n> >> +\t\t\tbreak;\n> >\n> > The convention in git.git is to avoid explicit NULL checks. So maybe\n> > something like this, which also avoids the assignment inside an \"if\"\n> >\n> >     key = getenv(envvar.buf);\n> >     if (!key)\n> >         break;\n> \n> All good suggestions, but...\n> \n> \"While not documented\" yes, for sure, but we do not document it for\n> a good reason---it is a pure implementation detail between Git\n> process that runs another one as its internal implementation detail.\n\nAgreed\n\n> I especially do not think we want to read from unbounded number of\n> GIT_CONFIG_KEY_<N> variables like this patch does.  How would a\n> script cleanse its environment to protect itself from stray such\n> environment variable pair?  Count up from 1 to forever?  Run \"env\"\n> and grep for \"GIT_CONFIG_KEY_[0-9]*=\" (the answer is No.  What if\n> some environment variables have newline in its values?)\n\nYou only have to unset `GIT_CONFIG_KEY_1` as the parser will stop\niterating on the first missing key. More generally, if you set `n` keys,\nit's sufficient to unset key `n+1`.\n\nPatrick\n"},{"id":"410070","messageId":"X7NvrmisOs4g036W@ncase","threadId":"54638","inReplyTo":"20201117023454.GA34754@coredump.intra.peff.net","subject":"Re: [PATCH 2/2] config: allow specifying config entries via envvar pairs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2020-11-17T06:37:34Z","receivedAt":"2020-11-17T06:38:04Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Nov 16, 2020 at 09:34:54PM -0500, Jeff King wrote:\n> On Mon, Nov 16, 2020 at 11:39:35AM -0800, Junio C Hamano wrote:\n> \n> > >> While not document, it is currently possible to specify config entries\n> > >> [in GIT_CONFIG_PARAMETERS]\n> > [...]\n> >\n> > \"While not documented\" yes, for sure, but we do not document it for\n> > a good reason---it is a pure implementation detail between Git\n> > process that runs another one as its internal implementation detail.\n> \n> I have actually been quite tempted to document and promise that it will\n> continue to work. Because it really is useful in some instances. The\n> thing that has held me back is that the documentation would reveal how\n> unforgiving the parser is. ;)\n> \n> It insists that key/value pairs are shell-quoted as a whole. I think if\n> we made it accept a some reasonable inputs:\n> \n>   - do not require quoting for values that do not need it\n> \n>   - allow any amount of shell-style single-quoting (whole parameters,\n>     just values, etc).\n> \n>   - do not bother allowing other quoting, like double-quotes with\n>     backslashes. However, document backslash and double-quote as\n>     meta-characters that must not appear outside of single-quotes, to\n>     allow for future expansion.\n> \n> then I'd feel comfortable making it a public-facing feature. And for\n> most cases it would be pretty pleasant to use (and for the unpleasant\n> ones, I'm not sure that a little quoting is any worse than the paired\n> environment variables found here).\n\nI tend to disagree there. As long as you control keys and values\nyourself it's not too hard, that's true. But as soon as you start\nprocessing untrusted keys or values, then it's getting a lot harder.\n\nE.g. suppose you create a fetch mirror for a user, where the source is\nprotected by a password. We don't want to write the password into the\ngitconfig of the mirror repository. Passing it via `-C` will show up in\nps(1). Using GIT_CONFIG_PARAMETERS requires you to quote the value,\nwhich contains arbitrary data, and if you fail to do that correctly you\nnow have an avenue for arbitrary config injection.\n\nThat scenario is roughly why I came up with the _KEY/_VALUE schema. It\nrequires no quoting, is trivial to set up (at least for its target\naudience, which is mostly scripts or programs) and wouldn't show up in\nps(1).\n\n> > I especially do not think we want to read from unbounded number of\n> > GIT_CONFIG_KEY_<N> variables like this patch does.  How would a\n> > script cleanse its environment to protect itself from stray such\n> > environment variable pair?  Count up from 1 to forever?  Run \"env\"\n> > and grep for \"GIT_CONFIG_KEY_[0-9]*=\" (the answer is No.  What if\n> > some environment variables have newline in its values?)\n> \n> Yeah, scripts can currently assume that:\n> \n>   unset $(git rev-parse --local-env-vars)\n> \n> will drop any config from the environment. In some cases, having\n> rev-parse enumerate the GIT_CONFIG_KEY_* variables that are set would be\n> sufficient. But that implies that rev-parse is seeing the same\n> environment we're planning to clear. As it is now, a script is free to\n> use rev-parse to generate that list, hold on to it, and then use it\n> later.\n\nGood point. Adjusting it would be trivial, though: unset all consecutive\nGIT_CONFIG_KEY_$n keys and potentially also GIT_CONFIG_VALUE_$n until we\nhit a gap. The parser would stop on the first gap anyway.\n\nPatrick\n\n> -Peff\n"},{"id":"410071","messageId":"X7NwaVNhUFMZzO3W@ncase","threadId":"54638","inReplyTo":"32a5617e-49b8-c6e9-4b1e-09e77bcae8fa@iee.email","subject":"Re: [PATCH 2/2] config: allow specifying config entries via envvar pairs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2020-11-17T06:40:41Z","receivedAt":"2020-11-17T06:40:55Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Nov 13, 2020 at 04:37:33PM +0000, Philip Oakley wrote:\n> On 13/11/2020 12:16, Patrick Steinhardt wrote:\n> > This commit thus adds a new way of adding config entries via the\n> > environment which doesn't require splitting of keys and values. The user\n> > can specify an config entry's key via `GIT_CONFIG_KEY_$n` and a value\n> > via `GIT_CONFIG_VALUE_$n`, where `n` is any number starting with 1. It\n> > is possible to add multiple entries via consecutively numbered envvars\n> > `GIT_CONFIG_KEY_1`, `GIT_CONFIG_KEY_2`, etc, where each of the keys may\n> > have a matching value. \n> \n> > When no matching value exists, it's assumed to be\n> > the empty value.\n> Is this a good choice of default in the face of potential mistyping when\n> entering commands, or cut&paste editing of scripts. It's easy to see\n> cases of mismatched KEY_2 VALUE_1 entries.\n> \n> Wouldn't it be better to warn about un-matched key/value pairs?\n\nGood point. I'll change this on the next iteration.\n\n> > +GIT_CONFIG_KEY_1,GIT_CONFIG_VALUE_1::\n> \n> Shouldn't the man page entry indicate that it's '<n>'  ?\n\nI wasn't quite sure how to document it, but using `<n>` would indicate\nbetter that this can be multiple envvars.\n\nPatrick\n\n> Philip\n"},{"id":"410073","messageId":"20201117070108.GB41136@coredump.intra.peff.net","threadId":"54638","inReplyTo":"X7NvrmisOs4g036W@ncase","subject":"Re: [PATCH 2/2] config: allow specifying config entries via envvar pairs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-11-17T07:01:08Z","receivedAt":"2020-11-17T07:01:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Nov 17, 2020 at 07:37:34AM +0100, Patrick Steinhardt wrote:\n\n> > then I'd feel comfortable making it a public-facing feature. And for\n> > most cases it would be pretty pleasant to use (and for the unpleasant\n> > ones, I'm not sure that a little quoting is any worse than the paired\n> > environment variables found here).\n> \n> I tend to disagree there. As long as you control keys and values\n> yourself it's not too hard, that's true. But as soon as you start\n> processing untrusted keys or values, then it's getting a lot harder.\n> \n> E.g. suppose you create a fetch mirror for a user, where the source is\n> protected by a password. We don't want to write the password into the\n> gitconfig of the mirror repository. Passing it via `-C` will show up in\n> ps(1). Using GIT_CONFIG_PARAMETERS requires you to quote the value,\n> which contains arbitrary data, and if you fail to do that correctly you\n> now have an avenue for arbitrary config injection.\n> \n> That scenario is roughly why I came up with the _KEY/_VALUE schema. It\n> requires no quoting, is trivial to set up (at least for its target\n> audience, which is mostly scripts or programs) and wouldn't show up in\n> ps(1).\n\nTrue. Shell-quoting is pretty easy, but it's still a thing that the\ncaller has to do (and get right). Within Git we have nice routines for\nthat, but if you're calling from another program, you probably don't.\n\n> > Yeah, scripts can currently assume that:\n> > \n> >   unset $(git rev-parse --local-env-vars)\n> > \n> > will drop any config from the environment. In some cases, having\n> > rev-parse enumerate the GIT_CONFIG_KEY_* variables that are set would be\n> > sufficient. But that implies that rev-parse is seeing the same\n> > environment we're planning to clear. As it is now, a script is free to\n> > use rev-parse to generate that list, hold on to it, and then use it\n> > later.\n> \n> Good point. Adjusting it would be trivial, though: unset all consecutive\n> GIT_CONFIG_KEY_$n keys and potentially also GIT_CONFIG_VALUE_$n until we\n> hit a gap. The parser would stop on the first gap anyway.\n\nThat doesn't help this case, which currently works:\n\n  # remember the list so we don't have to invoke rev-parse in\n  # each iteration of the loop\n  vars=$(git rev-parse --local-env-vars)\n\n  for repo in $repos; do\n\t# start with a clean slate\n\tunset $vars\n\n\texport GIT_DIR=$repo\n\tgit do-some-thing\n\n\t# oops, these won't get cleared in the next go-round because\n\t# they weren't set when we called rev-parse\n\texport GIT_CONFIG_KEY_1=foo.bar\n\texport GIT_CONFIG_VALUE_1=whatever\n\tgit do-another-thing\n  done\n\nNow I have no idea if anybody is doing something along those lines, and\nit's a bit convoluted (I'd probably use a subshell). And obviously\nnobody is doing this _exact_ thing yet, because the key/value variables\ndon't exist yet. So maybe it would all work out (the caller of this\nscript could have set git vars, but in that case we'd pick them up in\nthe rev-parse call).\n\nSo I dunno. It's probably unlikely to bite somebody, but it is a\ndeparture from the current design.\n\n-Peff\n"},{"id":"410074","messageId":"xmqq4klo7992.fsf@gitster.c.googlers.com","threadId":"54638","inReplyTo":"X7NtovvfE7IjWzie@ncase","subject":"Re: [PATCH 2/2] config: allow specifying config entries via envvar pairs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-17T07:06:17Z","receivedAt":"2020-11-17T07:06:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n>> I especially do not think we want to read from unbounded number of\n>> GIT_CONFIG_KEY_<N> variables like this patch does.  How would a\n>> script cleanse its environment to protect itself from stray such\n>> environment variable pair?  Count up from 1 to forever?  Run \"env\"\n>> and grep for \"GIT_CONFIG_KEY_[0-9]*=\" (the answer is No.  What if\n>> some environment variables have newline in its values?)\n>\n> You only have to unset `GIT_CONFIG_KEY_1` as the parser will stop\n> iterating on the first missing key. More generally, if you set `n` keys,\n> it's sufficient to unset key `n+1`.\n\nYes, but those who want to set N keys would likely to be content\nwith setting 1..N and happily forget unsetting N+1, and that is\nwhere \"how would one cleanse the environment to give a clean slate?\"\ncomes from.\n"},{"id":"410080","messageId":"875z64f5cb.fsf@evledraar.gmail.com","threadId":"54638","inReplyTo":"xmqqy2j1851k.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 2/2] config: allow specifying config entries via envvar pairs","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2020-11-17T14:03:32Z","receivedAt":"2020-11-17T14:04:21Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Nov 16 2020, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n>\n>> On Fri, Nov 13 2020, Patrick Steinhardt wrote:\n>>\n>>> While not document, it is currently possible to specify config entries\n>>\n>> \"While not documented...\"\n>>\n>>> +\t\tstrbuf_addf(&envvar, \"GIT_CONFIG_KEY_%d\", i);\n>>> +\t\tif ((key = getenv(envvar.buf)) == NULL)\n>>> +\t\t\tbreak;\n>>\n>> The convention in git.git is to avoid explicit NULL checks. So maybe\n>> something like this, which also avoids the assignment inside an \"if\"\n>>\n>>     key = getenv(envvar.buf);\n>>     if (!key)\n>>         break;\n>\n> All good suggestions, but...\n>\n> \"While not documented\" yes, for sure, but we do not document it for\n> a good reason---it is a pure implementation detail between Git\n> process that runs another one as its internal implementation detail.\n\n*nod* I didn't mean it should be treated as some API, just \"it happens\n to work\". I do agree with Jeff downthread that it would be nice to have\n it explicitly supported.\n\n> I especially do not think we want to read from unbounded number of\n> GIT_CONFIG_KEY_<N> variables like this patch does.  How would a\n> script cleanse its environment to protect itself from stray such\n> environment variable pair?  Count up from 1 to forever?  Run \"env\"\n> and grep for \"GIT_CONFIG_KEY_[0-9]*=\" (the answer is No.  What if\n> some environment variables have newline in its values?)\n\nPurely on an implementation note, if we went that route we could provide\nsomething based on compat/unsetenv.c (or environ iteration in general)\nthat would loop over the env, but I agree it would be better to make\nGIT_CONFIG_PARAMETERS nice.\n"},{"id":"410081","messageId":"873618f4he.fsf@evledraar.gmail.com","threadId":"54638","inReplyTo":"20201117023454.GA34754@coredump.intra.peff.net","subject":"Re: [PATCH 2/2] config: allow specifying config entries via envvar pairs","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2020-11-17T14:22:05Z","receivedAt":"2020-11-17T14:22:12Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Nov 17 2020, Jeff King wrote:\n\n> On Mon, Nov 16, 2020 at 11:39:35AM -0800, Junio C Hamano wrote:\n>\n>> >> While not document, it is currently possible to specify config entries\n>> >> [in GIT_CONFIG_PARAMETERS]\n>> [...]\n>>\n>> \"While not documented\" yes, for sure, but we do not document it for\n>> a good reason---it is a pure implementation detail between Git\n>> process that runs another one as its internal implementation detail.\n>\n> I have actually been quite tempted to document and promise that it will\n> continue to work. Because it really is useful in some instances. The\n> thing that has held me back is that the documentation would reveal how\n> unforgiving the parser is. ;)\n>\n> It insists that key/value pairs are shell-quoted as a whole. I think if\n> we made it accept a some reasonable inputs:\n>\n>   - do not require quoting for values that do not need it\n>\n>   - allow any amount of shell-style single-quoting (whole parameters,\n>     just values, etc).\n>\n>   - do not bother allowing other quoting, like double-quotes with\n>     backslashes. However, document backslash and double-quote as\n>     meta-characters that must not appear outside of single-quotes, to\n>     allow for future expansion.\n>\n> then I'd feel comfortable making it a public-facing feature. And for\n> most cases it would be pretty pleasant to use (and for the unpleasant\n> ones, I'm not sure that a little quoting is any worse than the paired\n> environment variables found here).\n\nI wonder if something like the git config -z format wouldn't be easier,\nwith the twist that we'd obviously not support \\0. So we'd need an\noptional length prefix. : = unspecified.\n\n    :user.name\n    Jeff K\n    :alias.ci\n    commit\n    :10:bin.ary\n    <10 byte string, might have a \\n>\n    :other.key\n    Other Value\n\nMaybe that's overly fragile, or maybe another format would be better. I\nwas trying to come up with one where the common case wouldn't require\nknowing about shell quoting/unquoting, and where you could still do:\n\n    GIT_CONFIG_PARAMETERS=\":my.new\\nvalue\\n$GIT_CONFIG_PARAMETERS\"\n\nOr equivalent, and still just keep $GIT_CONFIG_PARAMETERS as-is to pass\nit along.\n\nYour \"do not require quoting\" accomplishes that, and it's arguably a lot\n"},{"id":"410181","messageId":"20201117235736.GC642410@coredump.intra.peff.net","threadId":"54638","inReplyTo":"873618f4he.fsf@evledraar.gmail.com","subject":"Re: [PATCH 2/2] config: allow specifying config entries via envvar pairs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-11-17T23:57:36Z","receivedAt":"2020-11-17T23:58:01Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Nov 17, 2020 at 03:22:05PM +0100, Ævar Arnfjörð Bjarmason wrote:\n\n> > then I'd feel comfortable making it a public-facing feature. And for\n> > most cases it would be pretty pleasant to use (and for the unpleasant\n> > ones, I'm not sure that a little quoting is any worse than the paired\n> > environment variables found here).\n> \n> I wonder if something like the git config -z format wouldn't be easier,\n> with the twist that we'd obviously not support \\0. So we'd need an\n> optional length prefix. : = unspecified.\n> \n>     :user.name\n>     Jeff K\n>     :alias.ci\n>     commit\n>     :10:bin.ary\n>     <10 byte string, might have a \\n>\n>     :other.key\n>     Other Value\n> \n> Maybe that's overly fragile, or maybe another format would be better.\n\nYeah, length-delimited strings are an alternative that some people think\nis less error-prone than quoting. And we do use pkt-lines. They're also\na pain for humans to write (it's nicer if they're optional, but when you\n_do_ have to start using them, now you are stuck counting things up).\n\n> I was trying to come up with one where the common case wouldn't\n> require knowing about shell quoting/unquoting, and where you could\n> still do:\n> \n>     GIT_CONFIG_PARAMETERS=\":my.new\\nvalue\\n$GIT_CONFIG_PARAMETERS\"\n> \n> Or equivalent, and still just keep $GIT_CONFIG_PARAMETERS as-is to pass\n> it along.\n> \n> Your \"do not require quoting\" accomplishes that, and it's arguably a lot\n\nLooks like your mail got cut off. But yeah, the goal of making the\nquoting optional was to make it easier for humans to use for simple\ncases. It doesn't help at all with other programs inserting values,\nwhich can just as easily err on the side of caution.\n\nBTW, there is another problem with GIT_CONFIG_PARAMETERS (and \"git -c\"\nin general). The dotted config-key format:\n\n  section.subsection.key\n\nis unambiguous by itself, even though \"subsection\" can contain arbitrary\nbytes, including dots. Because neither \"section\" nor \"key\" can contain\ndots, we can parse from either end, and take the whole middle as a\nsubsection (and this is how we do it in the code).\n\nBut an assignment string like:\n\n  section.subsection.key=value\n\n_is_ ambiguous. We have to parse left-to-right up to the first equals\n(since \"value\" can contain arbitrary characters, including an equals).\nBut \"subsection\" can have one, too, so we want to parse right-to-left\nthere. E.g., in:\n\n  one.two=three.four=five\n\nthis could be either of:\n\n  - section is \"one\", key is \"two\", value is \"three.four=five\"\n\n  - section is \"one\", subsection is \"two=three\", key is \"four\", value is\n    \"five\"\n\nWe currently always parse it as the former (which I think is least-bad\nof the two, since values are more likely than subsections to contain\narbitrary text with an equals).\n\n-Peff\n"},{"id":"410186","messageId":"20201118005014.GC389879@camp.crustytoothpaste.net","threadId":"54638","inReplyTo":"20201117023454.GA34754@coredump.intra.peff.net","subject":"Re: [PATCH 2/2] config: allow specifying config entries via envvar pairs","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-11-18T00:50:14Z","receivedAt":"2020-11-18T00:50:46Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-11-17 at 02:34:54, Jeff King wrote:\n> On Mon, Nov 16, 2020 at 11:39:35AM -0800, Junio C Hamano wrote:\n> \n> > >> While not document, it is currently possible to specify config entries\n> > >> [in GIT_CONFIG_PARAMETERS]\n> > [...]\n> >\n> > \"While not documented\" yes, for sure, but we do not document it for\n> > a good reason---it is a pure implementation detail between Git\n> > process that runs another one as its internal implementation detail.\n> \n> I have actually been quite tempted to document and promise that it will\n> continue to work. Because it really is useful in some instances. The\n> thing that has held me back is that the documentation would reveal how\n> unforgiving the parser is. ;)\n> \n> It insists that key/value pairs are shell-quoted as a whole. I think if\n> we made it accept a some reasonable inputs:\n> \n>   - do not require quoting for values that do not need it\n> \n>   - allow any amount of shell-style single-quoting (whole parameters,\n>     just values, etc).\n> \n>   - do not bother allowing other quoting, like double-quotes with\n>     backslashes. However, document backslash and double-quote as\n>     meta-characters that must not appear outside of single-quotes, to\n>     allow for future expansion.\n> \n> then I'd feel comfortable making it a public-facing feature. And for\n> most cases it would be pretty pleasant to use (and for the unpleasant\n> ones, I'm not sure that a little quoting is any worse than the paired\n> environment variables found here).\n\nWhat if we didn't document it but provided a command that produced a\nsuitable value?  Maybe something like this:\n\n  GIT_CONFIG_PARAMETERS=$(git rev-parse --quote-parameters a.b.c ENV_VAR d.e.f OTHER_ENV_VAR)\n\nOr whatever we decide.\n\nI don't personally love shell quoting as an interchange mechanism; I'd\nprefer something like URI-encoding, which is a bit more standardized and\neasier to reason about from a security perspective.  But if we decide to\nchange it, it doesn't matter, since it's still undocumented and this\nwould be the only acceptable way to pass config through the environment.\n\nAlternatively, we could just do this:\n\n  git with-config --key a.b.c --value ENV_VAR --key d.e.f --value OTHER_ENV_VAR --command git foo\n\nThat would also leave it undocumented, but make it easier to work with.\n\n> > I especially do not think we want to read from unbounded number of\n> > GIT_CONFIG_KEY_<N> variables like this patch does.  How would a\n> > script cleanse its environment to protect itself from stray such\n> > environment variable pair?  Count up from 1 to forever?  Run \"env\"\n> > and grep for \"GIT_CONFIG_KEY_[0-9]*=\" (the answer is No.  What if\n> > some environment variables have newline in its values?)\n> \n> Yeah, scripts can currently assume that:\n> \n>   unset $(git rev-parse --local-env-vars)\n> \n> will drop any config from the environment. In some cases, having\n> rev-parse enumerate the GIT_CONFIG_KEY_* variables that are set would be\n> sufficient. But that implies that rev-parse is seeing the same\n> environment we're planning to clear. As it is now, a script is free to\n> use rev-parse to generate that list, hold on to it, and then use it\n> later.\n\nI'm also uncomfortable with an arbitrary number of keys and values.  It\nbecomes very tricky to cleanse the environment, and even if the code\nstops at the first gap, if you then add more entries, then you have to\ncleanse again or risk a security problem.  I feel like this is only\ngoing to bite us in the future.\n-- \nbrian m. carlson (he/him or they/them)\nHouston, Texas, US\n"},{"id":"410191","messageId":"20201118015907.GD650959@coredump.intra.peff.net","threadId":"54638","inReplyTo":"20201118005014.GC389879@camp.crustytoothpaste.net","subject":"Re: [PATCH 2/2] config: allow specifying config entries via envvar pairs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-11-18T01:59:07Z","receivedAt":"2020-11-18T01:59:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 18, 2020 at 12:50:14AM +0000, brian m. carlson wrote:\n\n> > then I'd feel comfortable making it a public-facing feature. And for\n> > most cases it would be pretty pleasant to use (and for the unpleasant\n> > ones, I'm not sure that a little quoting is any worse than the paired\n> > environment variables found here).\n> \n> What if we didn't document it but provided a command that produced a\n> suitable value?  Maybe something like this:\n> \n>   GIT_CONFIG_PARAMETERS=$(git rev-parse --quote-parameters a.b.c ENV_VAR d.e.f OTHER_ENV_VAR)\n> \n> Or whatever we decide.\n\nI think we mostly already have that tooling.\n\n  $ GIT_CONFIG_PARAMETERS=$(\n      git rev-parse --sq-quote \\\n        foo.bar=value \\\n        'section.key=with spaces' \\\n        \"or.even=embedded 'quotes'\" |\n      sed 's/^ //'; # yuck\n    )\n  $ export GIT_CONFIG_PARAMETERS\n  $ git config --list --show-scope | grep ^command\n  command\tfoo.bar=value\n  command\tsection.key=with spaces\n  command\tor.even=embedded 'quotes'\n\nThe \"yuck\" there is because --sq-quote insists on putting a space at the\nfront. Our parser should probably ignore that, but right now it's quite\npicky.\n\nThough I suppose:\n\n  - do you mean ENV_VAR to literally look up an environment variable?\n    That solves Patrick's original problem of not wanting to put\n    sensitive values onto the command line. But it's much more annoying\n    to use if you _don't_ already have the value in an environment\n    variable. I'm not sure if that original problem matters here, as a\n    program that does a lot of this would probably implement the quoting\n    itself.\n\n  - obviously it is doubling down on the shell-quoting as a public\n    thing, and part of your point is that we would leave it opaque.\n    I'm OK with that aspect (even if it ends up as an alias for\n    --sq-quote for now). I'm not entirely sure people aren't using this\n    in the wild already, though. Saying \"it was undocumented\" may give\n    us a leg to stand on if we change the format, but it will still be\n    annoying to people we break.\n\n  - my example above still has the \"a.b=c.d=e\" ambiguity that I\n    mentioned earlier. Fixing that requires changing the format in the\n    environment variable.\n\n> I don't personally love shell quoting as an interchange mechanism; I'd\n> prefer something like URI-encoding, which is a bit more standardized and\n> easier to reason about from a security perspective.  But if we decide to\n> change it, it doesn't matter, since it's still undocumented and this\n> would be the only acceptable way to pass config through the environment.\n\nYes, I think concatenating uri_encode(key) + \"=\" + uri_encode(value)\nwould be easier to reason about, and solves the ambiguity problem. If we\nare willing to break from the existing format, it's probably a\nreasonable direction.\n\n> Alternatively, we could just do this:\n> \n>   git with-config --key a.b.c --value ENV_VAR --key d.e.f --value OTHER_ENV_VAR --command git foo\n> \n> That would also leave it undocumented, but make it easier to work with.\n\nI wondered at first how this is different from:\n\n  git -c a.b.c=value foo\n\nbut I guess it is meant to do the same environment-lookup? We could\nprobably add:\n\n  git --env-config a.b.c=ENV_VAR foo\n\nto have Git internally stick $ENV_VAR into $GIT_CONFIG_PARAMETERS. That\navoids all of the rev-parse rigamarole, keeps the format of the\nenvironment variable opaque, and solves Patrick's problem of putting the\nvalue onto the command-line.\n\nIt doesn't solve the ambiguity with \"=\" in the subsection, but IMHO that\nis not all that important a problem.\n\n-Peff\n"},{"id":"410196","messageId":"20201118022532.GD389879@camp.crustytoothpaste.net","threadId":"54638","inReplyTo":"20201118015907.GD650959@coredump.intra.peff.net","subject":"Re: [PATCH 2/2] config: allow specifying config entries via envvar pairs","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-11-18T02:25:32Z","receivedAt":"2020-11-18T02:26:10Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-11-18 at 01:59:07, Jeff King wrote:\n> I think we mostly already have that tooling.\n> \n>   $ GIT_CONFIG_PARAMETERS=$(\n>       git rev-parse --sq-quote \\\n>         foo.bar=value \\\n>         'section.key=with spaces' \\\n>         \"or.even=embedded 'quotes'\" |\n>       sed 's/^ //'; # yuck\n>     )\n>   $ export GIT_CONFIG_PARAMETERS\n>   $ git config --list --show-scope | grep ^command\n>   command\tfoo.bar=value\n>   command\tsection.key=with spaces\n>   command\tor.even=embedded 'quotes'\n> \n> The \"yuck\" there is because --sq-quote insists on putting a space at the\n> front. Our parser should probably ignore that, but right now it's quite\n> picky.\n\nI feel that this approach leaves a few things to be desired, yes.\n\n> Though I suppose:\n> \n>   - do you mean ENV_VAR to literally look up an environment variable?\n>     That solves Patrick's original problem of not wanting to put\n>     sensitive values onto the command line. But it's much more annoying\n>     to use if you _don't_ already have the value in an environment\n>     variable. I'm not sure if that original problem matters here, as a\n>     program that does a lot of this would probably implement the quoting\n>     itself.\n\nYeah, I did mean that, or various options to either read a literal value\nor from the environment.  Your proposal, while it obviously works,\ndoesn't solve Patrick's problem.\n\n>   - obviously it is doubling down on the shell-quoting as a public\n>     thing, and part of your point is that we would leave it opaque.\n>     I'm OK with that aspect (even if it ends up as an alias for\n>     --sq-quote for now). I'm not entirely sure people aren't using this\n>     in the wild already, though. Saying \"it was undocumented\" may give\n>     us a leg to stand on if we change the format, but it will still be\n>     annoying to people we break.\n\nI've been telling people for some time that Git doesn't have a way to do\nthis, so I'm comfortable sticking with that line until we have a\ndocumented way to do it.  I knew we had an internal environment variable\n(because I was curious and looked), but I knew from just looking at it\nthat it was internal.\n\n> Yes, I think concatenating uri_encode(key) + \"=\" + uri_encode(value)\n> would be easier to reason about, and solves the ambiguity problem. If we\n> are willing to break from the existing format, it's probably a\n> reasonable direction.\n\nWe'll have to deal with embedded NULs, but other than that it seems\nrobust and easy to reason about.  I like the proposal below better,\nthough.\n\n> I wondered at first how this is different from:\n> \n>   git -c a.b.c=value foo\n> \n> but I guess it is meant to do the same environment-lookup? We could\n> probably add:\n> \n>   git --env-config a.b.c=ENV_VAR foo\n> \n> to have Git internally stick $ENV_VAR into $GIT_CONFIG_PARAMETERS. That\n> avoids all of the rev-parse rigamarole, keeps the format of the\n> environment variable opaque, and solves Patrick's problem of putting the\n> value onto the command-line.\n\nSure, that could be an option.  It's the simplest, and we already know\nhow to handle config this way.  People will be able to figure out how to\nuse it pretty easily.\n\n> It doesn't solve the ambiguity with \"=\" in the subsection, but IMHO that\n> is not all that important a problem.\n\nI'm fine with saying that we don't support environment variable names\nwith equals signs and just going with that.  It solves the ambiguity\nproblem and I'm not sure that they could usefully work on Unix anyway.\n\nMoreover, people usually use the unrestricted data in the second chunk\nfor URLs, which shouldn't have unquoted equals signs.  You could\ntechnically name other second fields that way, but why would you when\nyou could just not?\n\nSo I'm not too worried about it either way.\n-- \nbrian m. carlson (he/him or they/them)\nHouston, Texas, US\n"},{"id":"410201","messageId":"xmqqeekr43sr.fsf@gitster.c.googlers.com","threadId":"54638","inReplyTo":"20201118005014.GC389879@camp.crustytoothpaste.net","subject":"Re: [PATCH 2/2] config: allow specifying config entries via envvar pairs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-18T05:44:36Z","receivedAt":"2020-11-18T05:44:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n\n> I don't personally love shell quoting as an interchange mechanism; I'd\n> prefer something like URI-encoding, which is a bit more standardized and\n> easier to reason about from a security perspective.  But if we decide to\n> change it, it doesn't matter, since it's still undocumented and this\n> would be the only acceptable way to pass config through the environment.\n\nSurely.  \n\nI am kind-of surprised that nobody has brought up json or yaml ;-)\n"},{"id":"410208","messageId":"X7THfjaP91+GV//V@ncase","threadId":"54638","inReplyTo":"20201118022532.GD389879@camp.crustytoothpaste.net","subject":"Re: [PATCH 2/2] config: allow specifying config entries via envvar pairs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2020-11-18T07:04:30Z","receivedAt":"2020-11-18T07:04:36Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Nov 18, 2020 at 02:25:32AM +0000, brian m. carlson wrote:\n> On 2020-11-18 at 01:59:07, Jeff King wrote:\n> > Yes, I think concatenating uri_encode(key) + \"=\" + uri_encode(value)\n> > would be easier to reason about, and solves the ambiguity problem. If we\n> > are willing to break from the existing format, it's probably a\n> > reasonable direction.\n> \n> We'll have to deal with embedded NULs, but other than that it seems\n> robust and easy to reason about.  I like the proposal below better,\n> though.\n\nThis would be easy enough to handle and fixes all the problems I\ncurrently have.\n\n> > I wondered at first how this is different from:\n> > \n> >   git -c a.b.c=value foo\n> > \n> > but I guess it is meant to do the same environment-lookup? We could\n> > probably add:\n> > \n> >   git --env-config a.b.c=ENV_VAR foo\n> > \n> > to have Git internally stick $ENV_VAR into $GIT_CONFIG_PARAMETERS. That\n> > avoids all of the rev-parse rigamarole, keeps the format of the\n> > environment variable opaque, and solves Patrick's problem of putting the\n> > value onto the command-line.\n> \n> Sure, that could be an option.  It's the simplest, and we already know\n> how to handle config this way.  People will be able to figure out how to\n> use it pretty easily.\n\nAt first, this idea sounds quite interesting. But only until one\nrealizes that it's got the exact same problem which I'm trying to solve:\nthere's still a point in time where one can observe config values via\nthe command line, even though that moment now is a lot shorter compared\nto running the \"real\" git command with those keys.\n\n> > It doesn't solve the ambiguity with \"=\" in the subsection, but IMHO that\n> > is not all that important a problem.\n> \n> I'm fine with saying that we don't support environment variable names\n> with equals signs and just going with that.  It solves the ambiguity\n> problem and I'm not sure that they could usefully work on Unix anyway.\n> \n> Moreover, people usually use the unrestricted data in the second chunk\n> for URLs, which shouldn't have unquoted equals signs.  You could\n> technically name other second fields that way, but why would you when\n> you could just not?\n> \n> So I'm not too worried about it either way.\n\nThere's not only URLs though, but also paths in 'includeIf.*.path'.\nSure, it's unlikely that paths have an equals sign embedded. But I think\nwe should try to not paint ourselves into a corner.\n\nThis problem also would be nicely solved by URI-encoding both key and\nvalue and then passing it via GIT_CONFIG_PARAMETER.\n\nPatrick\n"},{"id":"410224","messageId":"87ft56rd7o.fsf@evledraar.gmail.com","threadId":"54638","inReplyTo":"20201117235736.GC642410@coredump.intra.peff.net","subject":"Re: [PATCH 2/2] config: allow specifying config entries via envvar pairs","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2020-11-18T13:44:59Z","receivedAt":"2020-11-18T13:45:24Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Nov 18 2020, Jeff King wrote:\n\n> On Tue, Nov 17, 2020 at 03:22:05PM +0100, Ævar Arnfjörð Bjarmason wrote:\n>\n>> > then I'd feel comfortable making it a public-facing feature. And for\n>> > most cases it would be pretty pleasant to use (and for the unpleasant\n>> > ones, I'm not sure that a little quoting is any worse than the paired\n>> > environment variables found here).\n>> \n>> I wonder if something like the git config -z format wouldn't be easier,\n>> with the twist that we'd obviously not support \\0. So we'd need an\n>> optional length prefix. : = unspecified.\n>> \n>>     :user.name\n>>     Jeff K\n>>     :alias.ci\n>>     commit\n>>     :10:bin.ary\n>>     <10 byte string, might have a \\n>\n>>     :other.key\n>>     Other Value\n>> \n>> Maybe that's overly fragile, or maybe another format would be better.\n>\n> Yeah, length-delimited strings are an alternative that some people think\n> is less error-prone than quoting. And we do use pkt-lines. They're also\n> a pain for humans to write (it's nicer if they're optional, but when you\n> _do_ have to start using them, now you are stuck counting things up).\n>\n>> I was trying to come up with one where the common case wouldn't\n>> require knowing about shell quoting/unquoting, and where you could\n>> still do:\n>> \n>>     GIT_CONFIG_PARAMETERS=\":my.new\\nvalue\\n$GIT_CONFIG_PARAMETERS\"\n>> \n>> Or equivalent, and still just keep $GIT_CONFIG_PARAMETERS as-is to pass\n>> it along.\n>> \n>> Your \"do not require quoting\" accomplishes that, and it's arguably a lot\n>n\n> Looks like your mail got cut off.\n\nNothing important, probably :)\n\n> But yeah, the goal of making the quoting optional was to make it\n> easier for humans to use for simple cases. It doesn't help at all with\n> other programs inserting values, which can just as easily err on the\n> side of caution.\n>\n> BTW, there is another problem with GIT_CONFIG_PARAMETERS (and \"git -c\"\n> in general). The dotted config-key format:\n>\n>   section.subsection.key\n>\n> is unambiguous by itself, even though \"subsection\" can contain arbitrary\n> bytes, including dots. Because neither \"section\" nor \"key\" can contain\n> dots, we can parse from either end, and take the whole middle as a\n> subsection (and this is how we do it in the code).\n>\n> But an assignment string like:\n>\n>   section.subsection.key=value\n>\n> _is_ ambiguous. We have to parse left-to-right up to the first equals\n> (since \"value\" can contain arbitrary characters, including an equals).\n> But \"subsection\" can have one, too, so we want to parse right-to-left\n> there. E.g., in:\n>\n>   one.two=three.four=five\n>\n> this could be either of:\n>\n>   - section is \"one\", key is \"two\", value is \"three.four=five\"\n>\n>   - section is \"one\", subsection is \"two=three\", key is \"four\", value is\n>     \"five\"\n>\n> We currently always parse it as the former (which I think is least-bad\n> of the two, since values are more likely than subsections to contain\n> arbitrary text with an equals).\n\nYeah, it's a pain to parse if it's on one line. FWIW that's the main\nreason for why the format I suggested moved it to \\n-delimited, because\nkeys can't contain an \\n, so you can unambiguously have them be\n\\n-delimited (as git config -z does).\n\nYou do need to worry about a \\n in the value, but for the common case\nwhere you don't have a \\n there we wouldn't need to provide the length.\n\nOr just provide tooling as you suggested in\n<20201118015907.GD650959@coredump.intra.peff.net>, which I like better\nthan any one format suggestion (including the one I suggsted). I.e. we\ncan document that:\n\n - The variable exists\n - You read/write/add to it using a return value from this tool\n\nWhich allows for keeping the value itself opaque and open to a future\nchange.\n"},{"id":"410225","messageId":"87d00arczw.fsf@evledraar.gmail.com","threadId":"54638","inReplyTo":"xmqq4klo7992.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 2/2] config: allow specifying config entries via envvar pairs","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2020-11-18T13:49:39Z","receivedAt":"2020-11-18T13:49:57Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Nov 17 2020, Junio C Hamano wrote:\n\n> Patrick Steinhardt <ps@pks.im> writes:\n>\n>>> I especially do not think we want to read from unbounded number of\n>>> GIT_CONFIG_KEY_<N> variables like this patch does.  How would a\n>>> script cleanse its environment to protect itself from stray such\n>>> environment variable pair?  Count up from 1 to forever?  Run \"env\"\n>>> and grep for \"GIT_CONFIG_KEY_[0-9]*=\" (the answer is No.  What if\n>>> some environment variables have newline in its values?)\n>>\n>> You only have to unset `GIT_CONFIG_KEY_1` as the parser will stop\n>> iterating on the first missing key. More generally, if you set `n` keys,\n>> it's sufficient to unset key `n+1`.\n>\n> Yes, but those who want to set N keys would likely to be content\n> with setting 1..N and happily forget unsetting N+1, and that is\n> where \"how would one cleanse the environment to give a clean slate?\"\n> comes from.\n\nNot as an argument from whataboutism, but just to note a bug/existing\nprior art:\n\nNobody in this thread has mentioned GIT_PUSH_OPTION_* which works pretty\nmuch like Patrick's suggestion, and it looks like --local-env-vars\nmisses those:\n\n    $ GIT_PUSH_OPTION_0=foo GIT_PUSH_OPTION_COUNT=20 git rev-parse --local-env-vars | grep GIT_PUSH\n    $\n\nI haven't tested this, but I expect there's a bug where a push hook\nitself does a local push to another repo and that repo has a hook, that\nthe push options are erroneously carried forward to the sub-process.\n\nThat might also be a feature, depending on your point of view.\n"},{"id":"410226","messageId":"X7UoIjflsJe7xOx8@ncase","threadId":"54638","inReplyTo":"87d00arczw.fsf@evledraar.gmail.com","subject":"Re: [PATCH 2/2] config: allow specifying config entries via envvar pairs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2020-11-18T13:56:50Z","receivedAt":"2020-11-18T13:57:19Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Nov 18, 2020 at 02:49:39PM +0100, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Tue, Nov 17 2020, Junio C Hamano wrote:\n> \n> > Patrick Steinhardt <ps@pks.im> writes:\n> >\n> >>> I especially do not think we want to read from unbounded number of\n> >>> GIT_CONFIG_KEY_<N> variables like this patch does.  How would a\n> >>> script cleanse its environment to protect itself from stray such\n> >>> environment variable pair?  Count up from 1 to forever?  Run \"env\"\n> >>> and grep for \"GIT_CONFIG_KEY_[0-9]*=\" (the answer is No.  What if\n> >>> some environment variables have newline in its values?)\n> >>\n> >> You only have to unset `GIT_CONFIG_KEY_1` as the parser will stop\n> >> iterating on the first missing key. More generally, if you set `n` keys,\n> >> it's sufficient to unset key `n+1`.\n> >\n> > Yes, but those who want to set N keys would likely to be content\n> > with setting 1..N and happily forget unsetting N+1, and that is\n> > where \"how would one cleanse the environment to give a clean slate?\"\n> > comes from.\n> \n> Not as an argument from whataboutism, but just to note a bug/existing\n> prior art:\n> \n> Nobody in this thread has mentioned GIT_PUSH_OPTION_* which works pretty\n> much like Patrick's suggestion, and it looks like --local-env-vars\n> misses those:\n> \n>     $ GIT_PUSH_OPTION_0=foo GIT_PUSH_OPTION_COUNT=20 git rev-parse --local-env-vars | grep GIT_PUSH\n>     $\n> \n> I haven't tested this, but I expect there's a bug where a push hook\n> itself does a local push to another repo and that repo has a hook, that\n> the push options are erroneously carried forward to the sub-process.\n> \n> That might also be a feature, depending on your point of view.\n\nI didn't actually know about it, thanks for pointing it out!\n\nIf we're going to use the same `_COUNT` approach, then I think the\nissues which were discussed would mostly go away. No gaps needed, no\nrequirement to unset `$n+1`. Any properly behaving user would know to\nset it as otherwise the written code/script cannot work. Also,\ngit-rev-parse(1) wouldn't have to dynamically adjust based on whether a\npreviously existing gap was filled with new keys or not.\n\nI'd be happy to pursue that road, but I'll wait for some feedback first.\n\nPatrick\n"},{"id":"410239","messageId":"xmqqo8ju3b7w.fsf@gitster.c.googlers.com","threadId":"54638","inReplyTo":"87d00arczw.fsf@evledraar.gmail.com","subject":"Re: [PATCH 2/2] config: allow specifying config entries via envvar pairs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-18T16:01:55Z","receivedAt":"2020-11-18T16:02:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> Nobody in this thread has mentioned GIT_PUSH_OPTION_* which works pretty\n> much like Patrick's suggestion, and it looks like --local-env-vars\n> misses those:\n>\n>     $ GIT_PUSH_OPTION_0=foo GIT_PUSH_OPTION_COUNT=20 git rev-parse --local-env-vars | grep GIT_PUSH\n>     $\n>\n> I haven't tested this, but I expect there's a bug where a push hook\n> itself does a local push to another repo and that repo has a hook, that\n> the push options are erroneously carried forward to the sub-process.\n\nTrue.\n\nNobody mentioned the environment variable in the discussion, and\nnobody discovered and was motivated enough to report and/or fix it,\nmay be a good indication that these variables are not much used in\nreal life and certainly not in combination with hooks that further\npush things out.\n"},{"id":"410330","messageId":"20201119021116.GE389879@camp.crustytoothpaste.net","threadId":"54638","inReplyTo":"X7THfjaP91+GV//V@ncase","subject":"Re: [PATCH 2/2] config: allow specifying config entries via envvar pairs","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-11-19T02:11:16Z","receivedAt":"2020-11-19T02:11:44Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-11-18 at 07:04:30, Patrick Steinhardt wrote:\n> On Wed, Nov 18, 2020 at 02:25:32AM +0000, brian m. carlson wrote:\n> > Sure, that could be an option.  It's the simplest, and we already know\n> > how to handle config this way.  People will be able to figure out how to\n> > use it pretty easily.\n> \n> At first, this idea sounds quite interesting. But only until one\n> realizes that it's got the exact same problem which I'm trying to solve:\n> there's still a point in time where one can observe config values via\n> the command line, even though that moment now is a lot shorter compared\n> to running the \"real\" git command with those keys.\n\nI don't think that's the case.  This command:\n\n  git --env-config a.b.c=ENV_VAR\n\nwould be equivalent to this shell command:\n\n  git -c \"a.b.c=$ENV_VAR\"\n\nIn other words, ENV_VAR is the _name_ of a environment variable to read\nfor the config value.  Subprocesses would inherit it using the\nundocumented GIT_CONFIG_PARAMETERS.\n\nOr are you trying to hide the configuration key as well?\n-- \nbrian m. carlson (he/him or they/them)\nHouston, Texas, US\n"},{"id":"410334","messageId":"X7YSq9zgTgfFCF1N@ncase","threadId":"54638","inReplyTo":"20201119021116.GE389879@camp.crustytoothpaste.net","subject":"Re: [PATCH 2/2] config: allow specifying config entries via envvar pairs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2020-11-19T06:37:31Z","receivedAt":"2020-11-19T06:37:44Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Nov 19, 2020 at 02:11:16AM +0000, brian m. carlson wrote:\n> On 2020-11-18 at 07:04:30, Patrick Steinhardt wrote:\n> > On Wed, Nov 18, 2020 at 02:25:32AM +0000, brian m. carlson wrote:\n> > > Sure, that could be an option.  It's the simplest, and we already know\n> > > how to handle config this way.  People will be able to figure out how to\n> > > use it pretty easily.\n> > \n> > At first, this idea sounds quite interesting. But only until one\n> > realizes that it's got the exact same problem which I'm trying to solve:\n> > there's still a point in time where one can observe config values via\n> > the command line, even though that moment now is a lot shorter compared\n> > to running the \"real\" git command with those keys.\n> \n> I don't think that's the case.  This command:\n> \n>   git --env-config a.b.c=ENV_VAR\n> \n> would be equivalent to this shell command:\n> \n>   git -c \"a.b.c=$ENV_VAR\"\n> \n> In other words, ENV_VAR is the _name_ of a environment variable to read\n> for the config value.  Subprocesses would inherit it using the\n> undocumented GIT_CONFIG_PARAMETERS.\n> \n> Or are you trying to hide the configuration key as well?\n\nNo. I just didn't realize that it's supposed to be the name of an\nenvvar.\n\nPatrick\n"}]}