From d0a580c60cfa02e78ba4769caca83f54c2f133f1 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Thu, 15 Oct 2026 22:30:57 +0200 Subject: [PATCH] fix(19.3-06): warn at boot when turnstile routes have no secret (WR-08) - BuildRouter logs one warning with the route count and the production flag when routes name turnstile:open or turnstile:closed and the verifier has no secret; the secret is never logged --- docs/services/authentication.md | 2 +- modules/surf/README.md | 2 +- modules/surf/cors_profile_test.go | 59 +++++++++++++++++++++++++++++++ modules/surf/router.go | 27 ++++++++++++++ 4 files changed, 88 insertions(+), 2 deletions(-) diff --git a/docs/services/authentication.md b/docs/services/authentication.md index 5c8f2ba..e6a224e 100644 --- a/docs/services/authentication.md +++ b/docs/services/authentication.md @@ -183,7 +183,7 @@ The middleware reads the token from the `turnstile_token` field (`turnstile.fiel | Key | Default | Controls | |-----|---------|----------| -| `turnstile.secret` | empty | The site's secret key. Set it through `SUMMER_TURNSTILE__SECRET`; a PHP application's `TURNSTILE_SECRET` value moves to that variable. | +| `turnstile.secret` | empty | The site's secret key. Set it through `SUMMER_TURNSTILE__SECRET`; a PHP application's `TURNSTILE_SECRET` value moves to that variable. When it is empty and routes name `turnstile:open` or `turnstile:closed`, the start-up logs one warning, `surf: turnstile.secret is empty`, with the number of such routes, since they are then unprotected. | | `turnstile.field` | `turnstile_token` | The request field carrying the token. | Refusals are written with neutral bodies, `{"error":"turnstile_unavailable"}` (503) and `{"error":"turnstile_failed"}` (422). To keep an existing response envelope, publish a `bouncer.TurnstileWriter` on the app in your plugin's `Register`; it receives the status and the `bouncer.TurnstileVerdict`. A published `*bouncer.Turnstile` replaces the verifier built from config, for example to give it its own `http.Client`. diff --git a/modules/surf/README.md b/modules/surf/README.md index a8f36a6..9c58060 100644 --- a/modules/surf/README.md +++ b/modules/surf/README.md @@ -162,7 +162,7 @@ The generated application `main` wires surf in with `surf.ServeCommand` and `sur | `http.cors.max_age` | `0` | Preflight cache time in seconds. | | `http.cors.supports_credentials` | `false` | Sends `Access-Control-Allow-Credentials: true`. Combined with `*` in `allowed_origins` it fails boot. With credentials, a pattern or several origins, every response on a CORS path carries `Vary: Origin`, preflights and disallowed origins included. | | `http.cors_profiles..*` | none | A CORS profile for `cors:`: the `http.cors` keys except `paths` (`allowed_origins`, `allowed_origins_patterns`, `allowed_methods`, `allowed_headers`, `exposed_headers`, `max_age`, `supports_credentials`), validated the same way. Names use lower-case letters, digits, underscore and dash. | -| `turnstile.secret` | empty | Cloudflare Turnstile secret for `turnstile:open` and `turnstile:closed`. Without it `turnstile:open` passes and `turnstile:closed` answers 503 in the production environment (passes elsewhere). A `*bouncer.Turnstile` published on the app replaces the verifier built from this key. Environment: `SUMMER_TURNSTILE__SECRET`. | +| `turnstile.secret` | empty | Cloudflare Turnstile secret for `turnstile:open` and `turnstile:closed`. Without it `turnstile:open` passes and `turnstile:closed` answers 503 in the production environment (passes elsewhere), and when routes name either, `surf.BuildRouter` logs one start-up warning, `surf: turnstile.secret is empty`, with the route count. A `*bouncer.Turnstile` published on the app replaces the verifier built from this key. Environment: `SUMMER_TURNSTILE__SECRET`. | | `turnstile.field` | `turnstile_token` | Request field (query, JSON body or form body) carrying the Turnstile token. | | `http.rate_limit.store` | `memory` | Where `throttle` counters live: `memory` (per instance) or `redis` (shared by every instance through [cooler](../cooler/README.md), which then needs `redis.url`). Only `serve` installs the Redis store; `route:list` keeps the memory store and never dials Redis. Any other value fails at boot. Environment: `SUMMER_HTTP__RATE_LIMIT__STORE`. | | `http.api_docs.enabled` | `false` | Mounts `GET ` (Swagger UI page) and `GET .json` (the spec of the one `surf.APIDocsProvider` plugin). Both are public. Environment: `SUMMER_HTTP__API_DOCS__ENABLED`. | diff --git a/modules/surf/cors_profile_test.go b/modules/surf/cors_profile_test.go index 8d2d246..be639ed 100644 --- a/modules/surf/cors_profile_test.go +++ b/modules/surf/cors_profile_test.go @@ -1,8 +1,11 @@ package surf import ( + "bytes" + "encoding/json" "errors" "io" + "log/slog" "net/http" "net/http/httptest" "strings" @@ -659,3 +662,59 @@ func TestCORSProfileVaryKeptWithHandlerVary(t *testing.T) { t.Errorf("Vary = %q, want Accept-Language too", rec.Header().Values("Vary")) } } + +func TestTurnstileEmptySecretWarnsAtBoot(t *testing.T) { + // boot builds a router on an app whose published logger writes JSON + // lines to a buffer and returns the warnings naming turnstile.secret. + boot := func(t *testing.T, env string, verifier *bouncer.Turnstile, plugins ...party.Plugin) []map[string]any { + t.Helper() + app := transportApp(t, env, map[string]string{"http.yaml": transportBodyLimits}) + var buf bytes.Buffer + if err := app.Publish(slog.New(slog.NewJSONHandler(&buf, nil))); err != nil { + t.Fatal(err) + } + if verifier != nil { + if err := app.Publish[*bouncer.Turnstile](verifier); err != nil { + t.Fatal(err) + } + } + if _, err := BuildRouter(app, plugins); err != nil { + t.Fatal(err) + } + var out []map[string]any + for _, line := range strings.Split(strings.TrimSpace(buf.String()), "\n") { + if line == "" { + continue + } + var rec map[string]any + if err := json.Unmarshal([]byte(line), &rec); err != nil { + t.Fatalf("log line %q: %v", line, err) + } + if msg, _ := rec["msg"].(string); strings.HasPrefix(msg, "surf: turnstile.secret is empty") { + out = append(out, rec) + } + } + return out + } + + got := boot(t, "production", nil, turnstileRoutes("turnstile:closed", "turnstile:open")) + if len(got) != 1 { + t.Fatalf("warnings = %v, want one", got) + } + if got[0]["level"] != "WARN" || got[0]["routes"] != float64(2) || got[0]["production"] != true { + t.Fatalf("warning = %v, want WARN with routes 2 and production true", got[0]) + } + if msg := got[0]["msg"].(string); !strings.Contains(msg, "turnstile:open passes every request") || !strings.Contains(msg, "503 in production") { + t.Errorf("message = %q", msg) + } + + // A published verifier with a secret: no warning. + if got := boot(t, "production", bouncer.NewTurnstile("test-secret", true), turnstileRoutes("turnstile:closed", "turnstile:open")); len(got) != 0 { + t.Fatalf("warnings with a secret = %v", got) + } + // No turnstile routes: no warning. + plain := routesPluginFunc{id: "acme.plain", routes: func(r pact.Router) { r.Get("/x", okHandler) }} + if got := boot(t, "development", nil, plain); len(got) != 0 { + t.Fatalf("warnings without turnstile routes = %v", got) + } +} diff --git a/modules/surf/router.go b/modules/surf/router.go index 1b051c3..6c4d2b5 100644 --- a/modules/surf/router.go +++ b/modules/surf/router.go @@ -3,6 +3,7 @@ package surf import ( "bytes" "fmt" + "log/slog" "math" "net/http" "strings" @@ -651,6 +652,7 @@ func BuildRouter(app *backpack.App, plugins []party.Plugin) (*Router, error) { return nil, err } } + warnTurnstileWithoutSecret(app, turnstile, r.routes) if err := r.addCORSPreflights(); err != nil { return nil, err } @@ -676,6 +678,31 @@ func buildTurnstile(app *backpack.App) (*bouncer.Turnstile, string) { return bouncer.NewTurnstile(strings.TrimSpace(app.Config.String("turnstile.secret")), production), field } +// warnTurnstileWithoutSecret logs one warning at boot when routes name +// turnstile:open or turnstile:closed and the verifier has no secret: such +// routes are not protected. The secret itself is never logged. +func warnTurnstileWithoutSecret(app *backpack.App, verifier *bouncer.Turnstile, routes []route) { + if verifier != nil && strings.TrimSpace(verifier.Secret) != "" { + return + } + n := 0 + for _, rt := range routes { + for _, name := range rt.middleware { + if name == "turnstile:open" || name == "turnstile:closed" { + n++ + break + } + } + } + if n == 0 { + return + } + production := verifier != nil && verifier.Production + accessLogger(app).Warn("surf: turnstile.secret is empty; turnstile:open passes every request, "+ + "and turnstile:closed passes outside production and answers 503 in production", + slog.Int("routes", n), slog.Bool("production", production)) +} + // checkAdminPrefix fails boot when a plugin other than cabana owns a route at // or under the admin prefix: the admin SPA and API own that whole subtree. func (r *Router) checkAdminPrefix(prefix string) error {