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 <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 16d6432f911fce621c6adceae84a1411ea4dabc8..89250f0b782c7328cc411c7d19b86e3978b01032 100644 (file)
--- 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;
 }
 
        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 *);
 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},
        {"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)
 };
 
 int cmd_submodule__helper(int argc, const char **argv, const char *prefix)

diff --git a/git-submodule.sh b/git-submodule.sh
index 91f5856df8c2cb07c6d2849acfe9f107991e3763..b1c056c71567522e7b1bdb94bacf10f70743e024 100755 (executable)
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -197,9 +197,9 @@ isnumber()
 # of the settings from GIT_CONFIG_PARAMETERS.
 sanitize_submodule_env()
 {
 # of the settings from GIT_CONFIG_PARAMETERS.
 sanitize_submodule_env()
 {

-       sanitized_config=$(git submodule--helper sanitize-config)
+       save_config=$GIT_CONFIG_PARAMETERS

        clear_local_git_env
        clear_local_git_env

-       GIT_CONFIG_PARAMETERS=$sanitized_config
+       GIT_CONFIG_PARAMETERS=$save_config

        export GIT_CONFIG_PARAMETERS
 }
 
        export GIT_CONFIG_PARAMETERS
 }
 

diff --git a/submodule.c b/submodule.c
index c18ab9b3977e511f695770aafa43b11d0a673e63..d5988811142a29a56a20535b0532e36998d9020f 100644 (file)
--- 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);
 }
        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++) {
 
 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);
                        argv_array_push(out, *var);

-               }

        }
        }

-

 }
 }

diff --git a/submodule.h b/submodule.h
index 48690b156a2ef9c97a5fad5cad6e79a231becf2d..869d259fac5ecc74532549c57add0b692a995728 100644 (file)
--- a/submodule.h
+++ b/submodule.h
@@ -43,19 +43,10 @@ int find_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_nam
 int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name);
 void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);
 
 int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name);
 void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);
 

-/*
- * This function is intended as a callback for use with
- * git_config_from_parameters(). It ignores any config options which
- * are not suitable for passing along to a submodule, and accumulates the rest
- * in "data", which must be a pointer to a strbuf. The end result can
- * be put into $GIT_CONFIG_PARAMETERS for passing to a sub-process.
- */
-int sanitize_submodule_config(const char *var, const char *value, void *data);
-

 /*
  * Prepare the "env_array" parameter of a "struct child_process" for executing
  * a submodule by clearing any repo-specific envirionment variables, but
 /*
  * Prepare the "env_array" parameter of a "struct child_process" for executing
  * a submodule by clearing any repo-specific envirionment variables, but

- * retaining any config approved by sanitize_submodule_config().
+ * retaining any config in the environment.

  */
 void prepare_submodule_repo_env(struct argv_array *out);
 
  */
 void prepare_submodule_repo_env(struct argv_array *out);
 

diff --git a/t/t7412-submodule--helper.sh b/t/t7412-submodule--helper.sh
deleted file mode 100755 (executable)
index 149d428..0000000
--- a/t/t7412-submodule--helper.sh
+++ /dev/null
@@ -1,26 +0,0 @@
-#!/bin/sh
-#
-# Copyright (c) 2016 Jacob Keller
-#
-
-test_description='Basic plumbing support of submodule--helper
-
-This test verifies the submodule--helper plumbing command used to implement
-git-submodule.
-'
-
-. ./test-lib.sh
-
-test_expect_success 'sanitize-config clears configuration' '
-       git -c user.name="Some User" submodule--helper sanitize-config >actual &&
-       test_must_be_empty actual
-'
-
-sq="'"
-test_expect_success 'sanitize-config keeps credential.helper' '
-       git -c credential.helper=helper submodule--helper sanitize-config >actual &&
-       echo "${sq}credential.helper=helper${sq}" >expect &&
-       test_cmp expect actual
-'
-
-test_done