Drop 'unsafe-inline' from the Content-Security-Policy (closes #530)

The delete prompts on the webhook page were inline onsubmit handlers.
Each delete form now carries its question in a data-confirm attribute,
which one submit listener in static/js/app.js asks. The three style
attributes became Tailwind classes, with tailwind.css regenerated, and
the x-cloak rule moved to style.css. The policy now sends
script-src 'self' and style-src 'self'.

The browser test now answers a target's delete prompt no and then yes,
and loads the webhook list, settings and profile pages under the
policy. The README states the policy as sent.

Model: opus-5-5

README.md

@@ -1235,34 +1235,36 @@ 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, its edit pages and
+A browser test in `internal/server` loads the webhook page, its edit pages, the
-the event log under the real policy and checks that: the add entrypoint form
+event log, the webhook list, the settings page and the profile page under the
-stays hidden until Add is clicked; for every target type, the targets section's
+real policy and checks that: the add entrypoint form stays hidden until Add is
-Add shows only a choice of type with Next and Cancel, Next shows only that
+clicked; for every target type, the targets section's Add shows only a choice of
-type's fields (no url field for `database` or `log`), Cancel at either step
+type with Next and Cancel, Next shows only that type's fields (no url field for
-closes the form, and saving adds the target; a refused target comes back with
+`database` or `log`), Cancel at either step closes the form, and saving adds the
-its form open, the values entered and the reason, and after Cancel the next Add
+target; a refused target comes back with its form open, the values entered and
-starts with an empty form and no reason; a refused save on the target edit page
+the reason, and after Cancel the next Add starts with an empty form and no
-and on the webhook edit page comes back with the reason and every value entered;
+reason; a refused save on the target edit page and on the webhook edit page
-the Copy button beside an entrypoint URL reads "Copied" once clicked; an
+comes back with the reason and every value entered; the Copy button beside an
-entrypoint's Edit button shows its edit form in place of its description and
+entrypoint URL reads "Copied" once clicked; an entrypoint's Edit button shows
-hides until the form closes, Cancel hides the form and drops what was typed, as
+its edit form in place of its description and hides until the form closes,
-does leaving the page and going back to it, and Save changes the description; of
+Cancel hides the form and drops what was typed, as does leaving the page and
-the recent events on the webhook page only the newest starts expanded, each
+going back to it, and Save changes the description; of the recent events on the
-expands and collapses, and Open leads to the event's own page; of the events in
+webhook page only the newest starts expanded, each expands and collapses, and
-the event log only the newest starts expanded, and an event there expands and
+Open leads to the event's own page; of the events in the event log only the
-collapses when its row's caret or its ID is clicked, and from the keyboard, but
+newest starts expanded, and an event there expands and collapses when its row's
-not when its ID is selected with the mouse, and a delivery's attempts inside it
+caret or its ID is clicked, and from the keyboard, but not when its ID is
-expand and collapse; and at phone width the menu button opens and closes the
+selected with the mouse, and a delivery's attempts inside it expand and
-mobile menu, and neither the webhook page nor the event log, with a delivery's
+collapse; at phone width the menu button opens and closes the mobile menu, and
-attempts open, scrolls sideways or cuts anything off at the page's or a card's
+neither the webhook page nor the event log, with a delivery's attempts open,
-edge. It also fails if the browser reports a console warning or error, an
+scrolls sideways or cuts anything off at the page's or a card's edge; and a
-uncaught exception, or anything the policy refused. `make check` and the image
+target's Delete asks first, naming the target, sends nothing when the answer is
-build lint it but do not run it, and `make test` leaves it out (its file is
+no and deletes the target when it is yes. It also fails if the browser reports a
-built only with the `browser` build tag). Run it with `make test-browser` after
+console warning or error, an uncaught exception, or anything the policy refused.
-changing `templates/` or `static/js/`: that builds `Dockerfile.browser`, which
+`make check` and the image build lint it but do not run it, and `make test`
-runs the test in a digest-pinned headless browser image, so the host needs no
+leaves it out (its file is built only with the `browser` build tag). Run it with
-browser.
+`make test-browser` after changing `templates/` or `static/js/`: that builds
+`Dockerfile.browser`, which runs the test in a digest-pinned headless browser
+image, so the host needs no browser.
 
 The package's tarball is committed as `3p/alpinejs-csp-3.14.9.tgz`, byte for
 byte as the npm registry publishes it. It is a dependency, not this repo's build
@@ -1295,7 +1297,9 @@ and commit the regenerated file with the change. Tailwind takes classes only
 from the files that `input.css` names in its `@source` lines; a class written in
 any other file is not generated until that file is named there too. `make check`
 and the image build fail when the committed file differs from what `make css`
-generates. `static/css/style.css` is hand-written and is not generated.
+generates. `static/css/style.css` is hand-written and is not generated. The
+pages' Content-Security-Policy refuses inline styles, so a template styles an
+element with a class, never a `style` attribute.
 
 `make css` runs the Tailwind standalone CLI in Docker, at the version and sha256
 pinned in the Dockerfile's stylesheet stages; it is never installed on the host.
@@ -1317,22 +1321,20 @@ webhooker solves this by acting as a durable intermediary:
    nothing is lost if downstream targets are unavailable.
 
 2. **Guaranteed delivery** — Events are queued for delivery to each configured
-   target. Failed deliveries are retried with configurable backoff. Delivery
+   target. Failed deliveries are retried with configurable backoff. Every
-   attempts are logged with status codes, response bodies, and timing.
+   delivery attempt is logged with status codes, response bodies, and timing.
 
     **That guarantee is at-least-once, not exactly-once.** When a send reaches
     its target but the write recording that outcome fails, the delivery is
     deliberately left in a recoverable state rather than marked done — losing a
     delivery is the worse failure — so the pending sweep picks it up about
     fifteen minutes later, or the next restart does, and the target receives a
