@matheusbsbs / matheus-bagliano / commits / 89044baa8b

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>

Jeff King committed May 4, 2016 at 21:22 UTC 89044baa8b8a14b48e78a42ebdc43cfcd144ce28

5 files changed +4 -93

builtin/submodule--helper.c

-17

@@ -260,22 +260,6 @@ static int module_clone(int argc, const char **argv, const char *prefix)

260 return 0;

261 }

262

263 -static int module_sanitize_config(int argc, const char **argv, const char *prefix)

264 -{

265 - struct strbuf sanitized_config = STRBUF_INIT;

266 -

267 - if (argc > 1)

268 - usage(_("git submodule--helper sanitize-config"));

269 -

270 - git_config_from_parameters(sanitize_submodule_config, &sanitized_config);

271 - if (sanitized_config.len)

272 - printf("%s\n", sanitized_config.buf);

273 -

274 - strbuf_release(&sanitized_config);

275 -

276 - return 0;

277 -}

278 -

263 struct cmd_struct {

264 const char *cmd;

265 int (*fn)(int, const char **, const char *);

@@ -285,7 +269,6 @@ static struct cmd_struct commands[] = {

269 {"list", module_list},

270 {"name", module_name},

271 {"clone", module_clone},

288 - {"sanitize-config", module_sanitize_config},

272 };

273

274 int cmd_submodule__helper(int argc, const char **argv, const char *prefix)

git-submodule.sh

+2 -2

@@ -197,9 +197,9 @@ isnumber()

197 # of the settings from GIT_CONFIG_PARAMETERS.

198 sanitize_submodule_env()

199 {

200 - sanitized_config=$(git submodule--helper sanitize-config)

200 + save_config=$GIT_CONFIG_PARAMETERS

201 clear_local_git_env

202 - GIT_CONFIG_PARAMETERS=$sanitized_config

202 + GIT_CONFIG_PARAMETERS=$save_config

203 export GIT_CONFIG_PARAMETERS

204 }

205

submodule.c

+1 -38

@@ -1098,50 +1098,13 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)

1098 strbuf_release(&rel_path);

1099 free((void *)real_work_tree);

1100 }

1101 -/*

1102 - * Rules to sanitize configuration variables that are Ok to be passed into

1103 - * submodule operations from the parent project using "-c". Should only

1104 - * include keys which are both (a) safe and (b) necessary for proper

1105 - * operation.

1106 - */

1107 -static int submodule_config_ok(const char *var)

1108 -{

1109 - if (starts_with(var, "credential."))

1110 - return 1;

1111 - return 0;

1112 -}

1113 -

1114 -int sanitize_submodule_config(const char *var, const char *value, void *data)

1115 -{

1116 - struct strbuf *out = data;

1117 -

1118 - if (submodule_config_ok(var)) {

1119 - if (out->len)

1120 - strbuf_addch(out, ' ');

1121 -

1122 - if (value)

1123 - sq_quotef(out, "%s=%s", var, value);

1124 - else

1125 - sq_quote_buf(out, var);

1126 - }

1127 -

1128 - return 0;

1129 -}

1101

1102 void prepare_submodule_repo_env(struct argv_array *out)

1103 {

1104 const char * const *var;

1105

1106 for (var = local_repo_env; *var; var++) {

1136 - if (!strcmp(*var, CONFIG_DATA_ENVIRONMENT)) {

1137 - struct strbuf sanitized_config = STRBUF_INIT;

1138 - git_config_from_parameters(sanitize_submodule_config,

1139 - &sanitized_config);

1140 - argv_array_pushf(out, "%s=%s", *var, sanitized_config.buf);

1141 - strbuf_release(&sanitized_config);

1142 - } else {

1107 + if (strcmp(*var, CONFIG_DATA_ENVIRONMENT))

1108 argv_array_push(out, *var);

1144 - }

1109 }

1146 -

1110 }

submodule.h

+1 -10

@@ -43,19 +43,10 @@ int find_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_nam

43 int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name);

44 void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);

46 -/*

47 - * This function is intended as a callback for use with

48 - * git_config_from_parameters(). It ignores any config options which

49 - * are not suitable for passing along to a submodule, and accumulates the rest

50 - * in "data", which must be a pointer to a strbuf. The end result can

51 - * be put into $GIT_CONFIG_PARAMETERS for passing to a sub-process.

52 - */

53 -int sanitize_submodule_config(const char *var, const char *value, void *data);

54 -

46 /*

47 * Prepare the "env_array" parameter of a "struct child_process" for executing

48 * a submodule by clearing any repo-specific envirionment variables, but

58 - * retaining any config approved by sanitize_submodule_config().

49 + * retaining any config in the environment.

50 */

51 void prepare_submodule_repo_env(struct argv_array *out);

t/t7412-submodule--helper.sh deleted

-26

@@ -1,26 +0,0 @@

1 -#!/bin/sh

2 -#

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