probe with POST so auth-on-POST-only gates are detected

0322ea1→main·

Soph·1mo ago·4 files·+155 added/-30 removed

GET /git-receive-pack doesn't reliably exercise the same auth path as the real push. Servers that 404/405 the GET method while requiring auth on POST were slipping past EnsureAuthForService — the streaming push then 401'd with no way to retry. Cloudflare-style "Invalid or expired token" servers fall in this bucket, as do some Gerrit configurations.

Two changes:

1. Probe with POST and the smart-HTTP flush packet "0000" as body — a valid no-op (zero ref updates, zero pack data) by spec. The auth layer challenges this POST identically to the real push, so the 401 signal is reliable. On the anonymous-allowed case the server processes a no-op that touches no ref state.

2. Lookup helper credentials before probing, and skip the probe entirely when the helper has nothing to attach. Without this every anonymous sync would do a wasted no-op POST per push. With it, only syncs that have a configured helper with stored credentials for the host pay the round-trip cost.

cmd/git-sync's TestMain now overrides auth.GitCredentialCommand with a "no helper configured" stub. Without isolation, the developer's local credential store (osxkeychain etc.) could return cached credentials for 127.0.0.1 left over from a previous test run, turning the probe into a real POST and inflating receive-pack POST counts. The internal/syncer integration tests are unaffected — their server filters by metricPack which the probe's no-op body doesn't carry.

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

## Sessions

47657919e49aView transcript

## Changes

4

- cmd/git-sync
 
 
  - Mmain_test.go+17

- internal/gitproto

- Mpush.go+1/-2

- Msmarthttp.go+39/-26

- Msmarthttp_test.go+98/-2

3 unmodified lines

4 5 6 7 8 9 10 4 unmodified lines

15 16 17 18 19 20 21 9 unmodified lines

31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51

3 unmodified lines

"bytes" "context" "encoding/json" "errors" "fmt" "io" "net/http" 4 unmodified lines

"testing" "time"

"entire.io/entire/git-sync/internal/auth" "entire.io/entire/git-sync/internal/syncertest" "entire.io/entire/git-sync/unstable" billy "github.com/go-git/go-billy/v6" 9 unmodified lines

"github.com/go-git/go-git/v6/storage/memory"

// TestMain isolates the package's tests from the developer's local
// credential helper. Without this, `git credential fill` could find
// stored credentials for 127.0.0.1 (e.g. cached from an earlier test
// run) and turn EnsureAuthForService's would-be no-op into a real
// auth-probe POST, throwing off receive-pack POST counts.
//
// Tests that need to exercise helper behaviour explicitly should
// restore auth.GitCredentialCommand in their own setup.
func TestMain(m *testing.M) {
    auth.GitCredentialCommand = func(_ context.Context, _ auth.CredentialOp, _ string) ([]byte, error) {
        return nil, errors.New("no helper configured (test default)")
    }
    os.Exit(m.Run())
}

const testBranch = "master"
const modeReplicate = "replicate"

Mcmd/git-sync/main_test.go+17

154 unmodified lines

155 156 157 158 159 158 159 160 161

154 unmodified lines

// The push body is io.MultiReader(header, packData); packData comes // from a live upload-pack pipe and isn't rewindable, so a mid-stream // 401 can't trigger PostRPCStreamBody's normal helper retry. Probe // for auth requirements up front instead — for HTTP conns that have // a CredentialHelper configured but no Auth resolved yet. // for auth requirements with a same-shape POST first. if hc, ok := conn.(*HTTPConn); ok { hc.EnsureAuthForService(ctx, transport.ReceivePackService) }

Minternal/gitproto/push.go+1/-2

477 unmodified lines

478 479 480 481 481 482 483 484 485 485 486 487 488 489 486 487 491 492 493 494 495 488 489 490 491 492 493 494 495 496 497 498 499 500 501 502 497 498 503 504 505 506 507 508 509 503 504 505 506 507 508 509 510 511 512 510 511 512 513 514 515 516 517 518 519 520 521 522 523 524 525 20 unmodified lines

546 547 548 542 549 550 551 552 553 554 555 544 556 557 558 559 560 561 562 563 550 564 565 566

477 unmodified lines

