From 4638728c632e59715b7346ddeca83528d37a4894 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 28 Apr 2016 09:38:20 -0400 Subject: submodule--helper: move config-sanitizing to submodule.c These functions should be used by any code which spawns a submodule process, which may happen in submodule.c (e.g., for spawning fetch). Let's move them there and make them public so that submodule--helper can continue to use them. Since they're now public, let's also provide a basic overview of their intended use. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- submodule.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) (limited to 'submodule.c') diff --git a/submodule.c b/submodule.c index b83939c294..24e8182f59 100644 --- a/submodule.c +++ b/submodule.c @@ -13,6 +13,7 @@ #include "argv-array.h" #include "blob.h" #include "thread-utils.h" +#include "quote.h" static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND; static struct string_list changed_submodule_paths; @@ -1097,3 +1098,50 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir) strbuf_release(&rel_path); free((void *)real_work_tree); } +/* + * Rules to sanitize configuration variables that are Ok to be passed into + * submodule operations from the parent project using "-c". Should only + * include keys which are both (a) safe and (b) necessary for proper + * operation. + */ +static int submodule_config_ok(const char *var) +{ + if (starts_with(var, "credential.")) + return 1; + return 0; +} + +int sanitize_submodule_config(const char *var, const char *value, void *data) +{ + struct strbuf *out = data; + + if (submodule_config_ok(var)) { + if (out->len) + strbuf_addch(out, ' '); + + if (value) + sq_quotef(out, "%s=%s", var, value); + else + sq_quote_buf(out, var); + } + + return 0; +} + +void prepare_submodule_repo_env(struct argv_array *out) +{ + const char * const *var; + + for (var = local_repo_env; *var; var++) { + if (!strcmp(*var, CONFIG_DATA_ENVIRONMENT)) { + struct strbuf sanitized_config = STRBUF_INIT; + git_config_from_parameters(sanitize_submodule_config, + &sanitized_config); + argv_array_pushf(out, "%s=%s", *var, sanitized_config.buf); + strbuf_release(&sanitized_config); + } else { + argv_array_push(out, *var); + } + } + +} -- cgit v1.2.1 From c12e8656700be6084aec49df66447e701fda1ecf Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 28 Apr 2016 09:39:15 -0400 Subject: submodule: use prepare_submodule_repo_env consistently Before 14111fc (git: submodule honor -c credential.* from command line, 2016-02-29), it was sufficient for code which spawned a process in a submodule to just set the child process's "env" field to "local_repo_env" to clear the environment of any repo-specific variables. That commit introduced a more complicated procedure, in which we clear most variables but allow through sanitized config. For C code, we used that procedure only for cloning, but not for any of the programs spawned by submodule.c. As a result, things like "git fetch --recurse-submodules" behave differently than "git clone --recursive"; the former will not pass through the sanitized config. We can fix this by using prepare_submodule_repo_env() everywhere in submodule.c. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- submodule.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'submodule.c') diff --git a/submodule.c b/submodule.c index 24e8182f59..c18ab9b397 100644 --- a/submodule.c +++ b/submodule.c @@ -367,7 +367,7 @@ static int submodule_needs_pushing(const char *path, const unsigned char sha1[20 argv[1] = sha1_to_hex(sha1); cp.argv = argv; - cp.env = local_repo_env; + prepare_submodule_repo_env(&cp.env_array); cp.git_cmd = 1; cp.no_stdin = 1; cp.out = -1; @@ -454,7 +454,7 @@ static int push_submodule(const char *path) const char *argv[] = {"push", NULL}; cp.argv = argv; - cp.env = local_repo_env; + prepare_submodule_repo_env(&cp.env_array); cp.git_cmd = 1; cp.no_stdin = 1; cp.dir = path; @@ -500,7 +500,7 @@ static int is_submodule_commit_present(const char *path, unsigned char sha1[20]) argv[3] = sha1_to_hex(sha1); cp.argv = argv; - cp.env = local_repo_env; + prepare_submodule_repo_env(&cp.env_array); cp.git_cmd = 1; cp.no_stdin = 1; cp.dir = path; @@ -683,7 +683,7 @@ static int get_next_submodule(struct child_process *cp, if (is_directory(git_dir)) { child_process_init(cp); cp->dir = strbuf_detach(&submodule_path, NULL); - cp->env = local_repo_env; + prepare_submodule_repo_env(&cp->env_array); cp->git_cmd = 1; if (!spf->quiet) strbuf_addf(err, "Fetching submodule %s%s\n", @@ -796,7 +796,7 @@ unsigned is_submodule_modified(const char *path, int ignore_untracked) argv[2] = "-uno"; cp.argv = argv; - cp.env = local_repo_env; + prepare_submodule_repo_env(&cp.env_array); cp.git_cmd = 1; cp.no_stdin = 1; cp.out = -1; @@ -857,7 +857,7 @@ int submodule_uses_gitfile(const char *path) /* Now test that all nested submodules use a gitfile too */ cp.argv = argv; - cp.env = local_repo_env; + prepare_submodule_repo_env(&cp.env_array); cp.git_cmd = 1; cp.no_stdin = 1; cp.no_stderr = 1; @@ -890,7 +890,7 @@ int ok_to_remove_submodule(const char *path) return 0; cp.argv = argv; - cp.env = local_repo_env; + prepare_submodule_repo_env(&cp.env_array); cp.git_cmd = 1; cp.no_stdin = 1; cp.out = -1; -- cgit v1.2.1 From 89044baa8b8a14b48e78a42ebdc43cfcd144ce28 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 4 May 2016 21:22:19 -0400 Subject: submodule: stop sanitizing config options The point of having a whitelist of command-line config options to pass to submodules was two-fold: 1. It prevented obvious nonsense like using core.worktree for multiple repos. 2. It could prevent surprise when the user did not mean for the options to leak to the submodules (e.g., http.sslverify=false). For case 1, the answer is mostly "if it hurts, don't do that". For case 2, we can note that any such example has a matching inverted surprise (e.g., a user who meant http.sslverify=true to apply everywhere, but it didn't). So this whitelist is probably not giving us any benefit, and is already creating a hassle as people propose things to put on it. Let's just drop it entirely. Note that we still need to keep a special code path for "prepare the submodule environment", because we still have to take care to pass through $GIT_CONFIG_PARAMETERS (and block the rest of the repo-specific environment variables). We can do this easily from within the submodule shell script, which lets us drop the submodule--helper option entirely (and it's OK to do so because as a "--" program, it is entirely a private implementation detail). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- submodule.c | 39 +-------------------------------------- 1 file changed, 1 insertion(+), 38 deletions(-) (limited to 'submodule.c') diff --git a/submodule.c b/submodule.c index c18ab9b397..d598881114 100644 --- a/submodule.c +++ b/submodule.c @@ -1098,50 +1098,13 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir) strbuf_release(&rel_path); free((void *)real_work_tree); } -/* - * Rules to sanitize configuration variables that are Ok to be passed into - * submodule operations from the parent project using "-c". Should only - * include keys which are both (a) safe and (b) necessary for proper - * operation. - */ -static int submodule_config_ok(const char *var) -{ - if (starts_with(var, "credential.")) - return 1; - return 0; -} - -int sanitize_submodule_config(const char *var, const char *value, void *data) -{ - struct strbuf *out = data; - - if (submodule_config_ok(var)) { - if (out->len) - strbuf_addch(out, ' '); - - if (value) - sq_quotef(out, "%s=%s", var, value); - else - sq_quote_buf(out, var); - } - - return 0; -} void prepare_submodule_repo_env(struct argv_array *out) { const char * const *var; for (var = local_repo_env; *var; var++) { - if (!strcmp(*var, CONFIG_DATA_ENVIRONMENT)) { - struct strbuf sanitized_config = STRBUF_INIT; - git_config_from_parameters(sanitize_submodule_config, - &sanitized_config); - argv_array_pushf(out, "%s=%s", *var, sanitized_config.buf); - strbuf_release(&sanitized_config); - } else { + if (strcmp(*var, CONFIG_DATA_ENVIRONMENT)) argv_array_push(out, *var); - } } - } -- cgit v1.2.1