Targets section: one Add, then a type and Next, then its fields (closes #370)

The webhook page's targets section lists only its targets until Add is
clicked. Add shows a choice of target type with Next; Next shows only
that type's fields, with Save and Cancel. The `database` and `log`
types have no URL field, and the `slack` form gains max retries.

A refused target now shows the webhook page again with the form open on
its type, the values entered and the reason, instead of a bare text
page. Target validation returns that message rather than writing the
response; newTarget validates a whole new target for reuse by the
new-webhook page. The edit page still answers a refusal in plain text.

Model: opus-5-5

README.md

@@ -1323,13 +1323,14 @@ markup. The CSP build runs no expressions, so every Alpine directive in
 `x-data="{ open: false }"` or `@click="open = !open"`.
 
 A browser test in `internal/server` loads the webhook page and the event log
-under the real policy and checks that: both add forms stay hidden until Add is
+under the real policy and checks that: the add entrypoint form stays hidden
-clicked; choosing Slack in the add target form leaves the HTTP fields out of
+until Add is clicked; for every target type, the targets section's Add shows
-what it submits, also after leaving the page and going back to it, when the
+only a choice of type and Next, Next shows only that type's fields (no url field
-browser restores the choice; the Copy button beside an entrypoint URL reads
+for `database` or `log`), Cancel closes the form, and saving adds the target;
-"Copied" once clicked; an event expands and collapses, and so do a delivery's
+a refused target comes back with its form open, the values entered and the
-attempts inside it; and at phone width the menu button opens and closes the
+reason; the Copy button beside an entrypoint URL reads "Copied" once clicked; an
-mobile menu. It also fails if the browser reports a console warning or error,
+event expands and collapses, and so do a delivery's attempts inside it; and 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`

internal/handlers/export_test.go

