From: Kim Altintop <kim@eagain.st>
Date: Wed, 04 Aug 2021 21:00:37 GMT
Subject: [PATCH v4] upload-pack.c: treat want-ref relative to namespace
Message-ID: <20210804205951.668140-1-kim@eagain.st>
In-Reply-To: <20210804203829.661565-1-kim@eagain.st>

When 'upload-pack' runs within the context of a git namespace, treat any
'want-ref' lines the client sends as relative to that namespace.

Also check if the wanted ref is hidden via 'hideRefs'. If it is hidden,
respond with an error as if the ref didn't exist.

Helped-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Kim Altintop <kim@eagain.st>
---

Changes from v3:

    * t5703: make tests actually pass :)


 t/t5703-upload-pack-ref-in-want.sh | 140 +++++++++++++++++++++++++++++
 upload-pack.c                      |  18 ++--
 2 files changed, 151 insertions(+), 7 deletions(-)

diff --git a/t/t5703-upload-pack-ref-in-want.sh b/t/t5703-upload-pack-ref-in-want.sh
index e9e471621d..4da86d771b 100755
--- a/t/t5703-upload-pack-ref-in-want.sh
+++ b/t/t5703-upload-pack-ref-in-want.sh
@@ -298,6 +298,146 @@ test_expect_success 'fetching with wildcard that matches multiple refs' '
 	grep "want-ref refs/heads/o/bar" log
 '

+REPO="$(pwd)/repo-ns"
+
+write_fetch_want_ref() {
+	local wanted_ref="$1"
+
+	write_command fetch
+	echo "0001"
+	echo "no-progress"
+	echo "want-ref $1"
+	echo "done"
+	echo "0000"
+}
+
+test_expect_success 'setup namespaced repo' '
+	(
+		git init -b main "$REPO" &&
+		cd "$REPO" &&
+		test_commit a &&
+		test_commit b &&
+		git checkout a &&
+		test_commit c &&
+		git checkout a &&
+		test_commit d &&
+		git update-ref refs/heads/ns-no b &&
+		git update-ref refs/namespaces/ns/refs/heads/ns-yes c &&
+		git update-ref refs/namespaces/ns/refs/heads/hidden d
+	) &&
+	git -C "$REPO" config uploadpack.allowRefInWant true
+'
+
+test_expect_success 'with namespace: want-ref is considered relative to namespace' '
+	wanted_ref=refs/heads/ns-yes &&
+
+	oid=$(git -C "$REPO" rev-parse "refs/namespaces/ns/$wanted_ref") &&
+	cat >expected_refs <<-EOF &&
+	$oid $wanted_ref
+	EOF
+	cat >expected_commits <<-EOF &&
+	$oid
+	$(git -C "$REPO" rev-parse a)
+	EOF
+
+	test-tool pkt-line pack >in <<-EOF &&
+	$(write_fetch_want_ref $wanted_ref)
+	EOF
+
+	env GIT_NAMESPACE=ns test-tool -C "$REPO" serve-v2 --stateless-rpc >out <in &&
+	check_output
+'
+
+test_expect_success 'with namespace: want-ref outside namespace is unknown' '
+	wanted_ref=refs/heads/ns-no &&
+
+	test-tool pkt-line pack >in <<-EOF &&
+	$(write_fetch_want_ref $wanted_ref)
+	EOF
+
+	test_must_fail env GIT_NAMESPACE=ns \
+		test-tool -C "$REPO" serve-v2 --stateless-rpc >out <in &&
+	grep "unknown ref" out
+'
+
+# Cross-check refs/heads/ns-no indeed exists
+test_expect_success 'without namespace: want-ref outside namespace succeeds' '
+	wanted_ref=refs/heads/ns-no &&
+
+	oid=$(git -C "$REPO" rev-parse $wanted_ref) &&
+	cat >expected_refs <<-EOF &&
+	$oid $wanted_ref
+	EOF
+	cat >expected_commits <<-EOF &&
+	$oid
+	$(git -C "$REPO" rev-parse a)
+	EOF
+
+	test-tool pkt-line pack >in <<-EOF &&
+	$(write_fetch_want_ref $wanted_ref)
+	EOF
+
+	test-tool -C "$REPO" serve-v2 --stateless-rpc >out <in &&
+	check_output
+'
+
+test_expect_success 'with namespace: hideRefs is matched, relative to namespace' '
+	wanted_ref=refs/heads/hidden &&
+	git -C "$REPO" config transfer.hideRefs $wanted_ref &&
+
+	test-tool pkt-line pack >in <<-EOF &&
+	$(write_fetch_want_ref $wanted_ref)
+	EOF
+
+	test_must_fail env GIT_NAMESPACE=ns \
+		test-tool -C "$REPO" serve-v2 --stateless-rpc >out <in &&
+	grep "unknown ref" out
+'
+
+# Cross-check refs/heads/hidden indeed exists
+test_expect_success 'with namespace: want-ref succeeds if hideRefs is removed' '
+	wanted_ref=refs/heads/hidden &&
+	git -C "$REPO" config --unset transfer.hideRefs $wanted_ref &&
+
+	oid=$(git -C "$REPO" rev-parse "refs/namespaces/ns/$wanted_ref") &&
+	cat >expected_refs <<-EOF &&
+	$oid $wanted_ref
+	EOF
+	cat >expected_commits <<-EOF &&
+	$oid
+	$(git -C "$REPO" rev-parse a)
+	EOF
+
+	test-tool pkt-line pack >in <<-EOF &&
+	$(write_fetch_want_ref $wanted_ref)
+	EOF
+
+	env GIT_NAMESPACE=ns test-tool -C "$REPO" serve-v2 --stateless-rpc >out <in &&
+	check_output
+'
+
+test_expect_success 'without namespace: relative hideRefs does not match' '
+	wanted_ref=refs/namespaces/ns/refs/heads/hidden &&
+	git -C "$REPO" config transfer.hideRefs refs/heads/hidden &&
+
+	oid=$(git -C "$REPO" rev-parse $wanted_ref) &&
+	cat >expected_refs <<-EOF &&
+	$oid $wanted_ref
+	EOF
+	cat >expected_commits <<-EOF &&
+	$oid
+	$(git -C "$REPO" rev-parse a)
+	EOF
+
+	test-tool pkt-line pack >in <<-EOF &&
+	$(write_fetch_want_ref $wanted_ref)
+	EOF
+
+	test-tool -C "$REPO" serve-v2 --stateless-rpc >out <in &&
+	check_output
+'
+
+
 . "$TEST_DIRECTORY"/lib-httpd.sh
 start_httpd

