Keep what was typed when a target or webhook edit is refused (closes #381)

A refused save on the target edit page now shows the edit form again,
with the reason above it and every value submitted, instead of a bare
text page; the status codes are unchanged. The webhook edit page keeps
the submitted name, description and retention the same way, while the
page still reports the stored retention.

Target edits are validated by setTargetFromForm, which newTarget now
uses too, so the add and edit forms accept and refuse the same things.
An empty max_retries keeps the target's own count.

The browser test also saves both edit pages with refused values.

Model: opus-5-5

README.md

@@ -1343,23 +1343,25 @@ markup. The CSP build runs no expressions, so every Alpine directive in
 `static/js/app.js`: `x-data="collapsible"` and `@click="toggle"`, never
 `x-data="{ open: false }"` or `@click="open = !open"`.
 
-A browser test in `internal/server` loads the webhook page and the event log
+A browser test in `internal/server` loads the webhook page, its edit pages and
-under the real policy and checks that: the add entrypoint form stays hidden
+the event log under the real policy and checks that: the add entrypoint form
-until Add is clicked; for every target type, the targets section's Add shows
+stays hidden until Add is clicked; for every target type, the targets section's
-only a choice of type with Next and Cancel, Next shows only that type's fields
+Add shows only a choice of type with Next and Cancel, Next shows only that
-(no url field for `database` or `log`), Cancel at either step closes the form,
+type's fields (no url field for `database` or `log`), Cancel at either step
-and saving adds the target; a refused target comes back with its form open, the
+closes the form, and saving adds the target; a refused target comes back with
-values entered and the reason, and after Cancel the next Add starts with an
+its form open, the values entered and the reason, and after Cancel the next Add
-empty form and no reason; the Copy button beside an entrypoint URL reads
+starts with an empty form and no reason; a refused save on the target edit page
-"Copied" once clicked; an entrypoint's Edit button shows its edit form in place
+and on the webhook edit page comes back with the reason and every value
-of its description and hides until the form closes, Cancel hides the form and
+entered; the Copy button beside an entrypoint URL reads "Copied" once clicked;
-drops what was typed, as does leaving the page and going back to it, and Save
+an entrypoint's Edit button shows its edit form in place of its description and
-changes the description; of the recent events on the webhook page only the
+hides until the form closes, Cancel hides the form and drops what was typed, as
-newest starts expanded, each expands and collapses, and Open leads to the
+does leaving the page and going back to it, and Save changes the description;
-event's own page; an event in the event log expands and collapses, and so do a
+of the recent events on the webhook page only the newest starts expanded, each
-delivery's attempts inside it; and at phone width the menu button opens and
+expands and collapses, and Open leads to the event's own page; an event in the
-closes the mobile menu. It also fails if the browser reports a console warning
+event log expands and collapses, and so do a delivery's attempts inside it; and
-or error, an uncaught exception, or anything the policy refused. `make check`
+at phone width the menu button opens and closes the mobile menu. It also fails
+if the browser reports a console warning or error, an uncaught exception, or
+anything the policy refused. `make check`
 and the image build lint it but do not run it, and `make test` leaves it out
 (its file is built only with the `browser` build tag). Run it with
 `make test-browser` after changing `templates/` or `static/js/`: that builds

internal/handlers/source_management.go

@@ -528,7 +528,7 @@ func (h *Handlers) renderSourceDetail(
 		"Events":          events,
 		"BaseURL":         baseURL,
 		"Stats":           h.loadWebhookStats(webhook.ID, entrypoints, targets),
-		"TargetForm":  targetForm,
+		tmplKeyTargetForm: targetForm,
 		"TargetError":     targetErr,
 	}
 
@@ -565,12 +565,12 @@ func (h *Handlers) HandleSourceEdit() http.HandlerFunc {
 			return
 		}
 
-		data := map[string]any{
+		h.renderWebhookEdit(
-			tmplKeyWebhook: &webhook,
+			w, r, &webhook,
-			tmplKeyError:   "",
+			webhook.Name, webhook.Description,
-		}
+			strconv.Itoa(webhook.RetentionDays),
-
+			"", http.StatusOK,
-		h.renderTemplate(w, r, "source_edit.html", data)
+		)
 	}
 }
 
@@ -616,7 +616,8 @@ func (h *Handlers) HandleSourceEditSubmit() http.HandlerFunc {
 	}
 }
 
