Artifact [6fb6cad367]

Artifact 6fb6cad3676d2553f88db0b30d4aabe043ed54753adc2489e2b3909f90783ed3:

• File 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: 4435) [annotate] [blame] [check-ins using]

package libfossil

import (
	"bytes"
	"context"
	"io"
	"net/http"
	"net/http/httptest"
	"strings"
	"sync/atomic"
	"testing"
)

// The HTTP transport follows redirects as fossil's http_exchange does
// (src/http.c, draft-fossil-sync-protocol-00 §4): never from HTTPS to HTTP,
// re-POSTing the same body for 301, 302, 307 and 308, stripping one trailing
// /xfer, and failing on the twentieth redirect.

// redirectTo answers every request with status code and a Location of to.
func redirectTo(code int, to func() string) http.HandlerFunc {
	return func(w http.ResponseWriter, r *http.Request) {
		w.Header().Set("Location", to())
		w.WriteHeader(code)
	}
}

func TestHTTPTransportRefusesHTTPSToHTTPRedirect(t *testing.T) {
	var plainHits atomic.Int32
	plain := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		plainHits.Add(1)
		w.Header().Set("Content-Type", "application/x-fossil")
	}))
	defer plain.Close()
	secure := httptest.NewTLSServer(redirectTo(http.StatusTemporaryRedirect,
		func() string { return plain.URL }))
	defer secure.Close()

	tr := NewHTTPTransport(secure.URL, WithHTTPClient(secure.Client()))
	_, err := tr.RoundTrip(context.Background(), []byte("login card"))
	if err == nil {
		t.Fatal("redirect from HTTPS to HTTP was followed")
	}
	if n := plainHits.Load(); n != 0 {
		t.Fatalf("the request body went to the plain-HTTP target %d times", n)
	}
}

func TestHTTPTransportRePostsAfterRedirect(t *testing.T) {
	payload := []byte("cards to send")
	for _, code := range []int{301, 302, 307, 308} {
		var method, ctype string
		var got []byte
		target := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
			method, ctype = r.Method, r.Header.Get("Content-Type")
			got, _ = io.ReadAll(r.Body)
			w.Header().Set("Content-Type", "application/x-fossil")
			w.Write([]byte("reply"))
		}))
		origin := httptest.NewServer(redirectTo(code, func() string { return target.URL }))

		tr := NewHTTPTransport(origin.URL)
		resp, err := tr.RoundTrip(context.Background(), payload)
		origin.Close()
		target.Close()
		if err != nil {
			t.Fatalf("%d: RoundTrip: %v", code, err)
		}
		if method != http.MethodPost || ctype != "application/x-fossil" {
			t.Errorf("%d: redirected as %s %q, want POST application/x-fossil", code, method, ctype)
		}
		if !bytes.Equal(got, payload) {
			t.Errorf("%d: redirected body = %q, want %q", code, got, payload)
		}
		if string(resp) != "reply" {
			t.Errorf("%d: reply = %q", code, resp)
		}
	}
}

func TestHTTPTransportStripsOneTrailingXfer(t *testing.T) {
	for _, tc := range []struct{ location, want string }{
		{"/repo/xfer", "/repo"},
		{"/repo/xfer/xfer", "/repo/xfer"},
	} {
		var path string
		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
			if r.URL.Path == "/" {
				w.Header().Set("Location", tc.location)
				w.WriteHeader(http.StatusFound)
				return
			}
			path = r.URL.Path
			w.Header().Set("Content-Type", "application/x-fossil")
		}))
		_, err := NewHTTPTransport(srv.URL+"/").RoundTrip(context.Background(), []byte("x"))
		srv.Close()
		if err != nil {
			t.Fatalf("%s: RoundTrip: %v", tc.location, err)
		}
		if path != tc.want {
			t.Errorf("redirect to %s posted to %s, want %s", tc.location, path, tc.want)
		}
	}
}

// chainServer redirects until it has answered hops requests, then replies.
func chainServer(hops int32) (*httptest.Server, *atomic.Int32) {
	var hits atomic.Int32
	var srv *httptest.Server
	srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		if hits.Add(1) <= hops {
			w.Header().Set("Location", srv.URL+"/r")
			w.WriteHeader(http.StatusTemporaryRedirect)
			return
		}
		w.Header().Set("Content-Type", "application/x-fossil")
	}))
	return srv, &hits
}

func TestHTTPTransportRedirectLimit(t *testing.T) {
	ok, _ := chainServer(19)
	defer ok.Close()
	if _, err := NewHTTPTransport(ok.URL).RoundTrip(context.Background(), []byte("x")); err != nil {
		t.Fatalf("19 redirects should be followed: %v", err)
	}

	endless, hits := chainServer(1 << 30)
	defer endless.Close()
	_, err := NewHTTPTransport(endless.URL).RoundTrip(context.Background(), []byte("x"))
	if err == nil || !strings.Contains(err.Error(), "redirect limit") {
		t.Fatalf("err = %v, want the redirect limit", err)
	}
	if n := hits.Load(); n != 20 {
		t.Fatalf("sent %d requests, want 20: the twentieth redirect is fatal", n)
	}
}