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 also loads the sign-in page, the webhook list, the settings and
profile pages and the 404 error page under the policy. The README
states the policy as sent.

Model: opus-5-5

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
-// application's own CSP allows inline script from 'self'.
+// inside the operator's authenticated origin.
 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
-// as the logged-in operator. The application's CSP does not
-// help here: script-src allows 'unsafe-inline' from 'self', so
-// a document served from this origin could run its own inline
-// script.
+// HTML or script. Without those a stored payload would be shown
+// as a page of the operator's own origin. The application's CSP
+// is a second defence behind them: it allows no inline script,
+// so such a page could not run script of its own.
 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
-// writes the slashes after http: as \/, which the browser reads as /.
+// entrypoint with no description is named by its URL. Each prompt is
+// 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/`+
-			unnamed.Path+`&quot;?`)
+		`data-confirm="Delete entrypoint &quot;http://example.com/h/`+
+			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
-// browser reads back as the name typed: each quote and angle bracket
-// as a \u escape, the slash as \/, the newline as \n and the backslash
-// doubled. An unescaped newline would break the prompt's script, and
-// the form would then submit without asking.
+// newline reaches its delete prompt escaped for the attribute, which
+// the browser reads back as the name typed: each quote and angle
+// bracket as a character reference, the rest as typed. An unescaped
+// double quote would end the attribute and cut the prompt short.
 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"+
-			"\\u003c\\/script\\u003e\\nline two&quot;?")
+		"Delete webhook &quot;Bob&#39;s &#34;best&#34; \\ hook"+
+			"&lt;/script&gt;\nline two&quot;?")
 }

internal/middleware/middleware.go

@@ -572,7 +572,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) +
@@ -594,8 +596,8 @@ func (s *Middleware) SecurityHeaders() func(http.Handler) http.Handler {
 			w.Header().Set(
 				"Content-Security-Policy",
 				"default-src 'self'; "+
-					"script-src 'self' 'unsafe-inline'; "+
-					"style-src 'self' 'unsafe-inline'",
+					"script-src 'self'; "+
+					"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
-// a Config with the given HSTSMaxAge, and returns every
-// Strict-Transport-Security value on the response.
-func hstsHeaders(t *testing.T, maxAge time.Duration) []string {
+// securityHeaders serves one request through SecurityHeaders built
+// over a Config with the given HSTSMaxAge, and returns the response's
+// headers.
+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
-// row shows its delivery counts.
+// Alpine.js directives, the copy control and the delete prompts work,
+// and that a target's row shows its delivery counts.
 func TestAlpineRunsUnderTheSecurityPolicy(t *testing.T) {
 	t.Parallel()
 
@@ -74,6 +74,10 @@ func TestAlpineRunsUnderTheSecurityPolicy(t *testing.T) {
 	userID, _ := env.seedUser(t, "browser", "browser-password")
 	webhook, older, event, target := seedBrowserWebhook(t, env, userID)
 
+	// The sign-in page is loaded before the browser signs in, since a
+	// signed-in visit to it is sent on to another page.
+	checkOtherPages(ctx, t, srv.URL+"/pages/login")
+
 	require.NoError(t, chromedp.Run(
 		ctx, setCookies(srv.URL, env.authCookies(t, userID, "browser")),
 	))
@@ -99,6 +103,11 @@ 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",
+		// A path that does not exist, which gets the 404 error page.
+		srv.URL+"/no-such-page")
+	checkDeletePrompt(ctx, t, page, target)
 
 	assert.Empty(t, problems(), "the browser reported problems")
 }
@@ -1388,3 +1397,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")
+}