4 files changed, 75 insertions(+), 26 deletions(-)

M web/handlers_keys.go
M web/handlers_repo.go
M web/handlers_settings.go
M web/web_test.go

@@ 3,10 3,12 @@ package web
 import (
 	"errors"
 	"net/http"
+	"net/url"
 	"strconv"
 	"strings"
 
 	"sourcecraft.dev/bigbes/sr-ht-ecore/chrome"
+	"sourcecraft.dev/bigbes/sr-ht-ecore/pages"
 
 	"sourcecraft.dev/bigbes/sr-ht-dolt/db"
 )


@@ 55,22 57,23 @@ func (a *app) handleKeysPost(w http.ResponseWriter, r *http.Request) {
 	if ac == nil {
 		return
 	}
-	if err := r.ParseForm(); err != nil {
+	form, err := pages.FormValues(w, r, 0)
+	if err != nil {
 		a.renderKeys(w, r, ac, http.StatusBadRequest, "Malformed form submission.", "")
 		return
 	}
 
-	if idStr := r.PostFormValue("delete_id"); idStr != "" {
+	if idStr := form.Get("delete_id"); idStr != "" {
 		a.keysDelete(w, r, ac, idStr)
 		return
 	}
-	a.keysAdd(w, r, ac)
+	a.keysAdd(w, r, ac, form)
 }
 
 // keysAdd decodes and registers a dolt public key for the caller.
-func (a *app) keysAdd(w http.ResponseWriter, r *http.Request, ac *authContext) {
-	pubStr := strings.TrimSpace(r.PostFormValue("pubkey"))
-	comment := strings.TrimSpace(r.PostFormValue("comment"))
+func (a *app) keysAdd(w http.ResponseWriter, r *http.Request, ac *authContext, form url.Values) {
+	pubStr := strings.TrimSpace(form.Get("pubkey"))
+	comment := strings.TrimSpace(form.Get("comment"))
 
 	pubkey, kid, err := decodeDoltPubKey(pubStr)
 	if err != nil {

@@ 9,6 9,7 @@ import (
 	"sourcecraft.dev/bigbes/sr-ht-core/config"
 
 	"sourcecraft.dev/bigbes/sr-ht-ecore/chrome"
+	"sourcecraft.dev/bigbes/sr-ht-ecore/pages"
 
 	"sourcecraft.dev/bigbes/sr-ht-dolt/browse"
 	"sourcecraft.dev/bigbes/sr-ht-dolt/core"


@@ 76,15 77,16 @@ func (a *app) handleCreate(w http.ResponseWriter, r *http.Request) {
 	if ac == nil {
 		return
 	}
-	if err := r.ParseForm(); err != nil {
+	values, err := pages.FormValues(w, r, 0)
+	if err != nil {
 		a.renderCreate(w, r, http.StatusBadRequest, createForm{}, "Malformed form submission.")
 		return
 	}
 
 	form := createForm{
-		Name:        strings.TrimSpace(r.PostFormValue("name")),
-		Description: strings.TrimSpace(r.PostFormValue("description")),
-		Visibility:  r.PostFormValue("visibility"),
+		Name:        strings.TrimSpace(values.Get("name")),
+		Description: strings.TrimSpace(values.Get("description")),
+		Visibility:  values.Get("visibility"),
 	}
 
 	visibility, ok := parseVisibility(form.Visibility)

@@ 3,12 3,14 @@ package web
 import (
 	"errors"
 	"net/http"
+	"net/url"
 	"strconv"
 	"strings"
 
 	"github.com/go-chi/chi/v5"
 
 	"sourcecraft.dev/bigbes/sr-ht-ecore/chrome"
+	"sourcecraft.dev/bigbes/sr-ht-ecore/pages"
 
 	"sourcecraft.dev/bigbes/sr-ht-dolt/core"
 	"sourcecraft.dev/bigbes/sr-ht-dolt/db"


@@ 90,29 92,30 @@ func (a *app) handleSettingsPost(w http.ResponseWriter, r *http.Request) {
 	if !ok {
 		return
 	}
-	if err := r.ParseForm(); err != nil {
+	form, err := pages.FormValues(w, r, 0)
+	if err != nil {
 		a.renderSettings(w, r, http.StatusBadRequest, repo, "Malformed form submission.", "")
 		return
 	}
 
-	switch r.PostFormValue("action") {
+	switch form.Get("action") {
 	case "update":
-		a.settingsUpdate(w, r, repo)
+		a.settingsUpdate(w, r, repo, form)
 	case "acl_add":
-		a.settingsACLAdd(w, r, repo)
+		a.settingsACLAdd(w, r, repo, form)
 	case "acl_remove":
-		a.settingsACLRemove(w, r, repo)
+		a.settingsACLRemove(w, r, repo, form)
 	case "delete":
-		a.settingsDelete(w, r, repo)
+		a.settingsDelete(w, r, repo, form)
 	default:
 		a.renderSettings(w, r, http.StatusBadRequest, repo, "Unknown action.", "")
 	}
 }
 
 // settingsUpdate applies the description + visibility change.
-func (a *app) settingsUpdate(w http.ResponseWriter, r *http.Request, repo *core.Repo) {
-	description := strings.TrimSpace(r.PostFormValue("description"))
-	visibility, ok := parseVisibility(r.PostFormValue("visibility"))
+func (a *app) settingsUpdate(w http.ResponseWriter, r *http.Request, repo *core.Repo, form url.Values) {
+	description := strings.TrimSpace(form.Get("description"))
+	visibility, ok := parseVisibility(form.Get("visibility"))
 	if !ok {
 		a.renderSettings(w, r, http.StatusBadRequest, repo, "Invalid visibility.", "")
 		return


@@ 128,9 131,9 @@ func (a *app) settingsUpdate(w http.ResponseWriter, r *http.Request, repo *core.
 
 // settingsACLAdd grants (or updates) an ACL entry for a username. The grantee is
 // resolved via the user resolver, which mirrors the meta profile on first sight.
-func (a *app) settingsACLAdd(w http.ResponseWriter, r *http.Request, repo *core.Repo) {
-	username := strings.TrimPrefix(strings.TrimSpace(r.PostFormValue("username")), "~")
-	mode, ok := parseAccessMode(r.PostFormValue("mode"))
+func (a *app) settingsACLAdd(w http.ResponseWriter, r *http.Request, repo *core.Repo, form url.Values) {
+	username := strings.TrimPrefix(strings.TrimSpace(form.Get("username")), "~")
+	mode, ok := parseAccessMode(form.Get("mode"))
 	if !ok {
 		a.renderSettings(w, r, http.StatusBadRequest, repo, "Invalid access mode.", "")
 		return


@@ 159,8 162,8 @@ func (a *app) settingsACLAdd(w http.ResponseWriter, r *http.Request, repo *core.
 }
 
 // settingsACLRemove revokes an ACL entry by user id.
-func (a *app) settingsACLRemove(w http.ResponseWriter, r *http.Request, repo *core.Repo) {
-	userID, err := strconv.Atoi(r.PostFormValue("user_id"))
+func (a *app) settingsACLRemove(w http.ResponseWriter, r *http.Request, repo *core.Repo, form url.Values) {
+	userID, err := strconv.Atoi(form.Get("user_id"))
 	if err != nil {
 		a.renderSettings(w, r, http.StatusBadRequest, repo, "Invalid user.", "")
 		return


@@ 179,8 182,8 @@ func (a *app) settingsACLRemove(w http.ResponseWriter, r *http.Request, repo *co
 // settingsDelete deletes the database after a name-confirmation check: the row,
 // then the on-disk store, then the served-cache handle. The confirmation guards
 // against accidental deletion.
-func (a *app) settingsDelete(w http.ResponseWriter, r *http.Request, repo *core.Repo) {
-	if r.PostFormValue("confirm_name") != repo.Name {
+func (a *app) settingsDelete(w http.ResponseWriter, r *http.Request, repo *core.Repo, form url.Values) {
+	if form.Get("confirm_name") != repo.Name {
 		a.renderSettings(w, r, http.StatusBadRequest, repo,
 			"Type the database name exactly to confirm deletion.", "")
 		return

@@ 5,6 5,7 @@ import (
 	"crypto/rand"
 	"errors"
 	"fmt"
+	"io"
 	"net/http"
 	"net/http/httptest"
 	"net/url"


@@ 505,6 506,46 @@ func TestMutationsAreRefusedWithoutSameOriginEvidence(t *testing.T) {
 	}
 }
 
+// TestFormsReadTheBodyAndOnlyTheBody pins the two properties every mutation on
+// this surface now gets from pages.FormValues.
+//
+// The query string is not the form. r.Form would merge it into the body's
+// values, which would let a mutation be driven entirely from a URL somebody was
+// linked to — and that is exactly the request the same-origin guard sees nothing
+// wrong with, because it really did come from our own page.
+//
+// And the body is bounded. net/http's own ceiling is 10 MiB per request, three
+// orders of magnitude more than any form here sends.
+func TestFormsReadTheBodyAndOnlyTheBody(t *testing.T) {
+	post := func(t *testing.T, target string, body io.Reader) *httptest.ResponseRecorder {
+		t.Helper()
+		h := newHarness(t)
+		h.store.add(&core.Repo{Name: "db", OwnerID: 10, OwnerName: "owner", Path: "/d", Visibility: core.VisibilityPublic})
+		req := httptest.NewRequest("POST", target, body)
+		req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+		req.Header.Set("Origin", selfOrigin)
+		req = req.WithContext(authn.WithCaller(req.Context(), testCaller(10, "owner")))
+		rec := httptest.NewRecorder()
+		h.router.ServeHTTP(rec, req)
+		return rec
+	}
+
+	t.Run("the query string cannot supply a field", func(t *testing.T) {
+		// A delete driven from the URL: the confirmation the body does not carry
+		// is offered in the query string instead. It must not be read.
+		rec := post(t, "/~owner/db/settings?action=delete&confirm_name=db", strings.NewReader(""))
+		assert.Equal(t, http.StatusBadRequest, rec.Code)
+		assert.Contains(t, rec.Body.String(), "Unknown action.")
+	})
+
+	t.Run("an oversized body is refused", func(t *testing.T) {
+		huge := "description=" + strings.Repeat("x", pages.DefaultMaxFormBytes+1)
+		rec := post(t, "/~owner/db/settings", strings.NewReader(huge))
+		assert.Equal(t, http.StatusBadRequest, rec.Code)
+		assert.Contains(t, rec.Body.String(), "Malformed form submission.")
+	})
+}
+
 // Nothing behind the login cookie may be reused for the next viewer: these URLs
 // say nothing about who the page was rendered for.
 func TestPagesAreNotCacheable(t *testing.T) {