From: Patrick Steinhardt <ps@pks.im>
Date: Mon, 12 Jan 2026 08:21:57 GMT
Subject: Re: [PATCH] remote-curl: Use auth for probe_rpc() requests too
Message-ID: <aWSvJbYBWpJc-Vcc@pks.im>
In-Reply-To: <c09387eb-0847-4130-85d1-9da8a3f64164@nvidia.com>

On Fri, Jan 09, 2026 at 10:39:10AM -0800, Aaron Plattner wrote:
> On 1/9/26 9:57 AM, Aaron Plattner wrote:
> > On 1/9/26 6:51 AM, Patrick Steinhardt wrote:
> [...]
> > > > 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.
> 
> I asked AI to try generating a test case for me and it discovered that the
> problem doesn't reproduce with Basic auth because git sets CURLOPT_USERNAME
> and CURLOPT_PASSWORD and curl implicitly includes those in subsequent
> requests without git having to add them explicitly. If we used
> CURLOPT_XOAUTH2_BEARER like imap-send.c does, then curl would presumably do
> the same thing behind the scenes.
> 
> That said, I'm not sure using that makes sense since the credential helper
> just tells git to use Bearer auth and what the token is, but not whether
> it's OAuth2 or some other kind of token. I don't know if that matters.
> Rahul, do you have any opinions there since you're familiar with this stuff
> than I am?
> 
> Anyway, the test it came up with creates a repository with 2000 branches to
> get the reply to hit the large_request=1 case and then uses a simple
> credential helper with a dummy Bearer token to trigger the problem. If you
> think the current fix and that test scenario sound reasonable, I'll clean it
> up and send out a v2.

Creating 2000 branches can be done efficiently via a single
git-update-ref(1) call, so this wouldn't cause the test to become
prohibitively expensive. And if that manages to reproduce the problem it
sounds like a reasonable way forward.

Thanks!

Patrick