Docs: mark R01-N09 / R01-N13 / R01-N14 fixed, refresh SPEC §3 / §9 / §11 / §13

Per the maintenance contract in SPEC.md §14, the doc update lands as its
own commit so a `git revert` of the code commit leaves the doc honest.

doc/REVIEW_01.md:
  - R01-N09 → accepted-by-design with full rationale: SameSite=Strict
    would break the cross-site-initiated OIDC return navigation; real
    CSRF defence is `SessionGuard::verifyCsrf()` (per-session token,
    `hash_equals`). The split-cookie alternative was considered and
    rejected.
  - R01-N13 → fixed-in-`d7dbfb5` with the `FatalErrorHandler`
    description and test inventory.
  - R01-N14 → fixed-in-`d7dbfb5` with the cap details, the
    abandoned-token audit row schema, and the rationale for not
    relocating the blob to a temp file.
  - "Suggested ordering" footer updated.

SPEC.md:
  - §3: `src/Http/` line gained `TrustedProxies` (R01-N05/N07) and
    `FatalErrorHandler` (R01-N13).
  - §9: new "R01-N13 + R01-N14" Shipped entry covering both fixes
    plus the R01-N09 accepted-by-design decision.
  - §11: expected test count bumped 211 → 242.
  - §13: `d7dbfb5 Fix R01-N13 + R01-N14: ...` prepended.

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

SPEC.md

@@ -96,7 +96,8 @@ per-cell audit trail.
 │   │                    SprintWorkerDay, Task, TaskAssignment
 │   │   └── Import/      (Phase 20) ParsedSheet, ParsedWeek, ParsedWorker,
 │   │                    ParsedTask, ParsedAssignment, ImportResult
-│   ├── Http/            Request, Response, Router, View (+ e() helper)
+│   ├── Http/            Request, Response, Router, View (+ e() helper),
+│   │                    TrustedProxies (R01-N05/N07), FatalErrorHandler (R01-N13)
 │   ├── Repositories/    UserRepository, WorkerRepository, SprintRepository,
 │   │                    SprintWeekRepository, SprintWorkerRepository,
 │   │                    SprintWorkerDayRepository, TaskRepository,
@@ -1322,6 +1323,63 @@ is gone — see `src/Auth/BootstrapAdmin.php`.
       tests (existing `tests/Cascade` + `tests/Controllers` already
       exercise these paths). Tenth fix from `doc/REVIEW_01.md`.
 
