Skip to content

Commit bb69b3b

Browse files
sunshinecogitster
authored andcommitted
worktree: don't allow "add" validation to be fooled by suffix matching
"git worktree add <path>" performs various checks before approving <path> as a valid location for the new worktree. Aside from ensuring that <path> does not already exist, one of the questions it asks is whether <path> is already a registered worktree. To perform this check, it queries find_worktree() and disallows the "add" operation if find_worktree() finds a match for <path>. As a convenience, however, find_worktree() casts an overly wide net to allow users to identify worktrees by shorthand in order to keep typing to a minimum. For instance, it performs suffix matching which, given subtrees "foo/bar" and "foo/baz", can correctly select the latter when asked only for "baz". "add" validation knows the exact path it is interrogating, so this sort of heuristic-based matching is, at best, questionable for this use-case and, at worst, may may accidentally interpret <path> as matching an existing worktree and incorrectly report it as already registered even when it isn't. (In fact, validate_worktree_add() already contains a special case to avoid accidentally matching against the main worktree, precisely due to this problem.) Avoid the problem of potential accidental matching against an existing worktree by instead taking advantage of find_worktree_by_path() which matches paths deterministically, without applying any sort of magic shorthand matching performed by find_worktree(). Reported-by: Cameron Gunnin <[email protected]> Signed-off-by: Eric Sunshine <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent bb4995f commit bb69b3b

File tree

2 files changed

+10
-8
lines changed

2 files changed

+10
-8
lines changed

builtin/worktree.c

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -234,14 +234,7 @@ static void validate_worktree_add(const char *path, const struct add_opts *opts)
234234
die(_("'%s' already exists"), path);
235235

236236
worktrees = get_worktrees(0);
237-
/*
238-
* find_worktree()'s suffix matching may undesirably find the main
239-
* rather than a linked worktree (for instance, when the basenames
240-
* of the main worktree and the one being created are the same).
241-
* We're only interested in linked worktrees, so skip the main
242-
* worktree with +1.
243-
*/
244-
wt = find_worktree(worktrees + 1, NULL, path);
237+
wt = find_worktree_by_path(worktrees, path);
245238
if (!wt)
246239
goto done;
247240

t/t2400-worktree-add.sh

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -570,6 +570,15 @@ test_expect_success '"add" an existing locked but missing worktree' '
570570
git worktree add --force --force --detach gnoo
571571
'
572572

573+
test_expect_success '"add" not tripped up by magic worktree matching"' '
574+
# if worktree "sub1/bar" exists, "git worktree add bar" in distinct
575+
# directory `sub2` should not mistakenly complain that `bar` is an
576+
# already-registered worktree
577+
mkdir sub1 sub2 &&
578+
git -C sub1 --git-dir=../.git worktree add --detach bozo &&
579+
git -C sub2 --git-dir=../.git worktree add --detach bozo
580+
'
581+
573582
test_expect_success FUNNYNAMES 'sanitize generated worktree name' '
574583
git worktree add --detach ". weird*..?.lock.lock" &&
575584
test -d .git/worktrees/---weird-.-

0 commit comments

Comments
 (0)