-// applyWebhookEdit validates and saves webhook edits.
+// applyWebhookEdit validates and saves webhook edits. A refused save
+// shows the edit form again with the values submitted and the reason.
 func (h *Handlers) applyWebhookEdit(
 	w http.ResponseWriter,
 	r *http.Request,
@@ -625,52 +626,52 @@ func (h *Handlers) applyWebhookEdit(
 	// The body size cap is enforced by the MaxBodySize middleware,
 	// which runs before CSRF parses the form.
 	name := r.PostFormValue("name")
-	if name == "" {
+	description := r.PostFormValue("description")
-		data := map[string]any{
+	retention := r.PostFormValue("retention_days")
-			tmplKeyWebhook: webhook,
-			tmplKeyError:   "Name is required",
-		}
 
-		h.renderTemplateStatus(w, r, "source_edit.html", data, http.StatusBadRequest)
+	if name == "" {
+		h.renderWebhookEdit(
+			w, r, webhook, name, description, retention,
+			"Name is required", http.StatusBadRequest,
+		)
 
 		return
 	}
 
-	oldName := webhook.Name
-	webhook.Name = name
-	webhook.Description = r.PostFormValue("description")
-
 	// An empty field falls back to the stored value, so submitting the
 	// form without touching retention leaves the policy alone.
 	retentionDays, errMsg := parseRetentionDays(
-		r.PostFormValue("retention_days"), webhook.RetentionDays,
+		retention, webhook.RetentionDays,
 	)
 	if errMsg != "" {
-		data := map[string]any{
+		h.renderWebhookEdit(
-			tmplKeyWebhook: webhook,
+			w, r, webhook, name, description, retention,
-			tmplKeyError:   errMsg,
+			errMsg, http.StatusBadRequest,
-		}
+		)
-
-		h.renderTemplateStatus(w, r, "source_edit.html", data, http.StatusBadRequest)
 
 		return
 	}
 
-	webhook.RetentionDays = retentionDays
+	// edited is the webhook as the submission leaves it; webhook stays
+	// as stored, for the page shown again when the save is refused.
+	edited := *webhook
+	edited.Name = name
+	edited.Description = description
+	edited.RetentionDays = retentionDays
 
 	// A new name renames the archive files before it is saved (see
 	// delivery.Engine.Rename). If either step fails, the same targets'
 	// archives go back to the name that is still stored, without
 	// reading the main database again.
 	targets, err := h.renameWebhookArchives(
-		webhook.ID, oldName, webhook.Name,
+		webhook.ID, webhook.Name, edited.Name,
 	)
 	if err == nil {
-		err = h.db.DB().Save(webhook).Error
+		err = h.db.DB().Save(&edited).Error
 	}
 
 	if err != nil {
-		restoreErr := h.renameArchives(targets, oldName)
+		restoreErr := h.renameArchives(targets, webhook.Name)
 		if restoreErr != nil {
 			h.log.Error(
 				"failed to rename archives back",
@@ -680,15 +681,14 @@ func (h *Handlers) applyWebhookEdit(
 		}
 
 		if errors.Is(err, delivery.ErrArchiveNameTaken) {
-			data := map[string]any{
+			h.renderWebhookEdit(
-				tmplKeyWebhook: webhook,
+				w, r, webhook, name, description, retention,
-				tmplKeyError: "Not saved: " + err.Error() +
+				"Not saved: "+err.Error()+
-					". Move that archive out of the data directory, " +
+					". Move that archive out of the data directory, "+
-					"its .db together with any -wal and -shm beside " +
+					"its .db together with any -wal and -shm beside "+
 					"it, then save again.",
-			}
+				http.StatusConflict,
-
+			)
-			h.renderTemplateStatus(w, r, "source_edit.html", data, http.StatusConflict)
 
 			return
 		}
@@ -704,6 +704,27 @@ func (h *Handlers) applyWebhookEdit(
 	)
 }
 
+// renderWebhookEdit renders the webhook edit page for the webhook as
+// stored, its form showing name, description and retentionDays, with
+// an optional error message above it.
+func (h *Handlers) renderWebhookEdit(
+	w http.ResponseWriter,
+	r *http.Request,
+	webhook *database.Webhook,
+	name, description, retentionDays, errMsg string,
+	status int,
+) {
+	data := map[string]any{
+		tmplKeyWebhook:  webhook,
+		tmplKeyError:    errMsg,
+		"Name":          name,
+		"Description":   description,
+		"RetentionDays": retentionDays,
+	}
+
+	h.renderTemplateStatus(w, r, "source_edit.html", data, status)
+}
+
 // HandleSourceDelete handles webhook deletion.
 func (h *Handlers) HandleSourceDelete() http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
@@ -1575,49 +1596,57 @@ func (h *Handlers) newTarget(
 	webhookID string,
 	in targetFormInput,
 ) (*database.Target, string, error) {
-	if in.Name == "" {
+	target := &database.Target{
-		return nil, "Name is required", nil
+		WebhookID: webhookID,
+		Type:      in.Type,
+		Active:    true,
 	}
 
-	if !isValidTargetType(in.Type) {
+	errMsg, err := h.setTargetFromForm(ctx, target, in)
-		return nil, "Invalid target type", nil
-	}
-
-	configJSON, errMsg, err := h.buildTargetConfig(ctx, in.Type, in)
 	if err != nil || errMsg != "" {
 		return nil, errMsg, err
 	}
 
-	// A new target has no stored retry count, so an absent field
+	return target, "", nil
-	// takes the fire-and-forget default. A field the operator filled
-	// in with something invalid is refused rather than becoming
-	// that default.
-	maxRetries, err := parseMaxRetries(in.MaxRetries, 0)
-	if err != nil {
-		return nil, "Invalid max retries: " + retriesErrorMessage(err), nil
-	}
-
-	return &database.Target{
-		WebhookID:  webhookID,
-		Name:       in.Name,
-		Type:       in.Type,
-		Active:     true,
-		Config:     configJSON,
-		MaxRetries: maxRetries,
-	}, "", nil
 }
 
-// isValidTargetType checks whether the target type is supported.
+// setTargetFromForm validates a target form against the target's type
-func isValidTargetType(tt database.TargetType) bool {
+// and, when it accepts it, sets the target's name, configuration and
-	switch tt {
+// retry count from it. It returns the message the form shows for
-	case database.TargetTypeHTTP,
+// anything it refuses, an unknown type among them, and then leaves the
-		database.TargetTypeDatabase,
+// target unchanged; an error is the server's fault, as for newTarget.
-		database.TargetTypeLog,
+// The add target form and the target edit form both go through here,
-		database.TargetTypeSlack:
+// so the two cannot come to disagree about what a target may be.
-		return true
+func (h *Handlers) setTargetFromForm(
-	default:
+	ctx context.Context,
-		return false
+	target *database.Target,
+	in targetFormInput,
+) (string, error) {
+	if in.Name == "" {
+		return "Name is required", nil
 	}
+
+	configJSON, errMsg, err := h.buildTargetConfig(ctx, target.Type, in)
+	if err != nil || errMsg != "" {
+		return errMsg, err
+	}
+
+	// An empty max_retries keeps the target's count: the
+	// fire-and-forget default of 0 for a new target, and the stored
+	// count for an edited one, since the forms for target types that
+	// do not retry have no such field. A value that is filled in but
+	// invalid is refused rather than becoming that count, so a typo
+	// cannot destroy the count a target is delivering with.
+	maxRetries, err := parseMaxRetries(in.MaxRetries, target.MaxRetries)
+	if err != nil {
+		return "Invalid max retries: " + retriesErrorMessage(err), nil
+	}
+
+	target.Name = in.Name
+	target.Config = configJSON
+	target.MaxRetries = maxRetries
+
+	return "", nil
 }
 
 // pageOrFirst parses a paginated page number, answering 1 for
@@ -1639,9 +1668,10 @@ func pageOrFirst(s string) int {
 }
 
 // targetFormInput carries the raw values of a target form. Both the
-// create and the edit path fill one and hand it to buildTargetConfig,
+// create and the edit path fill one and hand it to setTargetFromForm,
-// so neither can come to validate a destination differently from the
+// so neither can come to validate a target differently from the
-// other. A refused add target form is shown again from it.
+// other. Both forms are filled from one: the edit form with the
+// stored values, and a refused form with the values submitted.
 type targetFormInput struct {
 	// Name is the target's name.
 	Name string

internal/handlers/source_management_test.go

@@ -509,6 +509,51 @@ func TestHandleSourceEditSubmit_InvalidRetentionIsRejected(
 	)
 }
 
+// TestHandleSourceEditSubmit_RefusedFormComesBack refuses an edit for
+// each reason the form can give and checks that the form comes back
+// with the reason and the name, description and retention submitted,
+// that the page still reports the stored retention, and that nothing
+// is saved.
+func TestHandleSourceEditSubmit_RefusedFormComesBack(t *testing.T) {
+	t.Parallel()
+
+	env := setupSourceTest(t)
+
+	refused := func(name, retention, reason string) {
+		t.Helper()
+
+		wh := seedWebhookWithRetention(t, env.db, 30)
+
+		submitted := wh
+		submitted.Name = name
+		submitted.Description = "a description worth keeping"
+
+		w := submitEdit(t, env, submitted, retention)
+		assert.Equal(t, http.StatusBadRequest, w.Code)
+
+		page := w.Body.String()
+		assert.Contains(t, page, `class="alert-error">`+reason)
+		assert.Contains(t, page, `name="name" value="`+name+`"`)
+		assert.Contains(t, page, ">a description worth keeping</textarea>")
+		assert.Contains(
+			t, page, `name="retention_days" value="`+retention+`"`,
+		)
+		assert.Contains(t, page, "Currently 30 days.")
+
+		var stored database.Webhook
+
+		require.NoError(
+			t, env.db.DB().First(&stored, "id = ?", wh.ID).Error,
+		)
+		assert.Equal(t, wh.Name, stored.Name)
+		assert.Empty(t, stored.Description)
+		assert.Equal(t, 30, stored.RetentionDays)
+	}
+
+	refused("", "45", "Name is required")
+	refused("kept-name", "nonsense", "Retention must be")
+}
+
 func TestHandleSourceEditSubmit_EmptyRetentionLeavesValueUnchanged(
 	t *testing.T,
 ) {
@@ -809,6 +854,10 @@ func TestHandleSourceEditSubmit_ArchiveNameTaken(t *testing.T) {
 	w := submitEdit(t, env, wh, "")
 	require.Equal(t, http.StatusConflict, w.Code)
 	assert.Contains(t, w.Body.String(), "archive-taken.db")
+	assert.Contains(
+		t, w.Body.String(), `name="name" value="`+renamedWebhookName+`"`,
+		"the form comes back with the name submitted",
+	)
 
 	var stored database.Webhook
 

internal/handlers/target_edit.go

@@ -3,6 +3,7 @@ package handlers
 import (
 	"errors"
 	"net/http"
+	"strconv"
 
 	"github.com/go-chi/chi"
 	"sneak.berlin/go/webhooker/internal/database"
@@ -13,10 +14,13 @@ import (
 const targetEditTemplate = "target_edit.html"
 
 // tmplKeyTarget is the template data key for the target being
-// edited, and tmplKeyMaxTimeout for the timeout ceiling the form
+// edited, tmplKeyTargetForm for the values its form shows, and
-// tells the user about.
+// tmplKeyMaxTimeout for the timeout ceiling the form tells the user
+// about. The add target form on the webhook page takes its values
+// under the same key as the edit form.
 const (
 	tmplKeyTarget     = "Target"
+	tmplKeyTargetForm = "TargetForm"
 	tmplKeyMaxTimeout = "MaxTimeout"
 )
 
@@ -28,20 +32,19 @@ const configUnreadableMessage = "The stored configuration for this " +
 	"target could not be read. Enter the values below; saving " +
 	"replaces the stored configuration."
 
-// targetEditView is the display model for the target edit page.
+// targetEditView is the display model for the target edit page: the
+// target's row fields as stored. The values the form shows, the
+// UNMASKED configuration among them, come separately, as a
+// targetFormInput.
 //
-// It carries the target's row fields alongside its UNMASKED
+// It deliberately omits database.Target's raw Config blob: the form
-// configuration, and deliberately omits database.Target's raw
+// renders named fields, and giving the template the blob as well
-// Config blob: the form renders named fields, and giving the
+// would put an unreviewed second path to the credential on the page.
-// template the blob as well would put an unreviewed second path to
-// the credential on the page.
 type targetEditView struct {
 	ID     string
 	Name   string
 	Type   database.TargetType
 	Active bool
-	MaxRetries int
-	Config     delivery.TargetConfigForm
 }
 
 // HandleTargetEdit shows the form to edit a target.
@@ -73,7 +76,18 @@ func (h *Handlers) HandleTargetEdit() http.HandlerFunc {
 			msg = configUnreadableMessage
 		}
 
-		h.renderTargetEdit(w, r, webhook, target, cfg, msg)
+		form := targetFormInput{
+			Name:       target.Name,
+			URL:        cfg.URL,
+			Headers:    cfg.Headers,
+			Timeout:    cfg.Timeout,
+			MaxRetries: strconv.Itoa(target.MaxRetries),
+			Expiry:     cfg.Expiry,
+		}
+
+		h.renderTargetEdit(
+			w, r, webhook, target, form, msg, http.StatusOK,
+		)
 	}
 }
 
@@ -101,11 +115,12 @@ func (h *Handlers) HandleTargetEditSubmit() http.HandlerFunc {
 	}
 }
 
-// applyTargetEdit validates and saves target edits.
+// applyTargetEdit validates and saves target edits. A refused save
+// shows the edit form again with the values submitted and the reason.
 //
-// The submitted configuration goes through buildTargetConfig, the
+// The submission goes through setTargetFromForm, as a new target
-// same builder the create path uses, so an edited destination is
+// does, so an edited destination is SSRF-validated exactly as a new
-// SSRF-validated exactly as a new one is.
+// one is.
 //
 // The target's type is not editable. Each type stores a different
 // configuration shape and its delivery history is recorded against
@@ -118,16 +133,13 @@ func (h *Handlers) applyTargetEdit(
 	webhook database.Webhook,
 	target *database.Target,
 ) {
-	name := r.PostFormValue("name")
+	in := targetFormInputFrom(r)
-	if name == "" {
-		http.Error(w, "Name is required", http.StatusBadRequest)
 
-		return
+	// edited is the target as the submission leaves it; target stays
-	}
+	// as stored, for the page shown again when the save is refused.
+	edited := *target
 
-	configJSON, errMsg, err := h.buildTargetConfig(
+	errMsg, err := h.setTargetFromForm(r.Context(), &edited, in)
-		r.Context(), target.Type, targetFormInputFrom(r),
-	)
 	if err != nil {
 		h.serverError(w, r, "failed to encode target config", err)
 
@@ -135,45 +147,26 @@ func (h *Handlers) applyTargetEdit(
 	}
 
 	if errMsg != "" {
-		http.Error(w, errMsg, http.StatusBadRequest)
+		h.renderTargetEdit(
+			w, r, webhook, target, in, errMsg, http.StatusBadRequest,
+		)
 
 		return
 	}
 
-	// Retries are offered only by the forms for target types that
-	// retry, so an absent field means "this form does not edit
-	// retries" rather than "set them to zero". Reading it
-	// unconditionally would silently disable retries on any target
-	// saved from a form that does not render the input.
-	//
-	// A field that IS submitted but does not parse is a 400, through
-	// the same validator the create path uses. It is rejected before
-	// anything is written, so a typo cannot destroy the retry count
-	// the target is already delivering with.
-	if r.PostForm.Has("max_retries") {
-		retries, ok := targetMaxRetries(w, r, target.MaxRetries)
-		if !ok {
-			return
-		}
-
-		target.MaxRetries = retries
-	}
-
-	oldName := target.Name
-	target.Name = name
-	target.Config = configJSON
-
 	// A new name renames the archive file before it is saved (see
 	// delivery.Engine.Rename). If either step fails, it goes back to
 	// the name that is still stored.
-	err = h.renameTargetArchive(target, webhook.Name, oldName, name)
+	err = h.renameTargetArchive(
+		target, webhook.Name, target.Name, edited.Name,
+	)
 	if err == nil {
-		err = h.db.DB().Save(target).Error
+		err = h.db.DB().Save(&edited).Error
 	}
 
 	if err != nil {
 		restoreErr := h.renameTargetArchive(
-			target, webhook.Name, name, oldName,
+			target, webhook.Name, edited.Name, target.Name,
 		)
 		if restoreErr != nil {
 			h.log.Error(
@@ -184,8 +177,8 @@ func (h *Handlers) applyTargetEdit(
 		}
 
 		if errors.Is(err, delivery.ErrArchiveNameTaken) {
-			http.Error(
+			h.renderTargetEdit(
-				w,
+				w, r, webhook, target, in,
 				"Not saved: "+err.Error()+
 					". Move that archive out of the data directory, "+
 					"its .db together with any -wal and -shm beside "+
@@ -222,15 +215,17 @@ func (h *Handlers) renameTargetArchive(
 	return h.archives.Rename(target.ID, webhookName, newName)
 }
 
-// renderTargetEdit renders the target edit page with an optional
+// renderTargetEdit renders the target edit page for the target as
-// error message.
+// stored, its form showing form's values, with an optional error
+// message above it.
 func (h *Handlers) renderTargetEdit(
 	w http.ResponseWriter,
 	r *http.Request,
 	webhook database.Webhook,
 	target *database.Target,
-	cfg delivery.TargetConfigForm,
+	form targetFormInput,
 	errMsg string,
+	status int,
 ) {
 	// The template calls Webhook methods, which take pointer
 	// receivers; html/template cannot address a value stored in a
@@ -242,14 +237,13 @@ func (h *Handlers) renderTargetEdit(
 			Name:   target.Name,
 			Type:   target.Type,
 			Active: target.Active,
-			MaxRetries: target.MaxRetries,
-			Config:     cfg,
 		},
+		tmplKeyTargetForm: form,
 		tmplKeyMaxTimeout: delivery.MaxTargetTimeoutSeconds,
 		tmplKeyError:      errMsg,
 	}
 
-	h.renderTemplate(w, r, targetEditTemplate, data)
+	h.renderTemplateStatus(w, r, targetEditTemplate, data, status)
 }
 
 // ownedTarget resolves the request's sourceID and targetID

internal/handlers/target_edit_test.go

@@ -565,6 +565,73 @@ func assertEditRejectsTimeout(
 	)
 }
 
+// TestHandleTargetEditSubmit_RefusedFormComesBack refuses an edit of
+// a target of each type and checks that the edit form comes back with
+// the reason and every value submitted, and that nothing is saved.
+func TestHandleTargetEditSubmit_RefusedFormComesBack(t *testing.T) {
+	t.Parallel()
+
+	env := setupSourceTest(t)
+
+	// fields is what the operator submitted, as a query string.
+	cases := []struct {
+		targetType database.TargetType
+		fields     string
+		reason     string
+	}{
+		{
+			database.TargetTypeHTTP,
+			"name=edited&url=" + editBlockedURL +
+				"&headers=X-Edited:+kept&timeout=12&max_retries=3",
+			"Invalid target URL",
+		},
+		{
+			database.TargetTypeSlack,
+			"name=edited&url=" + editOriginalURL + "&max_retries=25",
+			"Invalid max retries",
+		},
+		{
+			database.TargetTypeDatabase, "name=edited&expiry=7d",
+			"Invalid archive expiry",
+		},
+		{database.TargetTypeLog, "name=", "Name is required"},
+	}
+
+	for _, tc := range cases {
+		t.Run(string(tc.targetType), func(t *testing.T) {
+			t.Parallel()
+
+			webhook := seedWebhookWithRetention(t, env.db, 30)
+			target := seedTarget(t, env.db, webhook.ID, tc.targetType)
+
+			form, err := url.ParseQuery(tc.fields)
+			require.NoError(t, err)
+
+			w := submitTargetEdit(env, webhook.ID, target.ID, form)
+			assert.Equal(t, http.StatusBadRequest, w.Code)
+
+			page := w.Body.String()
+			assert.Contains(t, page, `class="alert-error">`+tc.reason)
+
+			// headers is the form's one textarea; every other field is
+			// an input.
+			for field := range form {
+				shown := `name="` + field + `" value="` + form.Get(field) + `"`
+				if field == "headers" {
+					shown = ">" + form.Get(field) + "</textarea>"
+				}
+
+				assert.Contains(t, page, shown)
+			}
+
+			assert.Equal(
+				t, target.Name, storedTarget(t, env, target.ID).Name,
+				"a refused edit must save nothing",
+			)
+		})
+	}
+}
+
 // TestHandleTargetEdit_Scoping keeps the edit routes scoped the way
 // the delete and toggle routes are: ownership is decided by the
 // webhook, and the target is then scoped to it.
@@ -700,6 +767,10 @@ func TestHandleTargetEditSubmit_RenamesArchive(t *testing.T) {
 	w = submitTargetEdit(env, wh.ID, archive.ID, again)
 	require.Equal(t, http.StatusConflict, w.Code)
 	assert.Contains(t, w.Body.String(), "archive-taken.db")
+	assert.Contains(
+		t, w.Body.String(), `name="name" value="Again"`,
+		"the form comes back with the name submitted",
+	)
 	assert.Equal(
 		t, renamedTargetName, storedTarget(t, env, archive.ID).Name,
 	)

internal/handlers/target_private_refusal_test.go

@@ -44,9 +44,9 @@ func TestTargetRefusal_PrivateDestinationSaysHowToAllowIt(
 			form.Set("type", string(targetType))
 			form.Set("url", editBlockedURL)
 
-			// A refused add shows the webhook page again, where
+			// A refused add shows the webhook page again, and a
-			// the hint is HTML-escaped; a refused edit answers in
+			// refused edit the edit page, where the hint is
-			// plain text.
+			// HTML-escaped.
 			added := serveTarget(
 				env, http.MethodPost, targetsPath, form,
 			)
@@ -76,7 +76,8 @@ func TestTargetRefusal_PrivateDestinationSaysHowToAllowIt(
 			)
 			assert.Equal(t, http.StatusBadRequest, edited.Code)
 			assert.Contains(
-				t, edited.Body.String(), privateRefusalHint,
+				t, edited.Body.String(),
+				html.EscapeString(privateRefusalHint),
 			)
 		})
 	}

internal/handlers/target_retries.go

@@ -2,7 +2,6 @@ package handlers
 
 import (
 	"errors"
-	"net/http"
 	"strconv"
 	"strings"
 )
@@ -89,32 +88,3 @@ func retriesErrorMessage(err error) string {
 	return errRetriesInvalid.Error() +
 		", or 0 for fire-and-forget"
 }
-
-// targetMaxRetries reads and validates max_retries from a target edit
-// submission, answering the request with a 400 and reporting false
-// when the value is set but invalid.
-//
-// It and the create path (newTarget) both use parseMaxRetries and
-// retriesErrorMessage, so the two cannot come to disagree about what a
-// valid retry count is. The wording matches the timeout control on
-// the same submission.
-func targetMaxRetries(
-	w http.ResponseWriter,
-	r *http.Request,
-	fallback int,
-) (int, bool) {
-	retries, err := parseMaxRetries(
-		r.PostFormValue("max_retries"), fallback,
-	)
-	if err != nil {
-		http.Error(
-			w,
-			"Invalid max retries: "+retriesErrorMessage(err),
-			http.StatusBadRequest,
-		)
-
-		return 0, false
-	}
-
-	return retries, true
-}

internal/handlers/ui_copy_test.go

@@ -398,9 +398,9 @@ func TestTargetFormMaxRetriesCopyMatchesBehaviour(t *testing.T) {
 	)
 
 	// A slack target exercises the same max_retries field while needing
-	// only Config.URL from the edit template, so the test data stays
+	// only a URL from the edit template, so the test data stays
-	// minimal. The Target key mirrors the field names the template reads
+	// minimal. The Target and TargetForm keys mirror the field names
-	// off the handler's view value.
+	// the template reads off the handler's values.
 	editBody := renderPage(
 		t, h, sess, "target_edit.html", map[string]any{
 			dataKeyWebhook: webhook,
@@ -409,10 +409,10 @@ func TestTargetFormMaxRetriesCopyMatchesBehaviour(t *testing.T) {
 				"Name":   "t",
 				"Type":   "slack",
 				"Active": true,
-				"MaxRetries": 3,
-				"Config": map[string]any{
-					"URL": "https://hooks.slack.com/services/x",
 			},
+			"TargetForm": map[string]any{
+				"URL":        "https://hooks.slack.com/services/x",
+				"MaxRetries": "3",
 			},
 			dataKeyError: "",
 		},

internal/server/alpine_browser_test.go

@@ -118,6 +118,7 @@ func TestAlpineRunsUnderTheSecurityPolicy(t *testing.T) {
 	}
 
 	checkRefusedTarget(ctx, t, page)
+	checkRefusedEdits(ctx, t, page, target.ID)
 	checkCopy(ctx, t, page)
 	checkEntrypointEdit(ctx, t, page, page+"/events")
 	checkRecentEvents(ctx, t, page)
@@ -452,6 +453,64 @@ func checkRefusedTarget(ctx context.Context, t *testing.T, url string) {
 	assert.Empty(t, typed, "after Cancel, the next Add keeps the url entered")
 }
 
+// checkRefusedEdits fills in the target edit page and the webhook edit
+// page of a webhook page with values the server refuses, a loopback
+// destination and a retention above the longest finite one, which the
+// browser lets through. It saves each and checks that the page comes
+// back with the reason and every value still in its field. The values
+// are keyed by the id of their field.
+func checkRefusedEdits(
+	ctx context.Context, t *testing.T, page, targetID string,
+) {
+	t.Helper()
+
+	const reason = `//div[@class="alert-error"]`
+
+	edits := []struct {
+		url    string
+		values map[string]string
+	}{
+		{page + "/targets/" + targetID + "/edit", map[string]string{
+			"#name":        "edited-target",
+			"#url":         "http://127.0.0.1/hook",
+			"#headers":     "X-Edited: kept",
+			"#timeout":     "12",
+			"#max_retries": "3",
+		}},
+		{page + "/edit", map[string]string{
+			"#name":           "edited-webhook",
+			"#description":    "kept description",
+			"#retention_days": "200000",
+		}},
+	}
+
+	for _, edit := range edits {
+		require.NoError(t, chromedp.Run(ctx, loadPage(edit.url)))
+
+		for field, value := range edit.values {
+			require.NoError(t, chromedp.Run(
+				ctx, chromedp.SetValue(field, value, chromedp.ByQuery),
+			))
+		}
+
+		click(ctx, t, `//button[text()="Save Changes"]`)
+
+		assert.Truef(t, shown(ctx, reason),
+			"%s: a refused save does not show the reason", edit.url)
+
+		for field, value := range edit.values {
+			var kept string
+
+			require.NoError(t, chromedp.Run(
+				ctx, chromedp.Value(field, &kept, chromedp.ByQuery),
+			))
+			assert.Equalf(t, value, kept,
+				"%s: a refused save does not keep the %s entered",
+				edit.url, field)
+		}
+	}
+}
+
 // checkCopy loads a webhook page and checks that the Copy control beside
 // its entrypoint's URL is a button, and that clicking it copies the URL
 // and says so: the button reads "Copied" only once the copy succeeded.

templates/source_edit.html

@@ -18,17 +18,17 @@
             <input type="hidden" name="csrf_token" value="{{.CSRFToken}}">
             <div class="form-group">
                 <label for="name" class="label">Name</label>
-                <input type="text" id="name" name="name" value="{{.Webhook.Name}}" required class="input">
+                <input type="text" id="name" name="name" value="{{.Name}}" required class="input">
             </div>
 
             <div class="form-group">
                 <label for="description" class="label">Description</label>
-                <textarea id="description" name="description" rows="3" class="input">{{.Webhook.Description}}</textarea>
+                <textarea id="description" name="description" rows="3" class="input">{{.Description}}</textarea>
             </div>
 
             <div class="form-group">
                 <label for="retention_days" class="label">Retention (days)</label>
-                <input type="number" id="retention_days" name="retention_days" value="{{.Webhook.RetentionDays}}" min="0" class="input">
+                <input type="number" id="retention_days" name="retention_days" value="{{.RetentionDays}}" min="0" class="input">
                 <p class="text-xs text-gray-500 mt-1">Currently {{.Webhook.RetentionLabel}}.{{if .Webhook.RetainsForever}} No events are deleted while retention is set to forever.{{else}} A periodic cleanup permanently deletes events older than this, along with their delivery records.{{end}} Enter 0 to retain events forever; leave blank to keep the current setting.</p>
             </div>
 

templates/target_edit.html

@@ -26,25 +26,25 @@
 
             <div class="form-group">
                 <label for="name" class="label">Name</label>
-                <input type="text" id="name" name="name" value="{{.Target.Name}}" required class="input">
+                <input type="text" id="name" name="name" value="{{.TargetForm.Name}}" required class="input">
             </div>
 
             {{if eq .Target.Type "http"}}
             <div class="form-group">
                 <label for="url" class="label">Destination URL</label>
-                <input type="url" id="url" name="url" value="{{.Target.Config.URL}}" required class="input">
+                <input type="url" id="url" name="url" value="{{.TargetForm.URL}}" required class="input">
                 <p class="text-xs text-gray-500 mt-1">Revalidated on save; destinations that resolve to private or link-local addresses are rejected.</p>
             </div>
 
             <div class="form-group">
                 <label for="headers" class="label">Headers</label>
-                <textarea id="headers" name="headers" rows="4" class="input" placeholder="Authorization: Bearer ...">{{.Target.Config.Headers}}</textarea>
+                <textarea id="headers" name="headers" rows="4" class="input" placeholder="Authorization: Bearer ...">{{.TargetForm.Headers}}</textarea>
                 <p class="text-xs text-gray-500 mt-1">One <code>Name: value</code> per line, sent with every delivery. Leave blank for none. <code>Host</code>, <code>Content-Length</code>, <code>Transfer-Encoding</code>, <code>Connection</code>, <code>Trailer</code> and <code>User-Agent</code> are set by the delivery engine and are rejected here rather than silently ignored. Headers set here are dropped if a redirect leaves the destination's own origin, so a credential cannot follow one to another host.</p>
             </div>
 
             <div class="form-group">
                 <label for="timeout" class="label">Timeout (seconds)</label>
-                <input type="number" id="timeout" name="timeout" value="{{.Target.Config.Timeout}}" min="0" max="{{.MaxTimeout}}" class="input">
+                <input type="number" id="timeout" name="timeout" value="{{.TargetForm.Timeout}}" min="0" max="{{.MaxTimeout}}" class="input">
                 <p class="text-xs text-gray-500 mt-1">Per-request timeout, at most {{.MaxTimeout}} seconds. Leave blank to use the default.</p>
             </div>
             {{end}}
@@ -52,7 +52,7 @@
             {{if eq .Target.Type "slack"}}
             <div class="form-group">
                 <label for="url" class="label">Webhook URL</label>
-                <input type="url" id="url" name="url" value="{{.Target.Config.URL}}" required class="input">
+                <input type="url" id="url" name="url" value="{{.TargetForm.URL}}" required class="input">
                 <p class="text-xs text-gray-500 mt-1">Slack or Mattermost incoming webhook URL. Revalidated on save.</p>
             </div>
             {{end}}
@@ -60,7 +60,7 @@
             {{if eq .Target.Type "database"}}
             <div class="form-group">
                 <label for="expiry" class="label">Archive Expiry</label>
-                <input type="text" id="expiry" name="expiry" value="{{.Target.Config.Expiry}}" placeholder="never" class="input">
+                <input type="text" id="expiry" name="expiry" value="{{.TargetForm.Expiry}}" placeholder="never" class="input">
                 <p class="text-xs text-gray-500 mt-1">"never" (the default when blank) keeps archived rows forever, or a Go duration like "720h" prunes older rows.</p>
             </div>
             {{end}}
@@ -68,7 +68,7 @@
             {{if or (eq .Target.Type "http") (eq .Target.Type "slack")}}
             <div class="form-group">
                 <label for="max_retries" class="label">Max retries</label>
-                <input type="number" id="max_retries" name="max_retries" value="{{.Target.MaxRetries}}" min="0" max="20" class="input">
+                <input type="number" id="max_retries" name="max_retries" value="{{.TargetForm.MaxRetries}}" min="0" max="20" class="input">
                 <p class="text-xs text-gray-500 mt-1">This is the total number of delivery attempts, not retries on top of the first: a value of 3 makes three attempts in all. 0 means a single attempt with no retries and no circuit breaker.</p>
             </div>
             {{end}}