[PATCH 1/3] cgit: skip the notes system when the repository failed to open

[Konstantin Ryabitsev]

prepare_repo_env() calls setup_git_directory_gently() and then loads the
display notes unconditionally. When the setup fails, because repo.path is
not a valid git directory, the notes loader still resolves
refs/notes/commits, which reads the_repository->hash_algo. Git leaves that
pointer NULL outside a repository since v2.46, so cgit dereferences NULL
and dies before prepare_repo_cmd() gets a chance to report the failure.

The gap is easy to reach through scan-path, because cgit's own is_git_dir()
only stats objects/ and HEAD while git additionally requires refs/ and a
HEAD it can validate. A repository that is being created or replicated
therefore appears in the index and crashes every page that touches it.

Skip the notes system when the setup did not find a repository, and let
prepare_repo_cmd() print the config error page it already has. Cover the
three shapes a broken repo.path can take with a test.

Fixes: 8a92df0 ("Do not load user or system gitconfig and gitattributes")
Assisted-by: LLM [analysis, codegen, tests]
Signed-off-by: Konstantin Ryabitsev <konstantin at linuxfoundation.org>
---
 cgit.c                           |  7 ++++--
 tests/t0023-invalid-repo-path.sh | 51 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/cgit.c b/cgit.c
index ca318e8..47471b0 100644
--- a/cgit.c
+++ b/cgit.c
@@ -587,9 +587,12 @@ static void prepare_repo_env(int *nongit)
 
 	/* Setup the git directory and initialize the notes system. Both of these
 	 * load local configuration from the git repository, so we do them both while
-	 * the HOME variables are unset. */
+	 * the HOME variables are unset. The notes system resolves a ref, which
+	 * needs a repository: skip it when the setup above failed, and let
+	 * prepare_repo_cmd() report the failure instead. */
 	setup_git_directory_gently(nongit);
-	load_display_notes(NULL);
+	if (!*nongit)
+		load_display_notes(NULL);
 }
 
 static int prepare_repo_cmd(int nongit)
diff --git a/tests/t0023-invalid-repo-path.sh b/tests/t0023-invalid-repo-path.sh
new file mode 100755
index 0000000..891c6c0
--- /dev/null
+++ b/tests/t0023-invalid-repo-path.sh
@@ -0,0 +1,51 @@
+#!/bin/sh
+
+test_description='Check handling of a repo.path that is not a git directory'
+. ./setup.sh
+
+# A repo.path can stop being a valid git directory at any time: the directory
+# is removed, a symlink goes dangling, or the repository format is one this
+# build refuses. cgit is supposed to answer with its "config error" page, so
+# every page has to survive the failed setup rather than dereference a
+# repository that was never opened.
+
+setup_broken_repos() {
+	rm -rf broken && mkdir -p broken/empty-dir &&
+	git init -q --bare broken/badformat &&
+	git -C broken/badformat config core.repositoryformatversion 1 &&
+	git -C broken/badformat config extensions.frobnicate true &&
+	cat >>cgitrc <<-EOF
+	repo.url=gone
+	repo.path=$PWD/broken/does-not-exist
+	repo.desc=vanished repo
+
+	repo.url=empty-dir
+	repo.path=$PWD/broken/empty-dir
+	repo.desc=not a git directory
+
+	repo.url=badformat
+	repo.path=$PWD/broken/badformat
+	repo.desc=unsupported repository format
+	EOF
+}
+
+test_expect_success 'setup' 'setup_broken_repos'
+
+for repo in gone empty-dir badformat
+do
+	for page in summary log tree commit diff refs
+	do
+		test_expect_success "$page of a $repo repo reports a config error" '
+			cgit_url "'"$repo"'/'"$page"'" >output &&
+			grep -q "config error" output &&
+			grep -q "Failed to open" output
+		'
+	done
+done
+
+test_expect_success 'a bogus repo does not crash with an object id either' '
+	cgit_url "gone/commit/&id=HEAD" >output &&
+	grep -q "config error" output
+'
+
+test_done

-- 
2.55.0