diff --git a/upload-pack.c b/upload-pack.c
index 297b76fcb4..6ce07231d3 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -1417,21 +1417,25 @@ static int parse_want_ref(struct packet_writer *writer, const char *line,
 			  struct string_list *wanted_refs,
 			  struct object_array *want_obj)
 {
-	const char *arg;
-	if (skip_prefix(line, "want-ref ", &arg)) {
+	const char *refname_nons;
+	if (skip_prefix(line, "want-ref ", &refname_nons)) {
 		struct object_id oid;
 		struct string_list_item *item;
 		struct object *o;
+		struct strbuf refname = STRBUF_INIT;

-		if (read_ref(arg, &oid)) {
-			packet_writer_error(writer, "unknown ref %s", arg);
-			die("unknown ref %s", arg);
+		strbuf_addf(&refname, "%s%s", get_git_namespace(), refname_nons);
+		if (ref_is_hidden(refname_nons, refname.buf) ||
+		    read_ref(refname.buf, &oid)) {
+			packet_writer_error(writer, "unknown ref %s", refname_nons);
+			die("unknown ref %s", refname_nons);
 		}
+		strbuf_release(&refname);

-		item = string_list_append(wanted_refs, arg);
+		item = string_list_append(wanted_refs, refname_nons);
 		item->util = oiddup(&oid);

-		o = parse_object_or_die(&oid, arg);
+		o = parse_object_or_die(&oid, refname_nons);
 		if (!(o->flags & WANTED)) {
 			o->flags |= WANTED;
 			add_object_array(o, NULL, want_obj);
--
2.32.0