Docs: mark R01-N32/N33/N34 accepted-by-design

All three findings ship with audit-side "Suggested fix: None" / "None
required" — closing disposition was the only thing missing. R01-N32:
server-side confirm_name check at SprintController::delete is
authoritative; data-confirm-name is JS UX. R01-N33: data-csrf is the
standard fetch/htmx pattern, token rotation + strict CSP keep it safe.
R01-N34: migrations/ is committed code, post-R01-N22 apply runs at
deploy time only. No open findings remain in REVIEW_01.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

doc/REVIEW_01.md

@@ -1061,40 +1061,61 @@ note. Do not delete entries — they're history.
 
 ### R01-N32 — Sprint settings exposes the to-be-deleted name in plain HTML
 - **Severity**: LOW.
-- **Status**: open.
-- **Where**: `views/sprints/settings.twig` line 181 (`data-confirm-name=…`).
-- **What**: The expected confirmation string (the sprint's own name) is
-  emitted in a `data-` attribute so the JS can compare against typed input.
-  An attacker who can read the page already has the name from the title
-  bar — but the JS guard is bypassable trivially. The server-side check at
-  `SprintController::delete` lines 1001-1004 is the real defence.
-- **Suggested fix**: None (server check is authoritative, JS is UX). The
-  spec already calls this out as defence-in-depth.
+- **Status**: accepted-by-design — confirmed by re-reading the code at
+  `src/Controllers/SprintController.php::delete` (lines 1001-1023): the
+  `confirm_name` server-side check runs AFTER `requireAdmin` and
+  `verifyCsrf`, comparing `trim($req->postString('confirm_name'))`
+  against the persisted `$sprint->name` and 302'ing back to the settings
+  page with `?error=name_mismatch` on any divergence. The
+  `data-confirm-name` value in `views/sprints/settings.twig` line 181 is
+  consumed only by the in-page JS to keep the submit button disabled
+  until the typed value matches — a UX nicety, not a guard. An attacker
+  who can read the settings page is already authenticated as admin (the
+  view itself is admin-gated by `requireAdmin` in `public/index.php`)
+  and can read the sprint name from `<title>` / `<h1>` regardless. No
+  code change. The audit's own Suggested fix is "None"; recording the
+  closing disposition here matches the convention used for N29/N30.
 
 ### R01-N33 — `data-csrf` attribute reads CSRF token in HTML
 - **Severity**: LOW.
-- **Status**: open — necessary for AJAX usage.
-- **Where**: `views/sprints/show.twig` line 9, `present.twig` line 7,
-  `settings.twig` line 9.
-- **What**: The CSRF token is rendered into a `data-csrf` attribute so
-  vanilla JS (`fetch` calls) can attach it as `X-CSRF-Token`. This is the
-  standard pattern; not a leak (the token is per-session and JS-readable
-  by design).
-- **Suggested fix**: None. Confirmed acceptable.
+- **Status**: accepted-by-design — re-confirmed by tracing the consumers:
+  `public/assets/js/app.js` line 65 (htmx `htmx:configRequest` hook),
+  `public/assets/js/sprint-settings.js` line 31, and
+  `public/assets/js/sprint-planner.js` line 114 all read `data-csrf` and
+  attach it as `X-CSRF-Token` on `fetch` / htmx requests. The token is
+  per-session (rotated on `SessionGuard::login()` per R01-N08), 256
+  bits of entropy via `bin2hex(random_bytes(32))`, and the strict CSP
+  (`script-src 'self'`, no `unsafe-inline` / `unsafe-eval`,
+  `frame-ancestors 'none'`) means no foreign script can read the
+  attribute from the rendered DOM. Removing the attribute would force
+  inline-script injection of the token (forbidden by CSP) or a separate
+  fetch endpoint that returns the token (a worse pattern — the cookie
+  already authenticates that fetch, so the endpoint adds no defence).
+  The audit's own Suggested fix is "None. Confirmed acceptable.";
+  recording the closing disposition here matches the convention used
+  for N29/N30. No code change.
 
 ### R01-N34 — `Migrator` discovery regex permits unintended filenames
 - **Severity**: LOW.
-- **Status**: open.
-- **Where**: `src/Db/Migrator.php` line 91.
-- **What**: `^(\d{3,})_[A-Za-z0-9_\-]+\.sql$` — accepts e.g.
-  `001_evil_inject.sql` if dropped into `migrations/`. That is, a bad actor
-  with write access to `migrations/` can run arbitrary SQL on next request.
-  This is an "if attacker has filesystem access, game over" scenario, but
-  worth noting that the migration runner does no signature / allow-list
-  check.
-- **Suggested fix**: None required (server filesystem is trusted). If this
-  ever changes (e.g. a `gh-pr-deploys` workflow that lets non-admins land
-  files in `migrations/`), reconsider.
+- **Status**: accepted-by-design — same trust-boundary disposition as
+  R01-N29: the `migrations/` directory is committed source code,
+  reviewed in PRs, and shares the trust boundary with the rest of
+  `src/`. After R01-N22 the discovery and apply path runs at deploy
+  time inside `bin/migrate.php` (called by `bin/docker-entrypoint.sh`)
+  rather than on a live request, so even a hostile file would only
+  fire on container start where the operator sees the
+  `RuntimeException` in `docker logs`. The current six committed
+  files (`001_init.sql` through `006_users_tombstoned.sql`) all match
+  the regex by intention; tightening the pattern to a strict
+  allow-list would force a code edit on every legitimate migration.
+  Filesystem write access on the deploy host is already arbitrary
+  code execution by other means (PHP source under `src/`, Apache
+  config, the entrypoint script itself). Revisit only if
+  `migrations/` ever accepts non-reviewed input — same revisit
+  point R01-N29 already calls out. The audit's own Suggested fix is
+  "None required (server filesystem is trusted)"; recording the
+  closing disposition here matches the convention used for N29/N30.
+  No code change.
 
 ---
 
@@ -1189,7 +1210,16 @@ A reasonable cadence (do not treat as binding):
     accepted-by-design; phpdoc on `Migrator::apply()`.
 26. ~~**R01-N30** (import_upload colour-mapping disclosure)~~ —
     accepted-by-design; admin-gated, listed for completeness.
-27. The rest as time permits.
+27. ~~**R01-N32** (sprint name in `data-confirm-name`)~~ —
+    accepted-by-design; server-side `confirm_name` check is the
+    real defence, JS guard is UX.
+28. ~~**R01-N33** (`data-csrf` attribute)~~ — accepted-by-design;
+    standard pattern, strict CSP prevents foreign-origin reads.
+29. ~~**R01-N34** (Migrator regex permits filenames)~~ —
+    accepted-by-design; `migrations/` is committed code, post-N22
+    apply runs at deploy time only.
+
+All findings now resolved (`fixed-in-<sha>` or `accepted-by-design`).
 
 Each fix should ship as its own commit per SPEC.md §14, with a follow-up
 SPEC update if behaviour or config surface changes.