@matheusbsbs / matheus-bagliano / commits / a5a727c448

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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Jul 8, 2025 at 18:59 UTC a5a727c448e56b212546e047031166ced82c7d1e

2 files changed +31

builtin/remote.c

+17

@@ -157,6 +157,21 @@ static int parse_mirror_opt(const struct option *opt, const char *arg, int not)

157 return 0;

158 }

159

160 +static int check_remote_collision(struct remote *remote, void *data)

161 +{

162 + const char *name = data;

163 + const char *p;

164 +

165 + if (skip_prefix(name, remote->name, &p) && *p == '/')

166 + die(_("remote name '%s' is a subset of existing remote '%s'"),

167 + name, remote->name);

168 + if (skip_prefix(remote->name, name, &p) && *p == '/')

169 + die(_("remote name '%s' is a superset of existing remote '%s'"),

170 + name, remote->name);

171 +

172 + return 0;

173 +}

174 +

175 static int add(int argc, const char **argv, const char *prefix,

176 struct repository *repo UNUSED)

177 {

@@ -208,6 +223,8 @@ static int add(int argc, const char **argv, const char *prefix,

223 if (!valid_remote_name(name))

224 die(_("'%s' is not a valid remote name"), name);

225

226 + for_each_remote(check_remote_collision, (void *)name);

227 +

228 strbuf_addf(&buf, "remote.%s.url", name);

229 git_config_set(buf.buf, url);

230

t/t5505-remote.sh

+14

@@ -1644,4 +1644,18 @@ test_expect_success 'empty config clears remote.*.pushurl list' '

1644 test_cmp expect actual

1645 '

1646

1647 +test_expect_success 'forbid adding subset of existing remote' '

1648 + test_when_finished "git remote rm outer" &&

1649 + git remote add outer url &&

1650 + test_must_fail git remote add outer/inner url 2>err &&

1651 + test_grep ".outer/inner. is a subset of existing remote .outer." err

1652 +'

1653 +

1654 +test_expect_success 'forbid adding superset of existing remote' '

1655 + test_when_finished "git remote rm outer/inner" &&

1656 + git remote add outer/inner url &&

1657 + test_must_fail git remote add outer url 2>err &&

1658 + test_grep ".outer. is a superset of existing remote .outer/inner." err

1659 +'

1660 +

1661 test_done