4 files changed, 203 insertions(+), 0 deletions(-)

A bearer/status.go
A bearer/status_test.go
A pages/form.go
A pages/form_test.go

@@ 0,0 1,64 @@
+package bearer
+
+import (
+	"errors"
+	"net/http"
+	"strconv"
+)
+
+// StatusFor is the instance's answer to "a token was refused — what does the
+// caller see?", as one table rather than one per service.
+//
+// The mapping itself is not subtle. What makes it worth sharing is the arm
+// that is easy to get wrong and impossible to notice: ErrUnavailable is 503,
+// never 401. Reading an unreachable token daemon as "revoked" tells every CI
+// job on the instance that its credential is bad for as long as tokens.sr.ht
+// takes to restart, and somebody spends the evening re-minting tokens that
+// were never broken. bench alone had this switch written out three times — in
+// its REST surface, its MCP surface and its resolver — which is three chances
+// to fold the unreachable case into the invalid one.
+//
+// The three refusals that ARE the caller's fault share one status and one
+// sentence on purpose: telling a prober "that token exists but is revoked" is
+// information they have not earned. Which of them it was belongs in the log.
+//
+// ErrNotOurs is the one arm a service must decide before asking: it means a
+// well-formed token from another issuer, almost certainly a meta.sr.ht PAT,
+// and SPEC ch. 6 step 2 leaves each service to accept it (dolt) or refuse it
+// (bench, cover). Handle it first; reaching here it is a refusal, so it maps
+// to 401.
+//
+// A nil error maps to 200 so a caller can write the status unconditionally.
+func StatusFor(err error) int {
+	switch {
+	case err == nil:
+		return http.StatusOK
+	case errors.Is(err, ErrForbidden):
+		return http.StatusForbidden
+	case errors.Is(err, ErrUnavailable):
+		return http.StatusServiceUnavailable
+	default:
+		// ErrInvalid, ErrRevoked, ErrNotOurs, and anything a future validator
+		// step adds: an unrecognised failure is the caller's credential, not
+		// the instance's health. A new sentinel that deserves 503 has to say
+		// so here, which is the point of the default going this way — a
+		// forgotten arm refuses a request rather than declaring the service
+		// unwell.
+		return http.StatusUnauthorized
+	}
+}
+
+// Challenge is the WWW-Authenticate value a 401 carries: the scheme, and the
+// service's own config section as the realm.
+//
+// RFC 9110 requires the header on a 401, and every service on the instance was
+// assembling the same string from the same constant. The realm is the section
+// name ("bench.sr.ht") because that is what identifies the service everywhere
+// else on this instance — in the config, in the nav, in a grant.
+//
+// The realm is quoted per RFC 9110 §11.6.1; a quote or backslash in it would
+// end the parameter early, so the value is escaped rather than trusted. In
+// practice a section name contains neither.
+func Challenge(realm string) string {
+	return "Bearer realm=" + strconv.Quote(realm)
+}