// EnsureAuthForService tentatively attaches helper credentials before a // non-rewindable request body is committed. It's a no-op when no helper // is configured or Auth is already set. // is configured, when Auth is already set, or when the helper has no // credentials to offer. // // Used from push.go (and other streaming-body POST paths) where the body // is built from a live upstream stream (e.g. io.MultiReader over a pack // reader) and can't be replayed on a mid-stream 401. An anonymous GET // / probe asks the server whether it requires auth here. If it // 401s, the helper is consulted and the returned credentials are stored // in c.Auth tentatively — the next real operation (PostRPCStreamBody or // RequestInfoRefs) then Approves them on 2xx or Rejects them on 401/403. // reader) and can't be replayed on a mid-stream 401. // // Approval is deliberately not done from the probe itself: many servers // return 405 Method Not Allowed for GET /git-receive-pack without ever // checking the Authorization header, so a 405 with credentials attached // proves nothing about credential validity. Letting the real operation // validate keeps helper state honest. // The flow is: // 1. Ask the helper if it has credentials for this endpoint. If not, // bail — no point probing for an auth requirement we can't satisfy. // (This also keeps anonymous syncs from doing a wasted no-op POST.) // 2. Probe with a POST to / using the smart-HTTP flush packet // "0000" as body — a valid no-op (zero ref updates, zero pack data) // by spec. We probe with POST rather than GET because the auth layer // may only gate the POST handler; a GET probe would slip past on // servers that 404/405 GET while requiring auth on POST. // 3. If the probe gets 401, attach the helper credentials tentatively. // The next real operation (PostRPCStreamBody or RequestInfoRefs) // calls resolvePendingHelperCreds, which Approves them on 2xx or // Rejects them on 401/403 — helper state only changes based on the // actual outcome, never on the probe response alone. // // Servers that allow anonymous GET but only 401 on POST will slip past // this probe; for those, callers must pass explicit credentials. // If the probe doesn't 401 (200, 404, 405, etc.) we don't attach; the // server either accepts anonymous POSTs here or returns ambiguously, // and either way attaching unvalidated credentials could leak them. func (c *HTTPConn) EnsureAuthForService(ctx context.Context, service string) { if c.Auth != nil || c.CredentialHelper == nil { return } res, err := c.doServiceProbe(ctx, service, nil) if err != nil { return } defer res.Body.Close() if res.StatusCode != http.StatusUnauthorized { return } challengeURL := challengeURLFor(c.EndpointURL, res) user, pass, ok, lookupErr := c.CredentialHelper.Lookup(ctx, challengeURL) user, pass, ok, lookupErr := c.CredentialHelper.Lookup(ctx, c.EndpointURL) if lookupErr != nil || !ok { return } res, err := c.doServiceProbe(ctx, service) if err != nil { return } defer res.Body.Close() if res.StatusCode != http.StatusUnauthorized { return } challengeURL := challengeURLFor(c.EndpointURL, res) c.Auth = &transporthttp.BasicAuth{Username: user, Password: pass} c.pendingHelperCreds = &helperCreds{user: user, pass: pass, url: challengeURL} } 20 unmodified lines

// end of life is harmless. }

func (c *HTTPConn) doServiceProbe(ctx context.Context, service string, auth AuthMethod) (*http.Response, error) { // flushPacket is the smart-HTTP pkt-line "flush" marker. A request body // containing only a flush packet is a valid no-op for both upload-pack // (no wants/haves) and receive-pack (no ref updates, no pack data). var flushPacket = []byte("0000")

func (c *HTTPConn) doServiceProbe(ctx context.Context, service string) (*http.Response, error) { reqURL := fmt.Sprintf("%s/%s", c.EndpointURL.String(), service) req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL, nil) req, err := http.NewRequestWithContext(ctx, http.MethodPost, reqURL, bytes.NewReader(flushPacket)) if err != nil { return nil, fmt.Errorf("create auth-probe request: %w", err) } req.Header.Set("Content-Type", fmt.Sprintf("application/x-%s-request", service)) req.Header.Set("Accept", fmt.Sprintf("application/x-%s-result", service)) req.Header.Set("User-Agent", capability.DefaultAgent()) req.Header.Set(StatsPhaseHeader, service+" auth-probe") ApplyAuth(req, auth) res, err := c.HTTP.Do(req) if err != nil { return nil, fmt.Errorf("auth-probe request: %w", err) }

Minternal/gitproto/smarthttp.go+39/-26

900 unmodified lines

901 902 903 904 905 904 905 906 907 908 909 910 911 912 913 914 915 916 917 918 919 920 921 922 923 924 925 926 927 928 929 930 931 72 unmodified lines

1004 1005 1006 1007 1008 1009 1010 1011 1012 1013 1014 1015 1016 1017 1018 1019 1020 1021 1022 1023 1024 1025 1026 1027 1028 1029 1030 1031 1032 1033 1034 1035 1036 1037 1038 1039 1040 1041 1042 1043 1044 1045 1046 1047 1048 1049 1050 1051 1052 1053 1054 1055 1056 1057 1058 1059 1060 1061 1062 1063 1064 1065 1066 1067 1068 1069 1070 1071 1072 1073 1074 1075 1076 1077 1078 1079 1080 1081 1082

900 unmodified lines

if conn.Auth != nil { t.Error("expected conn.Auth to remain nil when probe gets non-401") } if got := helper.count("lookup"); got != 0 { t.Errorf("expected 0 helper lookups when probe didn't 401, got %d", got) } if got := helper.count("approve"); got != 0 { t.Errorf("must not Approve when probe didn't 401, got %d", got) } }