-    payload it already got. A send still in flight when the service stops is
+    payload it already got. webhooker adds no delivery identifier of its own to
-    left the same way: its attempt is not recorded, and the next start sends it
+    an outbound request, so **make your receiver idempotent** against whatever
-    again (see Shutdown). webhooker adds no delivery identifier of its own to an
+    the payload itself carries.
-    outbound request, so **make your receiver idempotent** against whatever the
-    payload itself carries.
 
 3. **Observability** — Full request/response logging for every webhook received
-   and for delivery attempts. Prometheus metrics expose volume, latency, and
+   and every delivery attempted. Prometheus metrics expose volume, latency, and
    error rates. The web UI provides real-time visibility into event flow.
 
 4. **Fan-out** — A single incoming webhook can be delivered to multiple targets
@@ -1770,7 +1772,7 @@ is what bounds it.
 #### DeliveryResult
 
 The result of a single delivery attempt. Every attempt (including retries) is
-individually logged, except one cut off by a stop (see Shutdown).
+individually logged for full observability.
 
 | Field           | Type    | Description                               |
 | --------------- | ------- | ----------------------------------------- |
@@ -2208,16 +2210,12 @@ This means:
   buffer rather than spawning more goroutines.
 - **Independent results** — each worker records its own delivery result in the
   per-webhook database without coordination.
-- **Graceful shutdown** — the stop hook cancels the workers' context. A worker
+- **Graceful shutdown** — cancel the context, workers finish their current task
-  may still take tasks already queued before it exits. An `http` or `slack`
+  and exit. The stop hook waits for the pool via `lifecycle.WaitForShutdown`,
-  request in flight is cut off, and one started after the cancel fails at once.
+  which bounds that wait by fx's stop timeout rather than blocking forever on a
-  Neither is recorded or counted in the metrics, and the next start sends the
+  wedged worker. On timeout it logs at `ERROR` and returns an error, and the
-  delivery again (see Shutdown). A `database` or `log` task runs as normal. The
+  goroutines that did not finish are still running — an unclean shutdown is
-  stop hook waits for the pool via `lifecycle.WaitForShutdown`, which bounds
+  reported rather than hidden.
-  that wait by fx's stop timeout rather than blocking forever on a wedged
-  worker. On timeout it logs at `ERROR` and returns an error, and the goroutines
-  that did not finish are still running — an unclean shutdown is reported rather
-  than hidden.
 
 **Recovery paths:**
 
@@ -2323,7 +2321,7 @@ getting anywhere.
 | Metric                                 | Type      | Meaning                                                                                                                                                                                                                                                                                                   |
 | -------------------------------------- | --------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
 | `webhooker_events_received_total`      | counter   | Events received and durably stored. Compare against the delivery counters on one dashboard                                                                                                                                                                                                                |
-| `webhooker_delivery_attempts_total`    | counter   | Delivery attempts actually dispatched to a target. A delivery a circuit breaker refused is not one: it is counted as a retry instead, but only when the refusal moves it into `retrying`. An attempt cut off by a stop is not counted (see Shutdown)                                                      |
+| `webhooker_delivery_attempts_total`    | counter   | Delivery attempts actually dispatched to a target. A delivery a circuit breaker refused is not one: it is counted as a retry instead, but only when the refusal moves it into `retrying`                                                                                                                  |
 | `webhooker_deliveries_succeeded_total` | counter   | Deliveries that reached `delivered`                                                                                                                                                                                                                                                                       |
 | `webhooker_deliveries_failed_total`    | counter   | Deliveries that failed terminally and will not be retried                                                                                                                                                                                                                                                 |
 | `webhooker_delivery_retries_total`     | counter   | Deliveries put back into `retrying`                                                                                                                                                                                                                                                                       |
@@ -3082,8 +3080,8 @@ webhooker/
 │   ├── static.go              # //go:embed directive
 │   ├── css/input.css          # Tailwind input, source for tailwind.css (make css)
 │   ├── css/tailwind.css       # Generated stylesheet the pages load
-│   ├── css/style.css          # Hand-written, loaded after tailwind.css: btn-small, the pointer cursor for input.css's buttons, the webhook list cards' focus outline
+│   ├── css/style.css          # Hand-written, loaded after tailwind.css: btn-small, the pointer cursor for input.css's buttons, the webhook list cards' focus outline, the rule hiding x-cloak elements
-│   ├── js/app.js              # Copy-to-clipboard, and the Alpine.js components
+│   ├── js/app.js              # Copy-to-clipboard, the delete prompts, and the Alpine.js components
 │   └── js/alpine.min.js       # Alpine.js CSP build, extracted from 3p/ by make assets, not committed
 ├── templates/                 # Go HTML templates (base, login, sources, etc.)
 ├── script/                    # Scripts to Rule Them All entrypoints
