Thread (6 messages) 6 messages, 3 authors, 2026-01-12

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

From: Aaron Plattner <hidden>
Date: 2026-01-09 17:58:20

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:
quoted
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.
quoted
(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.
quoted
   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.
quoted
   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 <redacted>
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.
quoted
  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
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help