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

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 35ae85a7e1bcc3d3d12f36cae439f7fbf98d8a4a..3d37c3f1822db14d376a19afd443c3082e8c5a8e 100644 (file)
--- 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;
 }
 
        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)
 {
 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;
        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);
        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;
 }
 
        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 *);


@@ -268,6 +333,7 @@ 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 2dd29b3df0e65ae14c51952defe0184b43b2b9c8..1f132b489b81eb0bb7a4a6a5b7d3a5b88a77f53a 100755 (executable)
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -192,6 +192,16 @@ isnumber()
        n=$(($1 + 0)) 2>/dev/null && test "$n" = "$1"
 }
 
        n=$(($1 + 0)) 2>/dev/null && test "$n" = "$1"
 }
 

+# Sanitize the local git environment for use within a submodule. We
+# can't simply use clear_local_git_env since we want to preserve some
+# of the settings from GIT_CONFIG_PARAMETERS.
+sanitize_submodule_env()
+{
+       sanitized_config=$(git submodule--helper sanitize-config)
+       clear_local_git_env
+       GIT_CONFIG_PARAMETERS=$sanitized_config
+}
+

 #
 # Add a new submodule to the working tree, .gitmodules and the index
 #
 #
 # Add a new submodule to the working tree, .gitmodules and the index
 #


