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

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

[Yoichi NAKAYAMA via GitGitGadget]

'git worktree repair' does not sufficiently validate the cross-references
between a linked working tree and its administrative data before repairing
them. This can cause the repair to modify the wrong .git file or gitdir in
certain situations.

This series first refactors the code to read the .git file once and extract
the worktree ID, then uses that information to validate the repair target
before modifying the cross-references.

 * [1/2] Refactor the code without changing functionality before making the
   fix
 * [2/2] Validate the worktree ID and inferred gitdir path before repairing

Yoichi NAKAYAMA (2):
  worktree repair: refactor and reduce .git file reads
  worktree repair: avoid breaking unrelated .git file and gitdir

 t/t2406-worktree-repair.sh |  33 +++++++++---
 worktree.c                 | 105 ++++++++++++++++++++-----------------
 2 files changed, 83 insertions(+), 55 deletions(-)


base-commit: 47ce80527c56f462cb97db4ca8125342204d3783
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2225%2Fyoichi%2Fworktree-repair-keep-unrelated-gitfile-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2225/yoichi/worktree-repair-keep-unrelated-gitfile-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2225
-- 
gitgitgadget

[PATCH 1/2] worktree repair: refactor and reduce .git file reads

[Yoichi NAKAYAMA via GitGitGadget]

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

Remove the file reading and trimming logic from `infer_backlink()`,
and instead read the .git file once in its caller,
`repair_worktree_at_path()`, using `read_gitfile_raw()`. Since
`read_gitfile_gently()` is replaced with `read_gitfile_raw()`, restore
the logic for constructing the absolute path and replace the
READ_GITFILE_ERR_NOT_A_REPO handling with a check using
`is_git_directory()`. Simplify the logic for prioritizing
'inferred_backlink' over 'backlink'.

Extract `get_worktree_id()` to get the worktree ID from the contents
of the .git file. We are going to modify and use this function in
subsequent commits.

Signed-off-by: Yoichi NAKAYAMA <yoichi.nakayama@gmail.com>
---
 worktree.c | 89 ++++++++++++++++++++++++++----------------------------
 1 file changed, 43 insertions(+), 46 deletions(-)

diff --git a/worktree.c b/worktree.c
index 8cb8637b18..7af13898d0 100644
--- a/worktree.c
+++ b/worktree.c
@@ -637,6 +637,14 @@ int other_head_refs(struct repository *repo,
 	return ret;
 }
 
+static const char *get_worktree_id(const char *dotgit_contents)
+{
+	const char *slash = find_last_dir_sep(dotgit_contents);
+	if (!slash)
+		return "";
+	return slash + 1;
+}
+
 /*
  * Repair worktree's /path/to/worktree/.git file if missing, corrupt, or not
  * pointing at <repo>/worktrees/<id>.
@@ -798,30 +806,20 @@ static int is_main_worktree_path(struct repository *repo, const char *path)
  * Returns -1 on failure and strbuf.len on success.
  */
 static ssize_t infer_backlink(struct repository *repo,
-			      const char *gitfile,
+			      const char *dotgit_contents,
 			      struct strbuf *inferred)
 {
-	struct strbuf actual = STRBUF_INIT;
 	const char *id;
 
-	if (strbuf_read_file(&actual, gitfile, 0) < 0)
-		goto error;
-	if (!starts_with(actual.buf, "gitdir:"))
-		goto error;
-	if (!(id = find_last_dir_sep(actual.buf)))
-		goto error;
-	strbuf_trim(&actual);
-	id++; /* advance past '/' to point at <id> */
+	id = get_worktree_id(dotgit_contents);
 	if (!*id)
 		goto error;
 	repo_common_path_replace(repo, inferred, "worktrees/%s", id);
 	if (!is_directory(inferred->buf))
 		goto error;
 
-	strbuf_release(&actual);
 	return inferred->len;
 error:
-	strbuf_release(&actual);
 	strbuf_reset(inferred); /* clear invalid path */
 	return -1;
 }
