Subject: [PATCH v5 3/7] submodule: always validate gitdirs inside submodule_name_to_gitdir

From: Adrian Ratiu <[email-masked]>
Date: Wed, 19 Nov 2025 23:10:26 +0200

Move the ad-hoc validation checks sprinkled across the source tree,
after calling submodule_name_to_gitdir() into the function proper,
which now always validates the gitdir before returning it.

This also makes parallel operations a bit safer due to checking and
erroring out each time the unified API detects a problem instead of
having one extra hardcoded validation check in submodule--helper.c.

It simplifies the API usage as well since users who don't have to
validate the submodule_name_to_gitdir() result themselves anymore
and reduces the risks of API users forgetting to validate.

Signed-off-by: Adrian Ratiu <[email-masked]>
---
 builtin/submodule--helper.c | 21 ---------------------
 submodule.c                 | 30 ++++++------------------------
 submodule.h                 |  5 -----
 3 files changed, 6 insertions(+), 50 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 2873b2780e..9914ca0786 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1703,10 +1703,6 @@ static int clone_submodule(const struct module_clone_data *clone_data,
 		clone_data_path = to_free = xstrfmt("%s/%s", repo_get_work_tree(the_repository),
 						    clone_data->path);
 
-	if (validate_submodule_git_dir(sm_gitdir, clone_data->name) < 0)
-		die(_("refusing to create/use '%s' in another submodule's "
-		      "git dir"), sm_gitdir);
-
 	if (!file_exists(sm_gitdir)) {
 		if (clone_data->require_init && !stat(clone_data_path, &st) &&
 		    !is_empty_dir(clone_data_path))
@@ -1780,23 +1776,6 @@ static int clone_submodule(const struct module_clone_data *clone_data,
 		free(path);
 	}
 
-	/*
-	 * We already performed this check at the beginning of this function,
-	 * before cloning the objects. This tries to detect racy behavior e.g.
-	 * in parallel clones, where another process could easily have made the
-	 * gitdir nested _after_ it was created.
-	 *
-	 * To prevent further harm coming from this unintentionally-nested
-	 * gitdir, let's disable it by deleting the `HEAD` file.
-	 */
-	if (validate_submodule_git_dir(sm_gitdir, clone_data->name) < 0) {
-		char *head = xstrfmt("%s/HEAD", sm_gitdir);
-		unlink(head);
-		free(head);
-		die(_("refusing to create/use '%s' in another submodule's "
-		      "git dir"), sm_gitdir);
-	}
-
 	connect_work_tree_and_git_dir(clone_data_path, sm_gitdir, 0);
 
 	p = repo_submodule_path(the_repository, clone_data_path, "config");
diff --git a/submodule.c b/submodule.c
index 35c55155f7..8ef028f26b 100644
--- a/submodule.c
+++ b/submodule.c
@@ -2153,30 +2153,11 @@ int submodule_move_head(const char *path, const char *super_prefix,
 
 	if (!(flags & SUBMODULE_MOVE_HEAD_DRY_RUN)) {
 		if (old_head) {
-			if (!submodule_uses_gitfile(path))
-				absorb_git_dir_into_superproject(path,
-								 super_prefix);
-			else {
-				char *dotgit = xstrfmt("%s/.git", path);
-				char *git_dir = xstrdup(read_gitfile(dotgit));
-
-				free(dotgit);
-				if (validate_submodule_git_dir(git_dir,
-							       sub->name) < 0)
-					die(_("refusing to create/use '%s' in "
-					      "another submodule's git dir"),
-					    git_dir);
-				free(git_dir);
-			}
+			absorb_git_dir_into_superproject(path, super_prefix);
 		} else {
 			struct strbuf gitdir = STRBUF_INIT;
 			submodule_name_to_gitdir(&gitdir, the_repository,
 						 sub->name);
-			if (validate_submodule_git_dir(gitdir.buf,
-						       sub->name) < 0)
-				die(_("refusing to create/use '%s' in another "
-				      "submodule's git dir"),
-				    gitdir.buf);
 			connect_work_tree_and_git_dir(path, gitdir.buf, 0);
 			strbuf_release(&gitdir);
 
@@ -2256,7 +2237,7 @@ int submodule_move_head(const char *path, const char *super_prefix,
 	return ret;
 }
 
-int validate_submodule_git_dir(char *git_dir, const char *submodule_name)
+static int validate_submodule_git_dir(char *git_dir, const char *submodule_name)
 {
 	size_t len = strlen(git_dir), suffix_len = strlen(submodule_name);
 	char *p;
@@ -2355,9 +2336,6 @@ static void relocate_single_git_dir_into_superproject(const char *path,
 		die(_("could not lookup name for submodule '%s'"), path);
 
 	submodule_name_to_gitdir(&new_gitdir, the_repository, sub->name);
-	if (validate_submodule_git_dir(new_gitdir.buf, sub->name) < 0)
-		die(_("refusing to move '%s' into an existing git dir"),
-		    real_old_git_dir);
 	if (safe_create_leading_directories_const(the_repository, new_gitdir.buf) < 0)
 		die(_("could not create directory '%s'"), new_gitdir.buf);
 	real_new_git_dir = real_pathdup(new_gitdir.buf, 1);
@@ -2606,4 +2584,8 @@ void submodule_name_to_gitdir(struct strbuf *buf, struct repository *r,
 	 */
 	repo_git_path_append(r, buf, "modules/");
 	strbuf_addstr(buf, submodule_name);
+
+	if (validate_submodule_git_dir(buf->buf, submodule_name) < 0)
+		die(_("refusing to create/use '%s' in another submodule's "
+		      "git dir"), buf->buf);
 }
diff --git a/submodule.h b/submodule.h
index b10e16e6c0..0b7692bc20 100644
--- a/submodule.h
+++ b/submodule.h
@@ -137,11 +137,6 @@ int submodule_to_gitdir(struct repository *repo,
 void submodule_name_to_gitdir(struct strbuf *buf, struct repository *r,
 			      const char *submodule_name);
 
-/*
- * Make sure that no submodule's git dir is nested in a sibling submodule's.
- */
-int validate_submodule_git_dir(char *git_dir, const char *submodule_name);
-
 /*
  * Make sure that the given submodule path does not follow symlinks.
  */
-- 
2.51.0