public inbox for git@vger.kernel.org
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: Aaron Plattner <aplattner@nvidia.com>
Cc: git@vger.kernel.org, Rahul Rameshbabu <rrameshbabu@nvidia.com>,
	Lucas De Marchi <demarchi@kernel.org>
Subject: Re: [PATCH v2] remote-curl: Use auth for probe_rpc() requests too
Date: Tue, 13 Jan 2026 07:49:00 +0100	[thread overview]
Message-ID: <aWXq3Lrzx9NGnwGb@pks.im> (raw)
In-Reply-To: <20260113031929.3746753-1-aplattner@nvidia.com>

On Mon, Jan 12, 2026 at 07:19:28PM -0800, Aaron Plattner wrote:
> If a large request requires post_rpc() to call probe_rpc(), the latter
> does not use the authorization credentials used for other requests. If
> this fails with an HTTP 401 error and http_auth.multistage isn't set,
> then the whole request just fails.
> 
> For example, using git-credential-msal [1], the following attempt to clone a
> large repository fails partway through because the initial request to download
> the commit history and promisor packs succeeds, but the
> subsequent request to download the blobs needed to construct the working
> tree fails with a 401 error and the checkout fails.
> 
> (lines removed for brevity)
> 
>   git clone --filter=blob:none https://secure-server.example/repo
>   11:03:26.855369 git.c:502               trace: built-in: git clone --filter=blob:none https://secure-server.example/repo
>   Cloning into 'sw'...
>   warning: templates not found in /home/aaron/share/git-core/templates
>   11:03:26.857169 run-command.c:673       trace: run_command: git remote-https origin https://secure-server.example/repo
>   11:03:27.012104 http.c:849              => Send header: GET repo/info/refs?service=git-upload-pack HTTP/1.1
>   11:03:27.049243 http.c:849              <= Recv header: HTTP/1.1 401 Unauthorized
>   11:03:27.049270 http.c:849              <= Recv header: WWW-Authenticate: Bearer error="invalid_request", error_description="No bearer token found in the request", msal-tenant-id="<tenant>", msal-client-id="<client>"
>   11:03:27.053786 run-command.c:673       trace: run_command: 'git credential-msal get'
>   11:03:27.952830 http.c:849              => Send header: GET repo/info/refs?service=git-upload-pack HTTP/1.1
>   11:03:27.952849 http.c:849              => Send header: Authorization: Bearer <redacted>
>   11:03:27.995419 http.c:849              <= Recv header: HTTP/1.1 200 OK
>   11:03:28.230039 http.c:890              == Info: Reusing existing https: connection with host secure-server.example
>   11:03:28.230208 http.c:849              => Send header: POST repo/git-upload-pack HTTP/1.1
>   11:03:28.230216 http.c:849              => Send header: Content-Type: application/x-git-upload-pack-request
>   11:03:28.230221 http.c:849              => Send header: Authorization: Bearer <redacted>
>   11:03:28.269085 http.c:849              <= Recv header: HTTP/1.1 200 OK
>   11:03:28.684163 http.c:890              == Info: Reusing existing https: connection with host secure-server.example
>   11:03:28.684379 http.c:849              => Send header: POST repo/git-upload-pack HTTP/1.1
>   11:03:28.684391 http.c:849              => Send header: Accept: application/x-git-upload-pack-result
>   11:03:28.684393 http.c:849              => Send header: Authorization: Bearer <redacted>
>   11:03:28.869546 run-command.c:673       trace: run_command: git index-pack --stdin --fix-thin '--keep=fetch-pack 43856 on dgx-spark' --promisor
>   11:06:39.861237 run-command.c:673       trace: run_command: git -c fetch.negotiationAlgorithm=noop fetch origin --no-tags --no-write-fetch-head --recurse-submodules=no --filter=blob:none --stdin
>   11:06:39.865981 run-command.c:673       trace: run_command: git remote-https origin https://secure-server.example/repo
>   11:06:39.868039 run-command.c:673       trace: run_command: git-remote-https origin https://secure-server.example/repo
>   11:07:30.412575 http.c:849              => Send header: GET repo/info/refs?service=git-upload-pack HTTP/1.1
>   11:07:30.456285 http.c:849              <= Recv header: HTTP/1.1 401 Unauthorized
>   11:07:30.456318 http.c:849              <= Recv header: WWW-Authenticate: Bearer error="invalid_request", error_description="No bearer token found in the request", msal-tenant-id="<tenant>", msal-client-id="<client>"
>   11:07:30.456439 run-command.c:673       trace: run_command: 'git credential-cache get'
>   11:07:30.461266 http.c:849              => Send header: GET repo/info/refs?service=git-upload-pack HTTP/1.1
>   11:07:30.461282 http.c:849              => Send header: Authorization: Bearer <redacted>
>   11:07:30.501628 http.c:849              <= Recv header: HTTP/1.1 200 OK
>   11:07:34.725262 http.c:849              => Send header: POST repo/git-upload-pack HTTP/1.1
>   11:07:34.725279 http.c:849              => Send header: Content-Type: application/x-git-upload-pack-request
>   11:07:34.761407 http.c:849              <= Recv header: HTTP/1.1 401 Unauthorized
>   11:07:34.761443 http.c:890              == Info: Bearer authentication problem, ignoring.
>   11:07:34.761453 http.c:849              <= Recv header: WWW-Authenticate: Bearer error="invalid_request", error_description="No bearer token found in the request", msal-tenant-id="<tenant>", msal-client-id="<client>"
>   11:07:34.761509 http.c:890              == Info: The requested URL returned error: 401
>   11:07:34.761530 http.c:890              == Info: closing connection #0
>   11:07:34.761913 run-command.c:673       trace: run_command: 'git credential-cache erase'
>   11:07:34.761927 run-command.c:765       trace: start_command: /bin/sh -c 'git credential-cache erase' 'git credential-cache erase'
>   11:07:34.768069 git.c:502               trace: built-in: git credential-cache erase
>   11:07:34.768690 run-command.c:673       trace: run_command: 'git credential-msal erase'
>   11:07:34.768713 run-command.c:765       trace: start_command: /bin/sh -c 'git credential-msal erase' 'git credential-msal erase'
>   11:07:34.772742 git.c:808               trace: exec: git-credential-msal erase
>   11:07:34.772783 run-command.c:673       trace: run_command: git-credential-msal erase
>   11:07:34.772819 run-command.c:765       trace: start_command: /usr/bin/git-credential-msal erase
>   error: RPC failed; HTTP 401 curl 22 The requested URL returned error: 401
>   fatal: unable to write request to remote: Broken pipe
>   fatal: could not fetch c4fff0229c9be06ecf576356a4d39a8a755b8d81 from promisor remote
>   warning: Clone succeeded, but checkout failed.
>   You can inspect what was checked out with 'git status'
>   and retry with 'git restore --source=HEAD :/'
> 
> In this case, the HTTP_REAUTH retry logic is not used because the
> credential helper didn't set the 'continue' flag, so
> http_auth.multistage is false and handle_curl_result() fails with
> HTTP_NOAUTH instead.

