|
|
|
|
@@ -0,0 +1,309 @@
|
|
|
|
|
---
|
|
|
|
|
phase: quick-261020-42e
|
|
|
|
|
plan: 01
|
|
|
|
|
quick_id: 261020-42e
|
|
|
|
|
slug: branded-html-error-pages-for-browsers-un
|
|
|
|
|
type: execute
|
|
|
|
|
mode: quick
|
|
|
|
|
wave: 1
|
|
|
|
|
depends_on: []
|
|
|
|
|
files_modified:
|
|
|
|
|
- modules/surf/errorpage.go
|
|
|
|
|
- modules/surf/errorpage.html
|
|
|
|
|
- modules/surf/errorpage_sun.webp
|
|
|
|
|
- modules/surf/errorpage_test.go
|
|
|
|
|
- modules/surf/router.go
|
|
|
|
|
- modules/surf/params.go
|
|
|
|
|
- modules/surf/overlap.go
|
|
|
|
|
- modules/surf/uploads.go
|
|
|
|
|
- modules/surf/accesslog_test.go
|
|
|
|
|
- modules/surf/README.md
|
|
|
|
|
- docs/services/routing.md
|
|
|
|
|
- docs/architecture/request-lifecycle.md
|
|
|
|
|
- docs/setup/configuration.md
|
|
|
|
|
autonomous: true
|
|
|
|
|
requirements: [QUICK-261020-42e]
|
|
|
|
|
source:
|
|
|
|
|
- .planning/todos/pending/2026-10-20-branded-error-pages-for-browsers.md (g15office jh5qzjba)
|
|
|
|
|
- orchestrator brief (negotiation rule, plug-in points, override hook, details rule, admin exclusion, tests, docs)
|
|
|
|
|
- modules/surf/router.go:351-392 (compile; mux built at :375, wrapped by pathScopedCORS at :387), :405-489 (wrap; constrain at :408/:411, recoverBare/recoverJSON at :408/:476/:478), :502-660 (BuildRouter; r.app at :504, mountUploads :628, cabana mount :638-649), :778-806 (recoverJSON writes wire.WriteOpaque500, recoverBare a bare 500)
|
|
|
|
|
- modules/surf/params.go:81-94 (constrain, http.NotFound at :88)
|
|
|
|
|
- modules/surf/overlap.go:386-434 (familyDispatch, http.NotFound at :424/:429, 405 via http.Error at :432-433 after Allow), :469-483 (registerOverlapFamilies builds familyDispatch)
|
|
|
|
|
- modules/surf/uploads.go:28 (uploadsContentSecurityPolicy), :53-70 (mountUploads -> uploadsHandler), :107-122 (two http.NotFound while bucket or DB is unpublished)
|
|
|
|
|
- modules/surf/accesslog_test.go:171-183 (direct recoverJSON/recoverBare calls)
|
|
|
|
|
- modules/surf/transport_smoke_test.go:33-60 (transportApp, routesPluginFunc helpers), modules/surf/admin_prefix_test.go:103-140 (adminPrefixApp, adminPrefixPlugin)
|
|
|
|
|
- modules/surf/locale_resolver.go (pattern for an optional backpack-published surf interface)
|
|
|
|
|
- modules/wire/response.go:25-30 (WriteOpaque500 body, unchanged)
|
|
|
|
|
- modules/compass/config.go:50 (SUMMER_ENV defaults to production), :75 (Environment), :140 (Bool)
|
|
|
|
|
- live grep at planning time: no maintenance mode exists in surf; no request-id middleware exists in the framework
|
|
|
|
|
|
|
|
|
|
estimate:
|
|
|
|
|
tokens: 90000
|
|
|
|
|
raw_tokens: 90000
|
|
|
|
|
tasks: 3
|
|
|
|
|
confidence: low
|
|
|
|
|
|
|
|
|
|
must_haves:
|
|
|
|
|
truths:
|
|
|
|
|
- "A browser navigation (GET with Accept text/html, Sec-Fetch-Mode navigate) to a path no route matches gets status 404 with Content-Type text/html; charset=utf-8 and a self-contained SummerCMS page: navy background, yellow accent, the sun logo as a data: URI, the status code, a short message and a link to /; no script tag and no external URL."
|
|
|
|
|
- "A curl-style (Accept */* or none), fetch/XHR (Sec-Fetch-Mode cors, X-Requested-With), JSON-Accept, /api or /api/..., /_<name>/... or admin-prefix request gets byte-identical status, body, Content-Type and Allow to the same router built with http.error_pages.enabled false: stdlib '404 page not found\\n', 'Method Not Allowed\\n' with Allow, wire's opaque JSON 500, and the bare raw-group 500."
|
|
|
|
|
- "A browser navigation that hits a ServeMux 405, a failed Where/WhereIn constraint, an overlap-family miss or wrong method, the uploads route while its bucket or database is unpublished, or a recovered panic (JSON or raw group) gets the branded page with that status (404, 405 or 500); a 405 keeps its Allow header; the panic value never appears in the body."
|
|
|
|
|
- "Method, path and the X-Request-Id request header (at most 128 bytes) appear on the page only when app.debug is true or the environment is set and not production; values are HTML-escaped; production with app.debug false shows none of them."
|
|
|
|
|
- "An application or plugin that publishes a surf.ErrorPageRenderer through backpack replaces the page body for every covered status; a renderer that returns ok=false, returns an empty body or panics falls back to the built-in page with the correct status."
|
|
|
|
|
- "With http.error_pages.enabled false every response is exactly today's, browser navigations included."
|
|
|
|
|
- "go vet ./..., go test -short ./..., go -C examples/hello test -short ./..., TestDocsTree and summer docs:build --check pass; the work lands as two commits (feat with code and docs, then test) staged by explicit path with no co-author trailer; nothing pushed or tagged; .planning/phases/19.4.1-* untouched."
|
|
|
|
|
artifacts:
|
|
|
|
|
- path: modules/surf/errorpage.go
|
|
|
|
|
provides: "surf.ErrorPage, surf.ErrorPageRenderer, the unexported errorPages type (negotiation, page writer, mux fallback) and the embedded template and logo"
|
|
|
|
|
- path: modules/surf/errorpage.html
|
|
|
|
|
provides: "The built-in html/template page with inline CSS in the SummerCMS palette"
|
|
|
|
|
- path: modules/surf/errorpage_sun.webp
|
|
|
|
|
provides: "The 60x60 sun logo, embedded and served as a data: URI"
|
|
|
|
|
- path: modules/surf/errorpage_test.go
|
|
|
|
|
provides: "Tracer test, negotiation table, API-unchanged equivalence test, HTML page checks, details, override, kill switch, admin prefix and uploads cases"
|
|
|
|
|
- path: modules/surf/README.md
|
|
|
|
|
provides: "Features bullet, API rows for surf.ErrorPage and surf.ErrorPageRenderer, http.error_pages.enabled config row, embed in the stdlib list"
|
|
|
|
|
- path: docs/services/routing.md
|
|
|
|
|
provides: "An 'Error pages' section: negotiation rule, covered errors, details rule, override example"
|
|
|
|
|
key_links:
|
|
|
|
|
- from: "modules/surf/router.go compile()"
|
|
|
|
|
to: "errorPages.fallback(mux)"
|
|
|
|
|
via: "the mux is wrapped before pathScopedCORS, so CORS headers stay and the access log still sees the status"
|
|
|
|
|
pattern: "fallback\\(mux\\)"
|
|
|
|
|
- from: "modules/surf/router.go wrap()"
|
|
|
|
|
to: "constrain(r.pages, ...), recoverJSON(r.pages, ...), recoverBare(r.pages, ...)"
|
|
|
|
|
via: "explicit *errorPages argument; nil keeps today's behaviour"
|
|
|
|
|
pattern: "recoverJSON\\(r\\.pages"
|
|
|
|
|
- from: "modules/surf/overlap.go registerOverlapFamilies"
|
|
|
|
|
to: "familyDispatch.pages"
|
|
|
|
|
via: "new parameter passed from compile()"
|
|
|
|
|
pattern: "pages"
|
|
|
|
|
- from: "modules/surf/router.go BuildRouter"
|
|
|
|
|
to: "errorPages admin prefix"
|
|
|
|
|
via: "set right after the cabana admin is mounted, so admin paths are never negotiated"
|
|
|
|
|
pattern: "adminPrefix"
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
<objective>
|
|
|
|
|
Browsers that hit a framework-generated error (unmatched route, wrong method, failed constraint, recovered panic) get a good-looking, self-contained SummerCMS error page instead of the stdlib's raw `404 page not found`. Every API or programmatic caller keeps today's exact bytes, so the PHP-compatible contract does not move.
|
|
|
|
|
|
|
|
|
|
Purpose: `bin/demo serve` with no frontend in front currently answers a browser at `/` with raw stdlib text. WinterCMS ships generic error pages; SummerCMS gets nicer ones, and applications can replace them.
|
|
|
|
|
|
|
|
|
|
Output: `modules/surf/errorpage.go` + embedded template and logo, the call-site changes in surf, documentation (surf README, routing, request lifecycle, configuration docs) and unit tests. Two commits.
|
|
|
|
|
|
|
|
|
|
Settled design decisions (cite these IDs in code review and the SUMMARY):
|
|
|
|
|
|
|
|
|
|
- QD-01 Owner: surf. surf owns the ServeMux and every call site; a separate beach-named module would be imported only by surf. No new module, no new third-party dependency (stdlib `embed` and `html/template`, which surf already uses).
|
|
|
|
|
- QD-02 Negotiation. A request gets the HTML page only when ALL of these hold, otherwise it gets today's response:
|
|
|
|
|
(a) if `Sec-Fetch-Mode` is present it equals `navigate`; if it is absent the method is GET or HEAD;
|
|
|
|
|
(b) no `X-Requested-With` header (any value);
|
|
|
|
|
(c) the `Accept` header names `text/html` or `application/xhtml+xml` explicitly with q > 0, and that q is strictly greater than the q of every JSON type it names (`application/json`, `text/json`, any `application/*+json`); `*/*` and `text/*` count for neither side; only the first 32 comma-separated media ranges are read; a malformed q skips that range;
|
|
|
|
|
(d) the path is not API-style: not `/api`, not under `/api/`, and its first segment does not start with `_` (the `/_<plugin>/` prefixes);
|
|
|
|
|
(e) the path is not the admin prefix or under it (only when the cabana admin is mounted).
|
|
|
|
|
The path checked is the one routing sees (after the locale-prefix rewrite).
|
|
|
|
|
- QD-03 Covered errors: the ServeMux fallback for unmatched routes (404) and method mismatch (405, Allow kept); surf's own `http.NotFound` in `constrain` (params.go) and `familyDispatch` (overlap.go, plus its 405); the two `http.NotFound` calls in `uploadsHandler` (uploads.go); the panic recovery in `recoverJSON` and `recoverBare` (raw groups included: OAuth authorize is a browser navigation, and a webhook never matches QD-02). Not covered, by design: plugin handlers' own error bodies, middleware refusals (401/403/413/422/429/503), the api-docs page's own 500s, and the 404 that `attach.StaticHandlerPublic` writes for a missing upload row (it lives in lagoon/attach). There is no maintenance mode in surf, so none is added.
|
|
|
|
|
- QD-04 Unchanged bytes. For a request that fails QD-02, the code path is the exact pre-change call (`http.NotFound`, `http.Error`, `wire.WriteOpaque500`, bare `WriteHeader(500)`, `mux.ServeHTTP`); no header is added. A nil `*errorPages` means "behave exactly as before", so `surf.New` + `compile` routers in existing tests keep today's behaviour.
|
|
|
|
|
- QD-05 Override hook: an optional backpack-published `surf.ErrorPageRenderer` (same pattern as `surf.LocaleResolver`), looked up per error with `app.Lookup[surf.ErrorPageRenderer]()`. Its single method `RenderErrorPage(r *http.Request, page surf.ErrorPage) (body []byte, ok bool)` returns the full HTML document. surf still owns the status and headers. ok=false, an empty body or a panic inside the renderer falls back to the built-in page. The renderer chooses per `page.Status`, as WinterCMS themes do with their `404` and `error` pages.
|
|
|
|
|
- QD-06 Details (method, path, `X-Request-Id` request header trimmed to 128 bytes) are filled into `surf.ErrorPage` and shown only when `app.debug` is true, or when `app.Config.Environment()` is non-empty and not `production` (case-insensitive, trimmed). A nil app or config, or an empty environment, counts as production. The panic value and stack are never rendered in any mode.
|
|
|
|
|
- QD-07 Admin: paths at or under the mounted admin prefix (`surf.Router.AdminPrefix`) are never negotiated, so the `/backend` SPA and its API keep their own behaviour.
|
|
|
|
|
- QD-08 Kill switch: config key `http.error_pages.enabled`, default true when absent (read with `app.Config.Lookup`, then `app.Config.Bool`). False makes `newErrorPages` return nil, which restores today's behaviour everywhere. Read once in `BuildRouter`.
|
|
|
|
|
- QD-09 Look and headers. The page has inline CSS only, with no external request. The logo is embedded and inlined as a `data:image/webp;base64,...` URI (passed as `template.URL`, a constant), and it is also the favicon. The copy is English. The palette, from the summercms.io tokens: page `#233148`, card `#29344a`, deep panel `#18223a`, button text `#1d2940`, accent `#f5c55a`, accent hover `#fbd77e`, light accent `#fbe3a6`/`#fde9b4`, text `#e9edf3`, `#c5ccd6`, `#a3adbd`, captions `#8a94a3`, borders `rgba(255,255,255,0.08)`, radii 6px, 16px and 999px, font `Roboto, system-ui, -apple-system, "Segoe UI", sans-serif`, mono `"Roboto Mono", ui-monospace, monospace`. Headers on every HTML error response, built-in or override: `Content-Type: text/html; charset=utf-8`, `X-Content-Type-Options: nosniff`, `Cache-Control: no-store`, `Vary` gains `Accept`, `Sec-Fetch-Mode` and `X-Requested-With` (merged through the existing `addVary`), and `Content-Security-Policy: default-src 'none'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; font-src 'self'; base-uri 'none'; form-action 'self'; frame-ancestors 'none'` (Set, so it replaces the uploads route's sandbox CSP for the page; there is no script-src, so no script runs on an error page). The home link is always `/`.
|
|
|
|
|
</objective>
|
|
|
|
|
|
|
|
|
|
<context>
|
|
|
|
|
@.planning/STATE.md
|
|
|
|
|
@CLAUDE.md
|
|
|
|
|
@.planning/todos/pending/2026-10-20-branded-error-pages-for-browsers.md
|
|
|
|
|
@modules/surf/README.md
|
|
|
|
|
@modules/surf/router.go
|
|
|
|
|
@modules/surf/locale_resolver.go
|
|
|
|
|
|
|
|
|
|
Project rules that apply here: standard library first, no new dependency. A change to exported API or config keys updates the module README and the affected docs/ pages in the same commit. Every identifier in a README or docs page must exist in the package (the docs checker enforces it). Framework code, READMEs and docs never name a consuming application; use neutral names such as `acme` or `blog`. YAML through goccy/go-yaml only (not needed here). Stage files by explicit path, never `git add -A`. No co-author or attribution trailer in commit messages (the user's global rule overrides any harness attribution). Do not push or tag. Another session is planning Phase 19.4.1: never touch `.planning/phases/19.4.1-*`, `.planning/state.json` or `.planning/STATE.md`. The orchestrator handles STATE, the todo move and the g15office comment.
|
|
|
|
|
</context>
|
|
|
|
|
|
|
|
|
|
<tasks>
|
|
|
|
|
|
|
|
|
|
<task type="tracer" tdd="true">
|
|
|
|
|
<name>Task 1: Tracer: a browser at an unmatched path gets the branded 404, curl keeps the stdlib text</name>
|
|
|
|
|
<files>modules/surf/errorpage.go, modules/surf/errorpage.html, modules/surf/errorpage_sun.webp, modules/surf/router.go, modules/surf/errorpage_test.go</files>
|
|
|
|
|
<read_first>
|
|
|
|
|
- modules/surf/router.go lines 351-392 (compile) and 500-520 (start of BuildRouter)
|
|
|
|
|
- modules/surf/cors.go lines 284-295 (addVary, reused for Vary merging)
|
|
|
|
|
- modules/surf/transport_smoke_test.go lines 28-60 (transportApp and routesPluginFunc test helpers to reuse)
|
|
|
|
|
- Logo copy source (binary; copy with cp, do not open): ../sm-summercmsio-app/vue-summercmsio-app/public/sun-logo.webp (60x60 WebP, 3282 bytes)
|
|
|
|
|
- Brand token reference (read once for the palette in QD-09): ../sm-summercmsio-app/vue-summercmsio-app/app/assets/css/tokens.css
|
|
|
|
|
</read_first>
|
|
|
|
|
<behavior>
|
|
|
|
|
- TestErrorPageTracer: build a router with surf.Assemble on transportApp(t, "production", http.yaml with body_limits) and one routesPluginFunc route GET /api/ping. A GET /nowhere with Accept "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8" and Sec-Fetch-Mode navigate answers 404, Content-Type "text/html; charset=utf-8", Cache-Control "no-store", a Content-Security-Policy starting "default-src 'none'", and a body containing "404", `href="/"`, "data:image/webp;base64," and no "<script".
|
|
|
|
|
- The same GET /nowhere with Accept "*/*" (curl) answers 404 with body exactly "404 page not found\n" and Content-Type "text/plain; charset=utf-8".
|
|
|
|
|
</behavior>
|
|
|
|
|
<action>
|
|
|
|
|
Pre-flight: run `git status --short -- modules/surf docs`. If another session has uncommitted edits in any file of this plan, stop and report instead of editing over them.
|
|
|
|
|
|
|
|
|
|
RED: create modules/surf/errorpage_test.go (package surf) with TestErrorPageTracer as described in behavior, reusing transportApp and routesPluginFunc. Run it and watch it fail.
|
|
|
|
|
|
|
|
|
|
GREEN, in this order:
|
|
|
|
|
1. Copy the logo from read_first to modules/surf/errorpage_sun.webp (cp; binary file, committed as is).
|
|
|
|
|
2. Create modules/surf/errorpage.html, an html/template document rendered from the unexported page view (the exported surf.ErrorPage fields plus a Logo field of type template.URL). The structure: doctype; html lang en; meta charset utf-8; viewport meta; meta robots noindex; title "{{Status}} · {{Title}} · SummerCMS"; a link rel icon pointing at the logo data URI; and one inline style element. The body is a full-viewport flex container that centres a card. Body background is #233148 with a soft radial glow of rgba(245,197,90,0.10) near the top, and the text is #e9edf3. The card is at most 480px wide, background #29344a, 1px border rgba(255,255,255,0.08), radius 16px, padding 40px 32px, centred text and a soft shadow. Inside the card, in order:
|
|
|
|
|
- the 60x60 logo img (alt "SummerCMS") above a wordmark "Summer" + "CMS", where "CMS" is a #fde9b4 to #f5c55a gradient text with #fbd77e as the plain colour;
|
|
|
|
|
- the status code at about 72px, weight 700, in the same yellow gradient;
|
|
|
|
|
- an h1 Title at 22px;
|
|
|
|
|
- the Message paragraph in #a3adbd, line-height 1.6;
|
|
|
|
|
- a pill link to the Home value: background #f5c55a, colour #1d2940, radius 999px, weight 600, hover #fbd77e, focus-visible outline 2px #fbe3a6;
|
|
|
|
|
- when ShowDetails is true, a definition list in the mono font at 13px on #18223a with radius 6px, labels #8a94a3, values #c5ccd6 and word-break break-all, listing Status, Method, Path and Request ID (Request ID only when non-empty);
|
|
|
|
|
- a caption "Powered by SummerCMS" in #8a94a3.
|
|
|
|
|
Add a media query below 520px that reduces padding and the status size. No script element, no external URL, no web font loading.
|
|
|
|
|
3. Create modules/surf/errorpage.go:
|
|
|
|
|
- `//go:embed` the template and the logo. Parse the template once at package level with template.Must. Build the logo data URI once (encoding/base64) as template.URL.
|
|
|
|
|
- Exported type ErrorPage with fields Status int, Title string, Message string, Home string, ShowDetails bool, Method string, Path string, RequestID string. Doc comments say that Method, Path and RequestID are empty unless ShowDetails is true.
|
|
|
|
|
- Unexported type errorPages with fields app *backpack.App and adminPrefix string, and newErrorPages(app *backpack.App) *errorPages returning a non-nil value. Task 2 adds the kill switch.
|
|
|
|
|
- browserNavigation(r *http.Request) bool implementing QD-02 (a) to (c), with a small hand-written Accept parser (strings.Split on comma, strings.Cut on semicolon, strconv.ParseFloat for q, case-insensitive types, first 32 ranges only). apiStylePath(path string) bool implementing QD-02 (d). Method (p *errorPages) wants(r *http.Request) bool returns false on a nil receiver, otherwise browserNavigation && !apiStylePath (Task 2 adds the admin check).
|
|
|
|
|
- pageFor(status int) returns the title and message. 404: "Page not found" / "The page you are looking for does not exist or has moved.". 405: "Method not allowed" / "This address does not accept that kind of request.". 500: "Something went wrong" / "The server hit an unexpected error. Please try again in a moment.". Any other status: http.StatusText with a generic sentence.
|
|
|
|
|
- Method (p *errorPages) write(w http.ResponseWriter, r *http.Request, status int). It fills ErrorPage with Home "/", executes the template into a bytes.Buffer, sets the QD-09 headers (Set for Content-Type, X-Content-Type-Options, Cache-Control and Content-Security-Policy, and addVary for Accept, Sec-Fetch-Mode and X-Requested-With), deletes Content-Length, then calls WriteHeader(status) and writes the body. If template execution fails, it falls back to http.Error(w, http.StatusText(status), status).
|
|
|
|
|
- Method (p *errorPages) fallback(mux *http.ServeMux) http.Handler. When p.wants(req) is false it calls mux.ServeHTTP directly (QD-04). Otherwise it takes h, pattern := mux.Handler(req). A non-empty pattern (a matched route or a ServeMux redirect) goes to mux.ServeHTTP(w, req). An empty pattern means ServeMux would answer 404 or 405 itself: run h into a small header-and-status capturing ResponseWriter that discards the body, copy a captured Allow header onto w, then call p.write(w, req, captured status).
|
|
|
|
|
4. In modules/surf/router.go add a `pages *errorPages` field to Router. Set r.pages = newErrorPages(app) in BuildRouter right after r.app = app. In compile(), wrap the mux: `r.stripLocalePrefix(pathScopedCORS(r.corsCfg, r.pages.fallback(mux)))`. fallback must be nil-safe: a nil receiver returns mux itself.
|
|
|
|
|
5. Add `embed` to the Standard library list in modules/surf/README.md (Dependencies section, alphabetical). This is the only README change in this task.
|
|
|
|
|
|
|
|
|
|
Do NOT commit; Task 2 commits.
|
|
|
|
|
</action>
|
|
|
|
|
<verify>
|
|
|
|
|
<automated>go vet ./modules/surf && go test -short ./modules/surf -run 'TestErrorPageTracer' -count=1</automated>
|
|
|
|
|
<fails_when>The browser request still gets "404 page not found", the HTML lacks the status, the home link or the data: logo, the HTML contains a script tag, or the curl request's body or Content-Type differs from the stdlib's; go vet reports an issue.</fails_when>
|
|
|
|
|
</verify>
|
|
|
|
|
<acceptance_criteria>
|
|
|
|
|
- TestErrorPageTracer passes; `go test -short ./modules/surf -count=1` still passes as a whole (no existing surf test regresses).
|
|
|
|
|
- `test -s modules/surf/errorpage_sun.webp` succeeds and `file modules/surf/errorpage_sun.webp` reports a WebP image.
|
|
|
|
|
- `grep -c '<script' modules/surf/errorpage.html` prints 0, and `grep -cE 'https?://' modules/surf/errorpage.html` prints 0 (no external asset).
|
|
|
|
|
- `grep -n 'fallback(mux)' modules/surf/router.go` finds the compile() wrap.
|
|
|
|
|
</acceptance_criteria>
|
|
|
|
|
<done>An unmatched browser navigation renders the branded 404 end to end through surf.Assemble, while a curl request gets the stdlib bytes unchanged. Nothing committed yet.</done>
|
|
|
|
|
</task>
|
|
|
|
|
|
|
|
|
|
<task type="auto" tdd="true">
|
|
|
|
|
<name>Task 2: Every covered error site, admin exclusion, details, override hook, kill switch and docs, committed</name>
|
|
|
|
|
<files>modules/surf/errorpage.go, modules/surf/router.go, modules/surf/params.go, modules/surf/overlap.go, modules/surf/uploads.go, modules/surf/accesslog_test.go, modules/surf/errorpage_test.go, modules/surf/README.md, docs/services/routing.md, docs/architecture/request-lifecycle.md, docs/setup/configuration.md</files>
|
|
|
|
|
<read_first>
|
|
|
|
|
- modules/surf/params.go lines 81-94 (constrain)
|
|
|
|
|
- modules/surf/overlap.go lines 386-434 and 469-490 (familyDispatch, registerOverlapFamilies)
|
|
|
|
|
- modules/surf/uploads.go lines 53-122 (mountUploads, uploadsHandler)
|
|
|
|
|
- modules/surf/router.go lines 405-489 (wrap), 634-649 (cabana mount), 778-806 (recoverJSON, recoverBare)
|
|
|
|
|
- modules/surf/accesslog_test.go lines 165-190 (direct recoverJSON/recoverBare calls)
|
|
|
|
|
- docs/services/routing.md sections "Responses" and "Testing and listing routes" (lines 110-215); docs/architecture/request-lifecycle.md lines 34-48; docs/setup/configuration.md lines 50-64
|
|
|
|
|
</read_first>
|
|
|
|
|
<behavior>
|
|
|
|
|
- A browser navigation to a constrained route whose parameter fails Where answers the branded 404; the same request from curl answers the stdlib 404 text.
|
|
|
|
|
- A browser navigation that panics in a normal route and in a raw group answers the branded 500 without the panic message; curl gets wire's opaque JSON 500 and the empty raw 500 respectively.
|
|
|
|
|
- With http.error_pages.enabled false a browser navigation to an unmatched path answers the stdlib 404 text.
|
|
|
|
|
</behavior>
|
|
|
|
|
<action>
|
|
|
|
|
Expand from the tracer. Add each behavior case to errorpage_test.go first as a quick RED check, then:
|
|
|
|
|
|
|
|
|
|
1. errorpage.go, kill switch (QD-08): newErrorPages returns nil when app and app.Config are non-nil, `http.error_pages.enabled` is present and false. Missing app, missing config or an absent key keep error pages on.
|
|
|
|
|
2. errorpage.go, admin exclusion (QD-07): add a nil-safe setAdminPrefix(prefix string). wants() also returns false when adminPrefix is non-empty and the path equals it or starts with prefix + "/" (`/backendx` is not under `/backend`). In router.go BuildRouter, call r.pages.setAdminPrefix(admin.Prefix) right after r.adminPrefix = admin.Prefix.
|
|
|
|
|
3. errorpage.go, details (QD-06): add a helper showDetails(app) bool. In write(), when it is true, fill Method, Path (r.URL.Path) and RequestID (strings.TrimSpace of the X-Request-Id request header, cut to 128 bytes) and set ShowDetails. Never pass the recovered panic value anywhere near the page.
|
|
|
|
|
4. errorpage.go, override (QD-05): add the exported interface ErrorPageRenderer with RenderErrorPage(r *http.Request, page ErrorPage) (body []byte, ok bool). Its doc comment says it is optional and published through backpack, that surf sets the status and the QD-09 headers, and that ok=false keeps the built-in page. In write(), when p.app is non-nil and app.Lookup[ErrorPageRenderer]() finds a non-nil renderer, call it inside a func with a deferred recover. Use its body when ok is true and the body is non-empty; otherwise, or after a recovered panic, render the built-in template.
|
|
|
|
|
5. errorpage.go, call-site helpers: (p *errorPages) notFound(w, r) does p.write(w, r, 404) when p.wants(r), else http.NotFound(w, r). (p *errorPages) methodNotAllowed(w, r) assumes the caller already set Allow and does p.write(w, r, 405) when p.wants(r), else http.Error(w, http.StatusText(405), 405).
|
|
|
|
|
6. params.go: give constrain a first parameter pages *errorPages and replace its http.NotFound with pages.notFound. router.go wrap(): pass r.pages at both constrain calls.
|
|
|
|
|
7. overlap.go: add a pages *errorPages field to familyDispatch, and a pages *errorPages parameter to registerOverlapFamilies (passed from compile() as r.pages). Replace both http.NotFound calls with f.pages.notFound and the http.Error 405 with f.pages.methodNotAllowed (Allow is still set first).
|
|
|
|
|
8. uploads.go: give uploadsHandler a pages *errorPages parameter (mountUploads passes r.pages; mountUploads already runs after r.pages is set) and replace its two http.NotFound calls with pages.notFound. Leave attach.StaticHandlerPublic untouched (QD-03).
|
|
|
|
|
9. router.go: recoverJSON(pages *errorPages, next) writes p.write(w, r, 500) when pages.wants(r), else wire.WriteOpaque500(w). recoverBare(pages *errorPages, next) writes the page when pages.wants(r), else the bare WriteHeader(500). markPanicked stays first in both. Update the three wrap() call sites to pass r.pages, and the three direct calls in accesslog_test.go to pass nil.
|
|
|
|
|
10. Docs, in this same commit (the project rule for exported API and config keys):
|
|
|
|
|
- modules/surf/README.md: add a Features bullet "Branded error pages for browsers", stating the QD-02 rule in one or two sentences, the covered errors (QD-03), that every other request keeps the exact stdlib or wire body, details only with app.debug or outside production, the admin prefix excluded, the CSP and no-store headers, and the override through a published surf.ErrorPageRenderer. Add API reference rows for `surf.ErrorPage` and `surf.ErrorPageRenderer`. Add a Configuration row for `http.error_pages.enabled` (default `true`, environment `SUMMER_HTTP__ERROR_PAGES__ENABLED`). Leave the H1 summary sentence unchanged, so the root README table does not change.
|
|
|
|
|
- docs/services/routing.md: add a section `## Error pages` right after `## Responses`. Cover what a browser sees versus an API client, the exact negotiation rule, the covered and uncovered errors, the details rule, the kill switch, and a short Go example of a plugin publishing an ErrorPageRenderer in Register with `app.Publish[surf.ErrorPageRenderer](...)`. In the example the renderer returns its own page for status 404 and ok=false otherwise; use the neutral plugin id `acme.theme`. Also update the overlap bullet at line 59 ("the same bare 404") so it says browsers get the error page.
|
|
|
|
|
- docs/architecture/request-lifecycle.md: in step 4 and in the raw-group paragraph, say that a browser navigation gets the HTML 500 page while other callers keep the opaque JSON 500 or the bare 500. Add a sentence that unmatched paths and wrong methods are negotiated the same way at the ServeMux.
|
|
|
|
|
- docs/setup/configuration.md: add a table row for `http.error_pages.enabled` next to the `http.access_log` row: "HTML error pages for browser navigations (defaults to `true`); request details appear when `app.debug` is true or outside production", linking surf.
|
|
|
|
|
11. Run the full verify. Then commit by explicit path: the four new or changed modules/surf code files from Task 1 (errorpage.go, errorpage.html, errorpage_sun.webp, errorpage_test.go), router.go, params.go, overlap.go, uploads.go, accesslog_test.go, modules/surf/README.md, and the three docs pages. Message: `feat(surf): branded HTML error pages for browser navigations`, with a short body naming the negotiation rule and the unchanged API bodies. No trailer of any kind.
|
|
|
|
|
</action>
|
|
|
|
|
<verify>
|
|
|
|
|
<automated>go vet ./... && go test -short ./... && go -C examples/hello test -short ./... && go test ./cmd/summer -run TestDocsTree -count=1 && go run ./cmd/summer docs:build --check && bash -c '! grep -rniE "summercmsio|fonoteka|plytarium|wavepath|grzyby" modules/surf/errorpage.go modules/surf/errorpage.html modules/surf/README.md docs/services/routing.md docs/architecture/request-lifecycle.md docs/setup/configuration.md' && bash -c 'C=$(git log -1 --format=%H --fixed-strings --grep="feat(surf): branded HTML error pages for browser navigations") && test -n "$C" && B=$(git log -1 --format=%B "$C") && F=$(git diff --name-only "$C^..$C") && test -n "$F" && ! grep -qi -e co-authored -e "generated with" <<< "$B" && test -z "$(grep -v -e "^modules/surf/" -e "^docs/services/routing\.md$" -e "^docs/architecture/request-lifecycle\.md$" -e "^docs/setup/configuration\.md$" <<< "$F")"'</automated>
|
|
|
|
|
<fails_when>Any package fails vet or tests (including an existing surf test whose expected 404/405/500 bytes moved), the docs checker rejects an identifier or link, a consuming-application name appears in the new code or docs, or the feat commit is missing, carries an attribution trailer or includes files outside modules/surf and the three docs pages.</fails_when>
|
|
|
|
|
</verify>
|
|
|
|
|
<acceptance_criteria>
|
|
|
|
|
- `grep -c 'http.NotFound' modules/surf/params.go modules/surf/uploads.go` prints 0 for both files. In modules/surf/overlap.go, `grep -c 'http.NotFound(w, r)'` prints 0; the remaining http.NotFoundHandler reference at the noopHandler declaration is expected.
|
|
|
|
|
- `grep -n 'type ErrorPageRenderer interface' modules/surf/errorpage.go` and `grep -n 'type ErrorPage struct' modules/surf/errorpage.go` each find one line.
|
|
|
|
|
- `grep -n 'http.error_pages.enabled' modules/surf/README.md docs/setup/configuration.md docs/services/routing.md` finds at least one line per file; `grep -n '^## Error pages' docs/services/routing.md` finds one line.
|
|
|
|
|
- `grep -n 'setAdminPrefix' modules/surf/router.go` finds the call after the cabana mount.
|
|
|
|
|
- `git status --short -- .planning/phases` shows no change made by this task.
|
|
|
|
|
</acceptance_criteria>
|
|
|
|
|
<done>All covered sites negotiate, the admin is excluded, details follow QD-06, the override and the kill switch work, the docs describe it, and the whole repo is green. One feat commit with code and docs, nothing pushed.</done>
|
|
|
|
|
</task>
|
|
|
|
|
|
|
|
|
|
<task type="auto" tdd="true">
|
|
|
|
|
<name>Task 3: Unit tests: negotiation table, API bytes unchanged, page content, details, override, admin, uploads</name>
|
|
|
|
|
<files>modules/surf/errorpage_test.go</files>
|
|
|
|
|
<read_first>
|
|
|
|
|
- modules/surf/errorpage.go (as written in Tasks 1-2: browserNavigation, apiStylePath, errorPages, wants, write)
|
|
|
|
|
- modules/surf/admin_prefix_test.go lines 100-160 (adminPrefixApp, adminPrefixPlugin)
|
|
|
|
|
- modules/surf/uploads_test.go (how a storage.uploads.serve app is configured without a published bucket)
|
|
|
|
|
- modules/surf/overlap_test.go (an overlap family fixture to reuse: two routes such as /shelves/items/{id} and /shelves/{shelfId}/follow with numeric Where)
|
|
|
|
|
</read_first>
|
|
|
|
|
<behavior>
|
|
|
|
|
- TestErrorPageNegotiation (table on browserNavigation + wants with adminPrefix "/backend"). The HTML page applies to: Chrome navigation (Chrome Accept + Sec-Fetch-Mode navigate); POST with Sec-Fetch-Mode navigate; GET and HEAD with Accept text/html and no Sec-Fetch headers; "text/html, application/json;q=0.5"; path "/backendx".
|
|
|
|
|
Today's response applies to: Accept "*/*"; no Accept; Accept text/html with Sec-Fetch-Mode cors; X-Requested-With XMLHttpRequest; "application/json"; "application/json, text/html"; "text/html;q=0.5, application/json;q=0.9"; "text/html;q=0"; "application/vnd.api+json, text/html;q=0.1"; POST without Sec-Fetch-Mode; paths "/api", "/api/blog/posts", "/_acme/feed", "/backend", "/backend/users". A header of 40 ranges with text/html only at position 35 also gets today's response (only the first 32 are read).
|
|
|
|
|
- TestErrorPagesKeepAPIBodies: two routers from the same routes, one default and one with http.error_pages.enabled false, both via surf.Assemble on transportApp. The routes: GET /api/ping, POST /blog/submit, GET /posts/{id} with Where id numeric, a normal GET /boom and a GroupRaw GET /raw/boom that both panic, and the overlap family pair. For curl-style, fetch-style (Sec-Fetch-Mode cors, Accept */*), XHR and JSON-Accept requests to /nowhere, GET /blog/submit (405), /posts/abc, /boom, /raw/boom and the overlap miss and wrong-method paths, status, body bytes, Content-Type and Allow are identical between the two routers. Spot-check the literals "404 page not found\n", "Method Not Allowed\n" and {"error":true,"message":"Internal server error"}.
|
|
|
|
|
- TestErrorPageHTML: browser navigations to the same targets get text/html with statuses 404 (/nowhere), 405 (GET /blog/submit, Allow equal to the disabled router's Allow), 404 (/posts/abc), 500 (/boom), 500 (/raw/boom), 404 (overlap miss) and 405 (overlap wrong method, sent as a POST navigation with Sec-Fetch-Mode navigate because both family members are GET routes). Each body contains the status number, `href="/"`, "data:image/webp;base64," and no "<script"; the 500 bodies never contain the panic message. Headers: Cache-Control no-store, nosniff, Vary containing Accept, CSP containing "frame-ancestors 'none'". A HEAD navigation gets the 404 status and headers with an empty body.
|
|
|
|
|
- TestErrorPageDetails: in the production environment with app.debug absent, a navigation to /nowhere-detail-marker with X-Request-Id "req-123" contains neither the marker nor "req-123". The same in production with app.debug true, and in development, contains both plus "GET". A path containing "<b>x</b>" (URL-escaped in the request) appears HTML-escaped, never as a raw "<b>". A 300-byte X-Request-Id is cut to 128 bytes.
|
|
|
|
|
- TestErrorPageRendererOverride: a published renderer returning "<p>custom 404</p>" for status 404 and ok=false otherwise. /nowhere gets the custom body with status 404 and the QD-09 headers, and a panicking route still gets the built-in 500 page. A renderer that panics gives the built-in 404 page with status 404. A renderer that returns an empty body with ok=true gives the built-in page.
|
|
|
|
|
- TestErrorPageAdminUntouched: with adminPrefixApp and adminPrefixPlugin (admin at /acme-admin), a browser navigation to /acme-admin/no-such-screen gives byte-identical status, body and Content-Type between the default router and the disabled one.
|
|
|
|
|
- TestErrorPageUploads: storage.uploads.serve true with no bucket or DB published. A browser navigation to the uploads prefix + /a/b.png gets the branded 404 with the page CSP (not the uploads sandbox CSP); a request with Accept "image/avif,image/webp,*/*" and Sec-Fetch-Mode no-cors gets "404 page not found\n" with the uploads CSP.
|
|
|
|
|
</behavior>
|
|
|
|
|
<action>
|
|
|
|
|
Extend modules/surf/errorpage_test.go (keep TestErrorPageTracer). Write the seven tests in behavior as table-driven tests where it fits. Use small helpers: browserReq(method, path) sets the Chrome navigation headers, curlReq(method, path) sets Accept */*, and twoRouters(t, env, files, plugins) returns the default and the disabled handler built with surf.Assemble. Reuse transportApp, routesPluginFunc, adminPrefixApp and adminPrefixPlugin rather than new fixtures. Use only neutral names (acme, blog). Keep every test free of Docker so it runs under -short. If a test exposes a defect in Task 2's code, fix the code in errorpage.go (or the call site) and include that file in this commit, with a sentence in the commit body.
|
|
|
|
|
|
|
|
|
|
Run the full verify, then commit by explicit path (modules/surf/errorpage_test.go, plus any code file a defect fix touched) with message `test(surf): cover error page negotiation, unchanged API bodies and overrides`, and no trailer.
|
|
|
|
|
</action>
|
|
|
|
|
<verify>
|
|
|
|
|
<automated>go vet ./... && go test -short ./modules/surf -run 'TestErrorPage' -count=1 -v && go test -short ./... && bash -c 'C=$(git log -1 --format=%H --fixed-strings --grep="test(surf): cover error page negotiation, unchanged API bodies and overrides") && test -n "$C" && B=$(git log -1 --format=%B "$C") && F=$(git diff --name-only "$C^..$C") && ! grep -qi -e co-authored -e "generated with" <<< "$B" && grep -qx "modules/surf/errorpage_test.go" <<< "$F"'</automated>
|
|
|
|
|
<fails_when>Any of the seven tests fails or is missing from the -v output, an API-style request's bytes differ between the default and disabled routers, a detail leaks in production, an override or fallback case misbehaves, the repo-wide short tests fail, or the test commit is missing, carries an attribution trailer or does not contain errorpage_test.go.</fails_when>
|
|
|
|
|
</verify>
|
|
|
|
|
<acceptance_criteria>
|
|
|
|
|
- `go test -short ./modules/surf -run 'TestErrorPage' -count=1 -v` lists TestErrorPageTracer, TestErrorPageNegotiation, TestErrorPagesKeepAPIBodies, TestErrorPageHTML, TestErrorPageDetails, TestErrorPageRendererOverride, TestErrorPageAdminUntouched and TestErrorPageUploads, all PASS.
|
|
|
|
|
- `grep -c 'error_pages' modules/surf/errorpage_test.go` prints at least 1 (the disabled-router comparison exists).
|
|
|
|
|
- `git log -2 --format=%s` shows the test commit on top of the feat commit.
|
|
|
|
|
</acceptance_criteria>
|
|
|
|
|
<done>The negotiation rule, unchanged API bytes, page content and headers, details gating, the override and its fallbacks, the admin exclusion and the uploads case are pinned by tests; one test commit; nothing pushed or tagged.</done>
|
|
|
|
|
</task>
|
|
|
|
|
|
|
|
|
|
</tasks>
|
|
|
|
|
|
|
|
|
|
<threat_model>
|
|
|
|
|
## Trust Boundaries
|
|
|
|
|
|
|
|
|
|
| Boundary | Description |
|
|
|
|
|
|----------|-------------|
|
|
|
|
|
| client to surf | Request path, method and headers (Accept, Sec-Fetch-Mode, X-Requested-With, X-Request-Id) are attacker-controlled and drive both negotiation and page content |
|
|
|
|
|
| surf to shared caches/CDN | Error responses may be stored by an intermediary and replayed to other clients |
|
|
|
|
|
| application/plugin renderer to surf | A published ErrorPageRenderer is in-process code that runs on the error path, including inside panic recovery |
|
|
|
|
|
|
|
|
|
|
## STRIDE Threat Register
|
|
|
|
|
|
|
|
|
|
| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan |
|
|
|
|
|
|-----------|----------|-----------|----------|-------------|-----------------|
|
|
|
|
|
| T-42e-01 | Information disclosure | errorPages.write details (QD-06) | medium | mitigate | Method, path and request id only when app.debug is true or the environment is set and not production; nil/empty environment counts as production; the panic value and stack are never passed to the page; TestErrorPageDetails and TestErrorPageHTML assert absence |
|
|
|
|
|
| T-42e-02 | Tampering (reflected XSS) | errorpage.html Path / RequestID, override pages | high | mitigate | html/template contextual escaping for every dynamic value; RequestID trimmed to 128 bytes; CSP default-src 'none' with no script-src, so no script runs on any error page, override pages included; TestErrorPageDetails asserts an escaped `<b>` |
|
|
|
|
|
| T-42e-03 | Tampering (contract regression) | API and programmatic responses | high | mitigate | QD-02 needs explicit text/html beating every JSON type, Sec-Fetch-Mode navigate when present, no X-Requested-With, not /api or /_ paths, not the admin prefix; non-matching requests run the exact pre-change call; TestErrorPagesKeepAPIBodies compares bytes with the disabled router |
|
|
|
|
|
| T-42e-04 | Spoofing (clickjacking) | error page | low | mitigate | frame-ancestors 'none' in the page CSP |
|
|
|
|
|
| T-42e-05 | Tampering (cache poisoning) | shared caches serving HTML to API clients | medium | mitigate | HTML error responses send Cache-Control: no-store and Vary: Accept, Sec-Fetch-Mode, X-Requested-With; the API path is unchanged, so a cached plain response stays harmless |
|
|
|
|
|
| T-42e-06 | Denial of service | Accept parsing | low | mitigate | Only the first 32 media ranges are read, with no regex; runs only on error paths and the fallback check |
|
|
|
|
|
| T-42e-07 | Denial of service | ErrorPageRenderer panic inside recoverJSON's deferred recovery | medium | mitigate | The renderer call runs under its own deferred recover and falls back to the built-in page; TestErrorPageRendererOverride covers a panicking renderer |
|
|
|
|
|
| T-42e-08 | Elevation of privilege | uploads route CSP replaced on its 404 page | low | accept | The replaced response body is the framework's own page, not user content, and the page CSP still forbids scripts and framing; successful upload responses keep the sandbox CSP |
|
|
|
|
|
| T-42e-SC | Tampering | npm/pip/cargo installs | low | accept | No package installs and no new third-party Go dependency; only stdlib embed and html/template |
|
|
|
|
|
</threat_model>
|
|
|
|
|
|
|
|
|
|
<verification>
|
|
|
|
|
- go vet ./... && go test -short ./... && go -C examples/hello test -short ./...
|
|
|
|
|
- go test ./cmd/summer -run TestDocsTree -count=1 && go run ./cmd/summer docs:build --check
|
|
|
|
|
- Manual sanity (optional, not gating): run any application's `serve` with no frontend in front, open http://127.0.0.1:8080/ in a browser to see the branded 404, and check that `curl -i http://127.0.0.1:8080/` still prints "404 page not found".
|
|
|
|
|
- git log -2 shows the feat and test commits, both without trailers; git status shows no edits under .planning/phases/19.4.1-*.
|
|
|
|
|
</verification>
|
|
|
|
|
|
|
|
|
|
<success_criteria>
|
|
|
|
|
- Browser navigations to framework-generated 404/405/500 get the branded SummerCMS page; every other request gets byte-identical responses to the disabled router.
|
|
|
|
|
- Applications can replace the page through surf.ErrorPageRenderer and switch the feature off with http.error_pages.enabled.
|
|
|
|
|
- The surf README, routing, request-lifecycle and configuration docs describe it, and the docs checker passes.
|
|
|
|
|
- Two commits, nothing pushed or tagged.
|
|
|
|
|
</success_criteria>
|
|
|
|
|
|
|
|
|
|
<output>
|
|
|
|
|
Create `.planning/quick/261020-42e-branded-html-error-pages-for-browsers-un/261020-42e-SUMMARY.md` when done. List the QD-01..QD-09 decisions as implemented, the two commit hashes, and the known uncovered sites (attach's missing-row 404, middleware refusals). The orchestrator moves the todo to done, updates STATE through `msd_run query state.*` handlers and posts the g15office comment on jh5qzjba; the executor does none of these.
|
|
|
|
|
</output>
|
