auth: surface context cancellation from credential Lookup

bebc282→main·

Soph·1mo ago·2 files·+35 added/-4 removed

Previously every failure from GitCredentialCommand was swallowed into ok=false with err=nil, so a sync whose context was cancelled or timed out while git credential fill was running surfaced the original HTTP 401 instead of context.Canceled / DeadlineExceeded.

The CredentialHelper interface already had an err return (kept "in case" by a //nolint:unparam directive), and tryHelperRetry propagates it — only Lookup itself wasn't using it. Now, when the subprocess fails and ctx is done, we return the wrapped ctx.Err() so callers report the real cause.

Addresses Cursor Bugbot review comment on PR #65: "Cancel masked as HTTP 401".

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

Changes

2

• internal/auth

  • Mauth.go+11/-4

  • Mauth_test.go+24

94 unmodified lines

// Lookup queries the git credential helper for credentials for ep. Returns
// ok=false if no credentials are available so the caller can surface a
// clean 401 rather than block.
//
//nolint:unparam // err is always nil today but kept for the CredentialHelper interface.
// clean 401 rather than block. A non-nil error means the lookup itself
// couldn't complete (e.g. the context was cancelled) and the caller should
// surface that rather than fall back to the original 401.
func (GitCredentialHelper) Lookup(ctx context.Context, ep *url.URL) (username, password string, ok bool, err error) {
    if !isHTTPEndpoint(ep) {
        return "", "", false, nil
    }

output, helperErr := GitCredentialCommand(ctx, CredentialOpFill, input)
    if helperErr != nil {
        return "", "", false, nil //nolint:nilerr // helper failure means "no credentials available"
        // A cancelled or timed-out context kills the `git credential fill`
        // subprocess; surface that as the real cause instead of masking it
        // as "no credentials available", which would report the original
        // HTTP 401 rather than context.Canceled/DeadlineExceeded.
        if ctxErr := ctx.Err(); ctxErr != nil {
            return "", "", false, fmt.Errorf("git credential fill: %w", ctxErr)
        }
        return "", "", false, nil
    }
    values := parseCredentialOutput(output)
    password = values["password"]

// TestGitCredentialHelper_Lookup_ContextCanceledSurfacesError ensures a // cancelled context isn't masked as "no credentials available": when the // git credential fill subprocess dies because the context is gone, Lookup // must return the context error so callers report it instead of falling back // to the original HTTP 401. func TestGitCredentialHelper_Lookup_ContextCanceledSurfacesError(t *testing.T) { ep := &url.URL{Scheme: "https", Host: "example.com"} ctx, cancel := context.WithCancel(context.Background()) cancel(); withRecordingHelper(t, new([]recordedCredCall), func(_ CredentialOp, _ string) ([]byte, error) { // exec.CommandContext kills the subprocess once the context is done, // surfacing as a command error. return nil, errors.New("signal: killed") })

_, _, ok, err := GitCredentialHelper{}.Lookup(ctx, ep) if ok { t.Error("expected ok=false on a cancelled context") } if !errors.Is(err, context.Canceled) { t.Errorf("expected context.Canceled, got %v", err) } }

func TestGitCredentialHelper_Lookup_EmptyPasswordReturnsNotFound(t *testing.T) { ep := &url.URL{Scheme: "https", Host: "example.com"} withRecordingHelper(t, new([]recordedCredCall), func(_ CredentialOp, _ string) ([]byte, error) {