From ce81b1da230cf04e231ce337c2946c0671ffb303 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 12 Jan 2021 13:26:45 +0100 Subject: config: add new way to pass config via `--config-env` While it's already possible to pass runtime configuration via `git -c =`, it may be undesirable to use when the value contains sensitive information. E.g. if one wants to set `http.extraHeader` to contain an authentication token, doing so via `-c` would trivially leak those credentials via e.g. ps(1), which typically also shows command arguments. To enable this usecase without leaking credentials, this commit introduces a new switch `--config-env==`. Instead of directly passing a value for the given key, it instead allows the user to specify the name of an environment variable. The value of that variable will then be used as value of the key. Co-authored-by: Jeff King Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/t1300-config.sh | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) (limited to 't') diff --git a/t/t1300-config.sh b/t/t1300-config.sh index 825d9a184f..ba46d9559d 100755 --- a/t/t1300-config.sh +++ b/t/t1300-config.sh @@ -1316,6 +1316,54 @@ test_expect_success 'detect bogus GIT_CONFIG_PARAMETERS' ' git config --get-regexp "env.*" ' +test_expect_success 'git --config-env=key=envvar support' ' + cat >expect <<-\EOF && + value + value + false + EOF + { + ENVVAR=value git --config-env=core.name=ENVVAR config core.name && + ENVVAR=value git --config-env=foo.CamelCase=ENVVAR config foo.camelcase && + ENVVAR= git --config-env=foo.flag=ENVVAR config --bool foo.flag + } >actual && + test_cmp expect actual +' + +test_expect_success 'git --config-env fails with invalid parameters' ' + test_must_fail git --config-env=foo.flag config --bool foo.flag 2>error && + test_i18ngrep "invalid config format: foo.flag" error && + test_must_fail git --config-env=foo.flag= config --bool foo.flag 2>error && + test_i18ngrep "missing environment variable name for configuration ${SQ}foo.flag${SQ}" error && + sane_unset NONEXISTENT && + test_must_fail git --config-env=foo.flag=NONEXISTENT config --bool foo.flag 2>error && + test_i18ngrep "missing environment variable ${SQ}NONEXISTENT${SQ} for configuration ${SQ}foo.flag${SQ}" error +' + +test_expect_success 'git -c and --config-env work together' ' + cat >expect <<-\EOF && + bar.cmd cmd-value + bar.env env-value + EOF + ENVVAR=env-value git \ + -c bar.cmd=cmd-value \ + --config-env=bar.env=ENVVAR \ + config --get-regexp "^bar.*" >actual && + test_cmp expect actual +' + +test_expect_success 'git -c and --config-env override each other' ' + cat >expect <<-\EOF && + env + cmd + EOF + { + ENVVAR=env git -c bar.bar=cmd --config-env=bar.bar=ENVVAR config bar.bar && + ENVVAR=env git --config-env=bar.bar=ENVVAR -c bar.bar=cmd config bar.bar + } >actual && + test_cmp expect actual +' + test_expect_success 'git config --edit works' ' git config -f tmp test.value no && echo test.value=yes >expect && -- cgit v1.2.3 From f9dbb64fadf599c588a39d2251bb3f9a2f7d572a Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 12 Jan 2021 13:27:06 +0100 Subject: config: parse more robust format in GIT_CONFIG_PARAMETERS When we stuff config options into GIT_CONFIG_PARAMETERS, we shell-quote each one as a single unit, like: 'section.one=value1' 'section.two=value2' On the reading side, we de-quote to get the individual strings, and then parse them by splitting on the first "=" we find. This format is ambiguous, because an "=" may appear in a subsection. So the config represented in a file by both: [section "subsection=with=equals"] key = value and: [section] subsection = with=equals.key=value ends up in this flattened format like: 'section.subsection=with=equals.key=value' and we can't tell which was desired. We have traditionally resolved this by taking the first "=" we see starting from the left, meaning that we allowed arbitrary content in the value, but not in the subsection. Let's make our environment format a bit more robust by separately quoting the key and value. That turns those examples into: 'section.subsection=with=equals.key'='value' and: 'section.subsection'='with=equals.key=value' respectively, and we can tell the difference between them. We can detect which format is in use for any given element of the list based on the presence of the unquoted "=". That means we can continue to allow the old format to work to support any callers which manually used the old format, and we can even intermingle the two formats. The old format wasn't documented, and nobody was supposed to be using it. But it's likely that such callers exist in the wild, so it's nice if we can avoid breaking them. Likewise, it may be possible to trigger an older version of "git -c" that runs a script that calls into a newer version of "git -c"; that new version would see the intermingled format. This does create one complication, which is that the obvious format in the new scheme for [section] some-bool is: 'section.some-bool' with no equals. We'd mistake that for an old-style variable. And it even has the same meaning in the old style, but: [section "with=equals"] some-bool does not. It would be: 'section.with=equals=some-bool' which we'd take to mean: [section] with = equals=some-bool in the old, ambiguous style. Likewise, we can't use: 'section.some-bool'='' because that's ambiguous with an actual empty string. Instead, we'll again use the shell-quoting to give us a hint, and use: 'section.some-bool'= to show that we have no value. Note that this commit just expands the reading side. We'll start writing the new format via "git -c" in a future patch. In the meantime, the existing "git -c" tests will make sure we didn't break reading the old format. But we'll also add some explicit coverage of the two formats to make sure we continue to handle the old one after we move the writing side over. And one final note: since we're now using the shell-quoting as a semantically meaningful hint, this closes the door to us ever allowing arbitrary shell quoting, like: 'a'shell'would'be'ok'with'this'.key=value But we have never supported that (only what sq_quote() would produce), and we are probably better off keeping things simple, robust, and backwards-compatible, than trying to make it easier for humans. We'll continue not to advertise the format of the variable to users, and instead keep "git -c" as the recommended mechanism for setting config (even if we are trying to be kind not to break users who may be relying on the current undocumented format). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t1300-config.sh | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) (limited to 't') diff --git a/t/t1300-config.sh b/t/t1300-config.sh index ba46d9559d..efdf2bf997 100755 --- a/t/t1300-config.sh +++ b/t/t1300-config.sh @@ -1294,6 +1294,58 @@ test_expect_success 'git -c is not confused by empty environment' ' GIT_CONFIG_PARAMETERS="" git -c x.one=1 config --list ' +test_expect_success 'GIT_CONFIG_PARAMETERS handles old-style entries' ' + v="${SQ}key.one=foo${SQ}" && + v="$v ${SQ}key.two=bar${SQ}" && + v="$v ${SQ}key.ambiguous=section.whatever=value${SQ}" && + GIT_CONFIG_PARAMETERS=$v git config --get-regexp "key.*" >actual && + cat >expect <<-EOF && + key.one foo + key.two bar + key.ambiguous section.whatever=value + EOF + test_cmp expect actual +' + +test_expect_success 'GIT_CONFIG_PARAMETERS handles new-style entries' ' + v="${SQ}key.one${SQ}=${SQ}foo${SQ}" && + v="$v ${SQ}key.two${SQ}=${SQ}bar${SQ}" && + v="$v ${SQ}key.ambiguous=section.whatever${SQ}=${SQ}value${SQ}" && + GIT_CONFIG_PARAMETERS=$v git config --get-regexp "key.*" >actual && + cat >expect <<-EOF && + key.one foo + key.two bar + key.ambiguous=section.whatever value + EOF + test_cmp expect actual +' + +test_expect_success 'old and new-style entries can mix' ' + v="${SQ}key.oldone=oldfoo${SQ}" && + v="$v ${SQ}key.newone${SQ}=${SQ}newfoo${SQ}" && + v="$v ${SQ}key.oldtwo=oldbar${SQ}" && + v="$v ${SQ}key.newtwo${SQ}=${SQ}newbar${SQ}" && + GIT_CONFIG_PARAMETERS=$v git config --get-regexp "key.*" >actual && + cat >expect <<-EOF && + key.oldone oldfoo + key.newone newfoo + key.oldtwo oldbar + key.newtwo newbar + EOF + test_cmp expect actual +' + +test_expect_success 'old and new bools with ambiguous subsection' ' + v="${SQ}key.with=equals.oldbool${SQ}" && + v="$v ${SQ}key.with=equals.newbool${SQ}=" && + GIT_CONFIG_PARAMETERS=$v git config --get-regexp "key.*" >actual && + cat >expect <<-EOF && + key.with equals.oldbool + key.with=equals.newbool + EOF + test_cmp expect actual +' + test_expect_success 'detect bogus GIT_CONFIG_PARAMETERS' ' cat >expect <<-\EOF && env.one one -- cgit v1.2.3 From 1ff21c05ba99ed2d0ade8318e3cb0c1a3f8d4b80 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 12 Jan 2021 13:27:01 +0100 Subject: config: store "git -c" variables using more robust format The previous commit added a new format for $GIT_CONFIG_PARAMETERS which is able to robustly handle subsections with "=" in them. Let's start writing the new format. Unfortunately, this does much less than you'd hope, because "git -c" itself has the same ambiguity problem! But it's still worth doing: - we've now pushed the problem from the inter-process communication into the "-c" command-line parser. This would free us up to later add an unambiguous format there (e.g., separate arguments like "git --config key value", etc). - for --config-env, the parser already disallows "=" in the environment variable name. So: git --config-env section.with=equals.key=ENVVAR will robustly set section.with=equals.key to the contents of $ENVVAR. The new test shows the improvement for --config-env. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t1300-config.sh | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 't') diff --git a/t/t1300-config.sh b/t/t1300-config.sh index efdf2bf997..cc68b42b97 100755 --- a/t/t1300-config.sh +++ b/t/t1300-config.sh @@ -1416,6 +1416,14 @@ test_expect_success 'git -c and --config-env override each other' ' test_cmp expect actual ' +test_expect_success '--config-env handles keys with equals' ' + echo value=with=equals >expect && + ENVVAR=value=with=equals git \ + --config-env=section.subsection=with=equals.key=ENVVAR \ + config section.subsection=with=equals.key >actual && + test_cmp expect actual +' + test_expect_success 'git config --edit works' ' git config -f tmp test.value no && echo test.value=yes >expect && -- cgit v1.2.3 From d8d77153eafdb0fc334e827976f09e4bdff26b58 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 12 Jan 2021 13:27:14 +0100 Subject: config: allow specifying config entries via envvar pairs While we currently have the `GIT_CONFIG_PARAMETERS` environment variable which can be used to pass runtime configuration data to git processes, it's an internal implementation detail and not supposed to be used by end users. Next to being for internal use only, this way of passing config entries has a major downside: the config keys need to be parsed as they contain both key and value in a single variable. As such, it is left to the user to escape any potentially harmful characters in the value, which is quite hard to do if values are controlled by a third party. This commit thus adds a new way of adding config entries via the environment which gets rid of this shortcoming. If the user passes the `GIT_CONFIG_COUNT=$n` environment variable, Git will parse environment variable pairs `GIT_CONFIG_KEY_$i` and `GIT_CONFIG_VALUE_$i` for each `i` in `[0,n)`. While the same can be achieved with `git -c =`, one may wish to not do so for potentially sensitive information. E.g. if one wants to set `http.extraHeader` to contain an authentication token, doing so via `-c` would trivially leak those credentials via e.g. ps(1), which typically also shows command arguments. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/t1300-config.sh | 115 +++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 114 insertions(+), 1 deletion(-) (limited to 't') diff --git a/t/t1300-config.sh b/t/t1300-config.sh index cc68b42b97..51a0621027 100755 --- a/t/t1300-config.sh +++ b/t/t1300-config.sh @@ -1424,6 +1424,117 @@ test_expect_success '--config-env handles keys with equals' ' test_cmp expect actual ' +test_expect_success 'git config handles environment config pairs' ' + GIT_CONFIG_COUNT=2 \ + GIT_CONFIG_KEY_0="pair.one" GIT_CONFIG_VALUE_0="foo" \ + GIT_CONFIG_KEY_1="pair.two" GIT_CONFIG_VALUE_1="bar" \ + git config --get-regexp "pair.*" >actual && + cat >expect <<-EOF && + pair.one foo + pair.two bar + EOF + test_cmp expect actual +' + +test_expect_success 'git config ignores pairs without count' ' + test_must_fail env GIT_CONFIG_KEY_0="pair.one" GIT_CONFIG_VALUE_0="value" \ + git config pair.one 2>error && + test_must_be_empty error +' + +test_expect_success 'git config ignores pairs with zero count' ' + test_must_fail env \ + GIT_CONFIG_COUNT=0 \ + GIT_CONFIG_KEY_0="pair.one" GIT_CONFIG_VALUE_0="value" \ + git config pair.one +' + +test_expect_success 'git config ignores pairs exceeding count' ' + GIT_CONFIG_COUNT=1 \ + GIT_CONFIG_KEY_0="pair.one" GIT_CONFIG_VALUE_0="value" \ + GIT_CONFIG_KEY_1="pair.two" GIT_CONFIG_VALUE_1="value" \ + git config --get-regexp "pair.*" >actual && + cat >expect <<-EOF && + pair.one value + EOF + test_cmp expect actual +' + +test_expect_success 'git config ignores pairs with zero count' ' + test_must_fail env \ + GIT_CONFIG_COUNT=0 GIT_CONFIG_KEY_0="pair.one" GIT_CONFIG_VALUE_0="value" \ + git config pair.one >error && + test_must_be_empty error +' + +test_expect_success 'git config ignores pairs with empty count' ' + test_must_fail env \ + GIT_CONFIG_COUNT= GIT_CONFIG_KEY_0="pair.one" GIT_CONFIG_VALUE_0="value" \ + git config pair.one >error && + test_must_be_empty error +' + +test_expect_success 'git config fails with invalid count' ' + test_must_fail env GIT_CONFIG_COUNT=10a git config --list 2>error && + test_i18ngrep "bogus count" error && + test_must_fail env GIT_CONFIG_COUNT=9999999999999999 git config --list 2>error && + test_i18ngrep "too many entries" error +' + +test_expect_success 'git config fails with missing config key' ' + test_must_fail env GIT_CONFIG_COUNT=1 GIT_CONFIG_VALUE_0="value" \ + git config --list 2>error && + test_i18ngrep "missing config key" error +' + +test_expect_success 'git config fails with missing config value' ' + test_must_fail env GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0="pair.one" \ + git config --list 2>error && + test_i18ngrep "missing config value" error +' + +test_expect_success 'git config fails with invalid config pair key' ' + test_must_fail env GIT_CONFIG_COUNT=1 \ + GIT_CONFIG_KEY_0= GIT_CONFIG_VALUE_0=value \ + git config --list && + test_must_fail env GIT_CONFIG_COUNT=1 \ + GIT_CONFIG_KEY_0=missing-section GIT_CONFIG_VALUE_0=value \ + git config --list +' + +test_expect_success 'environment overrides config file' ' + test_when_finished "rm -f .git/config" && + cat >.git/config <<-EOF && + [pair] + one = value + EOF + GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=pair.one GIT_CONFIG_VALUE_0=override \ + git config pair.one >actual && + cat >expect <<-EOF && + override + EOF + test_cmp expect actual +' + +test_expect_success 'GIT_CONFIG_PARAMETERS overrides environment config' ' + GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=pair.one GIT_CONFIG_VALUE_0=value \ + GIT_CONFIG_PARAMETERS="${SQ}pair.one=override${SQ}" \ + git config pair.one >actual && + cat >expect <<-EOF && + override + EOF + test_cmp expect actual +' + +test_expect_success 'command line overrides environment config' ' + GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=pair.one GIT_CONFIG_VALUE_0=value \ + git -c pair.one=override config pair.one >actual && + cat >expect <<-EOF && + override + EOF + test_cmp expect actual +' + test_expect_success 'git config --edit works' ' git config -f tmp test.value no && echo test.value=yes >expect && @@ -1769,9 +1880,11 @@ test_expect_success '--show-origin with --list' ' file:.git/config user.override=local file:.git/config include.path=../include/relative.include file:.git/../include/relative.include user.relative=include + command line: user.environ=true command line: user.cmdline=true EOF - git -c user.cmdline=true config --list --show-origin >output && + GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=user.environ GIT_CONFIG_VALUE_0=true\ + git -c user.cmdline=true config --list --show-origin >output && test_cmp expect output ' -- cgit v1.2.3