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 <cameron.gunnin@synopsys.com>
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/worktree.c b/builtin/worktree.c
index d6bc5263f175cedf42a87423da246c0a2f7f6f81..24f22800f38c759d123d7e307de488f0c8ee852f 100644 (file)
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -234,14 +234,7 @@ static void validate_worktree_add(const char *path, const struct add_opts *opts)
                die(_("'%s' already exists"), path);
 
        worktrees = get_worktrees(0);
-       /*
-        * find_worktree()'s suffix matching may undesirably find the main
-        * rather than a linked worktree (for instance, when the basenames
-        * of the main worktree and the one being created are the same).
-        * We're only interested in linked worktrees, so skip the main
-        * worktree with +1.
-        */
-       wt = find_worktree(worktrees + 1, NULL, path);
+       wt = find_worktree_by_path(worktrees, path);
        if (!wt)
                goto done;
 

diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
index b5ece19460cde97e6102dc34097fe57fcee77ac9..5a7495474aa8723f77b296813381151ca39b4b8b 100755 (executable)
--- a/t/t2400-worktree-add.sh
+++ b/t/t2400-worktree-add.sh
@@ -570,6 +570,15 @@ test_expect_success '"add" an existing locked but missing worktree' '
        git worktree add --force --force --detach gnoo
 '
 
+test_expect_success '"add" not tripped up by magic worktree matching"' '
+       # if worktree "sub1/bar" exists, "git worktree add bar" in distinct
+       # directory `sub2` should not mistakenly complain that `bar` is an
+       # already-registered worktree
+       mkdir sub1 sub2 &&
+       git -C sub1 --git-dir=../.git worktree add --detach bozo &&
+       git -C sub2 --git-dir=../.git worktree add --detach bozo
+'
+
 test_expect_success FUNNYNAMES 'sanitize generated worktree name' '
        git worktree add --detach ".  weird*..?.lock.lock" &&
        test -d .git/worktrees/---weird-.-