Nice, this is a helpful little detail.

> diff --git a/remote-curl.c b/remote-curl.c
> index 69f919454a..1d0ae72521 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -877,6 +877,8 @@ static int probe_rpc(struct rpc_state *rpc, struct slot_results *results)
>  	headers = curl_slist_append(headers, rpc->hdr_content_type);
>  	headers = curl_slist_append(headers, rpc->hdr_accept);
>  
> +	headers = http_append_auth_header(&http_auth, headers);
> +
>  	curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0L);
>  	curl_easy_setopt(slot->curl, CURLOPT_POST, 1L);
>  	curl_easy_setopt(slot->curl, CURLOPT_URL, rpc->service_url);
> diff --git a/t/t5563-simple-http-auth.sh b/t/t5563-simple-http-auth.sh
> index c1febbae9d..adc210cdd3 100755
> --- a/t/t5563-simple-http-auth.sh
> +++ b/t/t5563-simple-http-auth.sh
> @@ -63,6 +63,26 @@ test_expect_success 'setup repository' '
>  	git push --mirror "$HTTPD_DOCUMENT_ROOT_PATH/repo.git"
>  '
>  
> +test_expect_success 'setup large repository for probe_rpc testing' '
> +	rm -rf large.git &&

No previous test creates this directory, so it shouldn't ever exist.

