Re: [PATCH] remote-curl: Use auth for probe_rpc() requests too

[Aaron Plattner <aplattner@nvidia.com>]

On 1/9/26 6:51 AM, Patrick Steinhardt wrote:
> Hi,
> 
> sorry for taking so long to review your patch, but I didn't really dare
> to review it as I'm not that familiar with the subsystem in question.
> But given that nobody else reviewed it, either, let me try my best to at
> least provide _some_ helpful feedback to hopefully move this forward.

Thanks Patrick!

> On Wed, Nov 12, 2025 at 02:37:18PM -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.
> 
> Okay.
> 
>> (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
> 
> Okay, here we see the 401 error code.
> 
>>    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
> 
> And as we think that we've already set up authentication, this error
> code will cause us to think that the credentials that we've got are
> invalid. Consequently, we invalidate the credentials that we've stored.
> Naturally, this will cause _all_ subsequent requests to fail as we're no
> longer authenticated at all.
> 
>>    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 :/'
>>
>> Fix the immediate problem by including the authorization headers in the
>> probe_rpc() request as well.
>>
>> Signed-off-by: Aaron Plattner <aplattner@nvidia.com>
>> Link: [1] https://github.com/Binary-Eater/git-credential-msal
>> ---
>> If http_auth.multistage were set in this scenario, then probe_rpc() would have
>> returned HTTP_REAUTH and this would have probably worked by generating a new
>> Bearer token. And we might need to use HTTP_REAUTH to handle the case where the
>> token expires between the initial request and this one, but I don't think
>> tackling that in this patch makes sense since the original Bearer token was
>> still valid and git just didn't try using it. And setting multistage (the
>> 'continue' parameter in git-credential(1)) doesn't make sense for Bearer tokens
>> since the token comes from an external agent.
> 
> This is something I was wondering about. Specifically, I saw the loop
> that we had around `HTTP_REAUTH`:
> 
> 		do {
> 			err = probe_rpc(rpc, &results);
> 			if (err == HTTP_REAUTH)
> 				credential_fill(the_repository, &http_auth, 0);
> 		} while (err == HTTP_REAUTH);
> 
> I then double-checked that we indeed get `HTTP_REAUTH` as an error code
> on a 401, so I was wondering why this doesn't lead to an infinite loop.
> I didn't connect it with the "multistage" thing though.

Right. I think we don't actually get HTTP_REAUTH because of this logic 
in handle_curl_result:

	else if (results->http_code == 401) {
		if ((http_auth.username && http_auth.password) ||\
		    (http_auth.authtype && http_auth.credential)) {
			if (http_auth.multistage) {
				credential_clear_secrets(&http_auth);
				return HTTP_REAUTH;
			}
			credential_reject(the_repository, &http_auth);
			if (always_auth_proactively())
				http_proactive_auth = PROACTIVE_AUTH_NONE;
			return HTTP_NOAUTH;
		} else {
			http_auth_methods &= ~CURLAUTH_GSSNEGOTIATE;
			if (results->auth_avail) {
				http_auth_methods &= results->auth_avail;
				http_auth_methods_restricted = 1;
			}
			return HTTP_REAUTH;
		}
	}

In this case, http_auth.authtype and http_auth.credential are set and 
http_auth.multistage is false. So it proceeds to call 
credential_reject() and return with HTTP_NOAUTH which causes the calling 
functions to fail immediately.

> In any case, I think this information would be useful to have in the
> commit message to help guide readers.
> 
>>   remote-curl.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> 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);
> 
> The change looks simple enough, and matches what we do in `post_rpc()`
> itself.
> 
> It would be great to have a test case for this. It might be possible to
> use t5563-simple-http-auth as an example, where we already know to set
> up an HTTP server with authentication.

I'll look into that. It wasn't obvious to me how to make it hit this RPC 
case specifically but I'll see if I can figure out a way.

-- Aaron

> 
> Thanks!
> 
> Patrick