Git development
 help / color / mirror / Atom feed
From: Christian Couder <christian.couder@gmail.com>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
	"brian m . carlson" <sandals@crustytoothpaste.net>,
	Patrick Steinhardt <ps@pks.im>,
	Karthik Nayak <karthik.188@gmail.com>, Jeff King <peff@peff.net>,
	Elijah Newren <newren@gmail.com>,
	Christian Couder <christian.couder@gmail.com>,
	Christian Couder <chriscool@tuxfamily.org>
Subject: [PATCH 2/5] setup: extract path_allowlist_apply()
Date: Fri,  7 Aug 2026 15:55:08 +0200	[thread overview]
Message-ID: <20260807135511.1818458-3-christian.couder@gmail.com> (raw)
In-Reply-To: <20260807135511.1818458-1-christian.couder@gmail.com>

In a following commit we are going to check whether a repository is
part of an allowlist specified in a config variable.

To prepare for that let's extract existing code from
safe_directory_cb() into a new path_allowlist_apply() helper that will
help with such checks.

While at it let's make the helper's code simpler and more generic.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 setup.c | 107 +++++++++++++++++++++++++++++++-------------------------
 1 file changed, 59 insertions(+), 48 deletions(-)

diff --git a/setup.c b/setup.c
index 95909e9603..39dfa1cc5f 100644
--- a/setup.c
+++ b/setup.c
@@ -1339,6 +1339,64 @@ static int canonicalize_ceiling_entry(struct string_list_item *item,
 	}
 }
 
+static void path_allowlist_apply(const char *key, const char *value,
+				 const char *target_path, int *is_match)
+{
+	char *allowed = NULL;
+	char *normalized = NULL;
+
+	if (!value || !*value) {
+		*is_match = 0;
+		return;
+	}
+
+	if (!strcmp(value, "*")) {
+		*is_match = 1;
+		return;
+	}
+
+	if (git_config_pathname(&allowed, key, value) || !allowed)
+		return;
+
+	/*
+	 * Setting the config variable to a non-absolute path makes
+	 * little sense---it won't be relative to the configuration
+	 * file the item is defined in.  Except for ".", which means
+	 * "if we are at the top level of a repository, then it is
+	 * OK", which is slightly tighter than "*" that allows
+	 * discovery.
+	 */
+	if (!is_absolute_path(allowed) && strcmp(allowed, ".")) {
+		warning(_("%s '%s' not absolute"), key, allowed);
+		goto end;
+	}
+
+	/*
+	 * A .gitconfig in $HOME may be shared across different
+	 * machines and the config variable entries may or may not
+	 * exist as paths on all of these machines.  In other words,
+	 * it is not a warning worthy event when there is no such path
+	 * on this machine---the entry may be useful elsewhere.
+	 */
+	normalized = real_pathdup(allowed, 0);
+	if (!normalized)
+		goto end;
+
+	if (ends_with(normalized, "/*")) {
+		size_t len = strlen(normalized);
+		if (!fspathncmp(normalized, target_path, len - 1))
+			*is_match = 1;
+		goto end;
+	}
+
+	if (!fspathcmp(target_path, normalized))
+		*is_match = 1;
+
+end:
+	free(normalized);
+	free(allowed);
+}
+
 struct safe_directory_data {
 	char *path;
 	int is_safe;
@@ -1352,54 +1410,7 @@ static int safe_directory_cb(const char *key, const char *value,
 	if (strcmp(key, "safe.directory"))
 		return 0;
 
-	if (!value || !*value) {
-		data->is_safe = 0;
-	} else if (!strcmp(value, "*")) {
-		data->is_safe = 1;
-	} else {
-		char *allowed = NULL;
-
-		if (!git_config_pathname(&allowed, key, value) && allowed) {
-			char *normalized = NULL;
-
-			/*
-			 * Setting safe.directory to a non-absolute path
-			 * makes little sense---it won't be relative to
-			 * the configuration file the item is defined in.
-			 * Except for ".", which means "if we are at the top
-			 * level of a repository, then it is OK", which is
-			 * slightly tighter than "*" that allows discovery.
-			 */
-			if (!is_absolute_path(allowed) && strcmp(allowed, ".")) {
-				warning(_("safe.directory '%s' not absolute"),
-					allowed);
-				goto next;
-			}
-
-			/*
-			 * A .gitconfig in $HOME may be shared across
-			 * different machines and safe.directory entries
-			 * may or may not exist as paths on all of these
-			 * machines.  In other words, it is not a warning
-			 * worthy event when there is no such path on this
-			 * machine---the entry may be useful elsewhere.
-			 */
-			normalized = real_pathdup(allowed, 0);
-			if (!normalized)
-				goto next;
-
-			if (ends_with(normalized, "/*")) {
-				size_t len = strlen(normalized);
-				if (!fspathncmp(normalized, data->path, len - 1))
-					data->is_safe = 1;
-			} else if (!fspathcmp(data->path, normalized)) {
-				data->is_safe = 1;
-			}
-		next:
-			free(normalized);
-			free(allowed);
-		}
-	}
+	path_allowlist_apply(key, value, data->path, &data->is_safe);
 
 	return 0;
 }
-- 
2.55.0.530.gdb3615d990.dirty


  parent reply	other threads:[~2026-08-07 13:55 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-10  8:51 [PATCH 0/3] Introduce a 'fromAccepted' option to GIT_NO_LAZY_FETCH Christian Couder
2026-07-10  8:51 ` [PATCH 1/3] promisor-remote: factor out lazy_fetch_objects() Christian Couder
2026-07-10  8:51 ` [PATCH 2/3] promisor-remote: introduce enum allow_lazy_fetch Christian Couder
2026-07-10  8:51 ` [PATCH 3/3] promisor-remote: teach 'fromAccepted' to GIT_NO_LAZY_FETCH Christian Couder
2026-07-10 19:50 ` [PATCH 0/3] Introduce a 'fromAccepted' option " brian m. carlson
2026-07-12  9:06   ` Christian Couder
2026-08-07 13:55 ` [PATCH 0/5] Introduce 'uploadpack.lazyFetchTrusted' Christian Couder
2026-08-07 13:55   ` [PATCH 1/5] promisor-remote: factor out lazy_fetch_objects() Christian Couder
2026-08-07 13:58     ` Christian Couder
2026-08-07 13:55   ` Christian Couder [this message]
2026-08-07 13:55   ` [PATCH 3/5] setup: add 'allow_dot' arg to path_allowlist_apply() Christian Couder
2026-08-07 13:55   ` [PATCH 4/5] upload-pack: read uploadpack.lazyFetchTrusted Christian Couder
2026-08-07 13:55   ` [PATCH 5/5] builtin/upload-pack: set GIT_NO_LAZY_FETCH to 0 on trusted repo Christian Couder
2026-08-07 18:31   ` [PATCH 0/5] Introduce 'uploadpack.lazyFetchTrusted' 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

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

  git send-email \
    --in-reply-to=20260807135511.1818458-3-christian.couder@gmail.com \
    --to=christian.couder@gmail.com \
    --cc=chriscool@tuxfamily.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=karthik.188@gmail.com \
    --cc=newren@gmail.com \
    --cc=peff@peff.net \
    --cc=ps@pks.im \
    --cc=sandals@crustytoothpaste.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.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox