git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Stefan Beller <sbeller@google.com>
To: bmwill@google.com, ao2@ao2.it, hvoigt@hvoigt.net
Cc: git@vger.kernel.org, Stefan Beller <sbeller@google.com>
Subject: [PATCH] RFC: submodule-config: introduce trust level
Date: Thu, 12 Jul 2018 12:04:56 -0700	[thread overview]
Message-ID: <20180712190456.29337-1-sbeller@google.com> (raw)

The series merged at 614ea03a71e (Merge branch
'bw/submodule-config-cleanup', 2017-08-26) went to great length to make it
explicit to the caller where a value came from, as that would help the
caller to be careful to decide which values to take from where, i.e. be
careful about security implications.

In practice we always want to stack the settings starting with the
.gitmodules file as a base and then put the config on top of it.
So let's manage the security aspects impolitely in the submodule-config
machinery directly where we implement its parsing as there is a good
place to reason about the trust that we need to put into a parsed value.

This patch implements the trust level that is passed to the parsing,'
currently we only pass in the 'in_repo' level of trust, which is the
.gitmodules file.

Follow up patches could add other sources that populate the submodule
config again.

Signed-off-by: Stefan Beller <sbeller@google.com>
---

 This is on top of ao/config-from-gitmodules.

 submodule-config.c | 31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/submodule-config.c b/submodule-config.c
index 77421a49719..09eab9f00e0 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -21,6 +21,11 @@ struct submodule_cache {
 	unsigned gitmodules_read:1;
 };
 
+enum submodule_config_trust_level {
+	in_repo = 1,
+	configured_by_user = 2
+};
+
 /*
  * thin wrapper struct needed to insert 'struct submodule' entries to
  * the hashmap
@@ -387,12 +392,14 @@ struct parse_config_parameter {
 	struct submodule_cache *cache;
 	const struct object_id *treeish_name;
 	const struct object_id *gitmodules_oid;
+	enum submodule_config_trust_level source;
 	int overwrite;
 };
 
 static int parse_config(const char *var, const char *value, void *data)
 {
 	struct parse_config_parameter *me = data;
+	enum submodule_config_trust_level source = me->source;
 	struct submodule *submodule;
 	struct strbuf name = STRBUF_INIT, item = STRBUF_INIT;
 	int ret = 0;
@@ -406,6 +413,7 @@ static int parse_config(const char *var, const char *value, void *data)
 					     name.buf);
 
 	if (!strcmp(item.buf, "path")) {
+		/* all sources allowed */
 		if (!value)
 			ret = config_error_nonbool(var);
 		else if (!me->overwrite && submodule->path)
@@ -419,6 +427,7 @@ static int parse_config(const char *var, const char *value, void *data)
 			cache_put_path(me->cache, submodule);
 		}
 	} else if (!strcmp(item.buf, "fetchrecursesubmodules")) {
+		/* all sources allowed */
 		/* when parsing worktree configurations we can die early */
 		int die_on_error = is_null_oid(me->gitmodules_oid);
 		if (!me->overwrite &&
@@ -430,6 +439,7 @@ static int parse_config(const char *var, const char *value, void *data)
 								var, value,
 								die_on_error);
 	} else if (!strcmp(item.buf, "ignore")) {
+		/* all sources allowed */
 		if (!value)
 			ret = config_error_nonbool(var);
 		else if (!me->overwrite && submodule->ignore)
@@ -446,6 +456,7 @@ static int parse_config(const char *var, const char *value, void *data)
 			submodule->ignore = xstrdup(value);
 		}
 	} else if (!strcmp(item.buf, "url")) {
+		/* all sources allowed */
 		if (!value) {
 			ret = config_error_nonbool(var);
 		} else if (!me->overwrite && submodule->url) {
@@ -456,16 +467,27 @@ static int parse_config(const char *var, const char *value, void *data)
 			submodule->url = xstrdup(value);
 		}
 	} else if (!strcmp(item.buf, "update")) {
+		struct submodule_update_strategy st;
 		if (!value)
 			ret = config_error_nonbool(var);
 		else if (!me->overwrite &&
 			 submodule->update_strategy.type != SM_UPDATE_UNSPECIFIED)
 			warn_multiple_config(me->treeish_name, submodule->name,
 					     "update");
-		else if (parse_submodule_update_strategy(value,
-			 &submodule->update_strategy) < 0)
-				die(_("invalid value for %s"), var);
+		else if (parse_submodule_update_strategy(value, &st) < 0)
+			die(_("invalid value for %s"), var);
+		else if (source <= in_repo) {
+			if (st.type == SM_UPDATE_COMMAND) {
+				submodule->update_strategy.type = st.type;
+				submodule->update_strategy.command = \
+				"!echo Not trusting command in submodule.<name>.update";
+			} else {
+				submodule->update_strategy.type = st.type;
+				submodule->update_strategy.command = st.command;
+			}
+		}
 	} else if (!strcmp(item.buf, "shallow")) {
+		/* all sources allowed */
 		if (!me->overwrite && submodule->recommend_shallow != -1)
 			warn_multiple_config(me->treeish_name, submodule->name,
 					     "shallow");
@@ -473,6 +495,7 @@ static int parse_config(const char *var, const char *value, void *data)
 			submodule->recommend_shallow =
 				git_config_bool(var, value);
 	} else if (!strcmp(item.buf, "branch")) {
+		/* all sources allowed */
 		if (!me->overwrite && submodule->branch)
 			warn_multiple_config(me->treeish_name, submodule->name,
 					     "branch");
@@ -560,6 +583,7 @@ static const struct submodule *config_from(struct submodule_cache *cache,
 	parameter.treeish_name = treeish_name;
 	parameter.gitmodules_oid = &oid;
 	parameter.overwrite = 0;
+	parameter.source = in_repo;
 	git_config_from_mem(parse_config, CONFIG_ORIGIN_SUBMODULE_BLOB, rev.buf,
 			config, config_size, &parameter);
 	strbuf_release(&rev);
@@ -617,6 +641,7 @@ static int gitmodules_cb(const char *var, const char *value, void *data)
 	parameter.treeish_name = NULL;
 	parameter.gitmodules_oid = &null_oid;
 	parameter.overwrite = 1;
+	parameter.source = in_repo;
 
 	return parse_config(var, value, &parameter);
 }
-- 
2.18.0.203.gfac676dfb9-goog


             reply	other threads:[~2018-07-12 19:05 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-12 19:04 Stefan Beller [this message]
2018-07-12 20:54 ` [PATCH] RFC: submodule-config: introduce trust level Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180712190456.29337-1-sbeller@google.com \
    --to=sbeller@google.com \
    --cc=ao2@ao2.it \
    --cc=bmwill@google.com \
    --cc=git@vger.kernel.org \
    --cc=hvoigt@hvoigt.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).