@@ -3137,8 +3135,9 @@ Applied to all routes in this order:
 
 1. **RequestID** — Generate unique request IDs (chi built-in)
 2. **SecurityHeaders** — Production security headers on every response (HSTS
-   unless `HSTS_MAX_AGE` is `0`, X-Content-Type-Options, X-Frame-Options, CSP,
+   unless `HSTS_MAX_AGE` is `0`, X-Content-Type-Options, X-Frame-Options, CSP
-   Referrer-Policy, Permissions-Policy)
+   `default-src 'self'; script-src 'self'; style-src 'self'`, Referrer-Policy,
+   Permissions-Policy)
 3. **Logging** — Structured request logging (method, URL, status, latency,
    remote IP, client IP, user agent, request ID)
 4. **Metrics** — Prometheus HTTP metrics (if `METRICS_USERNAME` and
@@ -3223,6 +3222,12 @@ credential check, see [The login endpoint](#the-login-endpoint).
 - Production security headers on all responses: HSTS (unless `HSTS_MAX_AGE` is
   `0`), X-Content-Type-Options (`nosniff`), X-Frame-Options (`DENY`),
   Content-Security-Policy, Referrer-Policy, and Permissions-Policy
+- The Content-Security-Policy is
+  `default-src 'self'; script-src 'self'; style-src 'self'`: a page takes
+  scripts and styles only from files webhooker serves, and the browser runs no
+  inline script or event handler and ignores any `<style>` element or `style`
+  attribute in a page. The pages need none: their scripts are in `static/js/`
+  and their styles in `static/css/`
 - Request body size limits (1 MB) on all form POST endpoints, enforced by
   middleware that runs before CSRF parses the form
 - **CSRF protection** via [gorilla/csrf](https://github.com/gorilla/csrf) on all
@@ -3290,8 +3295,7 @@ stop-hook log:
 3. `server` — the HTTP drain, bounded by `server.ShutdownTimeout` (**3
    seconds**) and by what the hooks before it left, then a Sentry flush if
    `SENTRY_DSN` is set
-4. `delivery.Engine` — cuts off any `http` or `slack` request in flight, waits
+4. `delivery.Engine` — waits for its workers, then closes the archive databases
-   for its workers, then closes the archive databases
 5. `healthcheck`
 6. `WebhookDBManager`
 7. the database close
@@ -3302,12 +3306,6 @@ by the stop context, so a wedge there consumes the 5 seconds before the HTTP
 server hook is ever entered. The hooks after the server are microsecond-scale in
 normal operation.
 
-The delivery engine cancels a delivery request in flight. That attempt is not
-recorded or counted in the metrics, and the delivery keeps its `pending` or
-`retrying` status, so startup recovery sends it again on the next start (see
-Recovery paths under Bounded Worker Pool). If the receiver already had it, it
-receives it twice: the at-least-once duplicate described under Rationale.
-
 The HTTP drain budget is deliberately **shorter** than the sequence budget. Were
 the two equal, a drain that used its whole budget would exhaust the sequence
 budget at the instant it finished, and every later hook — the delivery engine,

internal/delivery/engine_lifecycle_test.go

@@ -2,16 +2,11 @@ package delivery_test
 
 import (
 	"context"
-	"io"
-	"net/http"
-	"net/http/httptest"
 	"path/filepath"
-	"sync"
 	"testing"
 	"time"
 
 	"github.com/google/uuid"
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.uber.org/fx"
 	"gorm.io/gorm/clause"
@@ -277,154 +272,6 @@ func TestEngine_StopHookHonoursStopTimeout(t *testing.T) {
 	requireStopHookExpires(t, lc.hooks[0], "delivery engine")
 }
 
-// seedHeldDelivery gives the setup's webhook an http target with
-// maxRetries, pointed at a receiver that keeps every request open until
-// the client gives up on it, and seeds one delivery to that target at
-// status. It returns the task that sends attempt attemptNum of the
-// delivery, and a channel that is closed when a request reaches the
-// receiver.
-func seedHeldDelivery(
-	t *testing.T,
-	s iSetup,
-	maxRetries int,
-	status database.DeliveryStatus,
-	attemptNum int,
-) (delivery.Task, <-chan struct{}) {
-	t.Helper()
-
-	arrived := make(chan struct{})
-
-	var once sync.Once
-
-	ts := httptest.NewServer(http.HandlerFunc(
-		func(_ http.ResponseWriter, r *http.Request) {
-			// The server notices the client hanging up only once the
-			// body has been read; until then r.Context() never ends.
-			_, _ = io.Copy(io.Discard, r.Body)
-
-			once.Do(func() { close(arrived) })
-
-			<-r.Context().Done()
-		},
-	))
-	t.Cleanup(ts.Close)
-
-	cfg := iHTTPConfig(ts.URL)
-	targetID := uuid.New().String()
-
-	iCreateWebhook(t, s.MainDB, s.WebhookID, "hook")
-	iCreateTarget(
-		t, s.MainDB, targetID, s.WebhookID, "held",
-		database.TargetTypeHTTP, cfg, maxRetries,
-	)
-
-	event := iSeedEvent(t, s.WebhookDB, s.WebhookID, `{"held":true}`)
-	d := iSeedDelivery(t, s.WebhookDB, event.ID, targetID, status)
-
-	bodyStr := event.Body
-
-	return iTask(
-		d, event, s.WebhookID, targetID, "held", cfg,
-		maxRetries, attemptNum, &bodyStr,
-	), arrived
-}
-
-// stopDuringAttempt waits until the receiver holds an attempt open, then
-// stops the engine through its real stop hook.
-func stopDuringAttempt(
-	t *testing.T, lc *recordingLifecycle, arrived <-chan struct{},
-) {
-	t.Helper()
-
-	select {
-	case <-arrived:
-	case <-time.After(5 * time.Second):
-		t.Fatal("the delivery attempt never reached the receiver")
-	}
-
-	require.NoError(t, lc.hooks[0].OnStop(context.Background()))
-}
-
-// requireRequeued runs restart recovery on the stopped engine and
-// requires it to queue task's attempt again on ch.
-func requireRequeued(
-	t *testing.T, s iSetup, ch <-chan delivery.Task, task delivery.Task,
-) {
-	t.Helper()
-
-	s.Engine.ExportRecoverWebhookDeliveries(
-		context.Background(), s.WebhookID,
-	)
-
-	select {
-	case got := <-ch:
-		assert.Equal(t, task.DeliveryID, got.DeliveryID)
-		assert.Equal(t, task.AttemptNum, got.AttemptNum)
-	case <-time.After(5 * time.Second):
-		t.Fatal("restart recovery did not queue the delivery again")
-	}
-}
-
-// TestEngine_StopLeavesFireAndForgetAttemptPending is the regression
-// test for a stop that turned an attempt in flight into a terminal
-// failure. Stopping the engine cancels the request; for a target with
-// no retries that was the delivery's only attempt, recorded as failed
-// with "context canceled", and nothing ever sent it again. The
-// delivery must instead stay pending, with no result and no count in
-// the attempt metrics, for restart recovery to send.
-func TestEngine_StopLeavesFireAndForgetAttemptPending(t *testing.T) {
-	t.Parallel()
-
-	s := newISetup(t)
-	reg := mIsolate(t, s)
-	lc := startEngineViaHook(t, s.Engine)
-
-	task, arrived := seedHeldDelivery(
-		t, s, 0, database.DeliveryStatusPending, 1,
-	)
-
-	s.Engine.Notify([]delivery.Task{task})
-	stopDuringAttempt(t, lc, arrived)
-
-	iAssertStatus(
-		t, s.WebhookDB, task.DeliveryID, database.DeliveryStatusPending,
-	)
-	assert.Empty(t, iResults(t, s.WebhookDB, task.DeliveryID))
-	assert.Zero(t, mCounter(t, reg, mAttempts, mTypeHTTP))
-	assert.Zero(t, mHTTPDurations(t, reg))
-
-	requireRequeued(t, s, s.Engine.ExportDeliveryCh(), task)
-}
-
-// TestEngine_StopLeavesLastRetryAttemptRetrying is the same regression
-// for a target with retries, on its last attempt: the cancelled attempt
-// used to exhaust the retries and end the delivery failed. It must stay
-// retrying with only its earlier attempt recorded, and restart recovery
-// must send that last attempt again.
-func TestEngine_StopLeavesLastRetryAttemptRetrying(t *testing.T) {
-	t.Parallel()
-
-	const maxRetries = 2
-
-	s := newISetup(t)
-	lc := startEngineViaHook(t, s.Engine)
-
-	task, arrived := seedHeldDelivery(
-		t, s, maxRetries, database.DeliveryStatusRetrying, maxRetries,
-	)
-	iSeedFailedResult(t, s.WebhookDB, task.DeliveryID)
-
-	s.Engine.ExportScheduleRetry(task, 0)
-	stopDuringAttempt(t, lc, arrived)
-
-	iAssertStatus(
-		t, s.WebhookDB, task.DeliveryID, database.DeliveryStatusRetrying,
-	)
-	assert.Len(t, iResults(t, s.WebhookDB, task.DeliveryID), 1)
-
-	requireRequeued(t, s, s.Engine.ExportRetryCh(), task)
-}
-
 // deliverToArchive gives the setup's webhook a database target,
 // runs one delivery to it through the running engine, and returns
 // the target's ID and archive file path. The archive writer holds

internal/delivery/target.go

@@ -32,8 +32,7 @@ type Scheduler interface {
 // Engine.observeAttempt, alongside the DeliveryResult it records for
 // it. Deliver is also entered for attempts that never happen — an
 // open circuit breaker refuses one — so the count cannot be taken
-// from around this call. An attempt cut off by a stop is neither
+// from around this call.
-// counted nor recorded.
 type Target interface {
 	Deliver(
 		ctx context.Context,

internal/delivery/target_http.go

@@ -57,11 +57,7 @@ type httpCore struct {
 // recorded and no circuit breaker is consulted. A positive
 // maxRetries gates the attempt on the circuit breaker and
 // schedules a backed-off retry on failure.
-//
-// ctx is the context attempt sends its request with. Stopping the
-// engine cancels it; see interruptedByStop.
 func (c *httpCore) deliver(
-	ctx context.Context,
 	webhookDB *gorm.DB,
 	d *database.Delivery,
 	task *Task,
@@ -70,55 +66,21 @@ func (c *httpCore) deliver(
 	attempt func() attemptResult,
 ) {
 	if maxRetries == 0 {
-		c.fireAndForget(ctx, webhookDB, d, attempt())
+		c.fireAndForget(webhookDB, d, attempt())
 
 		return
 	}
 
 	c.withRetry(
-		ctx, webhookDB, d, task, sched, maxRetries, attempt,
+		webhookDB, d, task, sched, maxRetries, attempt,
 	)
 }
 
-// interruptedByStop reports whether an attempt failed because the
-// engine is stopping, which cancels the context its request is made
-// with. Such an attempt is not recorded: no DeliveryResult, no circuit
-// breaker outcome, no status change, no count in the attempt metrics.
-// The delivery stays pending or retrying, and startup recovery sends
-// it again on the next start.
-//
-// A target's own timeout leaves that context alone, so its attempt is
-// still recorded as the failure it is.
-func (c *httpCore) interruptedByStop(
-	ctx context.Context,
-	d *database.Delivery,
-	res attemptResult,
-) bool {
-	if res.success || ctx.Err() == nil {
-		return false
-	}
-
-	c.eng.log.Warn(
-		"delivery attempt interrupted by shutdown; not recorded, "+
-			"it is sent again on the next start",
-		"delivery_id", d.ID,
-		"event_id", d.EventID,
-		"target_id", d.TargetID,
-	)
-
-	return true
-}
-
 func (c *httpCore) fireAndForget(
-	ctx context.Context,
 	webhookDB *gorm.DB,
 	d *database.Delivery,
 	res attemptResult,
 ) {
-	if c.interruptedByStop(ctx, d, res) {
-		return
-	}
-
 	c.eng.observeAttempt(d.Target.Type, res.elapsed())
 
 	err := c.eng.recordResult(
@@ -148,7 +110,6 @@ func (c *httpCore) fireAndForget(
 }
 
 func (c *httpCore) withRetry(
-	ctx context.Context,
 	webhookDB *gorm.DB,
 	d *database.Delivery,
 	task *Task,
@@ -170,10 +131,6 @@ func (c *httpCore) withRetry(
 
 	res := attempt()
 
-	if c.interruptedByStop(ctx, d, res) {
-		return
-	}
-
 	c.eng.observeAttempt(d.Target.Type, res.elapsed())
 
 	err := c.eng.recordResult(
@@ -438,7 +395,7 @@ func (t *httpTarget) Deliver(
 	}
 
 	t.deliver(
-		ctx, webhookDB, d, task, sched,
+		webhookDB, d, task, sched,
 		d.Target.MaxRetries, attempt,
 	)
 }

internal/delivery/target_slack.go

@@ -82,7 +82,7 @@ func (t *slackTarget) Deliver(
 	}
 
 	t.deliver(
-		ctx, webhookDB, d, task, sched,
+		webhookDB, d, task, sched,
 		d.Target.MaxRetries, attempt,
 	)
 }

internal/handlers/delivery_result_view_test.go

@@ -158,8 +158,7 @@ func TestHandleSourceLogs_RendersFailedAttempt(t *testing.T) {
 // TestHandleSourceLogs_EscapesResponseBody proves the
 // response body is treated as the untrusted remote content it
 // is. The remote chooses these bytes and the page is rendered
-// inside the operator's authenticated origin, where the
+// inside the operator's authenticated origin.
-// application's own CSP allows inline script from 'self'.
 func TestHandleSourceLogs_EscapesResponseBody(t *testing.T) {
 	t.Parallel()
 

internal/handlers/event_body.go

@@ -35,11 +35,10 @@ const eventBodyQuery = "SELECT cast(body as blob) " +
 // response is deliberately not renderable. Content-Disposition
 // makes the browser download rather than display it, and the
 // octet-stream type plus nosniff stop it being interpreted as
-// HTML or script. Without those a stored payload would execute
+// HTML or script. Without those a stored payload would be shown
-// as the logged-in operator. The application's CSP does not
+// as a page of the operator's own origin. The application's CSP
-// help here: script-src allows 'unsafe-inline' from 'self', so
+// is a second defence behind them: it allows no inline script,
-// a document served from this origin could run its own inline
+// so such a page could not run script of its own.
-// script.
 func (h *Handlers) HandleEventBodyDownload() http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		webhook, ok := h.ownedWebhook(w, r)

internal/handlers/event_detail_test.go

@@ -84,7 +84,7 @@ func TestHandleEventDetail_ShowsEventWholeWithDeliveries(t *testing.T) {
 	assert.Contains(t, page, strconv.Itoa(len(body))+" bytes")
 	assert.Contains(t, page, "{\n  "pad": "xxx")
 	assert.Contains(t, page, ""tail": ""+sentinel+""\n}")
-	assert.Contains(t, page, `style="max-height: 32rem; overflow-y: auto"`)
+	assert.Contains(t, page, `break-all max-h-[32rem] overflow-y-auto"`)
 	assert.NotContains(t, page, "Showing the first")
 	assert.Contains(t, page, target.Name)
 	assert.Contains(t, page, ">failed</span>")

internal/handlers/source_detail_test.go

@@ -273,7 +273,7 @@ func TestHandleSourceDetail_FitsWideAndNarrowWindows(t *testing.T) {
 
 	assert.Contains(
 		t, body,
-		`<div class="mx-auto px-6 py-8" style="max-width: 108rem"`,
+		`<div class="max-w-[108rem] mx-auto px-6 py-8">`,
 	)
 	assert.Contains(
 		t, body,
@@ -286,8 +286,8 @@ func TestHandleSourceDetail_FitsWideAndNarrowWindows(t *testing.T) {
 // target and says what deleting it loses, that the webhook's gives its
 // number of stored events (5 received, 2 removed by retention, so 3,
 // the statistics pane's "Within retention" figure), and that an
-// entrypoint with no description is named by its URL. The template
+// entrypoint with no description is named by its URL. Each prompt is
-// writes the slashes after http: as \/, which the browser reads as /.
+// its form's data-confirm attribute, where &#10; is a line break.
 func TestHandleSourceDetail_DeletePromptsNameWhatIsLost(t *testing.T) {
 	t.Parallel()
 
@@ -325,29 +325,28 @@ func TestHandleSourceDetail_DeletePromptsNameWhatIsLost(t *testing.T) {
 	body := renderSourceDetailPage(t, h, sess, wh.ID)
 
 	assert.Contains(t, body,
-		`Delete webhook &quot;delete-me&quot;?\n\n`+
+		`data-confirm="Delete webhook &quot;delete-me&quot;?&#10;&#10;`+
 			`This deletes its stored events (3) and their deliveries. `+
-			`Any archive files it wrote are kept.`)
+			`Any archive files it wrote are kept."`)
 	assert.Contains(t, body,
-		`Delete entrypoint &quot;Stripe&quot;?\n\n`+
+		`data-confirm="Delete entrypoint &quot;Stripe&quot;?&#10;&#10;`+
 			`Senders using its URL get an error from now on, `+
-			`and the URL cannot be restored.`)
+			`and the URL cannot be restored."`)
 	assert.Contains(t, body,
-		`Delete entrypoint &quot;http:\/\/example.com/h/`+
+		`data-confirm="Delete entrypoint &quot;http://example.com/h/`+
-			unnamed.Path+`&quot;?`)
+			unnamed.Path+`&quot;?&#10;&#10;`)
 	assert.Contains(t, body,
-		`Delete target &quot;t-log&quot;?\n\n`+
+		`data-confirm="Delete target &quot;t-log&quot;?&#10;&#10;`+
 			`Nothing more is delivered to it. `+
-			`Its past deliveries stay in the event log.`)
+			`Its past deliveries stay in the event log."`)
 }
 
 // TestHandleSourceDetail_DeletePromptKeepsQuotesInName checks that a
 // webhook name with quotes, a backslash, a closing script tag and a
-// newline reaches its delete prompt escaped for the script, which the
+// newline reaches its delete prompt escaped for the attribute, which
-// browser reads back as the name typed: each quote and angle bracket
+// the browser reads back as the name typed: each quote and angle
-// as a \u escape, the slash as \/, the newline as \n and the backslash
+// bracket as a character reference, the rest as typed. An unescaped
-// doubled. An unescaped newline would break the prompt's script, and
+// double quote would end the attribute and cut the prompt short.
-// the form would then submit without asking.
 func TestHandleSourceDetail_DeletePromptKeepsQuotesInName(t *testing.T) {
 	t.Parallel()
 
@@ -373,6 +372,6 @@ func TestHandleSourceDetail_DeletePromptKeepsQuotesInName(t *testing.T) {
 	body := renderSourceDetailPage(t, h, sess, wh.ID)
 
 	assert.Contains(t, body,
-		"Delete webhook &quot;Bob\\u0027s \\u0022best\\u0022 \\\\ hook"+
+		"Delete webhook &quot;Bob&#39;s &#34;best&#34; \\ hook"+
-			"\\u003c\\/script\\u003e\\nline two&quot;?")
+			"&lt;/script&gt;\nline two&quot;?")
 }

internal/middleware/middleware.go

@@ -538,7 +538,9 @@ func (s *Middleware) MetricsAuth() func(http.Handler) http.Handler {
 // SecurityHeaders returns middleware that sets production security
 // headers on every response: HSTS (unless HSTS_MAX_AGE is zero),
 // X-Content-Type-Options, X-Frame-Options, CSP, Referrer-Policy, and
-// Permissions-Policy.
+// Permissions-Policy. The CSP takes scripts and styles only from
+// files on this origin, so a template may hold no inline script,
+// event handler or style attribute.
 func (s *Middleware) SecurityHeaders() func(http.Handler) http.Handler {
 	maxAge := s.params.Config.HSTSMaxAge
 	hsts := "max-age=" + strconv.FormatInt(int64(maxAge/time.Second), 10) +
@@ -560,8 +562,8 @@ func (s *Middleware) SecurityHeaders() func(http.Handler) http.Handler {
 			w.Header().Set(
 				"Content-Security-Policy",
 				"default-src 'self'; "+
-					"script-src 'self' 'unsafe-inline'; "+
+					"script-src 'self'; "+
-					"style-src 'self' 'unsafe-inline'",
+					"style-src 'self'",
 			)
 			w.Header().Set(
 				"Referrer-Policy",

internal/middleware/middleware_test.go

@@ -747,10 +747,10 @@ func TestNoCache_SetsHeaders(t *testing.T) {
 
 // --- SecurityHeaders Middleware Tests ---
 
-// hstsHeaders serves one request through SecurityHeaders built over
+// securityHeaders serves one request through SecurityHeaders built
-// a Config with the given HSTSMaxAge, and returns every
+// over a Config with the given HSTSMaxAge, and returns the response's
-// Strict-Transport-Security value on the response.
+// headers.
-func hstsHeaders(t *testing.T, maxAge time.Duration) []string {
+func securityHeaders(t *testing.T, maxAge time.Duration) http.Header {
 	t.Helper()
 
 	cfg := &config.Config{HSTSMaxAge: maxAge}
@@ -772,7 +772,7 @@ func hstsHeaders(t *testing.T, maxAge time.Duration) []string {
 
 	handler.ServeHTTP(w, req)
 
-	return w.Header().Values("Strict-Transport-Security")
+	return w.Header()
 }
 
 func TestSecurityHeaders_HSTS(t *testing.T) {
@@ -804,11 +804,28 @@ func TestSecurityHeaders_HSTS(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 
-			assert.Equal(t, tt.expected, hstsHeaders(t, tt.maxAge))
+			assert.Equal(
+				t, tt.expected,
+				securityHeaders(t, tt.maxAge).Values(
+					"Strict-Transport-Security",
+				),
+			)
 		})
 	}
 }
 
+// TestSecurityHeaders_CSP pins the Content-Security-Policy: a page
+// takes its scripts and styles only from files on its own origin,
+// never from an inline script, event handler or style.
+func TestSecurityHeaders_CSP(t *testing.T) {
+	t.Parallel()
+
+	assert.Equal(
+		t, "default-src 'self'; script-src 'self'; style-src 'self'",
+		securityHeaders(t, 0).Get("Content-Security-Policy"),
+	)
+}
+
 // --- MaxBodySize Middleware Tests ---
 
 const testBodyLimit int64 = 64

internal/server/alpine_browser_test.go

@@ -60,8 +60,8 @@ const (
 // TestAlpineRunsUnderTheSecurityPolicy loads the webhook page and the
 // event log in a headless browser, served by the real router and so
 // under the real Content-Security-Policy, and checks that the pages'
-// Alpine.js directives and the copy control work, and that a target's
+// Alpine.js directives, the copy control and the delete prompts work,
-// row shows its delivery counts.
+// and that a target's row shows its delivery counts.
 func TestAlpineRunsUnderTheSecurityPolicy(t *testing.T) {
 	t.Parallel()
 
@@ -99,6 +99,9 @@ func TestAlpineRunsUnderTheSecurityPolicy(t *testing.T) {
 	checkEventLog(ctx, t, page+"/events", event.ID, older.ID, target.Name)
 	checkMobileMenu(ctx, t, page)
 	checkPhoneWidth(ctx, t, page, page+"/events", target.Name)
+	checkOtherPages(ctx, t,
+		srv.URL+"/hooks", srv.URL+"/settings", srv.URL+"/user/browser")
+	checkDeletePrompt(ctx, t, page, target)
 
 	assert.Empty(t, problems(), "the browser reported problems")
 }
@@ -1388,3 +1391,75 @@ func checkPhoneWidth(
 	assert.False(t, sideways, "the event log scrolls sideways on a phone")
 	assert.Empty(t, cutOff, "the event log cuts these off on a phone")
 }
+
+// checkOtherPages loads the pages at urls, which no other check opens,
+// so that anything the browser reports going wrong on them fails the
+// test too.
+func checkOtherPages(ctx context.Context, t *testing.T, urls ...string) {
+	t.Helper()
+
+	for _, url := range urls {
+		require.NoError(t, chromedp.Run(ctx, loadPage(url)))
+	}
+}
+
+// answerPrompt returns a script that stands in for the user at the
+// browser's prompt: it makes window.confirm record the question in
+// window.asked and give answer. It also records in window.sent whether
+// a form is sent once its submit event is handled, from a listener on
+// window, which the event reaches after the listener static/js/app.js
+// adds to document.
+func answerPrompt(answer bool) string {
+	return fmt.Sprintf(`
+		window.asked = "";
+		window.sent = false;
+		window.confirm = (question) => {
+			window.asked = question;
+			return %t;
+		};
+		window.addEventListener("submit", (event) => {
+			window.sent = !event.defaultPrevented;
+		});`, answer)
+}
+
+// checkDeletePrompt loads a webhook page and clicks Delete beside
+// target. It checks that the browser asks first, naming the target,
+// that answering no leaves the form unsent, and that answering yes
+// sends it and the target is deleted.
+func checkDeletePrompt(
+	ctx context.Context, t *testing.T, url string, target *database.Target,
+) {
+	t.Helper()
+
+	form := `form[action$="/targets/` + target.ID + `/delete"]`
+	question := `Delete target "` + target.Name + `"?` + "\n\n" +
+		"Nothing more is delivered to it. " +
+		"Its past deliveries stay in the event log."
+
+	var (
+		asked  string
+		sent   bool
+		listed bool
+	)
+
+	require.NoError(t, chromedp.Run(
+		ctx, loadPage(url), chromedp.Evaluate(answerPrompt(false), nil),
+	))
+	click(ctx, t, form+" button")
+	require.NoError(t, chromedp.Run(
+		ctx,
+		chromedp.Evaluate(`window.asked`, &asked),
+		chromedp.Evaluate(`window.sent`, &sent),
+	))
+	assert.Equal(t, question, asked, "Delete does not ask about the target")
+	assert.False(t, sent, "answering no still sends the delete form")
+
+	require.NoError(t, chromedp.Run(
+		ctx, chromedp.Evaluate(answerPrompt(true), nil),
+	))
+	clickAndLoad(ctx, t, form+" button")
+	require.NoError(t, chromedp.Run(ctx, chromedp.Evaluate(
+		`document.querySelector('`+form+`') !== null`, &listed,
+	)))
+	assert.False(t, listed, "answering yes does not delete the target")
+}

static/css/style.css

@@ -1,7 +1,7 @@
 /*
- * The two shared styles for the controls a user clicks. Every page loads
+ * The two shared styles for the controls a user clicks, and the rule that
- * this file after tailwind.css. It is plain CSS: make css does not build
+ * hides an element marked x-cloak. Every page loads this file after
- * it.
+ * tailwind.css. It is plain CSS: make css does not build it.
  *
  * A button is btn-primary, btn-secondary or btn-danger, from input.css.
  * A secondary or inline action, such as Copy beside an entrypoint URL or
@@ -46,3 +46,11 @@
         outline-offset: 2px;
     }
 }
+
+/*
+ * Alpine.js removes x-cloak from an element once it has started, so
+ * something Alpine hides is not seen on the page while it loads.
+ */
+[x-cloak] {
+    display: none !important;
+}

static/js/app.js

@@ -58,6 +58,18 @@
     }
 })();
 
+// Asks before a form with a data-confirm attribute is sent, such as a
+// delete form on the webhook page. The browser's own prompt shows the
+// attribute's text, and the form is sent only if the user accepts.
+document.addEventListener("submit", function (event) {
+    "use strict";
+
+    const message = event.target.getAttribute("data-confirm");
+    if (message !== null && !window.confirm(message)) {
+        event.preventDefault();
+    }
+});
+
 // Alpine.js components.
 //
 // The pages' Content-Security-Policy forbids eval, so the UI loads

templates/event_body.html

@@ -5,7 +5,7 @@
 {{if .Binary}}
 <p class="text-xs text-gray-500">This body is binary ({{.Size}} bytes) and is not shown. <a href="{{.EventURL}}/body" class="btn-small">Download the body</a></p>
 {{else if .Text}}
-<pre class="rounded-md border border-gray-200 bg-white p-2 text-xs text-gray-700 overflow-x-auto whitespace-pre-wrap break-all"{{if .Scroll}} style="max-height: 32rem; overflow-y: auto"{{end}}>{{.Text}}</pre>
+<pre class="rounded-md border border-gray-200 bg-white p-2 text-xs text-gray-700 overflow-x-auto whitespace-pre-wrap break-all{{if .Scroll}} max-h-[32rem] overflow-y-auto{{end}}">{{.Text}}</pre>
 {{if .Cut}}
 <p class="mt-2 text-xs text-gray-500">Showing the first {{.ShownBytes}} of {{.Size}} bytes, unformatted. <a href="{{.EventURL}}" class="btn-small">Show the whole body</a> <a href="{{.EventURL}}/body" class="btn-small">Download the body</a></p>
 {{end}}

templates/htmlheader.html

@@ -4,6 +4,5 @@
 <title>{{block "title" .}}Webhooker{{end}}</title>
 <link rel="stylesheet" href="/s/css/tailwind.css">
 <link rel="stylesheet" href="/s/css/style.css">
-<style>[x-cloak] { display: none !important; }</style>
 {{block "head" .}}{{end}}
 {{end}}

templates/settings.html

@@ -13,13 +13,11 @@
             <div class="p-4">
                 <!-- A value too wide to sit beside its name moves to the
                      next line, where a list breaks only at the spaces
-                     between its entries. overflow-wrap: anywhere breaks
+                     between its entries. wrap-anywhere breaks inside a
-                     inside a value only when it alone is wider than the
+                     value only when it alone is wider than the line. -->
-                     line; an inline style, because the committed
-                     tailwind.css has no class for it. -->
                 <div class="flex flex-wrap justify-between items-start gap-4">
                     <code class="text-sm font-medium text-gray-900">{{.Name}}</code>
-                    <code class="text-sm text-gray-900" style="overflow-wrap: anywhere">{{.Value}}</code>
+                    <code class="text-sm text-gray-900 wrap-anywhere">{{.Value}}</code>
                 </div>
                 <p class="text-sm text-gray-500 mt-1">{{.Description}}</p>
             </div>

templates/source_detail.html

@@ -5,12 +5,10 @@
 {{define "content"}}
 <!-- 108rem, half again the 72rem (max-w-6xl) of the webhook list, the
      event log, the navbar and the footer, so an entrypoint URL fits on
-     one line. An inline style, because the committed tailwind.css has
+     one line. wrap-anywhere goes only on names and descriptions: a
-     no class this wide. wrap-anywhere goes only on names and
+     row too wide for a phone must still run past the edge, where the
-     descriptions: a row too wide for a phone must still run past the
+     browser test sees it, rather than break its controls mid-word. -->
-     edge, where the browser test sees it, rather than break its
+<div class="max-w-[108rem] mx-auto px-6 py-8">
-     controls mid-word. -->
-<div class="mx-auto px-6 py-8" style="max-width: 108rem">
     <div class="mb-6">
         <a href="/hooks" class="btn-small">&larr; Back to webhooks</a>
         <div class="flex flex-wrap justify-between items-center gap-2 mt-2">
@@ -23,11 +21,12 @@
             <div class="flex gap-2">
                 <a href="/hook/{{.Webhook.ID}}/events" class="btn-secondary">Full Event Log</a>
                 <a href="/hook/{{.Webhook.ID}}/edit" class="btn-secondary">Edit</a>
-                <!-- The delete prompts are the browser's own, so they
+                <!-- Before a delete form is sent, static/js/app.js
-                     work without the page's scripts. The template
+                     asks in the browser's own prompt, with the form's
-                     escapes each name for the script, so a quote or a
+                     data-confirm text; &#10; is a line break. The
-                     backslash in it shows as typed. -->
+                     template escapes each name for the attribute, so a
-                <form method="POST" action="/hook/{{.Webhook.ID}}/delete" onsubmit="return confirm('Delete webhook &quot;{{.Webhook.Name}}&quot;?\n\nThis deletes its stored events{{with .Stats}} ({{.WithinRetention.Events}}){{end}} and their deliveries. Any archive files it wrote are kept.')">
+                     quote in it shows as typed. -->
+                <form method="POST" action="/hook/{{.Webhook.ID}}/delete" data-confirm="Delete webhook &quot;{{.Webhook.Name}}&quot;?&#10;&#10;This deletes its stored events{{with .Stats}} ({{.WithinRetention.Events}}){{end}} and their deliveries. Any archive files it wrote are kept.">
                     <input type="hidden" name="csrf_token" value="{{.CSRFToken}}">
                     <button type="submit" class="btn-danger">Delete</button>
                 </form>
@@ -90,7 +89,7 @@
                                     {{if .Active}}Deactivate{{else}}Activate{{end}}
                                 </button>
                             </form>
-                            <form method="POST" action="/hook/{{$.Webhook.ID}}/entrypoints/{{.ID}}/delete" onsubmit="return confirm('Delete entrypoint &quot;{{if .Description}}{{.Description}}{{else}}{{$.BaseURL}}/h/{{.Path}}{{end}}&quot;?\n\nSenders using its URL get an error from now on, and the URL cannot be restored.')" class="inline">
+                            <form method="POST" action="/hook/{{$.Webhook.ID}}/entrypoints/{{.ID}}/delete" data-confirm="Delete entrypoint &quot;{{if .Description}}{{.Description}}{{else}}{{$.BaseURL}}/h/{{.Path}}{{end}}&quot;?&#10;&#10;Senders using its URL get an error from now on, and the URL cannot be restored." class="inline">
                                 <input type="hidden" name="csrf_token" value="{{$.CSRFToken}}">
                                 <button type="submit" class="btn-small text-red-600" title="Delete">Delete</button>
                             </form>
@@ -278,7 +277,7 @@
                                     {{if .Active}}Deactivate{{else}}Activate{{end}}
                                 </button>
                             </form>
-                            <form method="POST" action="/hook/{{$.Webhook.ID}}/targets/{{.ID}}/delete" onsubmit="return confirm('Delete target &quot;{{.Name}}&quot;?\n\nNothing more is delivered to it. Its past deliveries stay in the event log.')" class="inline">
+                            <form method="POST" action="/hook/{{$.Webhook.ID}}/targets/{{.ID}}/delete" data-confirm="Delete target &quot;{{.Name}}&quot;?&#10;&#10;Nothing more is delivered to it. Its past deliveries stay in the event log." class="inline">
                                 <input type="hidden" name="csrf_token" value="{{$.CSRFToken}}">
                                 <button type="submit" class="btn-small text-red-600" title="Delete">Delete</button>
                             </form>