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>

Jacob Keller committed Feb 29, 2016 at 14:58 UTC 14111fc49272a70ceaeb5039796fbceb8a6e1cb7
4 files changed +133 -14
builtin/submodule--helper.c
+67 -1
@@ -124,6 +124,55 @@ static int module_name(int argc, const char **argv, const char *prefix)
124
125 return 0;
126 }
127 +
128 +/*
129 + * Rules to sanitize configuration variables that are Ok to be passed into
130 + * submodule operations from the parent project using "-c". Should only
131 + * include keys which are both (a) safe and (b) necessary for proper
132 + * operation.
133 + */
134 +static int submodule_config_ok(const char *var)
135 +{
136 + if (starts_with(var, "credential."))
137 + return 1;
138 + return 0;
139 +}
140 +
141 +static int sanitize_submodule_config(const char *var, const char *value, void *data)
142 +{
143 + struct strbuf *out = data;
144 +
145 + if (submodule_config_ok(var)) {
146 + if (out->len)
147 + strbuf_addch(out, ' ');
148 +
149 + if (value)
150 + sq_quotef(out, "%s=%s", var, value);
151 + else
152 + sq_quote_buf(out, var);
153 + }
154 +
155 + return 0;
156 +}
157 +
158 +static void prepare_submodule_repo_env(struct argv_array *out)
159 +{
160 + const char * const *var;
161 +
162 + for (var = local_repo_env; *var; var++) {
163 + if (!strcmp(*var, CONFIG_DATA_ENVIRONMENT)) {
164 + struct strbuf sanitized_config = STRBUF_INIT;
165 + git_config_from_parameters(sanitize_submodule_config,
166 + &sanitized_config);
167 + argv_array_pushf(out, "%s=%s", *var, sanitized_config.buf);
168 + strbuf_release(&sanitized_config);
169 + } else {
170 + argv_array_push(out, *var);
171 + }
172 + }
173 +
174 +}
175 +
176 static int clone_submodule(const char *path, const char *gitdir, const char *url,
177 const char *depth, const char *reference, int quiet)
178 {
@@ -145,7 +194,7 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url
194 argv_array_push(&cp.args, path);
195
196 cp.git_cmd = 1;
148 - cp.env = local_repo_env;
197 + prepare_submodule_repo_env(&cp.env_array);
198 cp.no_stdin = 1;
199
200 return run_command(&cp);
@@ -259,6 +308,22 @@ static int module_clone(int argc, const char **argv, const char *prefix)
308 return 0;
309 }
310
311 +static int module_sanitize_config(int argc, const char **argv, const char *prefix)
312 +{
313 + struct strbuf sanitized_config = STRBUF_INIT;
314 +
315 + if (argc > 1)
316 + usage(_("git submodule--helper sanitize-config"));
317 +
318 + git_config_from_parameters(sanitize_submodule_config, &sanitized_config);
319 + if (sanitized_config.len)
320 + printf("%s\n", sanitized_config.buf);
321 +
322 + strbuf_release(&sanitized_config);
323 +
324 + return 0;
325 +}
326 +
327 struct cmd_struct {
328 const char *cmd;
329 int (*fn)(int, const char **, const char *);
@@ -268,6 +333,7 @@ static struct cmd_struct commands[] = {
333 {"list", module_list},
334 {"name", module_name},
335 {"clone", module_clone},
336 + {"sanitize-config", module_sanitize_config},
337 };
338
339 int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
git-submodule.sh
+23 -13
@@ -192,6 +192,16 @@ isnumber()
192 n=$(($1 + 0)) 2>/dev/null && test "$n" = "$1"
193 }
194
195 +# Sanitize the local git environment for use within a submodule. We
196 +# can't simply use clear_local_git_env since we want to preserve some
197 +# of the settings from GIT_CONFIG_PARAMETERS.
198 +sanitize_submodule_env()
199 +{
200 + sanitized_config=$(git submodule--helper sanitize-config)
201 + clear_local_git_env
202 + GIT_CONFIG_PARAMETERS=$sanitized_config
203 +}
204 +
205 #
206 # Add a new submodule to the working tree, .gitmodules and the index
207 #
@@ -349,7 +359,7 @@ Use -f if you really want to add it." >&2
359 fi
360 git submodule--helper clone ${GIT_QUIET:+--quiet} --prefix "$wt_prefix" --path "$sm_path" --name "$sm_name" --url "$realrepo" ${reference:+"$reference"} ${depth:+"$depth"} || exit
361 (
352 - clear_local_git_env
362 + sanitize_submodule_env
363 cd "$sm_path" &&
364 # ash fails to wordsplit ${branch:+-b "$branch"...}
365 case "$branch" in
@@ -418,7 +428,7 @@ cmd_foreach()
428 name=$(git submodule--helper name "$sm_path")
429 (
430 prefix="$prefix$sm_path/"
421 - clear_local_git_env
431 + sanitize_submodule_env
432 cd "$sm_path" &&
433 sm_path=$(relative_path "$sm_path") &&
434 # we make $path available to scripts ...
@@ -713,7 +723,7 @@ Maybe you want to use 'update --init'?")"
723 cloned_modules="$cloned_modules;$name"
724 subsha1=
725 else
716 - subsha1=$(clear_local_git_env; cd "$sm_path" &&
726 + subsha1=$(sanitize_submodule_env; cd "$sm_path" &&
727 git rev-parse --verify HEAD) ||
728 die "$(eval_gettext "Unable to find current revision in submodule path '\$displaypath'")"
729 fi
@@ -723,11 +733,11 @@ Maybe you want to use 'update --init'?")"
733 if test -z "$nofetch"
734 then
735 # Fetch remote before determining tracking $sha1
726 - (clear_local_git_env; cd "$sm_path" && git-fetch) ||
736 + (sanitize_submodule_env; cd "$sm_path" && git-fetch) ||
737 die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")"
738 fi
729 - remote_name=$(clear_local_git_env; cd "$sm_path" && get_default_remote)
730 - sha1=$(clear_local_git_env; cd "$sm_path" &&
739 + remote_name=$(sanitize_submodule_env; cd "$sm_path" && get_default_remote)
740 + sha1=$(sanitize_submodule_env; cd "$sm_path" &&
741 git rev-parse --verify "${remote_name}/${branch}") ||
742 die "$(eval_gettext "Unable to find current ${remote_name}/${branch} revision in submodule path '\$sm_path'")"
743 fi
@@ -745,7 +755,7 @@ Maybe you want to use 'update --init'?")"
755 then
756 # Run fetch only if $sha1 isn't present or it
757 # is not reachable from a ref.
748 - (clear_local_git_env; cd "$sm_path" &&
758 + (sanitize_submodule_env; cd "$sm_path" &&
759 ( (rev=$(git rev-list -n 1 $sha1 --not --all 2>/dev/null) &&
760 test -z "$rev") || git-fetch)) ||
761 die "$(eval_gettext "Unable to fetch in submodule path '\$displaypath'")"
@@ -787,7 +797,7 @@ Maybe you want to use 'update --init'?")"
797 die "$(eval_gettext "Invalid update mode '$update_module' for submodule '$name'")"
798 esac
799
790 - if (clear_local_git_env; cd "$sm_path" && $command "$sha1")
800 + if (sanitize_submodule_env; cd "$sm_path" && $command "$sha1")
801 then
802 say "$say_msg"
803 elif test -n "$must_die_on_failure"
@@ -803,7 +813,7 @@ Maybe you want to use 'update --init'?")"
813 then
814 (
815 prefix="$prefix$sm_path/"
806 - clear_local_git_env
816 + sanitize_submodule_env
817 cd "$sm_path" &&
818 eval cmd_update
819 )
@@ -841,7 +851,7 @@ Maybe you want to use 'update --init'?")"
851
852 set_name_rev () {
853 revname=$( (
844 - clear_local_git_env
854 + sanitize_submodule_env
855 cd "$1" && {
856 git describe "$2" 2>/dev/null ||
857 git describe --tags "$2" 2>/dev/null ||
@@ -1125,7 +1135,7 @@ cmd_status()
1135 else
1136 if test -z "$cached"
1137 then
1128 - sha1=$(clear_local_git_env; cd "$sm_path" && git rev-parse --verify HEAD)
1138 + sha1=$(sanitize_submodule_env; cd "$sm_path" && git rev-parse --verify HEAD)
1139 fi
1140 set_name_rev "$sm_path" "$sha1"
1141 say "+$sha1 $displaypath$revname"
@@ -1135,7 +1145,7 @@ cmd_status()
1145 then
1146 (
1147 prefix="$displaypath/"
1138 - clear_local_git_env
1148 + sanitize_submodule_env
1149 cd "$sm_path" &&
1150 eval cmd_status
1151 ) ||
@@ -1209,7 +1219,7 @@ cmd_sync()
1219 if test -e "$sm_path"/.git
1220 then
1221 (
1212 - clear_local_git_env
1222 + sanitize_submodule_env
1223 cd "$sm_path"
1224 remote=$(get_default_remote)
1225 git config remote."$remote".url "$sub_origin_url"
t/t5550-http-fetch-dumb.sh
+17
@@ -91,6 +91,23 @@ test_expect_success 'configured username does not override URL' '
91 expect_askpass pass user@host
92 '
93
94 +test_expect_success 'cmdline credential config passes into submodules' '
95 + git init super &&
96 + set_askpass user@host pass@host &&
97 + (
98 + cd super &&
99 + git submodule add "$HTTPD_URL/auth/dumb/repo.git" sub &&
100 + git commit -m "add submodule"
101 + ) &&
102 + set_askpass wrong pass@host &&
103 + test_must_fail git clone --recursive super super-clone &&
104 + rm -rf super-clone &&
105 + set_askpass wrong pass@host &&
106 + git -c "credential.$HTTP_URL.username=user@host" \
107 + clone --recursive super super-clone &&
108 + expect_askpass pass user@host
109 +'
110 +
111 test_expect_success 'fetch changes via http' '
112 echo content >>file &&
113 git commit -a -m two &&
t/t7412-submodule--helper.sh new
+26
@@ -0,0 +1,26 @@
1 +#!/bin/sh
2 +#
3 +# Copyright (c) 2016 Jacob Keller
4 +#
5 +
6 +test_description='Basic plumbing support of submodule--helper
7 +
8 +This test verifies the submodule--helper plumbing command used to implement
9 +git-submodule.
10 +'
11 +
12 +. ./test-lib.sh
13 +
14 +test_expect_success 'sanitize-config clears configuration' '
15 + git -c user.name="Some User" submodule--helper sanitize-config >actual &&
16 + test_must_be_empty actual
17 +'
18 +
19 +sq="'"
20 +test_expect_success 'sanitize-config keeps credential.helper' '
21 + git -c credential.helper=helper submodule--helper sanitize-config >actual &&
22 + echo "${sq}credential.helper=helper${sq}" >expect &&
23 + test_cmp expect actual
24 +'
25 +
26 +test_done