From 08e0970a862aaa3b42c532ad0086bcaabe5108cf Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Mon, 29 Feb 2016 14:58:31 -0800 Subject: submodule: check argc count for git submodule--helper clone Extra unused arguments to git submodule--helper clone subcommand were being silently ignored. Add a check to the argc count after options handling to ensure that no extra arguments were left on the argv array. Signed-off-by: Jacob Keller Reviewed-by: Stefan Beller Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'builtin') diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index f4c3eff179..1e18075ed9 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -194,6 +194,10 @@ static int module_clone(int argc, const char **argv, const char *prefix) argc = parse_options(argc, argv, prefix, module_clone_options, git_submodule_helper_usage, 0); + if (argc) + usage_with_options(git_submodule_helper_usage, + module_clone_options); + strbuf_addf(&sb, "%s/modules/%s", get_git_dir(), name); sm_gitdir = strbuf_detach(&sb, NULL); -- cgit v1.2.3 From 717416ca87edcd3889e40d8191fd4b6721e8a02e Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Mon, 29 Feb 2016 14:58:32 -0800 Subject: submodule: fix submodule--helper clone usage git submodule--helper clone usage stated that paths were added after the [--] argument. The actual implementation required use of --path argument and only supports one path at a time. Update the usage string to match the current implementation. Signed-off-by: Jacob Keller Reviewed-by: Stefan Beller Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'builtin') diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 1e18075ed9..3c4d3ff7f4 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -187,7 +187,7 @@ static int module_clone(int argc, const char **argv, const char *prefix) const char *const git_submodule_helper_usage[] = { N_("git submodule--helper clone [--prefix=] [--quiet] " "[--reference ] [--name ] [--url ]" - "[--depth ] [--] [...]"), + "[--depth ] [--path ]"), NULL }; -- cgit v1.2.3 From 7dad2633348423191844eef49022a9013242d6ef Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Mon, 29 Feb 2016 14:58:33 -0800 Subject: submodule: fix segmentation fault in submodule--helper clone The git submodule--helper clone command will fail with a segmentation fault when given a null url or null path variable. Since these are required for proper functioning of the submodule--helper clone subcommand, add checks to prevent running and fail gracefully when missing. Update the usage string to reflect the requirement that the --url and --path "options" are required. Signed-off-by: Jacob Keller Reviewed-by: Stefan Beller Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'builtin') diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 3c4d3ff7f4..35ae85a7e1 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -186,15 +186,15 @@ static int module_clone(int argc, const char **argv, const char *prefix) const char *const git_submodule_helper_usage[] = { N_("git submodule--helper clone [--prefix=] [--quiet] " - "[--reference ] [--name ] [--url ]" - "[--depth ] [--path ]"), + "[--reference ] [--name ] [--depth ] " + "--url --path "), NULL }; argc = parse_options(argc, argv, prefix, module_clone_options, git_submodule_helper_usage, 0); - if (argc) + if (argc || !url || !path) usage_with_options(git_submodule_helper_usage, module_clone_options); -- cgit v1.2.3 From 14111fc49272a70ceaeb5039796fbceb8a6e1cb7 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Mon, 29 Feb 2016 14:58:35 -0800 Subject: git: submodule honor -c credential.* from command line Due to the way that the git-submodule code works, it clears all local git environment variables before entering submodules. This is normally a good thing since we want to clear settings such as GIT_WORKTREE and other variables which would affect the operation of submodule commands. However, GIT_CONFIG_PARAMETERS is special, and we actually do want to preserve these settings. However, we do not want to preserve all configuration as many things should be left specific to the parent project. Add a git submodule--helper function, sanitize-config, which shall be used to sanitize GIT_CONFIG_PARAMETERS, removing all key/value pairs except a small subset that are known to be safe and necessary. Replace all the calls to clear_local_git_env with a wrapped function that filters GIT_CONFIG_PARAMETERS using the new helper and then restores it to the filtered subset after clearing the rest of the environment. Signed-off-by: Jacob Keller Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 68 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 67 insertions(+), 1 deletion(-) (limited to 'builtin') diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 35ae85a7e1..3d37c3f182 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -124,6 +124,55 @@ static int module_name(int argc, const char **argv, const char *prefix) return 0; } + +/* + * 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; +} + +static 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; +} + +static 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); + } + } + +} + static int clone_submodule(const char *path, const char *gitdir, const char *url, const char *depth, const char *reference, int quiet) { @@ -145,7 +194,7 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url argv_array_push(&cp.args, path); cp.git_cmd = 1; - cp.env = local_repo_env; + prepare_submodule_repo_env(&cp.env_array); cp.no_stdin = 1; return run_command(&cp); @@ -259,6 +308,22 @@ static int module_clone(int argc, const char **argv, const char *prefix) return 0; } +static int module_sanitize_config(int argc, const char **argv, const char *prefix) +{ + struct strbuf sanitized_config = STRBUF_INIT; + + if (argc > 1) + usage(_("git submodule--helper sanitize-config")); + + git_config_from_parameters(sanitize_submodule_config, &sanitized_config); + if (sanitized_config.len) + printf("%s\n", sanitized_config.buf); + + strbuf_release(&sanitized_config); + + return 0; +} + struct cmd_struct { const char *cmd; int (*fn)(int, const char **, const char *); @@ -268,6 +333,7 @@ static struct cmd_struct commands[] = { {"list", module_list}, {"name", module_name}, {"clone", module_clone}, + {"sanitize-config", module_sanitize_config}, }; int cmd_submodule__helper(int argc, const char **argv, const char *prefix) -- cgit v1.2.3 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 --- builtin/submodule--helper.c | 48 --------------------------------------------- 1 file changed, 48 deletions(-) (limited to 'builtin') diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 3d37c3f182..16d6432f91 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -125,54 +125,6 @@ static int module_name(int argc, const char **argv, const char *prefix) return 0; } -/* - * 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; -} - -static 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; -} - -static 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); - } - } - -} - static int clone_submodule(const char *path, const char *gitdir, const char *url, const char *depth, const char *reference, int quiet) { -- cgit v1.2.3 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 --- builtin/submodule--helper.c | 17 ----------------- 1 file changed, 17 deletions(-) (limited to 'builtin') diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 16d6432f91..89250f0b78 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -260,22 +260,6 @@ static int module_clone(int argc, const char **argv, const char *prefix) return 0; } -static int module_sanitize_config(int argc, const char **argv, const char *prefix) -{ - struct strbuf sanitized_config = STRBUF_INIT; - - if (argc > 1) - usage(_("git submodule--helper sanitize-config")); - - git_config_from_parameters(sanitize_submodule_config, &sanitized_config); - if (sanitized_config.len) - printf("%s\n", sanitized_config.buf); - - strbuf_release(&sanitized_config); - - return 0; -} - struct cmd_struct { const char *cmd; int (*fn)(int, const char **, const char *); @@ -285,7 +269,6 @@ static struct cmd_struct commands[] = { {"list", module_list}, {"name", module_name}, {"clone", module_clone}, - {"sanitize-config", module_sanitize_config}, }; int cmd_submodule__helper(int argc, const char **argv, const char *prefix) -- cgit v1.2.3