@@ 0,0 1,39 @@
+package bearer
+
+import (
+	"errors"
+	"fmt"
+	"net/http"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestStatusForKeepsTheUnreachableDaemonOutOfThe401(t *testing.T) {
+	// The arm the whole table exists for: a token service that cannot be
+	// reached must not read as a bad credential.
+	assert.Equal(t, http.StatusServiceUnavailable, StatusFor(ErrUnavailable))
+	assert.Equal(t, http.StatusServiceUnavailable,
+		StatusFor(fmt.Errorf("ask tokens.sr.ht: %w", ErrUnavailable)),
+		"a wrapped one too — services wrap before returning")
+
+	assert.Equal(t, http.StatusForbidden, StatusFor(ErrForbidden))
+	assert.Equal(t, http.StatusForbidden,
+		StatusFor(fmt.Errorf("check the grant: %w", ErrForbidden)))
+
+	// The three the caller is responsible for answer alike, on purpose.
+	for _, err := range []error{ErrInvalid, ErrRevoked, ErrNotOurs} {
+		assert.Equal(t, http.StatusUnauthorized, StatusFor(err), "%v", err)
+	}
+
+	assert.Equal(t, http.StatusOK, StatusFor(nil))
+	assert.Equal(t, http.StatusUnauthorized, StatusFor(errors.New("something new")),
+		"an unrecognised failure refuses the request rather than declaring the service unwell")
+}
+
+func TestChallengeNamesTheServiceAndQuotesIt(t *testing.T) {
+	assert.Equal(t, `Bearer realm="bench.sr.ht"`, Challenge("bench.sr.ht"))
+	assert.Equal(t, `Bearer realm="dolt.sr.ht"`, Challenge("dolt.sr.ht"))
+	// A quote in the realm would end the parameter early if it were pasted in.
+	assert.Equal(t, `Bearer realm="a\"b"`, Challenge(`a"b`))
+}

@@ 0,0 1,44 @@
+package pages
+
+import (
+	"errors"
+	"fmt"
+	"net/http"
+	"net/url"
+)
+
+// DefaultMaxFormBytes bounds a urlencoded body. net/http's own ceiling for one
+// is 10 MiB per request, which is three orders of magnitude more than any form
+// on this instance sends and enough to be worth refusing on a page anyone can
+// reach without logging in.
+const DefaultMaxFormBytes = 1 << 16
+
+// ErrInvalidForm is returned when the body could not be read as a form: it was
+// malformed, or it exceeded the limit. A service maps this to 400.
+var ErrInvalidForm = errors.New("pages: the form could not be read")
+
+// FormValues reads a urlencoded body, bounded at max (DefaultMaxFormBytes when
+// max <= 0), and returns the body's values.
+//
+// It returns r.PostForm and never r.Form, and that is the whole reason this
+// three-line function is shared rather than copied. r.Form merges the query
+// string into the body's values, so a mutation could be driven entirely from a
+// URL somebody was linked to — and that request is precisely the one the
+// same-origin guard sees nothing wrong with, because it really did come from
+// our own page. A form posts its fields in its body; anything in the query
+// string of such a POST is not that form.
+//
+// The difference between the safe version and the hole is one character in a
+// field name, in a function every service with a form writes for itself, and
+// nothing at review time makes its absence visible. That is what makes it
+// belong beside csrf rather than in each service.
+func FormValues(w http.ResponseWriter, r *http.Request, max int64) (url.Values, error) {
+	if max <= 0 {
+		max = DefaultMaxFormBytes
+	}
+	r.Body = http.MaxBytesReader(w, r.Body, max)
+	if err := r.ParseForm(); err != nil {
+		return nil, fmt.Errorf("%w (%v)", ErrInvalidForm, err)
+	}
+	return r.PostForm, nil
+}

@@ 0,0 1,56 @@
+package pages
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func postForm(body string) *http.Request {
+	r := httptest.NewRequest(http.MethodPost, "/tokens?grants=%2A&revoke=all", strings.NewReader(body))
+	r.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	return r
+}
+
+// TestFormValuesIgnoresTheQueryString is the reason this function is shared.
+// A mutation must not be drivable from a URL a viewer was linked to, which the
+// same-origin guard would happily let through.
+func TestFormValuesIgnoresTheQueryString(t *testing.T) {
+	r := postForm("grants=bench%3Aupload")
+
+	got, err := FormValues(httptest.NewRecorder(), r, 0)
+	require.NoError(t, err)
+
+	assert.Equal(t, "bench:upload", got.Get("grants"), "the body's value, not the URL's")
+	assert.Empty(t, got.Get("revoke"), "a field present only in the query is not this form's")
+	// r.Form, the wrong one, would have carried it — that merge is the hole.
+	assert.Equal(t, "all", r.Form.Get("revoke"), "the merge is what we are avoiding")
+	assert.Equal(t, []string{"bench:upload", "*"}, r.Form["grants"],
+		"and it would have appended the URL's value to the body's")
+}
+
+func TestFormValuesRefusesAnOversizedBody(t *testing.T) {
+	r := postForm("grants=" + strings.Repeat("x", 4096))
+
+	_, err := FormValues(httptest.NewRecorder(), r, 128)
+	require.Error(t, err)
+	assert.ErrorIs(t, err, ErrInvalidForm)
+
+	// The same body inside the limit reads fine, so the refusal is the limit
+	// and not the shape.
+	got, err := FormValues(httptest.NewRecorder(), postForm("grants=ok"), 128)
+	require.NoError(t, err)
+	assert.Equal(t, "ok", got.Get("grants"))
+}
+
+func TestFormValuesDefaultsTheLimit(t *testing.T) {
+	r := postForm("grants=" + strings.Repeat("x", DefaultMaxFormBytes+1))
+
+	_, err := FormValues(httptest.NewRecorder(), r, 0)
+	require.Error(t, err, "max <= 0 means the default, not unbounded")
+	assert.ErrorIs(t, err, ErrInvalidForm)
+}