[PATCH 3/3] promisor-remote: teach 'fromAccepted' to GIT_NO_LAZY_FETCH

[Christian Couder <christian.couder@gmail.com>]

The `GIT_NO_LAZY_FETCH` environment variable can be set to 'true' or
'false' to enable or disable lazy fetching. By default it is set to
'true' when calling `git upload-pack` to avoid security issues, see
7b70e9efb1 (upload-pack: disable lazy-fetching by default, 2024-04-16).

Recently though, the "promisor-remote" capability was introduced into
protocol v2, which allows a server to advertise some promisor remotes
and clients to accept them or not.

When promisor remotes are advertised by the server and accepted by the
client, it means that they are quite trusted. So the security risks
which come from lazy fetching from them could be considered much more
acceptable.

Let's introduce a 'fromAccepted' option on top of 'true' and 'false'
for `GIT_NO_LAZY_FETCH` to allow lazy fetching only from accepted
promisor remotes.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/git-upload-pack.adoc    |  5 +++
 Documentation/git.adoc                |  6 ++--
 promisor-remote.c                     | 14 +++++++-
 promisor-remote.h                     |  1 +
 t/t5710-promisor-remote-capability.sh | 49 +++++++++++++++++++++++++++
 5 files changed, 71 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-upload-pack.adoc b/Documentation/git-upload-pack.adoc
index 9167a321d0..1c2ed9d7ba 100644
--- a/Documentation/git-upload-pack.adoc
+++ b/Documentation/git-upload-pack.adoc
@@ -71,6 +71,11 @@ This is implemented by having `upload-pack` internally set the
 (because you are fetching from a partial clone, and you are sure
 you trust it), you can explicitly set `GIT_NO_LAZY_FETCH` to
 `0`.
++
+`GIT_NO_LAZY_FETCH` can also be set to 'fromAccepted' which allows
+lazy fetching only from remotes that are advertised and accepted using
+the "promisor-remote" protocol v2 capability. See
+linkgit:gitprotocol-v2[5]. This is safer than setting it to `0`.
 
 SECURITY
 --------
diff --git a/Documentation/git.adoc b/Documentation/git.adoc
index 8a5cdd3b3d..14a083bcdb 100644
--- a/Documentation/git.adoc
+++ b/Documentation/git.adoc
@@ -947,9 +947,9 @@ for full details.
 	pathspecs as case-insensitive.
 
 `GIT_NO_LAZY_FETCH`::
-	Setting this Boolean environment variable to true tells Git
-	not to lazily fetch missing objects from the promisor remote
-	on demand.
+	Setting this environment variable controls whether Git is
+	allowed to lazily fetch missing objects from a promisor remote
+	on demand. See linkgit:git-upload-pack[1].
 
 `GIT_REFLOG_ACTION`::
 	When a ref is updated, reflog entries are created to keep
diff --git a/promisor-remote.c b/promisor-remote.c
index 56f57c5267..c80319f966 100644
--- a/promisor-remote.c
+++ b/promisor-remote.c
@@ -35,9 +35,11 @@ enum allow_lazy_fetch parse_allow_lazy_fetch_env(void)
 		return LAZY_FETCH_ALL;
 	if (val > 0)
 		return LAZY_FETCH_NONE;
+	if (!strcasecmp(v, "fromAccepted"))
+		return LAZY_FETCH_ACCEPTED;
 
 	die(_("bad environment value '%s' for '%s'; "
-	      "only 'false/0' and 'true/1' are valid"),
+	      "only 'false/0', 'true/1' and 'fromAccepted' are valid"),
 	    v, NO_LAZY_FETCH_ENVIRONMENT);
 }
 
@@ -337,6 +339,16 @@ static bool lazy_fetch_objects(struct repository *repo,
 				 to_free, true))
 		return true;
 
+	if (lf == LAZY_FETCH_ACCEPTED) {
+		static int warning_shown;
+		if (!warning_shown) {
+			warning_shown = 1;
+			warning(_("lazy fetching from accepted promisor remotes only; "
+				  "some objects may not be available"));
+		}
+		return false;
+	}
+
 	return try_promisor_remotes(repo, remaining_oids, remaining_nr,
 				    to_free, false);
 }
diff --git a/promisor-remote.h b/promisor-remote.h
index 87fc24c9eb..0d05ff9d84 100644
--- a/promisor-remote.h
+++ b/promisor-remote.h
@@ -28,6 +28,7 @@ int repo_has_promisor_remote(struct repository *r);
 /* Enum for lazy fetching parsing */
 enum allow_lazy_fetch {
 	LAZY_FETCH_NONE    = 0,  /* No lazy fetching */
+	LAZY_FETCH_ACCEPTED,     /* Lazy fetching only from accepted promisor remotes */
 	LAZY_FETCH_ALL           /* Lazy fetch from any promisor remotes */
 };
 
diff --git a/t/t5710-promisor-remote-capability.sh b/t/t5710-promisor-remote-capability.sh
index 549acff23f..1c61b100b9 100755
--- a/t/t5710-promisor-remote-capability.sh
+++ b/t/t5710-promisor-remote-capability.sh
@@ -173,6 +173,55 @@ test_expect_success "clone with promisor.acceptfromserver set to 'None'" '
 	initialize_server 1 "$oid"
 '
 
+test_expect_success "clone with GIT_NO_LAZY_FETCH=fromAccepted and accepted promisor remote" '
+	git -C server config promisor.advertise true &&
+	test_when_finished "rm -rf client" &&
+
+	# Clone from server to create a client
+	GIT_NO_LAZY_FETCH=fromAccepted git clone -c remote.lop.promisor=true \
+		-c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \
+		-c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \
+		-c promisor.acceptfromserver=All \
+		--no-local --filter="blob:limit=5k" server client &&
+
+	# Check that the largest object is still missing on the server
+	check_missing_objects server 1 "$oid"
+'
+
+test_expect_success "clone with GIT_NO_LAZY_FETCH=fromAccepted and no accepted promisor remote" '
+	git -C server config promisor.advertise true &&
+	test_when_finished "rm -rf client" &&
+
+	# Clone from server to create a client
+	# It should fail because the server cannot lazy fetch the missing blob
+	test_must_fail env GIT_NO_LAZY_FETCH=fromAccepted git clone -c remote.lop.promisor=true \
+		-c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \
+		-c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \
+		-c promisor.acceptfromserver=None \
+		--no-local --filter="blob:limit=5k" server client 2>err &&
+
+	test_grep "lazy fetching from accepted promisor remotes only" err &&
+
+	# Check that the largest object is still missing on the server
+	check_missing_objects server 1 "$oid"
+'
+
+test_expect_success "clone failure with GIT_NO_LAZY_FETCH=bogus" '
+	git -C server config promisor.advertise true &&
+	test_when_finished "rm -rf client" &&
+
+	test_must_fail env GIT_NO_LAZY_FETCH=bogus git clone -c remote.lop.promisor=true \
+		-c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \
+		-c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \
+		-c promisor.acceptfromserver=All \
+		--no-local --filter="blob:limit=5k" server client 2>err &&
+
+	test_grep "bad environment value" err &&
+
+	# Check that the largest object is still missing on the server
+	check_missing_objects server 1 "$oid"
+'
+
 test_expect_success "init + fetch with promisor.advertise set to 'true'" '
 	git -C server config promisor.advertise true &&
 	test_when_finished "rm -rf client" &&
-- 
2.55.0.125.g395cd2c8ec.dirty