// TestEnsureAuthForService_SkipsProbeWhenHelperHasNoCredentials avoids a // wasted no-op POST when there are no credentials to attach anyway — the // common shape for anonymous syncs and for syncs running in test/CI // environments with no credential helper configured. func TestEnsureAuthForService_SkipsProbeWhenHelperHasNoCredentials(t *testing.T) { helper := &fakeCredentialHelper{ok: false} called := false conn := newTestConn(t, roundTripperFunc(func(req *http.Request) (*http.Response, error) { called = true return newUnauthorizedResponse(req), nil })) conn.CredentialHelper = helper

conn.EnsureAuthForService(context.Background(), "git-receive-pack")

if called { t.Error("expected no probe when the helper has no credentials") } if conn.Auth != nil { t.Error("expected conn.Auth to remain nil") } }

72 unmodified lines

}

// TestEnsureAuthForService_ProbesWithPOSTAndFlushPacketBody verifies the // probe uses the same HTTP method as the real operation (POST), with a // minimal "0000" flush packet body — a valid no-op receive-pack push by // the smart-HTTP spec. Probing with POST is essential: servers that // gate only the POST handler (not GET) would otherwise slip past us. func TestEnsureAuthForService_ProbesWithPOSTAndFlushPacketBody(t *testing.T) { helper := &fakeCredentialHelper{user: "alice", pass: "s3cret", ok: true} var probeMethod string var probeBody []byte conn := newTestConn(t, roundTripperFunc(func(req *http.Request) (*http.Response, error) { probeMethod = req.Method if req.Body != nil { b, err := io.ReadAll(req.Body) if err != nil { t.Fatalf("read probe body: %v", err) } probeBody = b } return newUnauthorizedResponse(req), nil })) conn.CredentialHelper = helper

conn.EnsureAuthForService(context.Background(), "git-receive-pack")

if probeMethod != http.MethodPost { t.Errorf("expected probe method POST, got %q", probeMethod) } if string(probeBody) != "0000" { t.Errorf("expected probe body to be the flush packet '0000', got %q", probeBody) } }

// TestEnsureAuthForService_DetectsAuthGatedPostEvenWhenGetIsAnonymous: // the gap a GET-based probe would miss — server returns 404 to GET // (the receive-pack endpoint isn't a GET resource) but 401 to POST. // A POST probe correctly detects the auth requirement. func TestEnsureAuthForService_DetectsAuthGatedPostEvenWhenGetIsAnonymous(t *testing.T) { helper := &fakeCredentialHelper{user: "alice", pass: "s3cret", ok: true} var methods []string conn := newTestConn(t, roundTripperFunc(func(req *http.Request) (*http.Response, error) { methods = append(methods, req.Method) if req.Method == http.MethodGet { return &http.Response{ StatusCode: http.StatusNotFound, Request: req, Header: make(http.Header), Body: io.NopCloser(strings.NewReader("")), }, nil } // POST: server requires auth. if req.Header.Get("Authorization") == "" { return newUnauthorizedResponse(req), nil } return &http.Response{ StatusCode: http.StatusOK, Request: req, Header: make(http.Header), Body: io.NopCloser(strings.NewReader("")), }, nil })) conn.CredentialHelper = helper

conn.EnsureAuthForService(context.Background(), "git-receive-pack")

if conn.Auth == nil { t.Fatal("expected probe to detect POST auth requirement and attach helper creds") } if got := helper.count("lookup"); got != 1 { t.Errorf("expected 1 helper lookup, got %d", got) } for _, m := range methods { if m == http.MethodGet { t.Errorf("probe should not use GET — server may serve GET differently than POST") } } }

// TestEnsureAuthForService_405ProbeWithCredsDoesNotPoisonHelper is the // specific regression: previously, a 405 to the authenticated probe was // interpreted as "creds accepted" and Approve was called — even though