amadan

carlos / movement Public

The app's own CSP was refusing its own sign-in redirect

form-action governs the whole redirect chain a form submission starts, not
just the URL the form posts to. rastrillo ships form-action 'self', so the
POST to /signin succeeded, answered 303 to keymail.dev's approval ceremony,
and the browser declined to follow its own server's redirect.

Nothing failed server-side. The 303 was correct, the logs were clean, and
the only evidence anywhere was one line in a console. Paul found it by
opening the inspector on a spinner.

Options.CSP now carries the framework default with the sign-in provider
added to form-action and nothing else widened — scripts, styles and images
stay same-origin, framing stays refused, and a test asserts that separately
so a future fix cannot quietly loosen the rest.

The provider host is not generally knowable: rastrillo/auth sends the
browser wherever the submitted address delegates. It is knowable here only
because the admin allowlist is a closed set, which is why signinOrigins
carries a warning that adding an admin on another provider means adding
their host too — the failure mode is identical and just as silent.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Paul Campbell 7afdfdad177644756beeee2dbe1c3a9656d7f54e parent c2c8d2a
4 files changed, +101 −2
  • cmd/movement/main.go +4 −0
  • copy-review/server.log +2 −2
  • internal/movement/csp.go +34 −0
  • internal/movementtest/csp_test.go +61 −0
diff --git a/cmd/movement/main.go b/cmd/movement/main.go
index 9325abc..ee4d6df 100644
--- a/cmd/movement/main.go
+++ b/cmd/movement/main.go
@@ -54,6 +54,10 @@ func main() {
}
opts.Mux = mux
+ // rastrillo's default policy is form-action 'self', which a browser
+ // applies to the whole redirect chain a form submission starts — so the
+ // app's own CSP was refusing its own 303 to the sign-in provider.
+ opts.CSP = movement.CSP()
opts.DBPath = ""
// The same page a handler's own error path renders (render.go's
// ErrorPage — wire it to Ctx.ErrorPage too as the app grows
diff --git a/copy-review/server.log b/copy-review/server.log
index abcb850..e8cab15 100644
--- a/copy-review/server.log
+++ b/copy-review/server.log
@@ -1,3 +1,3 @@
-REVIEW_URL: http://docs-t1.tail5fd3ba.ts.net:40617
-REVIEW_URL_FALLBACK: http://100.119.180.57:40617
+REVIEW_URL: http://docs-t1.tail5fd3ba.ts.net:41343
+REVIEW_URL_FALLBACK: http://100.119.180.57:41343
Copy review running
diff --git a/internal/movement/csp.go b/internal/movement/csp.go
new file mode 100644
index 0000000..3747342
--- /dev/null
+++ b/internal/movement/csp.go
@@ -0,0 +1,34 @@
+package movement
+
+import "strings"
+
+// signinOrigins are the hosts a sign-in may hand the browser off to.
+//
+// This exists because the app's own Content-Security-Policy was breaking its
+// own admin sign-in. rastrillo ships `form-action 'self'`, and a browser
+// applies form-action to the whole redirect chain a form submission starts —
+// not just to the URL the form posts to. So the POST to /signin succeeded,
+// answered 303 to keymail.dev's approval ceremony, and the browser refused
+// to follow its own server's redirect. Nothing failed server-side; the only
+// evidence was a console line.
+//
+// The host is not ours to guess in general: rastrillo/auth sends the browser
+// to whatever provider the submitted address delegates to. It is knowable
+// here only because the admin allowlist is a closed set we control.
+//
+// ADDING AN ADMIN ON ANOTHER PROVIDER MEANS ADDING ITS HOST HERE. Miss it
+// and their sign-in fails the same silent way: a 303 the browser declines,
+// no server error, nothing in the logs.
+var signinOrigins = []string{"https://keymail.dev"}
+
+// CSP is the policy every response carries. It is rastrillo's default with
+// the sign-in providers added to form-action, and nothing else widened:
+// scripts, styles and images stay same-origin, framing stays refused.
+func CSP() string {
+ return "default-src 'self'; " +
+ "style-src 'self' 'unsafe-inline'; " +
+ "img-src 'self' data:; " +
+ "frame-ancestors 'none'; " +
+ "base-uri 'self'; " +
+ "form-action 'self' " + strings.Join(signinOrigins, " ")
+}
diff --git a/internal/movementtest/csp_test.go b/internal/movementtest/csp_test.go
new file mode 100644
index 0000000..1b40e63
--- /dev/null
+++ b/internal/movementtest/csp_test.go
@@ -0,0 +1,61 @@
+package movementtest
+
+import (
+ "strings"
+ "testing"
+
+ movement "amadan.net/carlos/movement/internal/movement"
+)
+
+// The app's own Content-Security-Policy broke its own admin sign-in, and did
+// it silently: the POST succeeded, the 303 to the provider was correct, and
+// the browser refused to follow it. form-action governs the whole redirect
+// chain a form submission starts, not only where the form posts.
+//
+// The only evidence was a line in somebody's console. That is why this is a
+// test: nothing server-side fails, so nothing else would ever catch it.
+func TestSigninProviderIsAllowedByFormAction(t *testing.T) {
+ csp := movement.CSP()
+
+ formAction := ""
+ for _, d := range strings.Split(csp, ";") {
+ if d = strings.TrimSpace(d); strings.HasPrefix(d, "form-action") {
+ formAction = d
+ }
+ }
+ if formAction == "" {
+ t.Fatal("no form-action directive at all, which is more permissive than the default")
+ }
+ if !strings.Contains(formAction, "'self'") {
+ t.Error("form-action must still allow this app's own forms")
+ }
+ if !strings.Contains(formAction, "https://keymail.dev") {
+ t.Errorf("the sign-in provider is not in form-action, so the browser "+
+ "will refuse the redirect that starts the ceremony: %q", formAction)
+ }
+}
+
+// Widening one directive must not quietly widen the rest. Everything except
+// form-action stays exactly as strict as the framework's default.
+func TestNothingElseInThePolicyWasWidened(t *testing.T) {
+ csp := movement.CSP()
+
+ for _, want := range []string{
+ "default-src 'self'",
+ "style-src 'self' 'unsafe-inline'",
+ "img-src 'self' data:",
+ "frame-ancestors 'none'",
+ "base-uri 'self'",
+ } {
+ if !strings.Contains(csp, want) {
+ t.Errorf("the policy no longer carries %q", want)
+ }
+ }
+ if strings.Contains(csp, "script-src") {
+ t.Error("scripts have been given their own directive; they should stay " +
+ "under default-src 'self'")
+ }
+ if strings.Contains(csp, "unsafe-eval") || strings.Contains(csp, "*") {
+ t.Error("the policy contains a wildcard or unsafe-eval")
+ }
+}