app: mount CSRF, add the list screen's create control
Fix round 1. Mounts csrf.Protect app-wide over the router (Config.Origin, defaulting to DefaultOrigin; main.go derives it from the resolved -addr) rather than leaving POST /documents unprotected — the prior refusal was right to flag the missing origin config but wrong to leave the route open; the fix is a same-origin test, not a skipped protection. TestCreateWrongOriginRejected proves it actually refuses a cross-origin POST and writes nothing. Also adds the create form itself to list.html's page header, outside the empty/populated branch so it reaches a person with no documents exactly as it reaches everyone else — the plan had scoped a create control out of every task, so the browser-drivable flow it exists for had no way to be driven from a browser at all.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011F5CivVm4CmL9ByfSSAaXA
6 files changed,
+146
−24
cmd/docs/main.go+14 −0internal/docs/app.go+34 −11internal/docs/handlers.go+18 −8internal/docs/list_test.go+41 −0internal/docs/render.go+10 −4internal/docs/views/list.html+29 −1
diff --git a/cmd/docs/main.go b/cmd/docs/main.go| index 02c4427..187b208 100644 |
| --- a/cmd/docs/main.go |
| +++ b/cmd/docs/main.go |
| @@ -25,6 +25,19 @@ func main() { |
| os.Exit(1) |
| } |
| + // Best-effort Origin for csrf.Protect (docs.Config.Origin's doc |
| + // comment): when the platform handed us a TCP -addr, that is |
| + // exactly the host a browser loading this instance sends back as |
| + // its Origin header, so derive from it rather than leaving the |
| + // placeholder default in place. Empty (unix socket / LISTEN_FDS, |
| + // or no -addr given) falls through to docs.DefaultOrigin — the |
| + // real per-instance origin (spec §9.2) is platform wiring Task 6 |
| + // adds, not this line. |
| + origin := docs.DefaultOrigin |
| + if opts.Addr != "" { |
| + origin = "http://" + opts.Addr |
| + } |
| + |
| // New opens its own database handle (SKILL.md §1's "app that opens |
| // its own database"), so opts.DBPath is blanked below before Serve |
| // — otherwise Serve would open a second connection to the file New |
| @@ -35,6 +48,7 @@ func main() { |
| app, err := docs.New(docs.Config{ |
| DBPath: opts.DBPath, |
| Env: "production", |
| + Origin: origin, |
| Logger: logger, |
| }) |
| if err != nil { |
diff --git a/internal/docs/app.go b/internal/docs/app.go| index 5619382..c9c8499 100644 |
| --- a/internal/docs/app.go |
| +++ b/internal/docs/app.go |
| @@ -17,6 +17,7 @@ import ( |
| "log/slog" |
| "net/http" |
| + "github.com/carlosframework/rastrillo/csrf" |
| "github.com/carlosframework/rastrillo/db" |
| "github.com/carlosframework/rastrillo/migrate" |
| "github.com/go-chi/chi/v5" |
| @@ -41,10 +42,31 @@ type Config struct { |
| // tasks' tests already depend on, never needs a signature change. |
| DevAuth bool |
| + // Origin is this app's own origin (scheme://host[:port], no path) |
| + // — csrf.Protect checks every POST/PUT/PATCH/DELETE's |
| + // Origin/Referer against exactly this string (rastrillo SKILL.md |
| + // §5). Empty defaults to DefaultOrigin. |
| + // |
| + // This is a placeholder, not a resolved platform value: Resolve's |
| + // Options carries Addr/Socket, not an origin, and the instance's |
| + // real public hostname (spec §9.2's <app>.<sqid>.oncarlos.com) |
| + // only exists once that platform wiring lands — Task 6's |
| + // dev-run/deploy work, not this one. cmd/docs/main.go derives a |
| + // best-effort Origin from the resolved -addr when one was given |
| + // (matching what a browser hitting that address sends), and falls |
| + // back to DefaultOrigin otherwise. |
| + Origin string |
| + |
| // Logger defaults to slog.Default(). |
| Logger *slog.Logger |
| } |
| +// DefaultOrigin is Config.Origin's fallback when empty — see its doc |
| +// comment. Exported so a caller that needs to know what origin this |
| +// app is protecting itself against (NewForTest's callers, notably) |
| +// does not have to duplicate the literal. |
| +const DefaultOrigin = "http://localhost:8080" |
| + |
| // App is the CARLOS Docs rastrillo app. |
| type App struct { |
| cfg Config |
| @@ -63,10 +85,10 @@ var BootSchema = migrate.Merge(Schema) |
| // New opens the database, applies migrations and wires the router. It |
| // is rastrillo SKILL.md's app.go shape: db.Open for the SQLite handle, |
| // migrate.Apply once at boot, a chi router wrapped as an |
| -// *http.ServeMux so httptest exercises the real router. It does not |
| -// wire sessions, CSRF or an identity plugin — this task has no |
| -// mutating route and no signed-in route yet, so there is nothing for |
| -// either to protect; both land with the tasks that add them. |
| +// *http.ServeMux so httptest exercises the real router, csrf.Protect |
| +// mounted app-wide over POST /documents (SKILL.md §5). It does not |
| +// wire sessions or an identity plugin — there is no signed-in route |
| +// yet; that lands with Task 5. |
| func New(cfg Config) (*App, error) { |
| logger := cfg.Logger |
| if logger == nil { |
| @@ -75,6 +97,9 @@ func New(cfg Config) (*App, error) { |
| if cfg.Env == "" { |
| cfg.Env = "production" |
| } |
| + if cfg.Origin == "" { |
| + cfg.Origin = DefaultOrigin |
| + } |
| d, err := db.Open(cfg.DBPath, logger) |
| if err != nil { |
| @@ -106,14 +131,12 @@ func New(cfg Config) (*App, error) { |
| h := &handlers{render: render, logger: logger, store: store} |
| r := chi.NewRouter() |
| + // Mounted above every route, per rastrillo SKILL.md §5: refuses a |
| + // cross-origin POST/PUT/PATCH/DELETE by Sec-Fetch-Site/Origin/ |
| + // Referer evidence; GET/HEAD/OPTIONS pass untouched, so this does |
| + // nothing to "/" or "/d/{sqid}" and everything to "/documents". |
| + r.Use(csrf.Protect(cfg.Origin)) |
| r.Get("/", h.list) |
| - // csrf.Protect(origin) is not mounted here — rastrillo SKILL.md §5 |
| - // asks for it app-wide, but Protect needs a fixed origin and this |
| - // app has none yet (no sessions, no Resolve-derived origin: both |
| - // land with Task 6's dev-run wiring). POST /documents is the |
| - // app's only mutating route so far; it gets the same origin-check |
| - // protection the moment Task 6 gives this app an origin to check |
| - // against. |
| r.Post("/documents", h.create) |
| r.Get("/d/{sqid}", h.show) |
| r.Handle("/static/*", render.assets.Handler()) |
diff --git a/internal/docs/handlers.go b/internal/docs/handlers.go| index b29f90c..4b86831 100644 |
| --- a/internal/docs/handlers.go |
| +++ b/internal/docs/handlers.go |
| @@ -26,11 +26,19 @@ type handlers struct { |
| // labels are resolved through rastrillo.T from Catalog (render.go); |
| // a row's Title and When are the document's own data, not authored |
| // copy, so list.html prints them directly rather than through T. |
| +// |
| +// CreateLabel and CreatePlaceholder back the create form list.html |
| +// renders in its page header — present regardless of Groups, so it |
| +// reaches a person with no documents yet exactly as it reaches |
| +// everyone else (fix round 1: the original version of this screen |
| +// had no path to POST /documents at all outside a hand-typed curl). |
| type listPage struct { |
| - Title string |
| - Groups []docGroup |
| - EmptyTitle string |
| - EmptyBody string |
| + Title string |
| + Groups []docGroup |
| + EmptyTitle string |
| + EmptyBody string |
| + CreateLabel string |
| + CreatePlaceholder string |
| } |
| // docGroup is one recency bucket ("Today", "This week", ...) and the |
| @@ -71,10 +79,12 @@ func (h *handlers) list(w http.ResponseWriter, r *http.Request) { |
| } |
| data := listPage{ |
| - Title: rastrillo.T(r, "docs.list.title"), |
| - Groups: groupByRecency(r, time.Now().UTC(), docs), |
| - EmptyTitle: rastrillo.T(r, "docs.list.empty.title"), |
| - EmptyBody: rastrillo.T(r, "docs.list.empty.body"), |
| + Title: rastrillo.T(r, "docs.list.title"), |
| + Groups: groupByRecency(r, time.Now().UTC(), docs), |
| + EmptyTitle: rastrillo.T(r, "docs.list.empty.title"), |
| + EmptyBody: rastrillo.T(r, "docs.list.empty.body"), |
| + CreateLabel: rastrillo.T(r, "docs.list.create.label"), |
| + CreatePlaceholder: rastrillo.T(r, "docs.list.create.placeholder"), |
| } |
| if err := h.render.page(w, r, "list", data); err != nil { |
| h.logger.Error("docs: render list", "err", err) |
diff --git a/internal/docs/list_test.go b/internal/docs/list_test.go| index dfc5d1a..9236f21 100644 |
| --- a/internal/docs/list_test.go |
| +++ b/internal/docs/list_test.go |
| @@ -69,12 +69,23 @@ func TestEmptyListSaysSo(t *testing.T) { |
| } |
| } |
| +// TestCreateThenRead posts as a real browser would: NewForTest's app |
| +// carries no explicit Config.Origin, so it protects itself with |
| +// docs.DefaultOrigin (New's own fallback), and a browser submitting a |
| +// same-origin <form method="post"> to that origin sends exactly this |
| +// Origin header — csrf.Protect checks it and must let the request |
| +// through. Fix round 1: the original version of this test sent no |
| +// Origin/Referer/Sec-Fetch-Site at all, which is not what any browser |
| +// sends and would (correctly) be refused once CSRF protection was |
| +// mounted — the test was the wrong model of a browser, not the |
| +// protection. |
| func TestCreateThenRead(t *testing.T) { |
| app, _ := docs.NewForTest(t) |
| form := strings.NewReader("title=Minutes") |
| req := httptest.NewRequest(http.MethodPost, "/documents", form) |
| req.Header.Set("Content-Type", "application/x-www-form-urlencoded") |
| + req.Header.Set("Origin", docs.DefaultOrigin) |
| rec := httptest.NewRecorder() |
| app.Handler().ServeHTTP(rec, req) |
| @@ -97,6 +108,36 @@ func TestCreateThenRead(t *testing.T) { |
| } |
| } |
| +// TestCreateWrongOriginRejected proves csrf.Protect is actually wired |
| +// rather than silently a no-op: a cross-origin POST — an attacker's |
| +// page submitting to this app in a signed-in visitor's browser, the |
| +// exact shape CSRF protection exists to stop — must be refused, not |
| +// merely "not tested for success". |
| +func TestCreateWrongOriginRejected(t *testing.T) { |
| + app, _ := docs.NewForTest(t) |
| + |
| + form := strings.NewReader("title=Minutes") |
| + req := httptest.NewRequest(http.MethodPost, "/documents", form) |
| + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") |
| + req.Header.Set("Origin", "https://evil.example") |
| + rec := httptest.NewRecorder() |
| + app.Handler().ServeHTTP(rec, req) |
| + |
| + if rec.Code != http.StatusForbidden { |
| + t.Fatalf("POST /documents with wrong Origin: status %d, want 403", rec.Code) |
| + } |
| + |
| + // And confirm nothing was actually created — the refusal is real, |
| + // not just a wrong status code on an otherwise-successful write. |
| + rows, err := app.Store().List(context.Background()) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if len(rows) != 0 { |
| + t.Errorf("a cross-origin POST created %d document(s); want 0", len(rows)) |
| + } |
| +} |
| + |
| func TestUnknownDocument404s(t *testing.T) { |
| app, _ := docs.NewForTest(t) |
| req := httptest.NewRequest(http.MethodGet, "/d/nosuchthing", nil) |
diff --git a/internal/docs/render.go b/internal/docs/render.go| index 45c0c51..c353638 100644 |
| --- a/internal/docs/render.go |
| +++ b/internal/docs/render.go |
| @@ -39,13 +39,19 @@ var Catalog = rastrillo.Catalog{ |
| "docs.list.group.earlier": "Earlier", |
| // docs.list.empty.* — the empty state (Paul's ruling: "An empty |
| - // list is a designed screen, not a blank page"). It does not |
| - // promise a button: Task 4 is what makes creating a document |
| - // possible, and this task must not render a call to action for a |
| - // route that does not exist yet. |
| + // list is a designed screen, not a blank page"). |
| "docs.list.empty.title": "No documents yet", |
| "docs.list.empty.body": "Documents you create will show up here, newest first.", |
| + // docs.list.create.* — the create control in list.html's page |
| + // header, present whether or not Groups is empty (fix round 1: |
| + // the plan had scoped a create *control* out of every task, so the |
| + // screen this plan exists for could not be driven from a browser |
| + // at all). label is both the submit button's text and the title |
| + // field's accessible name; placeholder is that field's hint text. |
| + "docs.list.create.label": "New document", |
| + "docs.list.create.placeholder": "Title", |
| + |
| // docs.error.generic is the one fallback error string this task |
| // has: a render failure with nowhere styled to land yet (SKILL.md |
| // §7's view.Fail/ErrorPage wiring is later work). Still a catalog |
diff --git a/internal/docs/views/list.html b/internal/docs/views/list.html| index c84292f..fd1167f 100644 |
| --- a/internal/docs/views/list.html |
| +++ b/internal/docs/views/list.html |
| @@ -3,13 +3,23 @@ |
| through rastrillo.T, EmptyTitle/EmptyBody the same; a row's Title |
| and When are the document's own data and are printed as-is. |
| + The header's <form> is the app's one create control (fix round |
| + 1): a plain POST to /documents, no JavaScript, sitting beside the |
| + titles the same way page-header's own single-link ActionHref |
| + would — [rst-page-header]'s flex/space-between rule lays it out |
| + as the second flex child without any markup of its own. It is |
| + outside the {{if .Groups}} branch below on purpose: the header |
| + renders unconditionally, so the control reaches a person with no |
| + documents yet exactly the same way it reaches everyone else, |
| + rather than only appearing once there is something to list. |
| + |
| Writes the bare rst-* attribute grammar throughout — the rst- |
| vocabulary always as a bare attribute, never as a class — which is |
| this app's whole-screen markup convention (Task 1's home page set |
| it). render_test.go's TestOurTemplatesUseTheAttributeGrammar walks |
| this file's own bytes to hold that; rastrillo's vendored shell and |
| partials keep their own class spelling, which is their output, not |
| - this app's, and is out of that test's reach on purpose. The three |
| + this app's, and is out of that test's reach on purpose. The |
| [docs-*] selectors below are this app's own vocabulary, not |
| rastrillo's — SKILL.md §7's "ui is rastrillo's vocabulary, not |
| yours" is why they are not spelled rst-*, and they carry their |
| @@ -45,11 +55,29 @@ |
| color: var(--rst-text-muted); |
| font-size: var(--rst-fs-sm); |
| } |
| + /* [docs-create-form]: the title field and its submit button as one |
| + inline row, matching page-header's own single-action slot — it is |
| + the header's second flex child, so [rst-page-header]'s |
| + space-between rule places it opposite the titles with no layout |
| + rule of its own beyond this. */ |
| + [docs-create-form] { |
| + display: flex; |
| + flex-wrap: wrap; |
| + gap: var(--rst-sp-2); |
| + } |
| + [docs-create-form] [rst-input] { |
| + inline-size: 14rem; |
| + max-inline-size: 100%; |
| + } |
| </style> |
| <header rst-page-header> |
| <div rst-page-header-titles> |
| <h1>{{.Title}}</h1> |
| </div> |
| + <form docs-create-form method="post" action="/documents"> |
| + <input rst-input type="text" name="title" placeholder="{{.CreatePlaceholder}}" aria-label="{{.CreateLabel}}" autocomplete="off"> |
| + <button rst-btn="primary" type="submit">{{.CreateLabel}}</button> |
| + </form> |
| </header> |
| {{if .Groups}} |
| <div rst-list> |