Artifact 84c1e7cae8161af0c65a283829bd5dd179af834fc36cbabda6da4529d375dea5:
- File
internal/sync/transport_redirect_test.go
— part of check-in
[3a1014cf60]
at
2026-10-11 13:28:30
on branch trunk
— fix(sync): send content only to readers; never follow HTTPS to HTTP (security) (#297)
* fix(sync): send content only to requesters allowed to read it
The sync server answered `gimme HASH` with the artifact whether or not a pull card had been accepted, and answered `pragma uv-hash`, `uvgimme` and `uvigot` with the unversioned catalog and files for any requester, anonymous ones included.
Match fossil's page_xfer:
- gimme is answered only when a pull card was accepted, or in a clone, which fossil also treats as pulling. - pragma uv-hash sends the catalog only to a requester with Read (o, i or Admin). uv-push-ok goes only to a requester with y; other readers get uv-pull-only, as fossil sends. - uvgimme and uvigot send nothing to a requester without Read, and uvigot adopts a client's mtime only for a requester with y. - Table sync follows the same rules: xgimme needs an accepted pull and pragma xtable-hash needs Read. - pragma ci-lock takes or reports a lock only for a requester with Write, so an anonymous requester cannot learn who holds one.
Pragma handling moves into handlePragma to keep handleControlCard short.
GHSA-mvx3-6793-8x4r
* docs(changelog): note that gating ci-lock on Write departs from fossil
GHSA-mvx3-6793-8x4r
* fix(sync): follow HTTP redirects as fossil does, never HTTPS to HTTP
Both HTTP transports used net/http's default redirect policy. It follows a redirect from HTTPS to plain HTTP, so a 307 or 308 resent the request, login card included, in cleartext. It also turns a 301 or 302 into a GET without the body and stops after 10 redirects, neither of which fossil does.
PostXfer now follows redirects the way fossil's http_exchange (src/http.c) and draft §4 describe. It replaces the client's redirect policy for these requests, then:
- refuses a redirect from HTTPS to HTTP, and to any scheme but http(s); - re-POSTs the same body and Content-Type for 301, 302, 307 and 308; - strips one trailing /xfer from the new location, as fossil's loop does (the draft says "repeatedly"; the code strips once); - fails on the twentieth redirect (MAX_REDIRECTS 20).
HTTPTransport and NewHTTPTransport both use it, including with a client passed through WithHTTPClient.
GHSA-3f8x-8fr5-mr98 (user: [email protected], size: 4700) [annotate] [blame] [check-ins using]
package sync import ( "bytes" "context" "errors" "io" "net/http" "net/http/httptest" "os/exec" "path/filepath" "sync/atomic" "testing" "time" "github.com/danmestas/go-libfossil/internal/xfer" "github.com/danmestas/go-libfossil/testutil" ) // HTTPTransport follows redirects as fossil's http_exchange does (src/http.c, // draft-fossil-sync-protocol-00 §4). See also transport_redirect_test.go in // the root package, which covers the public transport. // emptyReply answers with a decodable, empty xfer message. func emptyReply(t *testing.T, w http.ResponseWriter) { t.Helper() body, err := (&xfer.Message{}).Encode() if err != nil { t.Errorf("encode reply: %v", err) return } w.Header().Set("Content-Type", xfer.ContentTypeCompressed) w.Write(body) } func pingMessage() *xfer.Message { return &xfer.Message{Cards: []xfer.Card{&xfer.PragmaCard{Name: "client-version"}}} } func TestHTTPTransportRePostsAfter302(t *testing.T) { want, err := pingMessage().Encode() if err != nil { t.Fatalf("encode: %v", err) } var method string var got []byte target := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { method = r.Method got, _ = io.ReadAll(r.Body) emptyReply(t, w) })) defer target.Close() origin := httptest.NewServer(http.RedirectHandler(target.URL, http.StatusFound)) defer origin.Close() tr := &HTTPTransport{URL: origin.URL} if _, err := tr.Exchange(context.Background(), pingMessage()); err != nil { t.Fatalf("Exchange: %v", err) } if method != http.MethodPost || !bytes.Equal(got, want) { t.Fatalf("redirected as %s with %d bytes, want POST with the same %d bytes", method, len(got), len(want)) } } func TestHTTPTransportStripsTrailingXferOnce(t *testing.T) { var path string srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path == "/" { http.Redirect(w, r, "/repo/xfer/xfer", http.StatusTemporaryRedirect) return } path = r.URL.Path emptyReply(t, w) })) defer srv.Close() tr := &HTTPTransport{URL: srv.URL + "/"} if _, err := tr.Exchange(context.Background(), pingMessage()); err != nil { t.Fatalf("Exchange: %v", err) } if path != "/repo/xfer" { t.Fatalf("posted to %s, want /repo/xfer", path) } } func TestHTTPTransportStopsAtRedirectLimit(t *testing.T) { var hits atomic.Int32 var srv *httptest.Server srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { hits.Add(1) http.Redirect(w, r, srv.URL+"/again", http.StatusPermanentRedirect) })) defer srv.Close() tr := &HTTPTransport{URL: srv.URL} _, err := tr.Exchange(context.Background(), pingMessage()) if !errors.Is(err, ErrRedirectLimit) { t.Fatalf("err = %v, want ErrRedirectLimit", err) } if n := hits.Load(); n != 20 { t.Fatalf("sent %d requests, want 20", n) } } func TestPostXferRefusesDowngrade(t *testing.T) { var plainHits atomic.Int32 plain := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { plainHits.Add(1) })) defer plain.Close() secure := httptest.NewTLSServer(http.RedirectHandler(plain.URL, http.StatusPermanentRedirect)) defer secure.Close() _, err := PostXfer(context.Background(), secure.Client(), secure.URL, xfer.ContentTypeCompressed, []byte("login card")) if !errors.Is(err, ErrRedirectDowngrade) { t.Fatalf("err = %v, want ErrRedirectDowngrade", err) } if retryableExchangeFault(err) { t.Fatal("a refused downgrade must not be retried") } if n := plainHits.Load(); n != 0 { t.Fatalf("the request went to the plain-HTTP target %d times", n) } } // TestCloneFromRealFossilThroughRedirect clones from fossil serve behind a // front that answers 302 with the server's /xfer URL: the body must be // re-POSTed and /xfer stripped for fossil to answer the clone. func TestCloneFromRealFossilThroughRedirect(t *testing.T) { bin := testutil.RequireFossilBin(t) dir := t.TempDir() remote := filepath.Join(dir, "remote.fossil") if out, err := exec.Command(bin, "new", remote).CombinedOutput(); err != nil { t.Fatalf("fossil new: %v\n%s", err, out) } url := startFossilServer(t, remote) front := httptest.NewServer(http.RedirectHandler(url+"/xfer", http.StatusFound)) defer front.Close() ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() r, res, err := Clone(ctx, filepath.Join(dir, "local.fossil"), &HTTPTransport{URL: front.URL}, CloneOpts{}) if err != nil { t.Fatalf("Clone through redirect: %v", err) } defer r.Close() if res.BlobsRecvd == 0 || res.ProjectCode != getProjectCode(t, remote) { t.Fatalf("clone got %d blobs, project %q", res.BlobsRecvd, res.ProjectCode) } }