{"thread":{"id":"66320","subject":"[PATCH 0/2] worktree repair: avoid breaking unrelated .git file and gitdir","startedAt":"2026-09-13T03:20:15Z","lastAt":"2026-09-13T03:20:18Z","messageCount":3,"participants":["Yoichi NAKAYAMA via GitGitGadget"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"552632","messageId":"pull.2225.git.1789269613.gitgitgadget@gmail.com","threadId":"66320","inReplyTo":null,"subject":"[PATCH 0/2] worktree repair: avoid breaking unrelated .git file and gitdir","fromName":"Yoichi NAKAYAMA via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-09-13T03:20:11Z","receivedAt":"2026-09-13T03:20:15Z","isPatch":true,"body":"'git worktree repair' does not sufficiently validate the cross-references\nbetween a linked working tree and its administrative data before repairing\nthem. This can cause the repair to modify the wrong .git file or gitdir in\ncertain situations.\n\nThis series first refactors the code to read the .git file once and extract\nthe worktree ID, then uses that information to validate the repair target\nbefore modifying the cross-references.\n\n * [1/2] Refactor the code without changing functionality before making the\n   fix\n * [2/2] Validate the worktree ID and inferred gitdir path before repairing\n\nYoichi NAKAYAMA (2):\n  worktree repair: refactor and reduce .git file reads\n  worktree repair: avoid breaking unrelated .git file and gitdir\n\n t/t2406-worktree-repair.sh |  33 +++++++++---\n worktree.c                 | 105 ++++++++++++++++++++-----------------\n 2 files changed, 83 insertions(+), 55 deletions(-)\n\n\nbase-commit: 47ce80527c56f462cb97db4ca8125342204d3783\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2225%2Fyoichi%2Fworktree-repair-keep-unrelated-gitfile-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2225/yoichi/worktree-repair-keep-unrelated-gitfile-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/2225\n-- \ngitgitgadget\n"},{"id":"552633","messageId":"dc7ebb427bedc7318ebbf84c05ecd02063408353.1789269613.git.gitgitgadget@gmail.com","threadId":"66320","inReplyTo":"pull.2225.git.1789269613.gitgitgadget@gmail.com","subject":"[PATCH 1/2] worktree repair: refactor and reduce .git file reads","fromName":"Yoichi NAKAYAMA via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-09-13T03:20:12Z","receivedAt":"2026-09-13T03:20:17Z","isPatch":true,"body":"From: Yoichi NAKAYAMA <yoichi.nakayama@gmail.com>\n\nRemove the file reading and trimming logic from `infer_backlink()`,\nand instead read the .git file once in its caller,\n`repair_worktree_at_path()`, using `read_gitfile_raw()`. Since\n`read_gitfile_gently()` is replaced with `read_gitfile_raw()`, restore\nthe logic for constructing the absolute path and replace the\nREAD_GITFILE_ERR_NOT_A_REPO handling with a check using\n`is_git_directory()`. Simplify the logic for prioritizing\n'inferred_backlink' over 'backlink'.\n\nExtract `get_worktree_id()` to get the worktree ID from the contents\nof the .git file. We are going to modify and use this function in\nsubsequent commits.\n\nSigned-off-by: Yoichi NAKAYAMA <yoichi.nakayama@gmail.com>\n---\n worktree.c | 89 ++++++++++++++++++++++++++----------------------------\n 1 file changed, 43 insertions(+), 46 deletions(-)\n\ndiff --git a/worktree.c b/worktree.c\nindex 8cb8637b18..7af13898d0 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -637,6 +637,14 @@ int other_head_refs(struct repository *repo,\n \treturn ret;\n }\n \n+static const char *get_worktree_id(const char *dotgit_contents)\n+{\n+\tconst char *slash = find_last_dir_sep(dotgit_contents);\n+\tif (!slash)\n+\t\treturn \"\";\n+\treturn slash + 1;\n+}\n+\n /*\n  * Repair worktree's /path/to/worktree/.git file if missing, corrupt, or not\n  * pointing at <repo>/worktrees/<id>.\n@@ -798,30 +806,20 @@ static int is_main_worktree_path(struct repository *repo, const char *path)\n  * Returns -1 on failure and strbuf.len on success.\n  */\n static ssize_t infer_backlink(struct repository *repo,\n-\t\t\t      const char *gitfile,\n+\t\t\t      const char *dotgit_contents,\n \t\t\t      struct strbuf *inferred)\n {\n-\tstruct strbuf actual = STRBUF_INIT;\n \tconst char *id;\n \n-\tif (strbuf_read_file(&actual, gitfile, 0) < 0)\n-\t\tgoto error;\n-\tif (!starts_with(actual.buf, \"gitdir:\"))\n-\t\tgoto error;\n-\tif (!(id = find_last_dir_sep(actual.buf)))\n-\t\tgoto error;\n-\tstrbuf_trim(&actual);\n-\tid++; /* advance past '/' to point at <id> */\n+\tid = get_worktree_id(dotgit_contents);\n \tif (!*id)\n \t\tgoto error;\n \trepo_common_path_replace(repo, inferred, \"worktrees/%s\", id);\n \tif (!is_directory(inferred->buf))\n \t\tgoto error;\n \n-\tstrbuf_release(&actual);\n \treturn inferred->len;\n error:\n-\tstrbuf_release(&actual);\n \tstrbuf_reset(inferred); /* clear invalid path */\n \treturn -1;\n }\n@@ -840,7 +838,8 @@ void repair_worktree_at_path(struct repository *repo,\n \tstruct strbuf inferred_backlink = STRBUF_INIT;\n \tstruct strbuf gitdir = STRBUF_INIT;\n \tstruct strbuf olddotgit = STRBUF_INIT;\n-\tchar *dotgit_contents = NULL;\n+\tstruct strbuf contents = STRBUF_INIT;\n+\tconst char *dotgit_contents = NULL;\n \tconst char *repair = NULL;\n \tint err;\n \n@@ -856,51 +855,49 @@ void repair_worktree_at_path(struct repository *repo,\n \t\tgoto done;\n \t}\n \n-\tinfer_backlink(repo, dotgit.buf, &inferred_backlink);\n-\tstrbuf_realpath_forgiving(&inferred_backlink, inferred_backlink.buf, 0);\n-\tdotgit_contents = xstrdup_or_null(read_gitfile_gently(dotgit.buf, &err));\n-\tif (dotgit_contents) {\n-\t\tstrbuf_addstr(&backlink, dotgit_contents);\n-\t} else if (err == READ_GITFILE_ERR_NOT_A_FILE ||\n-\t\t\terr == READ_GITFILE_ERR_IS_A_DIR) {\n+\terr = read_gitfile_raw(&contents, dotgit.buf);\n+\tif (err == READ_GITFILE_ERR_NOT_A_FILE ||\n+\t    err == READ_GITFILE_ERR_IS_A_DIR) {\n \t\tfn(1, dotgit.buf, _(\"unable to locate repository; .git is not a file\"), cb_data);\n \t\tgoto done;\n-\t} else if (err == READ_GITFILE_ERR_NOT_A_REPO) {\n-\t\tif (inferred_backlink.len) {\n-\t\t\t/*\n-\t\t\t * Worktree's .git file does not point at a repository\n-\t\t\t * but we found a .git/worktrees/<id> in this\n-\t\t\t * repository with the same <id> as recorded in the\n-\t\t\t * worktree's .git file so make the worktree point at\n-\t\t\t * the discovered .git/worktrees/<id>.\n-\t\t\t */\n-\t\t\tstrbuf_swap(&backlink, &inferred_backlink);\n-\t\t} else {\n-\t\t\tfn(1, dotgit.buf, _(\"unable to locate repository; .git file does not reference a repository\"), cb_data);\n-\t\t\tgoto done;\n-\t\t}\n-\t} else {\n+\t} else if (err) {\n \t\tfn(1, dotgit.buf, _(\"unable to locate repository; .git file broken\"), cb_data);\n \t\tgoto done;\n \t}\n \n+\tdotgit_contents = contents.buf;\n+\tinfer_backlink(repo, dotgit_contents, &inferred_backlink);\n+\tstrbuf_realpath_forgiving(&inferred_backlink, inferred_backlink.buf, 0);\n+\n+\tif (is_absolute_path(dotgit_contents)) {\n+\t\tstrbuf_addstr(&backlink, dotgit_contents);\n+\t} else {\n+\t\tstrbuf_addbuf(&backlink, &dotgit);\n+\t\tstrbuf_strip_suffix(&backlink, \".git\");\n+\t\tstrbuf_addstr(&backlink, dotgit_contents);\n+\t\tstrbuf_realpath_forgiving(&backlink, backlink.buf, 0);\n+\t}\n+\n+\tif (!is_git_directory(backlink.buf) && !inferred_backlink.len) {\n+\t\tfn(1, dotgit.buf, _(\"unable to locate repository; .git file does not reference a repository\"), cb_data);\n+\t\tgoto done;\n+\t}\n+\n \t/*\n \t * If we got this far, either the worktree's .git file pointed at a\n-\t * valid repository (i.e. read_gitfile_gently() returned success) or\n+\t * valid repository (i.e. is_git_directory() returned true) or\n \t * the .git file did not point at a repository but we were able to\n \t * infer a suitable new value for the .git file by locating a\n \t * .git/worktrees/<id> in *this* repository corresponding to the <id>\n \t * recorded in the worktree's .git file.\n \t *\n-\t * However, if, at this point, inferred_backlink is non-NULL (i.e. we\n-\t * found a suitable .git/worktrees/<id> in *this* repository) *and* the\n-\t * worktree's .git file points at a valid repository *and* those two\n-\t * paths differ, then that indicates that the user probably *copied*\n-\t * the main and linked worktrees to a new location as a unit rather\n-\t * than *moving* them. Thus, the copied worktree's .git file actually\n-\t * points at the .git/worktrees/<id> in the *original* repository, not\n-\t * in the \"copy\" repository. In this case, point the \"copy\" worktree's\n-\t * .git file at the \"copy\" repository.\n+\t * Even if the worktree's .git file pointed at a valid repository,\n+\t * it doesn't always mean that the backlink is correct. For example,\n+\t * the user might have *copied* the main and linked worktrees to a\n+\t * new location as a unit rather than *moving* them (the copied\n+\t * worktree's .git file actually points at the .git/worktrees/<id>\n+\t * in the *original* repository, not in the \"copy\" repository).\n+\t * Therefore, we prioritize inferred_backlink over backlink.\n \t */\n \tif (inferred_backlink.len && fspathcmp(backlink.buf, inferred_backlink.buf))\n \t\tstrbuf_swap(&backlink, &inferred_backlink);\n@@ -926,12 +923,12 @@ void repair_worktree_at_path(struct repository *repo,\n \t\t\t\t\t     gitdir.buf, use_relative_paths);\n \t}\n done:\n-\tfree(dotgit_contents);\n \tstrbuf_release(&olddotgit);\n \tstrbuf_release(&backlink);\n \tstrbuf_release(&inferred_backlink);\n \tstrbuf_release(&gitdir);\n \tstrbuf_release(&dotgit);\n+\tstrbuf_release(&contents);\n }\n \n int should_prune_worktree(struct repository *repo,\n-- \ngitgitgadget\n\n"},{"id":"552634","messageId":"99aa34135c481e7cd7605788408055157d09fa19.1789269613.git.gitgitgadget@gmail.com","threadId":"66320","inReplyTo":"pull.2225.git.1789269613.gitgitgadget@gmail.com","subject":"[PATCH 2/2] worktree repair: avoid breaking unrelated .git file and gitdir","fromName":"Yoichi NAKAYAMA via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-09-13T03:20:13Z","receivedAt":"2026-09-13T03:20:18Z","isPatch":true,"body":"From: Yoichi NAKAYAMA <yoichi.nakayama@gmail.com>\n\nCurrently, `repair_gitfile()` does not verify whether the worktree ID\nrecorded in the .git file matches the worktree being repaired, which\ncan result in an unrelated .git file being corrupted. For instance,\nif two worktree directories are swapped without using 'git worktree\nmove', running 'git worktree repair' in the main worktree accidentally\nswaps the links between their .git files and gitdirs.\n\n`repair_worktree_at_path()` proceeds even if it fails to infer the\ngitdir path. This can result in the corruption of an unrelated\ngitdir. For instance, if we copied a linked worktree to a new location\nX, running 'git worktree repair X' in a working tree which does not\nbelong to the original repository can accidentally overwrite the\ngitdir in the original repository (the scope of impact should be\nlimited to the repository where the command was executed).\n\nResolve these issues by validating the worktree ID and stopping the\nrepair when the ID does not match or the gitdir path cannot be\ninferred.\n\nSigned-off-by: Yoichi NAKAYAMA <yoichi.nakayama@gmail.com>\n---\n t/t2406-worktree-repair.sh | 33 +++++++++++++++++++++++++++------\n worktree.c                 | 18 ++++++++++++++----\n 2 files changed, 41 insertions(+), 10 deletions(-)\n\ndiff --git a/t/t2406-worktree-repair.sh b/t/t2406-worktree-repair.sh\nindex d4e53d492b..2ffa123f42 100755\n--- a/t/t2406-worktree-repair.sh\n+++ b/t/t2406-worktree-repair.sh\n@@ -56,15 +56,12 @@ test_expect_success 'repair missing .git file' '\n '\n \n test_expect_success 'repair bogus .git file' '\n-\ttest_corrupt_gitfile \"echo \\\"gitdir: /nowhere\\\" >corrupt/.git\" \\\n+\ttest_corrupt_gitfile \"echo \\\"contents not started with gitdir:\\\" >corrupt/.git\" \\\n \t\t\".git file broken\"\n '\n \n-test_expect_success 'repair incorrect .git file' '\n-\ttest_when_finished \"rm -rf other && git worktree prune\" &&\n-\ttest_create_repo other &&\n-\tother=$(git -C other rev-parse --absolute-git-dir) &&\n-\ttest_corrupt_gitfile \"echo \\\"gitdir: $other\\\" >corrupt/.git\" \\\n+test_expect_success 'repair unlinked .git file' '\n+\ttest_corrupt_gitfile \"echo \\\"gitdir: /nowhere/worktrees/corrupt\\\" >corrupt/.git\" \\\n \t\t\".git file incorrect\"\n '\n \n@@ -89,6 +86,18 @@ test_expect_success 'repair .git file from bare.git' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'skip unrelated .git file' '\n+\ttest_when_finished \"rm -rf corrupt other && git worktree prune\" &&\n+\tgit worktree add --detach corrupt &&\n+\trm -rf corrupt &&\n+\tgit worktree add --detach other &&\n+\tmv other corrupt &&\n+\tcat corrupt/.git >expect &&\n+\ttest_must_fail git worktree repair 2>err &&\n+\ttest_cmp expect corrupt/.git &&\n+\ttest_grep \"unrelated .git file\" err\n+'\n+\n test_expect_success 'invalid worktree path' '\n \ttest_must_fail git worktree repair /notvalid >out 2>err &&\n \ttest_must_be_empty out &&\n@@ -113,6 +122,18 @@ test_expect_success 'repo not found; .git not referencing repo' '\n \ttest_grep \".git file does not reference a repository\" err\n '\n \n+test_expect_success 'repo not found; .git not for worktree' '\n+\ttest_when_finished \"rm -rf side other-repo && git worktree prune\" &&\n+\ttest_create_repo other-repo &&\n+\tgit worktree add --detach side &&\n+\tcat .git/worktrees/side/gitdir >expect &&\n+\tcp -R side other-repo/side &&\n+\ttest_must_fail git -C other-repo worktree repair side >out 2>err &&\n+\ttest_cmp expect .git/worktrees/side/gitdir &&\n+\ttest_must_be_empty out &&\n+\ttest_grep \".git file is not for a linked worktree\" err\n+'\n+\n test_expect_success 'repo not found; .git file broken' '\n \ttest_when_finished \"rm -rf orig moved && git worktree prune\" &&\n \tgit worktree add --detach orig &&\ndiff --git a/worktree.c b/worktree.c\nindex 7af13898d0..88da599ab6 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -640,7 +640,11 @@ int other_head_refs(struct repository *repo,\n static const char *get_worktree_id(const char *dotgit_contents)\n {\n \tconst char *slash = find_last_dir_sep(dotgit_contents);\n-\tif (!slash)\n+\tconst char *prefix = \"/worktrees\";\n+\tint prefixlen = strlen(prefix);\n+\tif (!slash ||\n+\t    slash - dotgit_contents < prefixlen ||\n+\t    strncmp(slash - prefixlen, prefix, prefixlen))\n \t\treturn \"\";\n \treturn slash + 1;\n }\n@@ -692,8 +696,10 @@ static void repair_gitfile(struct worktree *wt,\n \tif (err == READ_GITFILE_ERR_NOT_A_FILE ||\n \t\terr == READ_GITFILE_ERR_IS_A_DIR)\n \t\tfn(1, wt->path, _(\".git is not a file\"), cb_data);\n-\telse if (err || !is_git_directory(backlink.buf))\n+\telse if (err)\n \t\trepair = _(\".git file broken\");\n+\telse if (strcmp(get_worktree_id(dotgit_contents), wt->id))\n+\t\tfn(1, wt->path, _(\"unrelated .git file\"), cb_data);\n \telse if (fspathcmp(backlink.buf, repo.buf))\n \t\trepair = _(\".git file incorrect\");\n \telse if (use_relative_paths == is_absolute_path(dotgit_contents))\n@@ -815,7 +821,7 @@ static ssize_t infer_backlink(struct repository *repo,\n \tif (!*id)\n \t\tgoto error;\n \trepo_common_path_replace(repo, inferred, \"worktrees/%s\", id);\n-\tif (!is_directory(inferred->buf))\n+\tif (!is_git_directory(inferred->buf))\n \t\tgoto error;\n \n \treturn inferred->len;\n@@ -882,6 +888,10 @@ void repair_worktree_at_path(struct repository *repo,\n \t\tfn(1, dotgit.buf, _(\"unable to locate repository; .git file does not reference a repository\"), cb_data);\n \t\tgoto done;\n \t}\n+\tif (!inferred_backlink.len) {\n+\t\tfn(1, dotgit.buf, _(\"unable to locate repository; .git file is not for a linked worktree\"), cb_data);\n+\t\tgoto done;\n+\t}\n \n \t/*\n \t * If we got this far, either the worktree's .git file pointed at a\n@@ -899,7 +909,7 @@ void repair_worktree_at_path(struct repository *repo,\n \t * in the *original* repository, not in the \"copy\" repository).\n \t * Therefore, we prioritize inferred_backlink over backlink.\n \t */\n-\tif (inferred_backlink.len && fspathcmp(backlink.buf, inferred_backlink.buf))\n+\tif (fspathcmp(backlink.buf, inferred_backlink.buf))\n \t\tstrbuf_swap(&backlink, &inferred_backlink);\n \n \tstrbuf_addf(&gitdir, \"%s/gitdir\", backlink.buf);\n-- \ngitgitgadget\n"}]}