gitproto: defer credential approval until response passes validation · Entire

gitproto: defer credential approval until response passes validation

Changes

2

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

defer res.Body.Close()

data, err := c.readInfoRefsResponse(res, service)
// Settle helper credentials on the fully-validated outcome: approve only
// once the advertisement parsed and read within limits, reject on 401/403.
// Running this after validation stops a misleading 2xx (wrong content-type
// or oversized body) from persisting credentials for an operation that
// ultimately failed.
c.resolvePendingHelperCreds(ctx, res, err == nil)
if err != nil {
    return nil, err
}
return data, nil
}

// readInfoRefsResponse validates and reads an /info/refs response: it checks
// the HTTP status and advertisement content-type, applies any redirect to the
// endpoint, and reads the body under a size cap. The caller closes res.Body.
func (c *HTTPConn) readInfoRefsResponse(res *http.Response, service string) ([]byte, error) {
    if err := httpError(res); err != nil {
        return nil, err
    }
}

// TestRequestInfoRefs_OnUnauthorizedRetry2xxBadContentTypeDoesNotApprove
// guards the deferred-approval contract: a retry that authenticates (HTTP 200)
// but returns a non-advertisement body must surface a content-type error and
// must NOT persist credentials in the helper — the operation didn't actually
// succeed, so a misleading 2xx shouldn't approve the creds. It's also not an
// auth failure, so the helper isn't told to reject them either.
func TestRequestInfoRefs_OnUnauthorizedRetry2xxBadContentTypeDoesNotApprove(t *testing.T) {
    helper := &fakeCredentialHelper{user: "alice", pass: "s3cret", ok: true}
    attempts := 0
    conn := newTestConn(t, roundTripperFunc(func(req *http.Request) (*http.Response, error) {
        attempts++
        if attempts == 1 {
            return newUnauthorizedResponse(req), nil
        }
        res := &http.Response{
            StatusCode: http.StatusOK,
            Request:    req,
            Header:     make(http.Header),
            Body:       io.NopCloser(strings.NewReader("<html>login</html>")),
        }
        res.Header.Set("Content-Type", "text/html")
        return res, nil
    }))
    conn.CredentialHelper = helper

_, err := conn.RequestInfoRefs(context.Background(), "git-upload-pack", "")
    if err == nil {
        t.Fatal("expected content-type error after a 2xx retry with a non-advertisement body")
    }
    if !strings.Contains(err.Error(), "unexpected info/refs content-type") {
        t.Fatalf("error = %v, want content-type error", err)
    }
    if got := helper.count("approve"); got != 0 {
        t.Errorf("must not approve credentials for an operation that failed validation, got %d approve calls", got)
    }
    if got := helper.count("reject"); got != 0 {
        t.Errorf("a 2xx-but-invalid response is not an auth failure, got %d reject calls", got)
    }
}

// TestRequestInfoRefs_OnUnauthorizedRetry403CallsReject documents that some
// token services (notably Cloudflare) return 403 "Invalid or expired token"
// instead of 401 when stored credentials have expired.