> +	git init large.git &&
> +	(
> +		cd large.git &&
> +		git config set maintenance.auto false &&
> +		git commit --allow-empty --message "initial" &&
> +		# Create many refs to trigger probe_rpc, which is called when
> +		# the request body is larger than http.postBuffer.
> +		#
> +		# In the test later, http.postBuffer is set to 70000. Each
> +		# "want" line is ~45 bytes, so we need at least 70000/45 = ~1600
> +		# refs
> +		printf "create refs/heads/branch-%d @\n" $(test_seq 2000) |
> +		    git update-ref --stdin
> +	) &&
> +	git clone --bare large.git "$HTTPD_DOCUMENT_ROOT_PATH/large.git" &&
> +	rm -rf large.git

You can use "test_when_finished rm -rf large.git" before creating the
repo to ensure it's always getting pruned at the end of the test.

> +'
> +
>  test_expect_success 'access using basic auth' '
>  	test_when_finished "per_test_cleanup" &&
>  

I think we can simply inline this setup into the below test. We only use
it in that specific test anyway.

> @@ -605,6 +625,32 @@ test_expect_success 'access using bearer auth with invalid credentials' '
>  	EOF
>  '
>  
> +test_expect_success 'clone with bearer auth and probe_rpc' '
> +	test_when_finished "per_test_cleanup" &&
> +
> +	set_credential_reply get <<-EOF &&
> +	capability[]=authtype
> +	authtype=Bearer
> +	credential=YS1naXQtdG9rZW4=
> +	EOF
> +
> +	# Bearer token
> +	cat >"$HTTPD_ROOT_PATH/custom-auth.valid" <<-EOF &&
> +	id=1 creds=Bearer YS1naXQtdG9rZW4=
> +	EOF
> +
> +	cat >"$HTTPD_ROOT_PATH/custom-auth.challenge" <<-EOF &&
> +	id=1 status=200
> +	id=default response=WWW-Authenticate: Bearer authorize_uri="id.example.com"
> +	EOF
> +
> +	# Set a small buffer to force probe_rpc to be called
> +	# Must be > LARGE_PACKET_MAX (65520)
> +	test_config_global http.postBuffer 70000 &&
> +	test_config_global credential.helper test-helper &&
> +	git clone "$HTTPD_URL/custom_auth/large.git" partial-auth-clone 2>clone-error
> +'
> +

Yup, I've verified that this test fails when the bug fix is reverted.
Good.

You might want to reroll to address my two nits around the test, but
even without that reroll I'm already happy with this v2. Thanks!

Patrick

  reply	other threads:[~2026-01-13  6:49 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-13  3:19 [PATCH v2] remote-curl: Use auth for probe_rpc() requests too Aaron Plattner
2026-01-13  6:49 ` Patrick Steinhardt [this message]
2026-01-13 13:09 ` Junio C Hamano
2026-01-14  1:06   ` Aaron Plattner
2026-01-14  2:20     ` Jeff King
2026-01-14 14:11       ` Junio C Hamano
2026-01-14 16:33         ` Aaron Plattner
2026-01-14 17:30           ` Jeff King
2026-01-14 17:41           ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aWXq3Lrzx9NGnwGb@pks.im \
    --to=ps@pks.im \
    --cc=aplattner@nvidia.com \
    --cc=demarchi@kernel.org \
    --cc=git@vger.kernel.org \
    --cc=rrameshbabu@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox