git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 2/5] setup: extract path_allowlist_apply()

From
Christian Couder <christian.couder@gmail.com>
Date
Sep 8, 2026, 16:41 UTC
Message-ID
<20260908164129.560396-3-christian.couder@gmail.com>
In-Reply-To
<20260908164129.560396-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, by passing it a `bool (*allow_path)(const char *path, void *cbdata)` function that decides if a path is acceptable by the caller.

To further simplify how to reuse that new helper, and avoid duplicating the config-value handling in a future commit, let's also introduce a path_allowlist_config_apply() helper.

For clarity, let's change the `int is_safe` to `bool safe` in `struct safe_directory_data`.

Signed-off-by: Christian Couder <christian.couder@gmail.com>
---
 setup.c | 138 ++++++++++++++++++++++++++++++++++++--------------------
 setup.h |  50 ++++++++++++++++++++
 2 files changed, 138 insertions(+), 50 deletions(-)
diff --git a/setup.c b/setup.c
index dfe05d9a03..366a7dc5c0 100644
--- a/setup.c
+++ b/setup.c
@@ -1338,67 +1338,105 @@ static int canonicalize_ceiling_entry(struct string_list_item *item,
 	}
 }
 
+void path_allowlist_apply(const char *allowed, const char *target_path,
+			  bool *matches,
+			  bool (*allow_path)(const char *path, void *cbdata),
+			  void *allow_path_cbdata)
+{
+	char *normalized = NULL;
+
+	if (!allowed || !*allowed) {
+		*matches = false;
+		return;
+	}
+
+	if (!strcmp(allowed, "*")) {
+		*matches = true;
+		return;
+	}
+
+	if (!allow_path(allowed, allow_path_cbdata))
+		return;
+
+	/*
+	 * 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)
+		return;
+
+	if (ends_with(normalized, "/*")) {
+		size_t len = strlen(normalized);
+		if (!fspathncmp(normalized, target_path, len - 1))
+			*matches = true;
+	} else if (!fspathcmp(target_path, normalized)) {
+		*matches = true;
+	}
+
+	free(normalized);
+}
+
+void path_allowlist_config_apply(const char *key, const char *value,
+				 const char *target_path, bool *matches,
+				 bool (*allow_path)(const char *path, void *cbdata),
+				 void *allow_path_cbdata)
+{
+	char *allowed = NULL;
+
+	if (!value || !*value || !strcmp(value, "*")) {
+		path_allowlist_apply(value, target_path, matches,
+				     allow_path, allow_path_cbdata);
+		return;
+	}
+
+	if (git_config_pathname(&allowed, key, value) || !allowed)
+		return;
+
+	path_allowlist_apply(allowed, target_path, matches,
+			     allow_path, allow_path_cbdata);
+
+	free(allowed);
+}
+
+/*
+ * 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.
+ */
+static bool allow_safe_dir(const char *path, void *cbdata_)
+{
+	struct path_allowlist_cb_data *cbdata = cbdata_;
+
+	if (is_absolute_path(path) || !strcmp(path, "."))
+		return true;
+
+	warning(_("%s '%s' not absolute"), cbdata->key, path);
+	return false;
+}
+
 struct safe_directory_data {
 	char *path;
-	int is_safe;
+	bool safe;
 };
 
 static int safe_directory_cb(const char *key, const char *value,
 			     const struct config_context *ctx UNUSED, void *d)
 {
 	struct safe_directory_data *data = d;
+	struct path_allowlist_cb_data cbdata = { .key = key };
 
 	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_config_apply(key, value, data->path, &data->safe,
+				    allow_safe_dir, &cbdata);
 
 	return 0;
 }
@@ -1440,7 +1478,7 @@ static int ensure_valid_ownership(const char *gitfile,
 	git_protected_config(safe_directory_cb, &data);
 
 	free(data.path);
-	return data.is_safe;
+	return data.safe;
 }
 
 void die_upon_dubious_ownership(const char *gitfile, const char *worktree,
diff --git a/setup.h b/setup.h
index 763fd384e8..6b84fbe507 100644
--- a/setup.h
+++ b/setup.h
@@ -304,4 +304,54 @@ struct startup_info {
 extern struct startup_info *startup_info;
 extern const char *tmp_original_cwd;
 
+/* Path allowlist */
+
+struct path_allowlist_cb_data {
+	const char *key;
+};
+
+/*
+ * Check the allowlist entry in `allowed` against `target_path`,
+ * updating `*matches` accordingly.
+ *
+ * `allowed` is a single entry of an allowlist of paths, typically one
+ * value of a multi-valued config variable, already expanded by
+ * git_config_pathname(). `target_path` is the (normalized) path being
+ * tested. `*matches` is updated in place:
+ *
+ *   - an empty `allowed` resets it to 'false' (so a later, more
+ *     specific config scope can clear entries from a broader one),
+ *   - "*" sets it to 'true' (allow everything),
+ *   - "<path>" sets it to 'true' if <path> equals `target_path`,
+ *   - "<path>" + "/" + "*" sets it to 'true' if <path> is a leading
+ *     directory of `target_path`,
+ *   - anything else leaves `*matches` unchanged.
+ *
+ * `allow_path` is called with `allowed` and `allow_path_cbdata`, and
+ * should return 'true' if the entry is acceptable to the caller. It
+ * lets each caller decide which paths it is willing to consider, and
+ * whether to warn about the ones it rejects. Returning 'false' leaves
+ * `*matches` unchanged.
+ *
+ * Callers are expected to invoke this once per allowlist entry,
+ * typically from a protected-config callback, so that untrusted
+ * repository config cannot influence the decision.
+ */
+void path_allowlist_apply(const char *allowed, const char *target_path,
+			  bool *matches,
+			  bool (*allow_path)(const char *path, void *cbdata),
+			  void *allow_path_cbdata);
+
+/*
+ * Apply one value of a multi-valued config variable holding an
+ * allowlist of paths, expanding it with git_config_pathname() before
+ * checking it against `target_path`. Empty and "*" values are passed
+ * through without expansion, as interpolating them is not
+ * meaningful. See path_allowlist_apply().
+ */
+void path_allowlist_config_apply(const char *key, const char *value,
+				 const char *target_path, bool *matches,
+				 bool (*allow_path)(const char *path, void *cbdata),
+				 void *allow_path_cbdata);
+
 #endif /* SETUP_H */
-- 
2.55.0.792.ged91fccac1.dirty
Previous: Christian CouderNext: Junio C Hamano
Message 25 of 70 in “Introduce a 'fromAccepted' option to GIT_NO_LAZY_FETCH”
  1. 0/3 Introduce a 'fromAccepted' option to GIT_NO_LAZY_FETCHChristian Couder, Jul 10, 2026
  2. 1/3 promisor-remote: factor out lazy_fetch_objects()Christian Couder, Jul 10, 2026
  3. 2/3 promisor-remote: introduce enum allow_lazy_fetchChristian Couder, Jul 10, 2026
  4. 3/3 promisor-remote: teach 'fromAccepted' to GIT_NO_LAZY_FETCHChristian Couder, Jul 10, 2026
  5. brian m. carlsonJul 10, 2026
  6. Christian CouderJul 12, 2026
  7. 0/5 Introduce 'uploadpack.lazyFetchTrusted'Christian Couder, Aug 7, 2026
  8. 1/5 promisor-remote: factor out lazy_fetch_objects()Christian Couder, Aug 7, 2026
  9. Christian CouderAug 7, 2026
  10. 2/5 setup: extract path_allowlist_apply()Christian Couder, Aug 7, 2026
  11. 4/5 upload-pack: read uploadpack.lazyFetchTrustedChristian Couder, Aug 7, 2026
  12. 5/5 builtin/upload-pack: set GIT_NO_LAZY_FETCH to 0 on trusted repoChristian Couder, Aug 7, 2026
  13. 3/5 setup: add 'allow_dot' arg to path_allowlist_apply()Christian Couder, Aug 7, 2026
  14. Junio C HamanoAug 7, 2026
  15. Christian CouderAug 10, 2026
  16. Junio C HamanoAug 11, 2026
  17. 0/5 Introduce 'uploadpack.lazyFetchTrusted'Christian Couder, Aug 13, 2026
  18. Junio C HamanoAug 13, 2026
  19. Christian CouderAug 14, 2026
  20. Junio C HamanoAug 14, 2026
  21. 0/5 Introduce 'uploadpack.lazyFetchTrusted'Christian Couder, Sep 8, 2026
  22. 1/5 promisor-remote: factor out lazy_fetch_objects()Christian Couder, Sep 8, 2026
  23. Junio C HamanoSep 8, 2026
  24. Christian CouderSep 28, 2026
  25. 2/5 setup: extract path_allowlist_apply()Christian Couder, Sep 8, 2026
  26. Junio C HamanoSep 8, 2026
  27. Christian CouderSep 28, 2026
  28. 3/5 upload-pack: read uploadpack.lazyFetchTrustedChristian Couder, Sep 8, 2026
  29. 4/5 promisor-remote: prevent infinite recursion when lazy fetchingChristian Couder, Sep 8, 2026
  30. Junio C HamanoSep 8, 2026
  31. Christian CouderSep 9, 2026
  32. Junio C HamanoSep 9, 2026
  33. Christian CouderSep 28, 2026
  34. 5/5 builtin/upload-pack: set GIT_NO_LAZY_FETCH to 0 on trusted repoChristian Couder, Sep 8, 2026
  35. Junio C HamanoSep 8, 2026
  36. Christian CouderSep 28, 2026
  37. 0/5 Introduce 'uploadpack.lazyFetchTrusted'Christian Couder, Sep 28, 2026
  38. 1/5 promisor-remote: factor out lazy_fetch_objects()Christian Couder, Sep 28, 2026
  39. 2/5 setup: extract path_allowlist_apply()Christian Couder, Sep 28, 2026
  40. Junio C HamanoSep 29, 2026
  41. Christian CouderOct 2, 2026
  42. 3/5 upload-pack: read uploadpack.lazyFetchTrustedChristian Couder, Sep 28, 2026
  43. 4/5 promisor-remote: prevent infinite recursion when lazy fetchingChristian Couder, Sep 28, 2026
  44. 5/5 builtin/upload-pack: don't disable lazy fetching on trusted repoChristian Couder, Sep 28, 2026
  45. Junio C HamanoSep 29, 2026
  46. Christian CouderOct 2, 2026
  47. Christian CouderOct 2, 2026
  48. 0/5 Introduce 'uploadpack.lazyFetchTrusted'Christian Couder, Oct 2, 2026
  49. 1/5 promisor-remote: factor out lazy_fetch_objects()Christian Couder, Oct 2, 2026
  50. 2/5 setup: extract path_allowlist_apply()Christian Couder, Oct 2, 2026
  51. 3/5 upload-pack: read uploadpack.lazyFetchTrustedChristian Couder, Oct 2, 2026
  52. 4/5 promisor-remote: prevent infinite recursion when lazy fetchingChristian Couder, Oct 2, 2026
  53. 5/5 builtin/upload-pack: don't disable lazy fetching on trusted repoChristian Couder, Oct 2, 2026
  54. Junio C HamanoOct 5, 2026
  55. Christian CouderOct 6, 2026
  56. 1/5 promisor-remote: factor out lazy_fetch_objects()Christian Couder, Aug 13, 2026
  57. Junio C HamanoAug 14, 2026
  58. Christian CouderSep 8, 2026
  59. 2/5 setup: extract path_allowlist_apply()Christian Couder, Aug 13, 2026
  60. Junio C HamanoAug 14, 2026
  61. Christian CouderSep 8, 2026
  62. Junio C HamanoSep 8, 2026
  63. 3/5 setup: add 'allow_dot' arg to path_allowlist_apply()Christian Couder, Aug 13, 2026
  64. Junio C HamanoAug 14, 2026
  65. Christian CouderSep 8, 2026
  66. 4/5 upload-pack: read uploadpack.lazyFetchTrustedChristian Couder, Aug 13, 2026
  67. Junio C HamanoAug 14, 2026
  68. 5/5 builtin/upload-pack: set GIT_NO_LAZY_FETCH to 0 on trusted repoChristian Couder, Aug 13, 2026
  69. Junio C HamanoAug 14, 2026
  70. Christian CouderSep 8, 2026

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.