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

[PATCH] remote: detect collisions in remote names

From
Jeff King <peff@peff.net>
Date
Jul 5, 2025, 18:58 UTC
Message-ID
<20250705185842.GA2496172@coredump.intra.peff.net>
In-Reply-To
<20250705165750.GA1951664@coredump.intra.peff.net>
On Sat, Jul 05, 2025 at 12:57:50PM -0400, Jeff King wrote:
Show 6 quoted lines
> So I dunno. It feels like a configuration error in most cases, but not
> all. I'd probably say that people touching the config manually should be
> allowed to do what they want, but maybe "git remote" should be a bit
> more careful about names being proper subsets of existing remotes (it
> should already prevent the exact-match above, I'd think, because the ref
> namespace it uses will always match the configuration name).

So I'm not entirely convinced we should do anything here. The answer might just be "if it hurts, don't do it". But if we wanted any protections in the "git remote" porcelain, they might look like this:

-- >8 --
Subject: [PATCH] remote: detect collisions in remote names

When two remotes collide in the destinations of their fetch refspecs, the results can be confusing. For example, in this silly example:

  git config remote.one.url [...]
  git config remote.one.fetch +refs/heads/*:refs/remotes/collide/*
  git config remote.two.url [...]
  git config remote.two.fetch +refs/heads/*:refs/remotes/collide/*
  git fetch --all

we may try to write to the same ref twice (once for each remote we're fetching). There's also a more subtle version of this. If you have remotes "outer/inner" and "outer", then the ref "inner/branch" on the second remote will conflict with just "branch" on the former (they both want to write to "refs/remotes/outer/inner/branch").

We probably don't want to forbid this kind of overlap completely. While the results can be confusing, there are legitimate reasons to have multiple refs write into the same namespace (e.g., if one is a "backup" of the other that is rarely fetched from).

But it may be worth limiting the porcelain "git remote" command to avoid this confusion. The example above cannot be done with "git remote", because it always[1] matches the refspecs to the remote name, and you can only have one instance of each remote name. But you can still trigger the more subtle variant like this:

  git remote add outer [...]
  git remote add outer/inner [...]

So let's detect that kind of name collision (in both directions) and forbid it. You can still do whatever you like by manipulating the config directly, but this should prevent the most obvious foot-gun.

[1] Almost always. With the --mirror option, the resulting refspec will
    just write into "refs/*"; the remote name does not appear in the ref
    namespace at all.
    Our new "names must not overlap" rule is not necessary for that
    case, but it seems reasonable to enforce it consistently. We already
    require all remote names to be valid in the ref namespace, even
    though we won't ever use them in that context for --mirror remotes.
    Likewise, our new rule doesn't help with overlap here. Any two
    mirror remotes will always overlap (in fact, any mirror remote along
    with any other single one, since refs/remotes/ is a subset of the
    mirrored refs). I'm not sure this is worth worrying about, but if it
    is, we'd want an additional rule like "mirror remotes must be the
    only remote".
Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/remote.c  | 17 +++++++++++++++++
 t/t5505-remote.sh | 14 ++++++++++++++
 2 files changed, 31 insertions(+)
diff --git a/builtin/remote.c b/builtin/remote.c
index 0d6755bcb7..b18730ddb2 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -157,6 +157,21 @@ static int parse_mirror_opt(const struct option *opt, const char *arg, int not)
 	return 0;
 }
 
+static int check_remote_collision(struct remote *remote, void *vname)
+{
+	const char *name = vname;
+	const char *p;
+
+	if (skip_prefix(name, remote->name, &p) && *p == '/')
+		die(_("remote name '%s' is a subset of existing remote '%s'"),
+		    name, remote->name);
+	if (skip_prefix(remote->name, name, &p) && *p == '/')
+		die(_("remote name '%s' is a superset of existing remote '%s'"),
+		    name, remote->name);
+
+	return 0;
+}
+
 static int add(int argc, const char **argv, const char *prefix,
 	       struct repository *repo UNUSED)
 {
@@ -208,6 +223,8 @@ static int add(int argc, const char **argv, const char *prefix,
 	if (!valid_remote_name(name))
 		die(_("'%s' is not a valid remote name"), name);
 
+	for_each_remote(check_remote_collision, (void *)name);
+
 	strbuf_addf(&buf, "remote.%s.url", name);
 	git_config_set(buf.buf, url);
 
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index bef0250e89..2701eef85e 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -1644,4 +1644,18 @@ test_expect_success 'empty config clears remote.*.pushurl list' '
 	test_cmp expect actual
 '
 
+test_expect_success 'forbid adding subset of existing remote' '
+	test_when_finished "git remote rm outer" &&
+	git remote add outer url &&
+	test_must_fail git remote add outer/inner url 2>err &&
+	test_grep ".outer/inner. is a subset of existing remote .outer." err
+'
+
+test_expect_success 'forbid adding superset of existing remote' '
+	test_when_finished "git remote rm outer/inner" &&
+	git remote add outer/inner url &&
+	test_must_fail git remote add outer url 2>err &&
+	test_grep ".outer. is a superset of existing remote .outer/inner." err
+'
+
 test_done
-- 
2.50.0.438.g3b3bebd3e8
Previous: Jeff KingNext: Patrick Steinhardt
Message 9 of 18 in “Allowing "/" in the name of a git remote is a strange choice”
  1. Per CederqvistJul 3, 2025
  2. Junio C HamanoJul 4, 2025
  3. Patrick SteinhardtJul 4, 2025
  4. Lidong YanJul 4, 2025
  5. Lidong YanJul 4, 2025
  6. Junio C HamanoJul 4, 2025
  7. Per CederqvistJul 4, 2025
  8. Jeff KingJul 5, 2025
  9. remote: detect collisions in remote namesJeff King, Jul 5, 2025
  10. Patrick SteinhardtJul 7, 2025
  11. Jeff KingJul 7, 2025
  12. Junio C HamanoJul 7, 2025
  13. Jeff KingJul 8, 2025
  14. Jeff KingJul 8, 2025
  15. Junio C HamanoJul 8, 2025
  16. Jeff KingJul 9, 2025
  17. Junio C HamanoJul 7, 2025
  18. Raymond E. PascoJul 9, 2025

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.