Defer Probe Credential Approval to the Real Operation

d08bf33→main·

Soph·1mo ago·2 files·+144 added/-74 removed

The previous probe logic ran a second authenticated GET / and approved the credentials whenever that response wasn't 401/403. But many servers return 405 to GET /git-receive-pack without ever validating the Authorization header — so a 405 with attached credentials proves nothing. Approving in that case was a false positive: stale helper creds got blessed, c.Auth was set, and the streaming POST that followed skipped the helper retry path. The push then failed with no way to recover the bad helper state.

Fix: the probe is now strictly anonymous. Its only job is to detect whether the server requires auth here (the 401 signal). If it does, the helper-supplied credentials are attached to c.Auth tentatively and recorded as pendingHelperCreds. The next real operation (PostRPCStreamBody or RequestInfoRefs) calls resolvePendingHelperCreds on its response: Approve on 2xx, Reject + clear c.Auth on 401/403, no-op otherwise. So helper state only changes when the real operation provides a definitive signal — which is what git itself does.

Six new/updated tests:

• TentativelyAttachesHelperCredsOnAnonymous401: c.Auth set, no Approve
• 405ProbeWithCredsDoesNotPoisonHelper: explicit regression for the bug
• RealPostApprovesTentativeCreds: 2xx on the real POST → Approve
• RealPostRejectsTentativeCreds: 401 on the real POST → Reject + clear
• NoHelperIsNoOp, AnonymousServiceLeavesAuthNil: existing semantics preserved

Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com

Sessions

?\ can you take a look at https://github.com/entireio/git-sync/issues/63Claude Code·Opus 4.7[1m]·1 step

Changes

2

• internal/gitproto
  • Msmarthttp.go+54/-30
  • Msmarthttp_test.go+90/-44

220 unmodified lines

// coordinated writer here so server-side progress lines don't
// clobber the in-place ticker frame.
ProgressOut io.Writer

// pendingHelperCreds tracks credentials supplied by the helper via
// EnsureAuthForService but not yet validated against a real operation.
// The next RequestInfoRefs/PostRPCStreamBody approves on 2xx or rejects
// on 401/403, ensuring helper state reflects the actual outcome rather
// than an ambiguous probe response.
pendingHelperCreds *helperCreds
}

type helperCreds struct {
user, pass string
url        *url.URL
}

// NewHTTPConn creates a new connection to the given endpoint.

if err != nil {
return nil, err
}
c.resolvePendingHelperCreds(ctx, res)

defer res.Body.Close()
if err := httpError(res); err != nil {
return nil, err
}

Ensures Auth for Service

func (c *HTTPConn) EnsureAuthForService(ctx context.Context, service string) {
if c.Auth != nil || c.CredentialHelper == nil {
return
}

TestEnsureAuthForService_ResolvesAuthBeforePost

This test simulates a server that allows anonymous /info/refs but requires auth on the service endpoint itself — the case where the streaming push body (io.MultiReader over packData) can't trigger an on-the-fly 401 retry. The probe must resolve credentials so the upcoming POST is pre-authenticated.