[PATCH 2/2] worktree repair: avoid breaking unrelated .git file and gitdir

["Yoichi NAKAYAMA via GitGitGadget" <gitgitgadget@gmail.com>]

From: Yoichi NAKAYAMA <yoichi.nakayama@gmail.com>

Currently, `repair_gitfile()` does not verify whether the worktree ID
recorded in the .git file matches the worktree being repaired, which
can result in an unrelated .git file being corrupted. For instance,
if two worktree directories are swapped without using 'git worktree
move', running 'git worktree repair' in the main worktree accidentally
swaps the links between their .git files and gitdirs.

`repair_worktree_at_path()` proceeds even if it fails to infer the
gitdir path. This can result in the corruption of an unrelated
gitdir. For instance, if we copied a linked worktree to a new location
X, running 'git worktree repair X' in a working tree which does not
belong to the original repository can accidentally overwrite the
gitdir in the original repository (the scope of impact should be
limited to the repository where the command was executed).

Resolve these issues by validating the worktree ID and stopping the
repair when the ID does not match or the gitdir path cannot be
inferred.

Signed-off-by: Yoichi NAKAYAMA <yoichi.nakayama@gmail.com>
---
 t/t2406-worktree-repair.sh | 33 +++++++++++++++++++++++++++------
 worktree.c                 | 18 ++++++++++++++----
 2 files changed, 41 insertions(+), 10 deletions(-)

diff --git a/t/t2406-worktree-repair.sh b/t/t2406-worktree-repair.sh
index d4e53d492b..2ffa123f42 100755
--- a/t/t2406-worktree-repair.sh
+++ b/t/t2406-worktree-repair.sh
@@ -56,15 +56,12 @@ test_expect_success 'repair missing .git file' '
 '
 
 test_expect_success 'repair bogus .git file' '
-	test_corrupt_gitfile "echo \"gitdir: /nowhere\" >corrupt/.git" \
+	test_corrupt_gitfile "echo \"contents not started with gitdir:\" >corrupt/.git" \
 		".git file broken"
 '
 
-test_expect_success 'repair incorrect .git file' '
-	test_when_finished "rm -rf other && git worktree prune" &&
-	test_create_repo other &&
-	other=$(git -C other rev-parse --absolute-git-dir) &&
-	test_corrupt_gitfile "echo \"gitdir: $other\" >corrupt/.git" \
+test_expect_success 'repair unlinked .git file' '
+	test_corrupt_gitfile "echo \"gitdir: /nowhere/worktrees/corrupt\" >corrupt/.git" \
 		".git file incorrect"
 '
 
@@ -89,6 +86,18 @@ test_expect_success 'repair .git file from bare.git' '
 	test_cmp expect actual
 '
 
+test_expect_success 'skip unrelated .git file' '
+	test_when_finished "rm -rf corrupt other && git worktree prune" &&
+	git worktree add --detach corrupt &&
+	rm -rf corrupt &&
+	git worktree add --detach other &&
+	mv other corrupt &&
+	cat corrupt/.git >expect &&
+	test_must_fail git worktree repair 2>err &&
+	test_cmp expect corrupt/.git &&
+	test_grep "unrelated .git file" err
+'
+
 test_expect_success 'invalid worktree path' '
 	test_must_fail git worktree repair /notvalid >out 2>err &&
 	test_must_be_empty out &&
@@ -113,6 +122,18 @@ test_expect_success 'repo not found; .git not referencing repo' '
 	test_grep ".git file does not reference a repository" err
 '
 
+test_expect_success 'repo not found; .git not for worktree' '
+	test_when_finished "rm -rf side other-repo && git worktree prune" &&
+	test_create_repo other-repo &&
+	git worktree add --detach side &&
+	cat .git/worktrees/side/gitdir >expect &&
+	cp -R side other-repo/side &&
+	test_must_fail git -C other-repo worktree repair side >out 2>err &&
+	test_cmp expect .git/worktrees/side/gitdir &&
+	test_must_be_empty out &&
+	test_grep ".git file is not for a linked worktree" err
+'
+
 test_expect_success 'repo not found; .git file broken' '
 	test_when_finished "rm -rf orig moved && git worktree prune" &&
 	git worktree add --detach orig &&
diff --git a/worktree.c b/worktree.c
index 7af13898d0..88da599ab6 100644
--- a/worktree.c
+++ b/worktree.c
@@ -640,7 +640,11 @@ int other_head_refs(struct repository *repo,
 static const char *get_worktree_id(const char *dotgit_contents)
 {
 	const char *slash = find_last_dir_sep(dotgit_contents);
-	if (!slash)
+	const char *prefix = "/worktrees";
+	int prefixlen = strlen(prefix);
+	if (!slash ||
+	    slash - dotgit_contents < prefixlen ||
+	    strncmp(slash - prefixlen, prefix, prefixlen))
 		return "";
 	return slash + 1;
 }
@@ -692,8 +696,10 @@ static void repair_gitfile(struct worktree *wt,
 	if (err == READ_GITFILE_ERR_NOT_A_FILE ||
 		err == READ_GITFILE_ERR_IS_A_DIR)
 		fn(1, wt->path, _(".git is not a file"), cb_data);
-	else if (err || !is_git_directory(backlink.buf))
+	else if (err)
 		repair = _(".git file broken");
+	else if (strcmp(get_worktree_id(dotgit_contents), wt->id))
+		fn(1, wt->path, _("unrelated .git file"), cb_data);
 	else if (fspathcmp(backlink.buf, repo.buf))
 		repair = _(".git file incorrect");
 	else if (use_relative_paths == is_absolute_path(dotgit_contents))
@@ -815,7 +821,7 @@ static ssize_t infer_backlink(struct repository *repo,
 	if (!*id)
 		goto error;
 	repo_common_path_replace(repo, inferred, "worktrees/%s", id);
-	if (!is_directory(inferred->buf))
+	if (!is_git_directory(inferred->buf))
 		goto error;
 
 	return inferred->len;
@@ -882,6 +888,10 @@ void repair_worktree_at_path(struct repository *repo,
 		fn(1, dotgit.buf, _("unable to locate repository; .git file does not reference a repository"), cb_data);
 		goto done;
 	}
+	if (!inferred_backlink.len) {
+		fn(1, dotgit.buf, _("unable to locate repository; .git file is not for a linked worktree"), cb_data);
+		goto done;
+	}
 
 	/*
 	 * If we got this far, either the worktree's .git file pointed at a
@@ -899,7 +909,7 @@ void repair_worktree_at_path(struct repository *repo,
 	 * in the *original* repository, not in the "copy" repository).
 	 * Therefore, we prioritize inferred_backlink over backlink.
 	 */
-	if (inferred_backlink.len && fspathcmp(backlink.buf, inferred_backlink.buf))
+	if (fspathcmp(backlink.buf, inferred_backlink.buf))
 		strbuf_swap(&backlink, &inferred_backlink);
 
 	strbuf_addf(&gitdir, "%s/gitdir", backlink.buf);
-- 
gitgitgadget