@@ -137,22 +137,20 @@ func (s *Handlers) RenderTemplateForTest(
 // BuildSlackTargetConfigForTest exposes
 // buildSlackTargetConfig for use in the handlers_test package.
 func (s *Handlers) BuildSlackTargetConfigForTest(
-	w http.ResponseWriter,
+	ctx context.Context,
-	r *http.Request,
 	targetURL string,
-) (string, error) {
+) (string, string) {
-	return s.buildSlackTargetConfig(w, r, targetURL)
+	return s.buildSlackTargetConfig(ctx, targetURL)
 }
 
 // BuildHTTPTargetConfigForTest exposes buildHTTPTargetConfig
 // for use in the handlers_test package, taking the form fields
 // an HTTP target's configuration is built from.
 func (s *Handlers) BuildHTTPTargetConfigForTest(
-	w http.ResponseWriter,
+	ctx context.Context,
-	r *http.Request,
 	targetURL, headers, timeout string,
-) (string, error) {
+) (string, string) {
-	return s.buildHTTPTargetConfig(w, r, targetFormInput{
+	return s.buildHTTPTargetConfig(ctx, targetFormInput{
 		URL:     targetURL,
 		Headers: headers,
 		Timeout: timeout,
@@ -162,9 +160,6 @@ func (s *Handlers) BuildHTTPTargetConfigForTest(
 // BuildDatabaseTargetConfigForTest exposes
 // buildDatabaseTargetConfig for use in the handlers_test
 // package.
-func (s *Handlers) BuildDatabaseTargetConfigForTest(
+func BuildDatabaseTargetConfigForTest(expiry string) (string, string) {
-	w http.ResponseWriter,
+	return buildDatabaseTargetConfig(expiry)
-	expiry string,
-) (string, error) {
-	return s.buildDatabaseTargetConfig(w, newRequestForTest(), expiry)
 }

internal/handlers/handlers_test.go

@@ -314,16 +314,11 @@ func TestBuildSlackTargetConfig_AcceptsPublicURL(t *testing.T) {
 
 	t.Cleanup(app.RequireStop)
 
-	req := httptest.NewRequestWithContext(
+	cfg, errMsg := h.BuildSlackTargetConfigForTest(
-		context.Background(), http.MethodPost, "/", nil)
+		t.Context(), "http://93.184.216.34/services/T00/B00/xxx",
-	w := httptest.NewRecorder()
-
-	cfg, err := h.BuildSlackTargetConfigForTest(
-		w, req, "http://93.184.216.34/services/T00/B00/xxx",
 	)
 
-	require.NoError(t, err)
+	assert.Empty(t, errMsg)
-	assert.Equal(t, http.StatusOK, w.Code)
 	assert.Contains(t, cfg, "webhookUrl")
 }
 
@@ -337,17 +332,12 @@ func TestBuildSlackTargetConfig_RejectsReservedURL(t *testing.T) {
 
 	t.Cleanup(app.RequireStop)
 
-	req := httptest.NewRequestWithContext(
+	cfg, errMsg := h.BuildSlackTargetConfigForTest(
-		context.Background(), http.MethodPost, "/", nil)
+		t.Context(), "http://169.254.169.254/latest/meta-data/",
-	w := httptest.NewRecorder()
-
-	cfg, err := h.BuildSlackTargetConfigForTest(
-		w, req, "http://169.254.169.254/latest/meta-data/",
 	)
 
-	require.Error(t, err)
+	assert.Contains(t, errMsg, "Invalid target URL")
 	assert.Empty(t, cfg)
-	assert.Equal(t, http.StatusBadRequest, w.Code)
 }
 
 func TestRenderTemplate(t *testing.T) {
@@ -444,29 +434,19 @@ func TestRenderTemplateMidRenderErrorSendsNoPartialBody(t *testing.T) {
 func TestBuildDatabaseTargetConfig_Valid(t *testing.T) {
 	t.Parallel()
 
-	var h *handlers.Handlers
-
-	app := newTestApp(t, &h)
-	app.RequireStart()
-
-	t.Cleanup(app.RequireStop)
-
 	// Empty expiry: the keep-forever default, empty config.
-	w := httptest.NewRecorder()
+	cfg, errMsg := handlers.BuildDatabaseTargetConfigForTest("")
-	cfg, err := h.BuildDatabaseTargetConfigForTest(w, "")
+	assert.Empty(t, errMsg)
-	require.NoError(t, err)
 	assert.Empty(t, cfg)
 
 	// Explicit never is stored as config.
-	w = httptest.NewRecorder()
+	cfg, errMsg = handlers.BuildDatabaseTargetConfigForTest("never")
-	cfg, err = h.BuildDatabaseTargetConfigForTest(w, "never")
+	assert.Empty(t, errMsg)
-	require.NoError(t, err)
 	assert.JSONEq(t, `{"expiry":"never"}`, cfg)
 
 	// A positive duration is stored as config.
-	w = httptest.NewRecorder()
+	cfg, errMsg = handlers.BuildDatabaseTargetConfigForTest("720h")
-	cfg, err = h.BuildDatabaseTargetConfigForTest(w, "720h")
+	assert.Empty(t, errMsg)
-	require.NoError(t, err)
 	assert.JSONEq(t, `{"expiry":"720h"}`, cfg)
 }
 
@@ -475,22 +455,13 @@ func TestBuildDatabaseTargetConfig_RejectsBadExpiry(
 ) {
 	t.Parallel()
 
-	var h *handlers.Handlers
-
-	app := newTestApp(t, &h)
-	app.RequireStart()
-
-	t.Cleanup(app.RequireStop)
-
 	for _, bad := range []string{"nonsense", "7d", "-5h"} {
-		w := httptest.NewRecorder()
+		cfg, errMsg := handlers.BuildDatabaseTargetConfigForTest(bad)
-		cfg, err := h.BuildDatabaseTargetConfigForTest(w, bad)
 
-		require.Error(t, err, "expiry %q", bad)
+		assert.Contains(
-		assert.Empty(t, cfg)
+			t, errMsg, "Invalid archive expiry",
-		assert.Equal(
+			"expiry %q should be refused", bad,
-			t, http.StatusBadRequest, w.Code,
-			"expiry %q should be rejected with 400", bad,
 		)
+		assert.Empty(t, cfg)
 	}
 }

internal/handlers/source_management.go

@@ -1,6 +1,7 @@
 package handlers
 
 import (
+	"context"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -38,9 +39,6 @@ type WebhookListItem struct {
 	EventsUnreadable  bool
 }
 
-// errMissingURL signals that a required URL was not provided.
-var errMissingURL = errors.New("missing URL")
-
 // parseRetentionDays interprets a retention_days form value. It
 // returns the number of days, or, for a value it refuses, the message
 // the create and edit forms show; the message is empty when the value
@@ -460,15 +458,20 @@ func (h *Handlers) HandleSourceDetail() http.HandlerFunc {
 			return
 		}
 
-		h.renderSourceDetail(w, r, webhook)
+		h.renderSourceDetail(w, r, webhook, targetFormInput{}, "")
 	}
 }
 
-// renderSourceDetail loads and renders a source detail page.
+// renderSourceDetail loads and renders a source detail page. With a
+// targetErr, it is the page shown again for a refused add target
+// form: it answers 400, and the form opens on targetForm's type with
+// its values and the message.
 func (h *Handlers) renderSourceDetail(
 	w http.ResponseWriter,
 	r *http.Request,
 	webhook database.Webhook,
+	targetForm targetFormInput,
+	targetErr string,
 ) {
 	var entrypoints []database.Entrypoint
 
@@ -525,9 +528,16 @@ func (h *Handlers) renderSourceDetail(
 		"Events":      events,
 		"BaseURL":     baseURL,
 		"Stats":       h.loadWebhookStats(webhook.ID, entrypoints, targets),
+		"TargetForm":  targetForm,
+		"TargetError": targetErr,
 	}
 
-	h.renderTemplate(w, r, "source_detail.html", data)
+	status := http.StatusOK
+	if targetErr != "" {
+		status = http.StatusBadRequest
+	}
+
+	h.renderTemplateStatus(w, r, "source_detail.html", data, status)
 }
 
 // HandleSourceEdit shows the form to edit a webhook.
@@ -1437,67 +1447,24 @@ func (h *Handlers) HandleTargetCreate() http.HandlerFunc {
 	}
 }
 
-// processTargetCreate validates and creates a new target.
+// processTargetCreate validates and creates a new target. A refused
+// submission shows the webhook page again, with the add target form
+// open on the chosen type, the values entered, and the reason.
 func (h *Handlers) processTargetCreate(
 	w http.ResponseWriter,
 	r *http.Request,
 	webhook database.Webhook,
 ) {
-	// The body size cap is enforced by the MaxBodySize middleware,
+	in := targetFormInputFrom(r)
-	// which runs before CSRF parses the form.
-	//
-	// Every field here is read with PostFormValue, not FormValue.
-	// FormValue falls back to the query string, which would let
-	// `POST /hook/{id}/targets?url=https://hooks.slack.com/...`
-	// configure a target from a value the request line carries — and
-	// the request line, unlike the body, is what logs, proxies,
-	// Referer headers and error trackers record.
-	name := r.PostFormValue("name")
-	targetType := database.TargetType(r.PostFormValue("type"))
 
-	if name == "" {
+	target, errMsg := h.newTarget(r.Context(), webhook.ID, in)
-		http.Error(
+	if errMsg != "" {
-			w, "Name is required", http.StatusBadRequest,
+		h.renderSourceDetail(w, r, webhook, in, errMsg)
-		)
 
 		return
 	}
 
-	if !isValidTargetType(targetType) {
+	err := h.db.DB().Create(target).Error
-		http.Error(
-			w, "Invalid target type",
-			http.StatusBadRequest,
-		)
-
-		return
-	}
-
-	configJSON, err := h.buildTargetConfig(
-		w, r, targetType, targetFormInputFrom(r),
-	)
-	if err != nil {
-		return
-	}
-
-	// A new target has no stored retry count, so an absent field
-	// takes the fire-and-forget default. A field the operator filled
-	// in with something invalid is rejected rather than becoming
-	// that default.
-	maxRetries, ok := targetMaxRetries(w, r, 0)
-	if !ok {
-		return
-	}
-
-	target := &database.Target{
-		WebhookID:  webhook.ID,
-		Name:       name,
-		Type:       targetType,
-		Active:     true,
-		Config:     configJSON,
-		MaxRetries: maxRetries,
-	}
-
-	err = h.db.DB().Create(target).Error
 	if err != nil {
 		h.serverError(w, r, "failed to create target", err)
 
@@ -1510,6 +1477,47 @@ func (h *Handlers) processTargetCreate(
 	)
 }
 
+// newTarget validates a new target for a webhook and returns the row
+// to create, or, when it refuses the target, the message the form
+// shows. Every form that creates a target goes through here, so they
+// all accept and refuse the same things.
+func (h *Handlers) newTarget(
+	ctx context.Context,
+	webhookID string,
+	in targetFormInput,
+) (*database.Target, string) {
+	if in.Name == "" {
+		return nil, "Name is required"
+	}
+
+	if !isValidTargetType(in.Type) {
+		return nil, "Invalid target type"
+	}
+
+	configJSON, errMsg := h.buildTargetConfig(ctx, in.Type, in)
+	if errMsg != "" {
+		return nil, errMsg
+	}
+
+	// A new target has no stored retry count, so an absent field
+	// 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)
+	}
+
+	return &database.Target{
+		WebhookID:  webhookID,
+		Name:       in.Name,
+		Type:       in.Type,
+		Active:     true,
+		Config:     configJSON,
+		MaxRetries: maxRetries,
+	}, ""
+}
+
 // isValidTargetType checks whether the target type is supported.
 func isValidTargetType(tt database.TargetType) bool {
 	switch tt {
@@ -1541,11 +1549,16 @@ func pageOrFirst(s string) int {
 	return v
 }
 
-// targetFormInput carries the raw form values describing a target's
+// targetFormInput carries the raw values of a target form. Both the
-// configuration. Both the create and the edit path fill one and hand
+// create and the edit path fill one and hand it to buildTargetConfig,
-// it to buildTargetConfig, so neither can come to validate a
+// so neither can come to validate a destination differently from the
-// destination differently from the other.
+// other. A refused add target form is shown again from it.
 type targetFormInput struct {
+	// Name is the target's name.
+	Name string
+	// Type is the type chosen on the add target form. The edit form
+	// has none: a target's stored type decides.
+	Type database.TargetType
 	// URL is the destination for an HTTP target and the webhook URL
 	// for a Slack target.
 	URL string
@@ -1554,13 +1567,15 @@ type targetFormInput struct {
 	Headers string
 	// Timeout is an HTTP target's per-request timeout in seconds.
 	Timeout string
+	// MaxRetries is an HTTP or Slack target's max_retries.
+	MaxRetries string
 	// Expiry is a database (archive) target's row expiry.
 	Expiry string
 }
 
-// targetFormInputFrom reads the configuration fields from a request
+// targetFormInputFrom reads a target form from a request body. The
-// body. The body size cap is enforced by the MaxBodySize middleware,
+// body size cap is enforced by the MaxBodySize middleware, which runs
-// which runs before CSRF parses the form.
+// before CSRF parses the form.
 //
 // Every field is read with PostFormValue, not FormValue. FormValue
 // falls back to the query string, which would let
@@ -1572,38 +1587,36 @@ type targetFormInput struct {
 // tokens.
 func targetFormInputFrom(r *http.Request) targetFormInput {
 	return targetFormInput{
+		Name:       r.PostFormValue("name"),
+		Type:       database.TargetType(r.PostFormValue("type")),
 		URL:        r.PostFormValue("url"),
 		Headers:    r.PostFormValue("headers"),
 		Timeout:    r.PostFormValue("timeout"),
+		MaxRetries: r.PostFormValue("max_retries"),
 		Expiry:     r.PostFormValue("expiry"),
 	}
 }
 
 // buildTargetConfig builds the JSON config string for a target from
-// the submitted form values, writing its own 4xx response on
+// the submitted form values, or returns the message the form shows
-// rejection. Which fields of in apply depends on the target type.
+// for a value it refuses. Which fields of in apply depends on the
+// target type; a type without a URL ignores any URL submitted.
 func (h *Handlers) buildTargetConfig(
-	w http.ResponseWriter,
+	ctx context.Context,
-	r *http.Request,
 	targetType database.TargetType,
 	in targetFormInput,
-) (string, error) {
+) (string, string) {
 	switch targetType {
 	case database.TargetTypeHTTP:
-		return h.buildHTTPTargetConfig(w, r, in)
+		return h.buildHTTPTargetConfig(ctx, in)
 	case database.TargetTypeSlack:
-		return h.buildSlackTargetConfig(w, r, in.URL)
+		return h.buildSlackTargetConfig(ctx, in.URL)
 	case database.TargetTypeDatabase:
-		return h.buildDatabaseTargetConfig(w, r, in.Expiry)
+		return buildDatabaseTargetConfig(in.Expiry)
 	case database.TargetTypeLog:
-		return "", nil
+		return "", ""
 	default:
-		http.Error(
+		return "", "Invalid target type"
-			w, "Invalid target type",
-			http.StatusBadRequest,
-		)
-
-		return "", errMissingURL
 	}
 }
 
@@ -1611,40 +1624,27 @@ func (h *Handlers) buildTargetConfig(
 // SSRF-validated destination plus the optional headers and timeout
 // the delivery path honours.
 func (h *Handlers) buildHTTPTargetConfig(
-	w http.ResponseWriter,
+	ctx context.Context,
-	r *http.Request,
 	in targetFormInput,
-) (string, error) {
+) (string, string) {
-	err := h.validateTargetURL(
+	errMsg := h.validateTargetURL(
-		w, r, in.URL, "URL is required for HTTP targets",
+		ctx, in.URL, "URL is required for HTTP targets",
 	)
-	if err != nil {
+	if errMsg != "" {
-		return "", err
+		return "", errMsg
 	}
 
 	headers, err := delivery.ParseTargetHeaders(in.Headers)
 	if err != nil {
-		http.Error(
+		return "", "Invalid headers: " + err.Error()
-			w,
-			"Invalid headers: "+err.Error(),
-			http.StatusBadRequest,
-		)
-
-		return "", err
 	}
 
 	timeout, err := delivery.ParseTargetTimeout(in.Timeout)
 	if err != nil {
-		http.Error(
+		return "", "Invalid timeout: " + err.Error()
-			w,
-			"Invalid timeout: "+err.Error(),
-			http.StatusBadRequest,
-		)
-
-		return "", err
 	}
 
-	return h.marshalTargetConfig(w, r, delivery.HTTPTargetConfig{
+	return marshalTargetConfig(delivery.HTTPTargetConfig{
 		URL:     in.URL,
 		Headers: headers,
 		Timeout: timeout,
@@ -1654,49 +1654,39 @@ func (h *Handlers) buildHTTPTargetConfig(
 // buildSlackTargetConfig builds config JSON for a Slack target,
 // whose whole configuration is one SSRF-validated webhook URL.
 func (h *Handlers) buildSlackTargetConfig(
-	w http.ResponseWriter,
+	ctx context.Context,
-	r *http.Request,
 	targetURL string,
-) (string, error) {
+) (string, string) {
-	err := h.validateTargetURL(
+	errMsg := h.validateTargetURL(
-		w, r, targetURL,
+		ctx, targetURL,
 		"Webhook URL is required for Slack targets",
 	)
-	if err != nil {
+	if errMsg != "" {
-		return "", err
+		return "", errMsg
 	}
 
-	return h.marshalTargetConfig(w, r, delivery.SlackTargetConfig{
+	return marshalTargetConfig(delivery.SlackTargetConfig{
 		WebhookURL: targetURL,
 	})
 }
 
-// validateTargetURL rejects an empty or SSRF-blocked destination,
+// validateTargetURL refuses an empty or SSRF-blocked destination,
-// writing the 400 itself. missingMsg is the error shown when no URL
+// returning the message the form shows, or "" when the destination
-// is given.
+// is accepted. missingMsg is the message for no URL at all.
 //
 // It is the single point at which a user-supplied destination enters
 // the SSRF guard, on create and on edit alike. An edit path that
 // reached storage without passing through here would reopen the hole
 // the guard closes.
 func (h *Handlers) validateTargetURL(
-	w http.ResponseWriter,
+	ctx context.Context,
-	r *http.Request,
 	targetURL, missingMsg string,
-) error {
+) string {
 	if targetURL == "" {
-		http.Error(
+		return missingMsg
-			w,
-			missingMsg,
-			http.StatusBadRequest,
-		)
-
-		return errMissingURL
 	}
 
-	err := h.ssrf.ValidateTargetURL(
+	err := h.ssrf.ValidateTargetURL(ctx, targetURL)
-		r.Context(), targetURL,
-	)
 	if err != nil {
 		// The submitted URL can be a credential (a Slack
 		// incoming webhook URL is a bearer token), so the log
@@ -1722,61 +1712,41 @@ func (h *Handlers) validateTargetURL(
 				"egress to your own network\" in the README)."
 		}
 
-		http.Error(w, msg, http.StatusBadRequest)
+		return msg
-
-		return err
 	}
 
-	return nil
+	return ""
 }
 
 // marshalTargetConfig serialises a target configuration for storage,
-// writing a 500 itself if it cannot.
+// or returns the message the form shows if it cannot.
-func (h *Handlers) marshalTargetConfig(
+func marshalTargetConfig(cfg any) (string, string) {
-	w http.ResponseWriter,
-	r *http.Request,
-	cfg any,
-) (string, error) {
 	configBytes, err := json.Marshal(cfg)
 	if err != nil {
-		h.serverError(w, r, "failed to encode target config", err)
+		return "", "Could not encode the target configuration: " +
-
+			err.Error()
-		return "", err
 	}
 
-	return string(configBytes), nil
+	return string(configBytes), ""
 }
 
 // buildDatabaseTargetConfig builds config JSON for a database
-// (archive) target. The optional expiry (a form value read by
+// (archive) target. The optional expiry is validated here, at
-// the caller, which bounds the request body) is validated here,
+// creation time, so an unparseable value is refused instead of
-// at creation time, so an unparseable value is rejected with a
+// failing every subsequent delivery. An empty expiry yields an
-// 400 instead of failing every subsequent delivery. An empty
+// empty config (the keep-forever default).
-// expiry yields an empty config (the keep-forever default).
+func buildDatabaseTargetConfig(expiry string) (string, string) {
-func (h *Handlers) buildDatabaseTargetConfig(
-	w http.ResponseWriter,
-	r *http.Request,
-	expiry string,
-) (string, error) {
 	expiry = strings.TrimSpace(expiry)
 	if expiry == "" {
-		return "", nil
+		return "", ""
 	}
 
 	err := delivery.ValidateArchiveExpiry(expiry)
 	if err != nil {
-		http.Error(
+		return "", "Invalid archive expiry: " + err.Error()
-			w,
-			"Invalid archive expiry: "+err.Error(),
-			http.StatusBadRequest,
-		)
-
-		return "", err
 	}
 
-	return h.marshalTargetConfig(
+	return marshalTargetConfig(map[string]any{"expiry": expiry})
-		w, r, map[string]any{"expiry": expiry},
-	)
 }
 
 // HandleEntrypointDelete handles deleting an entrypoint.

internal/handlers/target_create_test.go

@@ -0,0 +1,144 @@
+package handlers_test
+
+import (
+	"html"
+	"net/http"
+	"net/url"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/webhooker/internal/database"
+)
+
+// TestHandleTargetCreate_EveryType adds a target of each type. Each
+// submission carries a url: only the http and slack types store one.
+func TestHandleTargetCreate_EveryType(t *testing.T) {
+	t.Parallel()
+
+	env := setupSourceTest(t)
+
+	// fields is the rest of each submission, as a query string.
+	cases := []struct {
+		targetType  database.TargetType
+		fields      string
+		wantConfig  string
+		wantRetries int
+	}{
+		{
+			database.TargetTypeHTTP, "timeout=12&max_retries=3",
+			`{"url":"` + editOriginalURL + `","timeout":12}`, 3,
+		},
+		{
+			database.TargetTypeSlack, "max_retries=4",
+			`{"webhookUrl":"` + editOriginalURL + `"}`, 4,
+		},
+		{
+			database.TargetTypeDatabase, "expiry=720h",
+			`{"expiry":"720h"}`, 0,
+		},
+		{database.TargetTypeLog, "", "", 0},
+	}
+
+	for _, tc := range cases {
+		t.Run(string(tc.targetType), func(t *testing.T) {
+			t.Parallel()
+
+			webhook := seedWebhookWithRetention(t, env.db, 30)
+
+			form, err := url.ParseQuery(tc.fields)
+			require.NoError(t, err)
+			form.Set("name", "every-type")
+			form.Set("type", string(tc.targetType))
+			form.Set("url", editOriginalURL)
+
+			w := serveTarget(
+				env, http.MethodPost,
+				"/hook/"+webhook.ID+"/targets", form,
+			)
+			require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String())
+
+			targets := targetsForWebhook(t, env.db, webhook.ID)
+			require.Len(t, targets, 1)
+			assert.Equal(t, tc.targetType, targets[0].Type)
+			assert.Equal(t, tc.wantRetries, targets[0].MaxRetries)
+
+			if tc.wantConfig == "" {
+				assert.Empty(t, targets[0].Config)
+			} else {
+				assert.JSONEq(t, tc.wantConfig, targets[0].Config)
+			}
+		})
+	}
+}
+
+// TestHandleTargetCreate_RefusedFormComesBack refuses a target of each
+// type and checks that the webhook page comes back with the add target
+// form open on that type, the values entered, and the reason.
+func TestHandleTargetCreate_RefusedFormComesBack(t *testing.T) {
+	t.Parallel()
+
+	env := setupSourceTest(t)
+
+	// fields is what the operator typed, as a query string.
+	cases := []struct {
+		targetType database.TargetType
+		fields     string
+		reason     string
+	}{
+		{
+			database.TargetTypeHTTP,
+			"name=private&url=" + editBlockedURL +
+				"&timeout=12&max_retries=3",
+			"Invalid target URL",
+		},
+		{
+			database.TargetTypeSlack, "name=no-url&max_retries=4",
+			"Webhook URL is required for Slack targets",
+		},
+		{
+			database.TargetTypeDatabase, "name=archive&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)
+
+			typed, err := url.ParseQuery(tc.fields)
+			require.NoError(t, err)
+
+			form := url.Values{}
+			form.Set("type", string(tc.targetType))
+
+			for field := range typed {
+				form.Set(field, typed.Get(field))
+			}
+
+			w := serveTarget(
+				env, http.MethodPost,
+				"/hook/"+webhook.ID+"/targets", form,
+			)
+			assert.Equal(t, http.StatusBadRequest, w.Code)
+
+			page := w.Body.String()
+			assert.Contains(
+				t, page, `data-type="`+string(tc.targetType)+`"`,
+			)
+			assert.Contains(t, page, html.EscapeString(tc.reason))
+
+			for field := range typed {
+				assert.Contains(
+					t, page, `name="`+field+`" value="`+
+						html.EscapeString(typed.Get(field))+`"`,
+				)
+			}
+
+			assert.Empty(t, targetsForWebhook(t, env.db, webhook.ID))
+		})
+	}
+}

internal/handlers/target_edit.go

@@ -127,11 +127,12 @@ func (h *Handlers) applyTargetEdit(
 		return
 	}
 
-	configJSON, err := h.buildTargetConfig(
+	configJSON, errMsg := h.buildTargetConfig(
-		w, r, target.Type, targetFormInputFrom(r),
+		r.Context(), target.Type, targetFormInputFrom(r),
 	)
-	if err != nil {
+	if errMsg != "" {
-		// buildTargetConfig has already written the response.
+		http.Error(w, errMsg, http.StatusBadRequest)
+
 		return
 	}
 
@@ -161,7 +162,7 @@ func (h *Handlers) applyTargetEdit(
 	// 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, oldName, name)
 	if err == nil {
 		err = h.db.DB().Save(target).Error
 	}

internal/handlers/target_private_refusal_test.go

@@ -1,6 +1,7 @@
 package handlers_test
 
 import (
+	"html"
 	"net/http"
 	"net/url"
 	"testing"
@@ -43,12 +44,16 @@ func TestTargetRefusal_PrivateDestinationSaysHowToAllowIt(
 			form.Set("type", string(targetType))
 			form.Set("url", editBlockedURL)
 
+			// A refused add shows the webhook page again, where
+			// the hint is HTML-escaped; a refused edit answers in
+			// plain text.
 			added := serveTarget(
 				env, http.MethodPost, targetsPath, form,
 			)
 			assert.Equal(t, http.StatusBadRequest, added.Code)
 			assert.Contains(
-				t, added.Body.String(), privateRefusalHint,
+				t, added.Body.String(),
+				html.EscapeString(privateRefusalHint),
 			)
 
 			form.Set("url", editOriginalURL)

internal/handlers/target_retries.go

@@ -90,13 +90,14 @@ func retriesErrorMessage(err error) string {
 		", or 0 for fire-and-forget"
 }
 
-// targetMaxRetries reads and validates max_retries from a target form
+// 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.
 //
-// Both the create and the edit path go through here, so the two
+// It and the create path (newTarget) both use parseMaxRetries and
-// cannot come to disagree about what a valid retry count is. The
+// retriesErrorMessage, so the two cannot come to disagree about what a
-// wording matches the timeout control on the same submission.
+// valid retry count is. The wording matches the timeout control on
+// the same submission.
 func targetMaxRetries(
 	w http.ResponseWriter,
 	r *http.Request,

internal/server/alpine_browser_test.go

@@ -83,8 +83,37 @@ func TestAlpineRunsUnderTheSecurityPolicy(t *testing.T) {
 
 	page := srv.URL + "/hook/" + webhook.ID
 
-	checkAddForms(ctx, t, page)
+	checkAddEntrypoint(ctx, t, page)
-	checkTargetType(ctx, t, page+"/events")
+
+	// Each target type, with the fields its add target form submits, in
+	// page order. Only http and slack have a url field.
+	targetTypes := []struct {
+		name   string
+		fields string
+		values map[string]string
+	}{
+		{
+			"http", "csrf_token name type url headers timeout max_retries",
+			map[string]string{"url": publicTargetURL},
+		},
+		{
+			"slack", "csrf_token name type url max_retries",
+			map[string]string{"url": publicTargetURL},
+		},
+		{
+			"database", "csrf_token name type expiry",
+			map[string]string{"expiry": "720h"},
+		},
+		{"log", "csrf_token name type", nil},
+	}
+
+	for _, tt := range targetTypes {
+		checkAddTarget(
+			ctx, t, page, tt.name, strings.Fields(tt.fields), tt.values,
+		)
+	}
+
+	checkRefusedTarget(ctx, t, page)
 	checkCopy(ctx, t, page)
 	checkEventLog(ctx, t, page+"/events", event.ID, target.Name)
 	checkMobileMenu(ctx, t, page)
@@ -227,115 +256,176 @@ func click(ctx context.Context, t *testing.T, xpath string) {
 	))
 }
 
-// checkAddForms loads a webhook page and checks that each section's add
+// checkAddEntrypoint loads a webhook page and checks that the add
-// form stays hidden until the Add button beside its heading is clicked.
+// entrypoint form stays hidden until the Add button beside its heading
-// The click looks for a button element there, so it also checks that
+// is clicked. The click looks for a button element there, so it also
-// Add is one.
+// checks that Add is one.
-func checkAddForms(ctx context.Context, t *testing.T, url string) {
+func checkAddEntrypoint(ctx context.Context, t *testing.T, url string) {
+	t.Helper()
+
+	form := `form[action$="/entrypoints"]`
+
+	require.NoError(t, chromedp.Run(ctx, loadPage(url)))
+
+	assert.True(t, hidden(ctx, form),
+		"the add entrypoint form shows before Add is clicked")
+
+	click(ctx, t, `//h2[text()="Entrypoints"]/following-sibling::button`)
+
+	assert.True(t, shown(ctx, form),
+		"the add entrypoint form stays hidden when Add is clicked")
+}
+
+// publicTargetURL is a destination the server accepts for an http or
+// slack target. It is a literal public address, so accepting it needs
+// no DNS.
+const publicTargetURL = "https://93.184.216.34/hook"
+
+// The parts of the targets section's add target form the checks below
+// find and click. Add is the button beside the Targets heading; each
+// Cancel is found from the button beside it, since both are on the
+// page at once.
+const (
+	addTarget     = `//h2[text()="Targets"]/following-sibling::button`
+	typeSelect    = `//select[@aria-label="Target type"]`
+	nextButton    = `//button[text()="Next"]`
+	cancelChoice  = nextButton + `/following-sibling::button[text()="Cancel"]`
+	saveButton    = `//form[contains(@action, "/targets")]//button[text()="Save"]`
+	cancelFields  = saveButton + `/following-sibling::button[text()="Cancel"]`
+	targetName    = `form[action$="/targets"] input[name="name"]`
+	submittedKeys = `[...new FormData(
+		document.querySelector('form[action$="/targets"]')).keys()]`
+)
+
+// checkAddTarget loads a webhook page and walks the add target form for
+// one target type. The form shows nothing until Add is clicked; Add
+// shows only the type choice; Cancel there closes it; Next shows the
+// type's own fields in place of the choice, and the form then submits
+// exactly fields, so a field another type uses, such as url, is absent;
+// Cancel closes it again. It then adds a target of the type, filling in
+// values, and checks that the section lists it with that type.
+func checkAddTarget(
+	ctx context.Context,
+	t *testing.T,
+	url, targetType string,
+	fields []string,
+	values map[string]string,
+) {
 	t.Helper()
 
 	require.NoError(t, chromedp.Run(ctx, loadPage(url)))
 
-	sections := []struct{ heading, form string }{
+	assert.Truef(t, hidden(ctx, typeSelect),
-		{"Entrypoints", `form[action$="/entrypoints"]`},
+		"%s: the type choice shows before Add is clicked", targetType)
-		{"Targets", `form[action$="/targets"]`},
+	assert.Truef(t, hidden(ctx, targetName),
+		"%s: the fields show before Add is clicked", targetType)
+
+	click(ctx, t, addTarget)
+	assert.Truef(t, shown(ctx, typeSelect),
+		"%s: Add does not show the type choice", targetType)
+	assert.Truef(t, hidden(ctx, targetName),
+		"%s: Add shows the fields before Next", targetType)
+
+	click(ctx, t, cancelChoice)
+	assert.Truef(t, hidden(ctx, typeSelect),
+		"%s: Cancel does not close the type choice", targetType)
+
+	chooseTargetType(ctx, t, targetType)
+
+	var submitted []string
+
+	require.NoError(t, chromedp.Run(
+		ctx, chromedp.Evaluate(submittedKeys, &submitted),
+	))
+	assert.Equalf(t, fields, submitted,
+		"%s: the form does not submit exactly the type's fields", targetType)
+
+	click(ctx, t, cancelFields)
+	assert.Truef(t, hidden(ctx, targetName),
+		"%s: Cancel does not close the fields", targetType)
+	assert.Truef(t, shown(ctx, addTarget),
+		"%s: Add does not come back after Cancel", targetType)
+
+	name := "added-" + targetType
+
+	chooseTargetType(ctx, t, targetType)
+	require.NoError(t, chromedp.Run(
+		ctx, chromedp.SetValue(targetName, name, chromedp.ByQuery),
+	))
+
+	for field, value := range values {
+		require.NoError(t, chromedp.Run(ctx, chromedp.SetValue(
+			`form[action$="/targets"] [name="`+field+`"]`, value,
+			chromedp.ByQuery,
+		)))
 	}
 
-	for _, s := range sections {
+	click(ctx, t, saveButton)
-		assert.Truef(
+	assert.Truef(t, shown(ctx, `//span[text()="`+name+
-			t, hidden(ctx, s.form),
+		`"]/following-sibling::div/span[text()="`+targetType+`"]`),
-			"%s: the add form shows before Add is clicked", s.heading,
+		"%s: the added target is not listed with its type", targetType)
-		)
-
-		click(ctx, t, `//h2[text()="`+s.heading+
-			`"]/following-sibling::button`)
-
-		assert.Truef(
-			t, shown(ctx, s.form),
-			"%s: the add form stays hidden when Add is clicked", s.heading,
-		)
-	}
 }
 
-// checkTargetType chooses Slack in the open add target form and checks
+// chooseTargetType clicks Add, picks targetType and clicks Next, and
-// what the form would then submit: one url field, the Slack one, and
+// checks that the type's fields then show in place of the type choice.
-// not the HTTP url, headers or timeout, which are hidden and disabled.
+func chooseTargetType(ctx context.Context, t *testing.T, targetType string) {
-//
+	t.Helper()
-// It then opens the page at elsewhere and goes back. The browser loads
+
-// the webhook page again and restores the form as it was left, Slack
+	click(ctx, t, addTarget)
-// chosen, without a change event; the form must again show and submit
+	require.NoError(t, chromedp.Run(
-// Slack's fields, not the HTTP ones.
+		ctx, chromedp.SetValue(typeSelect, targetType, chromedp.BySearch),
-func checkTargetType(ctx context.Context, t *testing.T, elsewhere string) {
+	))
+	click(ctx, t, nextButton)
+
+	assert.Truef(t, shown(ctx, targetName),
+		"%s: Next does not show the fields", targetType)
+	assert.Truef(t, hidden(ctx, typeSelect),
+		"%s: Next leaves the type choice showing", targetType)
+	assert.Truef(t, hidden(ctx, addTarget),
+		"%s: Add still shows while the form is open", targetType)
+}
+
+// checkRefusedTarget submits an http target the server refuses, a
+// loopback destination, and checks that the page comes back with the
+// form open on the http fields, the values entered and the reason.
+func checkRefusedTarget(ctx context.Context, t *testing.T, url string) {
 	t.Helper()
 
 	const (
-		chooseSlack = `(() => {
+		refusedURL = "http://127.0.0.1/hook"
-			const type = document.querySelector('select[name="type"]');
+		urlField   = `form[action$="/targets"] input[name="url"]`
-			type.value = "slack";
-			type.dispatchEvent(new Event("change"));
-		})()`
-		chosen    = `document.querySelector('select[name="type"]').value`
-		howLoaded = `performance.getEntriesByType("navigation")[0].type`
-		submitted = `[...new FormData(
-			document.querySelector('form[action$="/targets"]')).keys()]`
-		slackURL = `input[placeholder^="https://hooks.slack.com/"]`
-		httpURL  = `input[placeholder="https://example.com/webhook"]`
 	)
 
-	slackFields := strings.Fields("csrf_token name type max_retries url")
+	require.NoError(t, chromedp.Run(ctx, loadPage(url)))
 
-	var fields []string
+	chooseTargetType(ctx, t, "http")
+	require.NoError(t, chromedp.Run(
+		ctx,
+		chromedp.SetValue(targetName, "refused", chromedp.ByQuery),
+		chromedp.SetValue(urlField, refusedURL, chromedp.ByQuery),
+	))
+
+	click(ctx, t, saveButton)
+
+	assert.True(t, shown(ctx, `//div[@class="alert-error"]`),
+		"a refused target does not show the reason")
+
+	var name, typed string
 
 	require.NoError(t, chromedp.Run(
 		ctx,
-		chromedp.Evaluate(chooseSlack, nil),
+		chromedp.Value(targetName, &name, chromedp.ByQuery),
-		chromedp.Evaluate(submitted, &fields),
+		chromedp.Value(urlField, &typed, chromedp.ByQuery),
 	))
 
-	assert.Equal(
+	assert.Equal(t, "refused", name,
-		t, slackFields, fields,
+		"a refused target does not keep the name entered")
-		"with Slack chosen, the HTTP fields must not be submitted",
+	assert.Equal(t, refusedURL, typed,
-	)
+		"a refused target does not keep the url entered")
-
+	assert.True(t, shown(ctx, targetName),
-	var loaded, restored string
+		"a refused target does not come back with the form open")
-
+	assert.True(t, hidden(ctx, typeSelect),
-	// Going back waits for the load event, after which the browser has
+		"a refused target comes back on the type choice")
-	// restored the form.
-	require.NoError(t, chromedp.Run(
-		ctx,
-		loadPage(elsewhere),
-		chromedp.NavigateBack(),
-		chromedp.WaitNotPresent("[x-cloak]", chromedp.ByQuery),
-		chromedp.Evaluate(howLoaded, &loaded),
-		chromedp.Evaluate(chosen, &restored),
-	))
-
-	// A page the browser kept in memory and showed again as it was
-	// would prove nothing here.
-	require.Equal(
-		t, "back_forward", loaded,
-		"going back, the browser did not load the page again",
-	)
-	require.Equal(
-		t, "slack", restored,
-		"going back, the browser did not restore the chosen type",
-	)
-
-	click(ctx, t, `//h2[text()="Targets"]/following-sibling::button`)
-
-	assert.True(t, shown(ctx, slackURL),
-		"going back with Slack chosen, the Slack fields are not shown")
-	assert.True(t, hidden(ctx, httpURL),
-		"going back with Slack chosen, the HTTP fields are shown")
-
-	require.NoError(t, chromedp.Run(
-		ctx, chromedp.Evaluate(submitted, &fields),
-	))
-
-	assert.Equal(
-		t, slackFields, fields,
-		"going back with Slack chosen, the HTTP fields must not be submitted",
-	)
 }
 
 // checkCopy loads a webhook page and checks that the Copy control beside

static/js/app.js

@@ -87,24 +87,35 @@ document.addEventListener("alpine:init", function () {
         };
     });
 
-    // The add target form. Only the chosen type's fields show, and the
+    // The targets section's add target form, in three steps: closed,
-    // others are disabled so that the form does not submit them.
+    // choosing a type, then filling in that type's fields. targetType
+    // is empty until Next takes it from the type select.
     //
-    // The type is read from the type select when Alpine starts, when the
+    // A refused submission comes back with the type it was submitted
-    // select changes, and on pageshow. Going back to the page, the
+    // with in the section's data-type, and starts on that type's fields.
-    // browser restores the type chosen before without a change event,
-    // in some browsers only after Alpine has started, but always before
-    // pageshow.
     window.Alpine.data("targetForm", function () {
         return {
+            choosing: false,
             targetType: "",
             init() {
-                this.readType();
+                this.targetType = this.$root.dataset.type;
             },
-            readType() {
+            add() {
-                this.targetType = this.$root.querySelector(
+                this.choosing = true;
-                    'select[name="type"]'
+            },
-                ).value;
+            next() {
+                this.targetType = this.$refs.type.value;
+                this.choosing = false;
+            },
+            cancel() {
+                this.choosing = false;
+                this.targetType = "";
+            },
+            get filling() {
+                return this.targetType !== "";
+            },
+            get closed() {
+                return !this.choosing && !this.filling;
             },
             get isHttp() {
                 return this.targetType === "http";
@@ -115,14 +126,8 @@ document.addEventListener("alpine:init", function () {
             get isDatabase() {
                 return this.targetType === "database";
             },
-            get notHttp() {
+            get isLog() {
-                return !this.isHttp;
+                return this.targetType === "log";
-            },
-            get notSlack() {
-                return !this.isSlack;
-            },
-            get notDatabase() {
-                return !this.isDatabase;
             },
         };
     });

templates/source_detail.html

@@ -91,10 +91,10 @@
         </div>
 
         <!-- Targets -->
-        <div class="card" x-data="collapsible">
+        <div class="card" x-data="targetForm" data-type="{{.TargetForm.Type}}">
             <div class="p-4 border-b border-gray-200 flex justify-between items-center">
                 <h2 class="text-lg font-medium text-gray-900">Targets</h2>
-                <button type="button" @click="toggle" class="btn-small">
+                <button type="button" @click="add" x-show="closed" class="btn-small">
                     <svg class="w-3 h-3 mr-1" fill="none" stroke="currentColor" viewBox="0 0 24 24">
                         <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M12 4v16m8-8H4"/>
                     </svg>
@@ -102,48 +102,84 @@
                 </button>
             </div>
 
-            <!-- Add target form -->
+            <!-- Add target form. Add shows the type choice; Next replaces
-            <div x-show="open" x-cloak class="p-4 bg-gray-50 border-b border-gray-200">
+                 it with the chosen type's fields. Each type's fields,
-                <form method="POST" action="/hook/{{.Webhook.ID}}/targets" x-data="targetForm" @pageshow.window="readType" class="space-y-3">
+                 and the hidden type field submitted with them, exist
+                 only while that type is chosen. A refused submission
+                 comes back open on its type, with the values entered. -->
+            <form method="POST" action="/hook/{{.Webhook.ID}}/targets">
                 <input type="hidden" name="csrf_token" value="{{.CSRFToken}}">
-                    <div class="flex gap-2">
+                <div x-show="choosing" x-cloak class="p-4 bg-gray-50 border-b border-gray-200 flex flex-wrap gap-2">
-                        <input type="text" name="name" placeholder="Target name" required class="input text-sm flex-1">
+                    <select x-ref="type" aria-label="Target type" class="input text-sm w-40">
-                        <select name="type" @change="readType" class="input text-sm w-32">
                         <option value="http">HTTP</option>
                         <option value="slack">Slack</option>
                         <option value="database">Database</option>
                         <option value="log">Log</option>
                     </select>
+                    <button type="button" @click="next" class="btn-primary text-sm">Next</button>
+                    <button type="button" @click="cancel" class="btn-secondary text-sm">Cancel</button>
                 </div>
-                    <div x-show="isHttp">
+                <div x-show="filling" x-cloak class="p-4 bg-gray-50 border-b border-gray-200 space-y-3">
-                        <input type="url" name="url" placeholder="https://example.com/webhook" :disabled="notHttp" class="input text-sm">
+                    {{if .TargetError}}
-                    </div>
+                    <div class="alert-error">{{.TargetError}}</div>
-                    <div x-show="isHttp">
+                    {{end}}
-                        <textarea name="headers" rows="3" placeholder="Authorization: Bearer ..." :disabled="notHttp" class="input text-sm"></textarea>
+                    <input type="text" name="name" value="{{.TargetForm.Name}}" placeholder="Target name" required class="input text-sm">
+                    <template x-if="isHttp">
+                        <div class="space-y-3">
+                            <input type="hidden" name="type" value="http">
+                            <input type="url" name="url" value="{{.TargetForm.URL}}" placeholder="https://example.com/webhook" class="input text-sm">
+                            <div>
+                                <textarea name="headers" rows="3" placeholder="Authorization: Bearer ..." class="input text-sm">{{.TargetForm.Headers}}</textarea>
                                 <p class="text-xs text-gray-500 mt-1">Optional request headers, one <code>Name: value</code> per line, sent with every delivery.</p>
                             </div>
-                    <div x-show="isHttp" class="flex gap-2 items-center">
+                            <div class="flex gap-2 items-center">
                                 <label class="text-sm text-gray-700">Timeout (seconds, blank = default):</label>
-                        <input type="number" name="timeout" min="0" max="300" :disabled="notHttp" class="input text-sm w-24">
+                                <input type="number" name="timeout" value="{{.TargetForm.Timeout}}" min="0" max="300" class="input text-sm w-24">
                             </div>
-                    <div x-show="isHttp">
+                            <div>
                                 <div class="flex gap-2 items-center">
                                     <label class="text-sm text-gray-700">Max retries:</label>
-                            <input type="number" name="max_retries" value="0" min="0" max="20" class="input text-sm w-24">
+                                    <input type="number" name="max_retries" value="{{.TargetForm.MaxRetries}}" placeholder="0" min="0" max="20" class="input text-sm w-24">
                                 </div>
                                 <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>
-                    <div x-show="isSlack">
+                        </div>
-                        <input type="url" name="url" placeholder="https://hooks.slack.com/services/..." :disabled="notSlack" class="input text-sm">
+                    </template>
+                    <template x-if="isSlack">
+                        <div class="space-y-3">
+                            <input type="hidden" name="type" value="slack">
+                            <div>
+                                <input type="url" name="url" value="{{.TargetForm.URL}}" placeholder="https://hooks.slack.com/services/..." class="input text-sm">
                                 <p class="text-xs text-gray-500 mt-1">Slack or Mattermost incoming webhook URL. Payloads are pretty-printed in code blocks.</p>
                             </div>
-                    <div x-show="isDatabase">
+                            <div>
-                        <input type="text" name="expiry" placeholder="never" :disabled="notDatabase" class="input text-sm">
+                                <div class="flex gap-2 items-center">
+                                    <label class="text-sm text-gray-700">Max retries:</label>
+                                    <input type="number" name="max_retries" value="{{.TargetForm.MaxRetries}}" placeholder="0" min="0" max="20" class="input text-sm w-24">
+                                </div>
+                                <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>
+                        </div>
+                    </template>
+                    <template x-if="isDatabase">
+                        <div>
+                            <input type="hidden" name="type" value="database">
+                            <input type="text" name="expiry" value="{{.TargetForm.Expiry}}" placeholder="never" class="input text-sm">
                             <p class="text-xs text-gray-500 mt-1">Archive expiry: "never" (default) keeps rows forever, or a duration like "720h" prunes older rows.</p>
                         </div>
-                    <button type="submit" class="btn-primary text-sm">Add Target</button>
+                    </template>
-                </form>
+                    <template x-if="isLog">
+                        <div>
+                            <input type="hidden" name="type" value="log">
+                            <p class="text-xs text-gray-500">A log target writes each event to the application log. It has no settings beyond its name.</p>
                         </div>
+                    </template>
+                    <div class="flex gap-2">
+                        <button type="submit" class="btn-primary text-sm">Save</button>
+                        <button type="button" @click="cancel" class="btn-secondary text-sm">Cancel</button>
+                    </div>
+                </div>
+            </form>
 
             <div class="divide-y divide-gray-100">
                 {{range .Targets}}