@@ -349,7 +359,7 @@ Use -f if you really want to add it." >&2
                fi
                git submodule--helper clone ${GIT_QUIET:+--quiet} --prefix "$wt_prefix" --path "$sm_path" --name "$sm_name" --url "$realrepo" ${reference:+"$reference"} ${depth:+"$depth"} || exit
                (
                fi
                git submodule--helper clone ${GIT_QUIET:+--quiet} --prefix "$wt_prefix" --path "$sm_path" --name "$sm_name" --url "$realrepo" ${reference:+"$reference"} ${depth:+"$depth"} || exit
                (

-                       clear_local_git_env
+                       sanitize_submodule_env

                        cd "$sm_path" &&
                        # ash fails to wordsplit ${branch:+-b "$branch"...}
                        case "$branch" in
                        cd "$sm_path" &&
                        # ash fails to wordsplit ${branch:+-b "$branch"...}
                        case "$branch" in


@@ -418,7 +428,7 @@ cmd_foreach()
                        name=$(git submodule--helper name "$sm_path")
                        (
                                prefix="$prefix$sm_path/"
                        name=$(git submodule--helper name "$sm_path")
                        (
                                prefix="$prefix$sm_path/"

-                               clear_local_git_env
+                               sanitize_submodule_env

                                cd "$sm_path" &&
                                sm_path=$(relative_path "$sm_path") &&
                                # we make $path available to scripts ...
                                cd "$sm_path" &&
                                sm_path=$(relative_path "$sm_path") &&
                                # we make $path available to scripts ...


@@ -713,7 +723,7 @@ Maybe you want to use 'update --init'?")"
                        cloned_modules="$cloned_modules;$name"
                        subsha1=
                else
                        cloned_modules="$cloned_modules;$name"
                        subsha1=
                else

-                       subsha1=$(clear_local_git_env; cd "$sm_path" &&
+                       subsha1=$(sanitize_submodule_env; cd "$sm_path" &&

                                git rev-parse --verify HEAD) ||
                        die "$(eval_gettext "Unable to find current revision in submodule path '\$displaypath'")"
                fi
                                git rev-parse --verify HEAD) ||
                        die "$(eval_gettext "Unable to find current revision in submodule path '\$displaypath'")"
                fi


@@ -723,11 +733,11 @@ Maybe you want to use 'update --init'?")"
                        if test -z "$nofetch"
                        then
                                # Fetch remote before determining tracking $sha1
                        if test -z "$nofetch"
                        then
                                # Fetch remote before determining tracking $sha1

-                               (clear_local_git_env; cd "$sm_path" && git-fetch) ||
+                               (sanitize_submodule_env; cd "$sm_path" && git-fetch) ||

                                die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")"
                        fi
                                die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")"
                        fi

-                       remote_name=$(clear_local_git_env; cd "$sm_path" && get_default_remote)
-                       sha1=$(clear_local_git_env; cd "$sm_path" &&
+                       remote_name=$(sanitize_submodule_env; cd "$sm_path" && get_default_remote)
+                       sha1=$(sanitize_submodule_env; cd "$sm_path" &&

                                git rev-parse --verify "${remote_name}/${branch}") ||
                        die "$(eval_gettext "Unable to find current ${remote_name}/${branch} revision in submodule path '\$sm_path'")"
                fi
                                git rev-parse --verify "${remote_name}/${branch}") ||
                        die "$(eval_gettext "Unable to find current ${remote_name}/${branch} revision in submodule path '\$sm_path'")"
                fi


@@ -745,7 +755,7 @@ Maybe you want to use 'update --init'?")"
                        then
                                # Run fetch only if $sha1 isn't present or it
                                # is not reachable from a ref.
                        then
                                # Run fetch only if $sha1 isn't present or it
                                # is not reachable from a ref.

-                               (clear_local_git_env; cd "$sm_path" &&
+                               (sanitize_submodule_env; cd "$sm_path" &&

                                        ( (rev=$(git rev-list -n 1 $sha1 --not --all 2>/dev/null) &&
                                         test -z "$rev") || git-fetch)) ||
                                die "$(eval_gettext "Unable to fetch in submodule path '\$displaypath'")"
                                        ( (rev=$(git rev-list -n 1 $sha1 --not --all 2>/dev/null) &&
                                         test -z "$rev") || git-fetch)) ||
                                die "$(eval_gettext "Unable to fetch in submodule path '\$displaypath'")"


@@ -787,7 +797,7 @@ Maybe you want to use 'update --init'?")"
                                die "$(eval_gettext "Invalid update mode '$update_module' for submodule '$name'")"
                        esac
 
                                die "$(eval_gettext "Invalid update mode '$update_module' for submodule '$name'")"
                        esac
 

-                       if (clear_local_git_env; cd "$sm_path" && $command "$sha1")
+                       if (sanitize_submodule_env; cd "$sm_path" && $command "$sha1")

                        then
                                say "$say_msg"
                        elif test -n "$must_die_on_failure"
                        then
                                say "$say_msg"
                        elif test -n "$must_die_on_failure"


@@ -803,7 +813,7 @@ Maybe you want to use 'update --init'?")"
                then
                        (
                                prefix="$prefix$sm_path/"
                then
                        (
                                prefix="$prefix$sm_path/"

-                               clear_local_git_env
+                               sanitize_submodule_env

                                cd "$sm_path" &&
                                eval cmd_update
                        )
                                cd "$sm_path" &&
                                eval cmd_update
                        )


@@ -841,7 +851,7 @@ Maybe you want to use 'update --init'?")"
 
 set_name_rev () {
        revname=$( (
 
 set_name_rev () {
        revname=$( (

-               clear_local_git_env
+               sanitize_submodule_env

                cd "$1" && {
                        git describe "$2" 2>/dev/null ||
                        git describe --tags "$2" 2>/dev/null ||
                cd "$1" && {
                        git describe "$2" 2>/dev/null ||
                        git describe --tags "$2" 2>/dev/null ||


@@ -1125,7 +1135,7 @@ cmd_status()
                else
                        if test -z "$cached"
                        then
                else
                        if test -z "$cached"
                        then

-                               sha1=$(clear_local_git_env; cd "$sm_path" && git rev-parse --verify HEAD)
+                               sha1=$(sanitize_submodule_env; cd "$sm_path" && git rev-parse --verify HEAD)

                        fi
                        set_name_rev "$sm_path" "$sha1"
                        say "+$sha1 $displaypath$revname"
                        fi
                        set_name_rev "$sm_path" "$sha1"
                        say "+$sha1 $displaypath$revname"


@@ -1135,7 +1145,7 @@ cmd_status()
                then
                        (
                                prefix="$displaypath/"
                then
                        (
                                prefix="$displaypath/"

-                               clear_local_git_env
+                               sanitize_submodule_env

                                cd "$sm_path" &&
                                eval cmd_status
                        ) ||
                                cd "$sm_path" &&
                                eval cmd_status
                        ) ||


@@ -1209,7 +1219,7 @@ cmd_sync()
                        if test -e "$sm_path"/.git
                        then
                        (
                        if test -e "$sm_path"/.git
                        then
                        (

-                               clear_local_git_env
+                               sanitize_submodule_env

                                cd "$sm_path"
                                remote=$(get_default_remote)
                                git config remote."$remote".url "$sub_origin_url"
                                cd "$sm_path"
                                remote=$(get_default_remote)
                                git config remote."$remote".url "$sub_origin_url"

diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh
index 64146352ae20e9a5abdcda77be514d0f3264c3b8..48e2ab62da7a4293dd647524f75f3590783bcb02 100755 (executable)
--- a/t/t5550-http-fetch-dumb.sh
+++ b/t/t5550-http-fetch-dumb.sh
@@ -91,6 +91,23 @@ test_expect_success 'configured username does not override URL' '
        expect_askpass pass user@host
 '
 
        expect_askpass pass user@host
 '
 

+test_expect_success 'cmdline credential config passes into submodules' '
+       git init super &&
+       set_askpass user@host pass@host &&
+       (
+               cd super &&
+               git submodule add "$HTTPD_URL/auth/dumb/repo.git" sub &&
+               git commit -m "add submodule"
+       ) &&
+       set_askpass wrong pass@host &&
+       test_must_fail git clone --recursive super super-clone &&
+       rm -rf super-clone &&
+       set_askpass wrong pass@host &&
+       git -c "credential.$HTTP_URL.username=user@host" \
+               clone --recursive super super-clone &&
+       expect_askpass pass user@host
+'
+

 test_expect_success 'fetch changes via http' '
        echo content >>file &&
        git commit -a -m two &&
 test_expect_success 'fetch changes via http' '
        echo content >>file &&
        git commit -a -m two &&

diff --git a/t/t7412-submodule--helper.sh b/t/t7412-submodule--helper.sh
new file mode 100755 (executable)
index 0000000..149d428
--- /dev/null
+++ b/t/t7412-submodule--helper.sh
@@ -0,0 +1,26 @@
+#!/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