key helper on the challenged host; retry POSTs too · Entire
Key helper on the challenged host; retry POSTs too
642a774→main· Soph·1mo ago·2 files·+233 added/-45 removed
Two correctness gaps caught in review:
After a 3xx on /info/refs, the host that actually returns 401 is in res.Request.URL, which can differ from c.EndpointURL. Querying the helper with c.EndpointURL fetched (and on success approved) credentials under the wrong key — a miss on the next run, or a poisoned key if the retry happened to work via a follow-up redirect. New challengeURLFor() preserves the original repo path but swaps in the post-redirect scheme/host before consulting the helper.
Helper fallback existed only on GET /info/refs. Servers that allow anonymous discovery but require auth on the actual pack POST (e.g. Gerrit anonymous-readable + authenticated push) would fail hard. PostRPCStreamBody now does the same lookup → retry → approve/reject dance, gated on the body being io.Seeker so we can rewind it for the second attempt. PostRPC / PostRPCStream always pass bytes.NewReader, which is seekable; a caller that hands in a raw non-seekable Reader sees the 401 surface as-is (documented in the doc comment).
The retry lifecycle is now in tryHelperRetry, shared between GET and POST. Four new tests cover redirect-host keying and the POST 401-retry permutations.
Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com
Sessions
991254d4f73aView transcript
Changes
2
- internal/gitproto
- Msmarthttp.go+101/-34
- Msmarthttp_test.go+132/-11
309 unmodified lines
if err != nil {
return nil, err
}
// On 401, fall back to the credential helper — but only when no
// explicit auth was configured. Explicit auth that fails is a real
// error the user needs to see.
if res.StatusCode == http.StatusUnauthorized && c.Auth == nil && c.CredentialHelper != nil {
user, pass, ok, lookupErr := c.CredentialHelper.Lookup(ctx, c.EndpointURL)
if lookupErr != nil {
_ = res.Body.Close()
return nil, fmt.Errorf("look up credentials: %w", lookupErr)
}
if ok {
_ = res.Body.Close()
retryAuth := &transporthttp.BasicAuth{Username: user, Password: pass}
res, err = c.doInfoRefsRequest(ctx, service, gitProtocol, retryAuth)
if err != nil {
c.CredentialHelper.Reject(ctx, c.EndpointURL, user, pass)
return nil, err
}
switch {
case res.StatusCode == http.StatusUnauthorized || res.StatusCode == http.StatusForbidden:
// 403 included because some token services (e.g. Cloudflare)
// surface "Invalid or expired token" as 403 rather than 401.
c.CredentialHelper.Reject(ctx, c.EndpointURL, user, pass)
case res.StatusCode >= http.StatusOK && res.StatusCode < http.StatusMultipleChoices:
c.Auth = retryAuth
c.CredentialHelper.Approve(ctx, c.EndpointURL, user, pass)
}
}
res, err = c.tryHelperRetry(ctx, res, func(auth AuthMethod) (*http.Response, error) {
return c.doInfoRefsRequest(ctx, service, gitProtocol, auth)
})
if err != nil {
return nil, err
}
defer res.Body.Close()
}
// Caller must close the returned ReadCloser. // The body is sent as-is — streaming readers produce a chunked request. // On a 401 we consult the credential helper and retry, mirroring git's // own behaviour for servers that allow anonymous /info/refs but gate the // actual upload-pack/receive-pack POST behind auth. Retry is only possible // when body is an io.Seeker (so we can rewind it); callers that pass a raw // non-seekable Reader will see the 401 surface as-is.\nfunc (c *HTTPConn) PostRPCStreamBody(ctx context.Context, service string, body io.Reader, v2 bool, phase string) (io.ReadCloser, error) { res, err := c.doPostRPCRequest(ctx, service, body, v2, phase, c.Auth) if err != nil { return nil, err } if seeker, ok := body.(io.Seeker); ok { res, err = c.tryHelperRetry(ctx, res, func(auth AuthMethod) (*http.Response, error) { if _, seekErr := seeker.Seek(0, io.SeekStart); seekErr != nil { return nil, fmt.Errorf("rewind RPC body for credential-helper retry: %w", seekErr) } return c.doPostRPCRequest(ctx, service, body, v2, phase, auth) }) if err != nil { return nil, err } } if err := httpError(res); err != nil { _ = res.Body.Close() return nil, err } return res.Body, nil }
// doPostRPCRequest issues a single POST to /