+- [x] **R01-N13 + R01-N14 — Fatal-error safety net + XLSX session cap**
+      (`d7dbfb5`). Two findings, one commit (both touch
+      `public/index.php`).
+      *N13:* `public/index.php` calls `ob_start()` so a stray
+      warning/notice can't send headers before `Response::send()` runs,
+      but the flush is in the happy path — a fatal error mid-request
+      could let PHP flush whatever was buffered to the response,
+      without the security headers (CSP / HSTS / X-Frame /
+      X-Content-Type-Options / Referrer-Policy) that the regular path
+      applies. New `App\Http\FatalErrorHandler` registers BOTH
+      `set_exception_handler` (catches uncaught throwables escaping
+      `Router::dispatch()`) and `register_shutdown_function` (filtered
+      on a fatal mask: `E_ERROR | E_PARSE | E_CORE_ERROR |
+      E_COMPILE_ERROR | E_USER_ERROR | E_RECOVERABLE_ERROR`). On fire,
+      every open output buffer is drained (so a half-rendered Twig
+      template can't leak in front of the 500 page) and a minimal
+      `500 — Server error` HTML body is written with the same
+      security headers a normal response carries. Production hides
+      the throwable detail; non-production echoes class + message
+      (HTML-escaped). To prevent the happy/error paths from drifting
+      on CSP edits, `public/index.php` now sources its security
+      headers from `FatalErrorHandler::securityHeaders($isHttps)` —
+      single source of truth. The handler is registered TWICE: once
+      with `isHttps=false` immediately after autoload (so a fatal
+      during migrations or service wiring still produces a clean
+      500), and again with the resolved HTTPS flag right before
+      `dispatch()`. New `tests/Http/FatalErrorHandlerTest.php` (7
+      cases) pins production-vs-dev detail, HSTS-on-HTTPS-only,
+      headers-skipped-when-already-sent, drain-runs-once-and-first,
+      and `securityHeaders()` parity.
+      *N14:* The full parsed workbook (every cell value, status,
+      weeks, workers, tasks for every sheet) was JSON-stashed in PHP's
+      session file. Limit was 5 MB on the upload but parsed expansion
+      is unbounded. New
+      `ImportController::MAX_SESSION_PAYLOAD_BYTES = 2 * 1024 * 1024`
+      (2 MiB) bounds the JSON-encoded preview blob via the new
+      pure-static `encodedPayloadBytes(array): int` helper; a payload
+      past the cap is rejected with `?error=too_large_payload`. Both
+      pruning paths now record an
+      `IMPORT_PREVIEW_ABANDONED` audit row (entity_type=
+      `import_token`, entity_id=NULL, after_json carrying
+      `{file_name, sheet_count, payload_bytes, age_seconds,
+      created_at}` UTC ISO-8601). The pure-static
+      `abandonedAuditPayload(array, int): array` builder is guarded
+      against malformed entries (missing `created_at` → age 0; clock
+      skew → age 0). `ImportController` constructor gained an
+      `AuditLogger` dependency. `views/sprints/import_upload.twig`
+      gained the new error message. Test count: 4 → 12 in
+      `tests/Controllers/ImportControllerTest.php`.
+      *N09:* accepted-by-design in the same review — `SameSite=Lax`
+      stays. Strict would block the cross-site-initiated OIDC
+      callback navigation from carrying the cookie, killing the
+      library's `state`/PKCE check. Real CSRF defence is
+      `SessionGuard::verifyCsrf()` (per-session token, `hash_equals`).
+      Tests: 242 / 673 (was 227 / 590). Twelfth, thirteenth, and
+      doc-only fix from `doc/REVIEW_01.md`.
+
 - [x] **R01-N12 — Validate `/audit` date filters server-side**
       (`1b28469`). The audit viewer concatenated `T00:00:00Z` /
       `T23:59:59Z` onto the raw `from_date` / `to_date` query inputs
@@ -1417,7 +1475,7 @@ for f in $(git ls-files '*.php'); do php -l "$f" | tail -1 | sed "s|^|$f: |"; do
 Run the test suite:
 ```bash
 vendor/bin/phpunit
-# → OK (211 tests, 562 assertions)
+# → OK (242 tests, 673 assertions)
 ```
 
 The Phase 20 parser tests need `ext-dom`, `ext-zip`, `ext-xmlreader`,
@@ -1464,6 +1522,7 @@ before acting — nothing here is load-bearing once it grows stale.
 ## 13. Git history (as of this writing)
 
 ```
+d7dbfb5 Fix R01-N13 + R01-N14: fatal-error safety net + XLSX session cap
 1b28469 Fix R01-N12: validate audit date filters server-side
 d6b163d Docs: mark R01-N10 fixed, refresh SPEC §9 / §13
 c1dbfc1 Fix R01-N10: bind sprint_id with placeholder in MAX(sort_order) lookups

doc/REVIEW_01.md

@@ -330,14 +330,27 @@ note. Do not delete entries — they're history.
 
 ### R01-N09 — Session cookie `SameSite=Lax`, not `Strict`
 - **Severity**: MEDIUM.
-- **Status**: open — debatable; depends on threat model.
-- **Where**: `src/Auth/SessionGuard.php` lines 47, 56-58.
+- **Status**: accepted-by-design — `Lax` is required for the OIDC return.
+  The original "Suggested fix" understated the cost: when Entra redirects
+  the user back to `/auth/callback` it is a *cross-site initiated* top-
+  level navigation (initiator origin is `login.microsoftonline.com`), so a
+  `SameSite=Strict` session cookie would NOT be sent. The OIDC library
+  (`jumbojett/openid-connect-php`) reads the `state` and PKCE
+  `code_verifier` it stashed in `$_SESSION` BEFORE the bounce — without
+  the cookie those lookups fail and the callback errors out. We therefore
+  keep `Lax` (cookie sent on top-level cross-site GETs only — never on
+  cross-site sub-resource requests, image GETs, or any non-GET method).
+  The real CSRF defence is `SessionGuard::verifyCsrf()` (per-session
+  token, header or `_csrf` form field, `hash_equals()`); the OIDC
+  callback adds the library's `state`/PKCE check on top. A "split-cookie"
+  approach (Lax cookie just for the OIDC handshake, Strict for the main
+  session) was considered and rejected as not worth the complexity:
+  defence-in-depth gain is small and we'd have to teach two cookies to
+  every place that rotates session ids.
+- **Where**: `src/Auth/SessionGuard.php` lines 41-77.
 - **What**: `Lax` allows the cookie on top-level cross-site GETs (a click
-  from a phishing page). The OIDC redirect flow doesn't need cross-site GET
-  cookies (it's a top-level navigation back to the app's own origin).
-- **Suggested fix**: Set `SameSite=Strict`. Validate the OIDC callback
-  flow still works (the library's state/PKCE check is what actually defends
-  here; `Strict` is defence-in-depth).
+  from a phishing page).
+- **Suggested fix**: NONE. Decision recorded — see Status.
 
 ### R01-N10 — SQL string concatenation of integer parameters in repos
 - **Severity**: MEDIUM (non-exploitable today, brittle by design).
@@ -426,7 +439,32 @@ note. Do not delete entries — they're history.
 
 ### R01-N13 — Output buffer can leak partial responses on error
 - **Severity**: MEDIUM.
-- **Status**: open.
+- **Status**: fixed-in-`d7dbfb5` — new `App\Http\FatalErrorHandler`
+  registers BOTH `set_exception_handler` (catches uncaught throwables
+  escaping `Router::dispatch()`) and `register_shutdown_function`
+  (filtered on a fatal mask: `E_ERROR | E_PARSE | E_CORE_ERROR |
+  E_COMPILE_ERROR | E_USER_ERROR | E_RECOVERABLE_ERROR`, so suppressed
+  warnings/notices don't trigger). On fire, the handler drains every
+  open output buffer (so a half-rendered Twig template can't leak in
+  front of the 500 page), then emits a minimal `500 — Server error`
+  HTML body with the SAME security headers a normal response carries
+  — nosniff, X-Frame-Options DENY, Referrer-Policy, the strict CSP,
+  and HSTS when the resolved scheme is HTTPS. Production hides the
+  throwable class + message; non-production echoes them (HTML-escaped)
+  for debugging. To prevent the happy/error paths from drifting on
+  CSP edits, `public/index.php` now sources its security headers from
+  `FatalErrorHandler::securityHeaders($isHttps)` — single source of
+  truth. The handler is registered TWICE: once with `isHttps=false`
+  immediately after autoload (so a fatal during migrations or service
+  wiring still produces a clean 500), and again with the resolved
+  HTTPS flag right before `Router::dispatch()`. New
+  `tests/Http/FatalErrorHandlerTest.php` (7 cases) pins the contract
+  — production hides detail, dev surfaces it (HTML-escaped against
+  XSS payloads), HSTS rides along on HTTPS only, headers are skipped
+  cleanly when `headers_sent()` returns true (mid-flight fatal),
+  the buffer drain runs exactly once and BEFORE any header / body
+  write, and `securityHeaders($isHttps)` matches the emitted set.
+  Tests: 235 / 624 (was 227 / 590).
 - **Where**: `public/index.php` lines 38, 248-254.
 - **What**: `ob_start()` is called at the top, but the flush is in the
   happy path. On a fatal error mid-request (uncaught throwable that escapes
@@ -446,7 +484,46 @@ note. Do not delete entries — they're history.
 
 ### R01-N14 — XLSX import session blob held in `$_SESSION`
 - **Severity**: MEDIUM (memory / DoS vector, not an injection).
-- **Status**: open.
+- **Status**: fixed-in-`d7dbfb5` — went with the cap-only path.
+  `ImportController::MAX_SESSION_PAYLOAD_BYTES = 2 * 1024 * 1024` (2 MiB)
+  bounds the JSON-encoded preview blob; `upload()` encodes the
+  `ParsedSheet[]::toArray()` result once, and a payload past the cap
+  is rejected with `?error=too_large_payload` (new entry in
+  `views/sprints/import_upload.twig`'s message map). The encoded form
+  is reused as the size estimate via the new pure-static
+  `encodedPayloadBytes(array $sheetsArr): int` helper — same encoding
+  flags as `AuditLogger::encodeJson` (`JSON_UNESCAPED_UNICODE |
+  JSON_UNESCAPED_SLASHES`). The session entry also gains a
+  `payload_bytes` field so the abandoned-token audit row has a real
+  number rather than `unknown`. The "stream-parse + temp file"
+  alternative was considered and rejected — the cap is enough for
+  the threat model (admin-only upload, 5 MB raw cap already in place,
+  `www-data`-only session files), and the temp-file approach adds
+  a disk-hygiene problem the cap dodges entirely.
+- **R01-N14b** — IMPORT_PREVIEW_ABANDONED audit rows. Both pruning
+  paths now record the abandonment: `pruneSessionImports()` (called
+  on every fresh upload, drops aged-out tokens) and
+  `loadSessionEntry()` (called by `preview()` / `commit()`, drops a
+  token whose TTL elapsed mid-flow). The row is `action=
+  IMPORT_PREVIEW_ABANDONED, entity_type=import_token, entity_id=NULL`
+  with `after_json` carrying `{file_name, sheet_count, payload_bytes,
+  age_seconds, created_at}` (UTC ISO-8601). The pure-static
+  `abandonedAuditPayload(array $entry, int $now): array` builder is
+  guarded against malformed entries (`created_at` missing → age
+  clamps to 0 instead of leaking the unix-epoch offset; clock skew
+  giving a "future" entry → age also clamps to 0). The
+  `ImportController` constructor gained an `AuditLogger` dependency;
+  wiring updated in `public/index.php`.
+- **Tests**: `tests/Controllers/ImportControllerTest.php` grew from
+  4 to 12 cases. New cases pin: `encodedPayloadBytes([])` is 2 (just
+  `[]`); the helper agrees with a hand-rolled `json_encode + strlen`
+  on a real-shaped payload; the cap constant is exactly 2 MiB
+  (drift fence so a future bump must update REVIEW_01.md +
+  the upload form's error message); a payload of cap+1 length is
+  measurable as larger than the cap; the abandoned-audit payload
+  has the right keys + types + a strict ISO timestamp; missing
+  fields produce safe defaults; clock skew → age 0. Suite:
+  242 / 673 (was 227 / 590).
 - **Where**: `src/Controllers/ImportController.php` lines 102-115 (write),
   244-261 (read). Session storage path is on disk under
   `/var/www/data/sessions/`.
@@ -806,7 +883,12 @@ A reasonable cadence (do not treat as binding):
    `bc745cd`.
 10. ~~**R01-N10** (bind-not-concat sweep)~~ — fixed in `c1dbfc1`.
 11. ~~**R01-N12** (date filter validation)~~ — fixed in `1b28469`.
-12. The rest as time permits.
+12. ~~**R01-N09** (cookie SameSite)~~ — accepted-by-design; Lax is
+    required for the OIDC return navigation, see Status block.
+13. ~~**R01-N13** (fatal-error safety net)~~ — fixed in `d7dbfb5`.
+14. ~~**R01-N14** (XLSX session cap + abandoned-token audit)~~ —
+    fixed in `d7dbfb5`.
+15. The rest as time permits.
 
 Each fix should ship as its own commit per SPEC.md §14, with a follow-up
 SPEC update if behaviour or config surface changes.