@@ -840,7 +838,8 @@ void repair_worktree_at_path(struct repository *repo,
 	struct strbuf inferred_backlink = STRBUF_INIT;
 	struct strbuf gitdir = STRBUF_INIT;
 	struct strbuf olddotgit = STRBUF_INIT;
-	char *dotgit_contents = NULL;
+	struct strbuf contents = STRBUF_INIT;
+	const char *dotgit_contents = NULL;
 	const char *repair = NULL;
 	int err;
 
@@ -856,51 +855,49 @@ void repair_worktree_at_path(struct repository *repo,
 		goto done;
 	}
 
-	infer_backlink(repo, dotgit.buf, &inferred_backlink);
-	strbuf_realpath_forgiving(&inferred_backlink, inferred_backlink.buf, 0);
-	dotgit_contents = xstrdup_or_null(read_gitfile_gently(dotgit.buf, &err));
-	if (dotgit_contents) {
-		strbuf_addstr(&backlink, dotgit_contents);
-	} else if (err == READ_GITFILE_ERR_NOT_A_FILE ||
-			err == READ_GITFILE_ERR_IS_A_DIR) {
+	err = read_gitfile_raw(&contents, dotgit.buf);
+	if (err == READ_GITFILE_ERR_NOT_A_FILE ||
+	    err == READ_GITFILE_ERR_IS_A_DIR) {
 		fn(1, dotgit.buf, _("unable to locate repository; .git is not a file"), cb_data);
 		goto done;
-	} else if (err == READ_GITFILE_ERR_NOT_A_REPO) {
-		if (inferred_backlink.len) {
-			/*
-			 * Worktree's .git file does not point at a repository
-			 * but we found a .git/worktrees/<id> in this
-			 * repository with the same <id> as recorded in the
-			 * worktree's .git file so make the worktree point at
-			 * the discovered .git/worktrees/<id>.
-			 */
-			strbuf_swap(&backlink, &inferred_backlink);
-		} else {
-			fn(1, dotgit.buf, _("unable to locate repository; .git file does not reference a repository"), cb_data);
-			goto done;
-		}
-	} else {
+	} else if (err) {
 		fn(1, dotgit.buf, _("unable to locate repository; .git file broken"), cb_data);
 		goto done;
 	}
 
+	dotgit_contents = contents.buf;
+	infer_backlink(repo, dotgit_contents, &inferred_backlink);
+	strbuf_realpath_forgiving(&inferred_backlink, inferred_backlink.buf, 0);
+
+	if (is_absolute_path(dotgit_contents)) {
+		strbuf_addstr(&backlink, dotgit_contents);
+	} else {
+		strbuf_addbuf(&backlink, &dotgit);
+		strbuf_strip_suffix(&backlink, ".git");
+		strbuf_addstr(&backlink, dotgit_contents);
+		strbuf_realpath_forgiving(&backlink, backlink.buf, 0);
+	}
+
+	if (!is_git_directory(backlink.buf) && !inferred_backlink.len) {
+		fn(1, dotgit.buf, _("unable to locate repository; .git file does not reference a repository"), cb_data);
+		goto done;
+	}
+
 	/*
 	 * If we got this far, either the worktree's .git file pointed at a
-	 * valid repository (i.e. read_gitfile_gently() returned success) or
+	 * valid repository (i.e. is_git_directory() returned true) or
 	 * the .git file did not point at a repository but we were able to
 	 * infer a suitable new value for the .git file by locating a
 	 * .git/worktrees/<id> in *this* repository corresponding to the <id>
 	 * recorded in the worktree's .git file.
 	 *
-	 * However, if, at this point, inferred_backlink is non-NULL (i.e. we
-	 * found a suitable .git/worktrees/<id> in *this* repository) *and* the
-	 * worktree's .git file points at a valid repository *and* those two
-	 * paths differ, then that indicates that the user probably *copied*
-	 * the main and linked worktrees to a new location as a unit rather
-	 * than *moving* them. Thus, the copied worktree's .git file actually
-	 * points at the .git/worktrees/<id> in the *original* repository, not
-	 * in the "copy" repository. In this case, point the "copy" worktree's
-	 * .git file at the "copy" repository.
+	 * Even if the worktree's .git file pointed at a valid repository,
+	 * it doesn't always mean that the backlink is correct. For example,
+	 * the user might have *copied* the main and linked worktrees to a
+	 * new location as a unit rather than *moving* them (the copied
+	 * worktree's .git file actually points at the .git/worktrees/<id>
+	 * 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))
 		strbuf_swap(&backlink, &inferred_backlink);
@@ -926,12 +923,12 @@ void repair_worktree_at_path(struct repository *repo,
 					     gitdir.buf, use_relative_paths);
 	}
 done:
-	free(dotgit_contents);
 	strbuf_release(&olddotgit);
 	strbuf_release(&backlink);
 	strbuf_release(&inferred_backlink);
 	strbuf_release(&gitdir);
 	strbuf_release(&dotgit);
+	strbuf_release(&contents);
 }
 
 int should_prune_worktree(struct repository *repo,
-- 
gitgitgadget

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

[Yoichi NAKAYAMA via GitGitGadget]

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

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

[Yoichi NAKAYAMA]

Friendly ping on this :)

Re: [PATCH 1/2] worktree repair: refactor and reduce .git file reads

[Junio C Hamano]

"Yoichi NAKAYAMA via GitGitGadget" <gitgitgadget@gmail.com> writes:

> +static const char *get_worktree_id(const char *dotgit_contents)
> +{
> +	const char *slash = find_last_dir_sep(dotgit_contents);
> +	if (!slash)
> +		return "";
> +	return slash + 1;
> +}

This returns a pointer into dotgit_contents; it is the last
component of a pathname, similar to what basename(3) gives us.

> @@ -798,30 +806,20 @@ static int is_main_worktree_path(struct repository *repo, const char *path)

The unified diff is a bit hard to follow, so let's see if we can
compare preimage and postimage more easily.

>  static ssize_t infer_backlink(struct repository *repo,
> -			      const char *gitfile,
>  			      struct strbuf *inferred)
>  {
> -	struct strbuf actual = STRBUF_INIT;
>  	const char *id;
>  
> -	if (strbuf_read_file(&actual, gitfile, 0) < 0)
> -		goto error;
> -	if (!starts_with(actual.buf, "gitdir:"))
> -		goto error;
> -	if (!(id = find_last_dir_sep(actual.buf)))
> -		goto error;
> -	strbuf_trim(&actual);
> -	id++; /* advance past '/' to point at <id> */
>  	if (!*id)
>  		goto error;
>  	repo_common_path_replace(repo, inferred, "worktrees/%s", id);
>  	if (!is_directory(inferred->buf))
>  		goto error;
>  
> -	strbuf_release(&actual);
>  	return inferred->len;
>  error:
> -	strbuf_release(&actual);
>  	strbuf_reset(inferred); /* clear invalid path */
>  	return -1;

We used to receive the filename of ".git", read it and made sure we
have "gitdir:" prefix, and find the last component, but then trimmed
the actual buffer.  Which means a few things.

 - If the contents of the gitfile were "gitdir:foo/bar/baz \n", our
   id pointer found the slash after "foo/bar", trimmed the buffer to
   have "gitdir:foo/bar/baz", and then incremented id, which now
   points at "baz".

 - If the contents of the gitfile were "gitdir: foo/bar/   \n", then
   after triming, the buffer would have "gitdir: foo/bar/" and id
   would be pointing at the NUL at the end, which would have lead us
   to error.

Now let's look at the new code.

> @@ -798,30 +806,20 @@ static int is_main_worktree_path(struct repository *repo, const char *path)
>   * Returns -1 on failure and strbuf.len on success.
>   */
>  static ssize_t infer_backlink(struct repository *repo,
> +			      const char *dotgit_contents,
>  			      struct strbuf *inferred)
>  {
>  	const char *id;
>  
> +	id = get_worktree_id(dotgit_contents);
>  	if (!*id)
>  		goto error;
>  	repo_common_path_replace(repo, inferred, "worktrees/%s", id);
>  	if (!is_directory(inferred->buf))
>  		goto error;
>  
>  	return inferred->len;
>  error:
>  	strbuf_reset(inferred); /* clear invalid path */
>  	return -1;

The caller is expected to give us the contents of gitfile read by
setup.c:read_gitfile_raw(), which reads the file in full, validates
that the file begins with "gitdir: " (notice the trailing space),
removes arbitrary run of CR or LF from the end, and then returns
the string after skipping "gitdir: " prefix (8 bytes).

In the normal case, read_gitfile_raw() would see "gitdir: foo/bar/baz\n" 
in the file and returns "foo/bar/baz" to our caller.  In fishy cases
we examined for the preimage above:

 - If the contents of the gitfile were "gitdir:foo/bar/baz \n", our
   caller would have received an error from read_gitfile_raw() and
   wouldn't have called us.

 - If the contents of the gitfile were "gitdir: foo/bar/   \n", our
   caller would have given us "foo/bar/   ".

get_worktree_id() will give us "baz" in the normal case, and "   "
in the last case.  We fail to error out in the latter with "*id"
check, but is_directory() check will catch us, as the inferred
directory is "worktrees/   " in that bad case.

So there are certain differences in error cases, but they behave the
same in the most basic cases.


Now, this is the caller in the preimage (i.e., what we used to do).

> @@ -856,51 +855,49 @@ void repair_worktree_at_path(struct repository *repo,
>  		goto done;
>  	}
>  
> -	infer_backlink(repo, dotgit.buf, &inferred_backlink);
> -	strbuf_realpath_forgiving(&inferred_backlink, inferred_backlink.buf, 0);
> -	dotgit_contents = xstrdup_or_null(read_gitfile_gently(dotgit.buf, &err));

We used to have infer_backlink() read the .git file to compute "worktree/$id",
then again called read_gitfile_gently() to read it again.

> -	if (dotgit_contents) {
> -		strbuf_addstr(&backlink, dotgit_contents);

This is the happy path.  We successfully read from .git and use it.

> -	} else if (err == READ_GITFILE_ERR_NOT_A_FILE ||
> -			err == READ_GITFILE_ERR_IS_A_DIR) {
>  		fn(1, dotgit.buf, _("unable to locate repository; .git is not a file"), cb_data);
>  		goto done;

This is inherited badness, but overly long lines like this one needs
to be fixed.

> -	} else if (err == READ_GITFILE_ERR_NOT_A_REPO) {

The _gently() did read something, but that does not point at a git
directory.

> -		if (inferred_backlink.len) {
> -			/*
> -			 * Worktree's .git file does not point at a repository
> -			 * but we found a .git/worktrees/<id> in this
> -			 * repository with the same <id> as recorded in the
> -			 * worktree's .git file so make the worktree point at
> -			 * the discovered .git/worktrees/<id>.
> -			 */
> -			strbuf_swap(&backlink, &inferred_backlink);

If we had the "worktree/$id" thing, we use it.

> -		} else {
> -			fn(1, dotgit.buf, _("unable to locate repository; .git file does not reference a repository"), cb_data);
> -			goto done;
> -		}
> -	} else {
>  		fn(1, dotgit.buf, _("unable to locate repository; .git file broken"), cb_data);
>  		goto done;
>  	}

These lines to show error messages should also be folded to avoid
overly long lines.

So, what does the updated code in the postimage do?

> @@ -856,51 +855,49 @@ void repair_worktree_at_path(struct repository *repo,
>  		goto done;
>  	}
>  
> +	err = read_gitfile_raw(&contents, dotgit.buf);

We use read_gitfile_raw() just once.

> +	if (err == READ_GITFILE_ERR_NOT_A_FILE ||
> +	    err == READ_GITFILE_ERR_IS_A_DIR) {
>  		fn(1, dotgit.buf, _("unable to locate repository; .git is not a file"), cb_data);
>  		goto done;
> +	} else if (err) {
>  		fn(1, dotgit.buf, _("unable to locate repository; .git file broken"), cb_data);
>  		goto done;
>  	}

The original code handled the happy case that read_gitfile_gently()
successfully returned first.  Underlying read_gitfile_raw() would
not have given any of these errors when read_gitfile_gently()
succeeded, so handling the error cases first would not affect the
behaviour of the code in these cases.  Again, these overlong lines
are annoying.

Now the simplest error cases are behind us.  How would we do in the
happy case?

> +	dotgit_contents = contents.buf;
> +	infer_backlink(repo, dotgit_contents, &inferred_backlink);
> +	strbuf_realpath_forgiving(&inferred_backlink, inferred_backlink.buf, 0);

We reuse what we already read with read_gitfile_raw(), which
prepared "worktrees/$id", and do the same realpath_forgiving()
the original used to do a bit earlier.

> +	if (is_absolute_path(dotgit_contents)) {
> +		strbuf_addstr(&backlink, dotgit_contents);

I am not sure which part of the original this logic corresponds to.
If the result from read_gitfile_raw() is an absolute path, even if
it later turns out not to be is_git_directory(), the inferred backlink
is not given a chance to act as a fallback.  The original made a
call to read_gitfile_gently() which checked is_git_directory() to
give us an error, and that is how it allowed inferred backlink to
substitute for a bad contents stored in .git file.  Now we do not
allow that fallback if .git file has an absolute path?

Ah, outside the context of this patch, before we barf for "unable to
locate repository" when we complain backlink.buf is not naming a git
directory, there is the fallback logic, and in order to reach there,
we have "if (!is_git_directory(backlink.buf) && !inferred_backlink.len)"
there.  OK, so this may be doing the same thing as the original, but
it is rather hard to follow and convince readers that this is a
no-op conversion.

> +	} else {
> +		strbuf_addbuf(&backlink, &dotgit);
> +		strbuf_strip_suffix(&backlink, ".git");
> +		strbuf_addstr(&backlink, dotgit_contents);
> +		strbuf_realpath_forgiving(&backlink, backlink.buf, 0);

This converts dotgit_contents relative to the computed backlink,
which needs to be done here because read_gitfile_gently() used to do
that for us, which we no longer use.

> +	}
> +
> +	if (!is_git_directory(backlink.buf) && !inferred_backlink.len) {
> +		fn(1, dotgit.buf, _("unable to locate repository; .git file does not reference a repository"), cb_data);
> +		goto done;
> +	}

I'll stop here.