| Message ID | dfceffd8d4fbc3c99cfa7c5d838e4c3a2db6598a.1608104755.git.ps@pks.im (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | config: allow specifying config entries via env | expand |
Patrick Steinhardt <ps@pks.im> writes: > 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 <name>=<value>`, 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 <ps@pks.im> > --- > Documentation/git-config.txt | 16 +++++ > cache.h | 1 + > config.c | 67 +++++++++++++++++--- > environment.c | 1 + > t/t1300-config.sh | 115 ++++++++++++++++++++++++++++++++++- > 5 files changed, 191 insertions(+), 9 deletions(-) > > diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt > index 0e9351d3cb..72ccea4419 100644 > --- a/Documentation/git-config.txt > +++ b/Documentation/git-config.txt > @@ -346,6 +346,22 @@ GIT_CONFIG_NOSYSTEM:: > > See also <<FILES>>. > > +GIT_CONFIG_COUNT:: > +GIT_CONFIG_KEY_<n>:: > +GIT_CONFIG_VALUE_<n>:: > + If GIT_CONFIG_COUNT is set to a positive number, all environment pairs > + GIT_CONFIG_KEY_<n> and GIT_CONFIG_VALUE_<n> up to that number will be > + added to the process's runtime configuration. The config pairs are > + zero-indexed. Any missing key or value is treated as an error. An empty > + GIT_CONFIG_COUNT is treated the same as GIT_CONFIG_COUNT=0, namely no > + pairs are processed. These environment variables will override values > + in configuration files, but will be overridden by any explicit options > + passed via `git -c`. > + > + This is useful for cases where you want to spawn multiple git commands > + with a common configuration but cannot depend on a configuration file, > + for example when writing scripts. Dedent these three lines, and replace the blank lines before it with a line with a single '+' on it (an example is found in the paragraph that describes the "--get-color" option; look for "type=color" in the same file). Otherwise these subsequent paragraphs are treated differently from the first paragraph. The same problem may exist in new paragraphs in git.txt that describes the "--config-env" stuff. Thanks.
Junio C Hamano <gitster@pobox.com> writes: > The same problem may exist in new paragraphs in git.txt that > describes the "--config-env" stuff. Here is what I tentatively queued on top of these 8 patches as a fixup. Thanks. Documentation/git-config.txt | 8 ++++---- Documentation/git.txt | 29 +++++++++++++++-------------- 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt index b71c1ac7b8..67eb40f506 100644 --- a/Documentation/git-config.txt +++ b/Documentation/git-config.txt @@ -348,10 +348,10 @@ GIT_CONFIG_VALUE_<n>:: pairs are processed. These environment variables will override values in configuration files, but will be overridden by any explicit options passed via `git -c`. - - This is useful for cases where you want to spawn multiple git commands - with a common configuration but cannot depend on a configuration file, - for example when writing scripts. ++ +This is useful for cases where you want to spawn multiple git commands +with a common configuration but cannot depend on a configuration file, +for example when writing scripts. [[EXAMPLES]] diff --git a/Documentation/git.txt b/Documentation/git.txt index 80fb8fab11..3b0f87a71b 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -81,25 +81,26 @@ foo.bar= ...`) sets `foo.bar` to the empty string which `git config --type=bool` will convert to `false`. --config-env=<name>=<envvar>:: - Like `-c <name>=<var>` except the value is the name of an + Like `-c <name>=<value>`, give configuration variable + '<name>' a value, where <envvar> is the name of an environment variable from which to retrieve the value. Unlike `-c` there is no shortcut for directly setting the value to an empty string, instead the environment variable itself must be - set to the empty strin. Errors if the `<envvar>` does not exist + set to the empty string. It is an error if the `<envvar>` does not exist in the environment. `<envvar>` may not contain an equals sign to avoid ambiguity with `<name>`s which contain one. - - This is useful for cases where you want to pass transitory - configuration options to git, but are doing so on OS's where - other processes might be able to read your cmdline - (e.g. `/proc/self/cmdline`), but not your environ - (e.g. `/proc/self/environ`). That behavior is the default on - Linux, but may not be on your system. - - Note that this might add security for variables such as - `http.extraHeader` where the sensitive information is part of - the value, but not e.g. `url.<base.insteadOf` where the - sensitive information can be part of the key. ++ +This is useful for cases where you want to pass transitory +configuration options to git, but are doing so on OS's where +other processes might be able to read your cmdline +(e.g. `/proc/self/cmdline`), but not your environ +(e.g. `/proc/self/environ`). That behavior is the default on +Linux, but may not be on your system. ++ +Note that this might add security for variables such as +`http.extraHeader` where the sensitive information is part of +the value, but not e.g. `url.<base>.insteadOf` where the +sensitive information can be part of the key. --exec-path[=<path>]:: Path to wherever your core Git programs are installed.
On Wed, Dec 23, 2020 at 01:55:08PM -0800, Junio C Hamano wrote: > Junio C Hamano <gitster@pobox.com> writes: > > > The same problem may exist in new paragraphs in git.txt that > > describes the "--config-env" stuff. > > Here is what I tentatively queued on top of these 8 patches as a fixup. > > Thanks. Your changes look good to me, thanks! Patrick > > Documentation/git-config.txt | 8 ++++---- > Documentation/git.txt | 29 +++++++++++++++-------------- > 2 files changed, 19 insertions(+), 18 deletions(-) > > diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt > index b71c1ac7b8..67eb40f506 100644 > --- a/Documentation/git-config.txt > +++ b/Documentation/git-config.txt > @@ -348,10 +348,10 @@ GIT_CONFIG_VALUE_<n>:: > pairs are processed. These environment variables will override values > in configuration files, but will be overridden by any explicit options > passed via `git -c`. > - > - This is useful for cases where you want to spawn multiple git commands > - with a common configuration but cannot depend on a configuration file, > - for example when writing scripts. > ++ > +This is useful for cases where you want to spawn multiple git commands > +with a common configuration but cannot depend on a configuration file, > +for example when writing scripts. > > > [[EXAMPLES]] > diff --git a/Documentation/git.txt b/Documentation/git.txt > index 80fb8fab11..3b0f87a71b 100644 > --- a/Documentation/git.txt > +++ b/Documentation/git.txt > @@ -81,25 +81,26 @@ foo.bar= ...`) sets `foo.bar` to the empty string which `git config > --type=bool` will convert to `false`. > > --config-env=<name>=<envvar>:: > - Like `-c <name>=<var>` except the value is the name of an > + Like `-c <name>=<value>`, give configuration variable > + '<name>' a value, where <envvar> is the name of an > environment variable from which to retrieve the value. Unlike > `-c` there is no shortcut for directly setting the value to an > empty string, instead the environment variable itself must be > - set to the empty strin. Errors if the `<envvar>` does not exist > + set to the empty string. It is an error if the `<envvar>` does not exist > in the environment. `<envvar>` may not contain an equals sign > to avoid ambiguity with `<name>`s which contain one. > - > - This is useful for cases where you want to pass transitory > - configuration options to git, but are doing so on OS's where > - other processes might be able to read your cmdline > - (e.g. `/proc/self/cmdline`), but not your environ > - (e.g. `/proc/self/environ`). That behavior is the default on > - Linux, but may not be on your system. > - > - Note that this might add security for variables such as > - `http.extraHeader` where the sensitive information is part of > - the value, but not e.g. `url.<base.insteadOf` where the > - sensitive information can be part of the key. > ++ > +This is useful for cases where you want to pass transitory > +configuration options to git, but are doing so on OS's where > +other processes might be able to read your cmdline > +(e.g. `/proc/self/cmdline`), but not your environ > +(e.g. `/proc/self/environ`). That behavior is the default on > +Linux, but may not be on your system. > ++ > +Note that this might add security for variables such as > +`http.extraHeader` where the sensitive information is part of > +the value, but not e.g. `url.<base>.insteadOf` where the > +sensitive information can be part of the key. > > --exec-path[=<path>]:: > Path to wherever your core Git programs are installed. > -- > 2.30.0-rc1-197-ga312a798fc >
Patrick Steinhardt <ps@pks.im> writes: > On Wed, Dec 23, 2020 at 01:55:08PM -0800, Junio C Hamano wrote: >> Junio C Hamano <gitster@pobox.com> writes: >> >> > The same problem may exist in new paragraphs in git.txt that >> > describes the "--config-env" stuff. >> >> Here is what I tentatively queued on top of these 8 patches as a fixup. >> >> Thanks. > > Your changes look good to me, thanks! You're welcome. Looking forward to seeing a new round with these minor fixes squashed in, so that we do not have a series with known breakages in early parts that are fixed in later steps. Thanks.
diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt index 0e9351d3cb..72ccea4419 100644 --- a/Documentation/git-config.txt +++ b/Documentation/git-config.txt @@ -346,6 +346,22 @@ GIT_CONFIG_NOSYSTEM:: See also <<FILES>>. +GIT_CONFIG_COUNT:: +GIT_CONFIG_KEY_<n>:: +GIT_CONFIG_VALUE_<n>:: + If GIT_CONFIG_COUNT is set to a positive number, all environment pairs + GIT_CONFIG_KEY_<n> and GIT_CONFIG_VALUE_<n> up to that number will be + added to the process's runtime configuration. The config pairs are + zero-indexed. Any missing key or value is treated as an error. An empty + GIT_CONFIG_COUNT is treated the same as GIT_CONFIG_COUNT=0, namely no + pairs are processed. These environment variables will override values + in configuration files, but will be overridden by any explicit options + passed via `git -c`. + + This is useful for cases where you want to spawn multiple git commands + with a common configuration but cannot depend on a configuration file, + for example when writing scripts. + [[EXAMPLES]] EXAMPLES diff --git a/cache.h b/cache.h index 8d279bc110..294841fca7 100644 --- a/cache.h +++ b/cache.h @@ -472,6 +472,7 @@ static inline enum object_type object_type(unsigned int mode) #define TEMPLATE_DIR_ENVIRONMENT "GIT_TEMPLATE_DIR" #define CONFIG_ENVIRONMENT "GIT_CONFIG" #define CONFIG_DATA_ENVIRONMENT "GIT_CONFIG_PARAMETERS" +#define CONFIG_COUNT_ENVIRONMENT "GIT_CONFIG_COUNT" #define EXEC_PATH_ENVIRONMENT "GIT_EXEC_PATH" #define CEILING_DIRECTORIES_ENVIRONMENT "GIT_CEILING_DIRECTORIES" #define NO_REPLACE_OBJECTS_ENVIRONMENT "GIT_NO_REPLACE_OBJECTS" diff --git a/config.c b/config.c index 60a7261807..1742aefa3e 100644 --- a/config.c +++ b/config.c @@ -8,6 +8,7 @@ #include "cache.h" #include "branch.h" #include "config.h" +#include "environment.h" #include "repository.h" #include "lockfile.h" #include "exec-cmd.h" @@ -594,23 +595,73 @@ static int parse_config_env_list(char *env, config_fn_t fn, void *data) int git_config_from_parameters(config_fn_t fn, void *data) { - const char *env = getenv(CONFIG_DATA_ENVIRONMENT); + const char *env; + struct strbuf envvar = STRBUF_INIT; + struct strvec to_free = STRVEC_INIT; int ret = 0; - char *envw; + char *envw = NULL; struct config_source source; - if (!env) - return 0; - memset(&source, 0, sizeof(source)); source.prev = cf; source.origin_type = CONFIG_ORIGIN_CMDLINE; cf = &source; - /* sq_dequote will write over it */ - envw = xstrdup(env); - ret = parse_config_env_list(envw, fn, data); + env = getenv(CONFIG_COUNT_ENVIRONMENT); + if (env) { + unsigned long count; + char *endp; + int i; + count = strtoul(env, &endp, 10); + if (*endp) { + ret = error(_("bogus count in %s"), CONFIG_COUNT_ENVIRONMENT); + goto out; + } + if (count > INT_MAX) { + ret = error(_("too many entries in %s"), CONFIG_COUNT_ENVIRONMENT); + goto out; + } + + for (i = 0; i < count; i++) { + const char *key, *value; + + strbuf_addf(&envvar, "GIT_CONFIG_KEY_%d", i); + key = getenv_safe(&to_free, envvar.buf); + if (!key) { + ret = error(_("missing config key %s"), envvar.buf); + goto out; + } + strbuf_reset(&envvar); + + strbuf_addf(&envvar, "GIT_CONFIG_VALUE_%d", i); + value = getenv_safe(&to_free, envvar.buf); + if (!value) { + ret = error(_("missing config value %s"), envvar.buf); + goto out; + } + strbuf_reset(&envvar); + + if (config_parse_pair(key, value, fn, data) < 0) { + ret = -1; + goto out; + } + } + } + + env = getenv(CONFIG_DATA_ENVIRONMENT); + if (env) { + /* sq_dequote will write over it */ + envw = xstrdup(env); + if (parse_config_env_list(envw, fn, data) < 0) { + ret = -1; + goto out; + } + } + +out: + strbuf_release(&envvar); + strvec_clear(&to_free); free(envw); cf = source.prev; return ret; diff --git a/environment.c b/environment.c index 2234af462c..2f27008424 100644 --- a/environment.c +++ b/environment.c @@ -117,6 +117,7 @@ const char * const local_repo_env[] = { ALTERNATE_DB_ENVIRONMENT, CONFIG_ENVIRONMENT, CONFIG_DATA_ENVIRONMENT, + CONFIG_COUNT_ENVIRONMENT, DB_ENVIRONMENT, GIT_DIR_ENVIRONMENT, GIT_WORK_TREE_ENVIRONMENT, diff --git a/t/t1300-config.sh b/t/t1300-config.sh index 35a1a6e8b1..e06961767f 100755 --- a/t/t1300-config.sh +++ b/t/t1300-config.sh @@ -1421,6 +1421,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 && @@ -1766,9 +1877,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 '
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 <name>=<value>`, 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 <ps@pks.im> --- Documentation/git-config.txt | 16 +++++ cache.h | 1 + config.c | 67 +++++++++++++++++--- environment.c | 1 + t/t1300-config.sh | 115 ++++++++++++++++++++++++++++++++++- 5 files changed, 191 insertions(+), 9 deletions(-)