Explorar o código

Docs: mark R01-N25/N26/N27 fixed in f6ce13f/1f11117/32d03fc

Closes the three lowest-numbered LOW findings:
- N25 X-Permitted-Cross-Domain-Policies header
- N26 one-shot session flash for the post-delete chip
- N27 entrypoint session-file GC loop

Suggested-ordering tail updated to entries 21-23 with the new SHAs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chiappa hai 2 días
pai
achega
b706a17912
Modificáronse 1 ficheiros con 55 adicións e 4 borrados
  1. 55 4
      doc/REVIEW_01.md

+ 55 - 4
doc/REVIEW_01.md

@@ -916,7 +916,20 @@ note. Do not delete entries — they're history.
 
 ### R01-N25 — No `X-Permitted-Cross-Domain-Policies` header
 - **Severity**: LOW.
-- **Status**: open.
+- **Status**: fixed-in-`f6ce13f` —
+  `FatalErrorHandler::securityHeaders()` (the single source of truth
+  shared by the happy path and the 500 fallback per R01-N13) now emits
+  `X-Permitted-Cross-Domain-Policies: none` unconditionally — on HTTP
+  and HTTPS alike, unlike HSTS. The drift fence in
+  `FatalErrorHandlerTest` grew two assertions: the production-500 emit
+  carries the header, and the helper map carries it on both
+  `securityHeaders(true)` and `securityHeaders(false)`. The optional
+  `Permissions-Policy` directive from the Suggested-fix list was left
+  out — the app does not use any of the gated browser features
+  (geolocation, camera, mic, USB, …) so the directive would be a no-op
+  with maintenance cost; revisit if a future feature legitimately
+  needs one of those APIs and we want an explicit allow-list. Tests:
+  333 / 868 (was 331 / 860).
 - **Where**: `public/index.php` lines 240-243.
 - **What**: Adobe Flash / Acrobat is dead, but defence-in-depth headers are
   cheap.
@@ -925,7 +938,20 @@ note. Do not delete entries — they're history.
 
 ### R01-N26 — `home.twig` "deleted" flash relies on query string truthiness
 - **Severity**: LOW.
-- **Status**: open.
+- **Status**: fixed-in-`1f11117` — went with the audit's
+  Suggested-fix exactly: `SprintController::delete` stashes the
+  deleted sprint's name in `$_SESSION['flash_deleted_sprint_name']`
+  (matching the existing `flash_*` convention from
+  `ImportController`) and redirects to a bare `/`. The home handler
+  in `public/index.php` reads the key — `SessionGuard::currentUser`
+  has already started the session by that point — and `unset()`s it
+  on render, so a manual page refresh cannot re-show the chip and a
+  hand-crafted URL cannot trigger it at all. The view's `{% if
+  deletedSprintName|default('') != '' %}` guard is unchanged; only
+  the data source moved. New `TwigViewTest` cases (2): chip renders
+  when the flash is set (and the name is HTML-escaped, drift fence
+  for R01-N21's autoescape pin), chip stays absent when the flash
+  is empty. Tests: 333 / 868 (was 331 / 860).
 - **Where**: `views/home.twig` lines 11-15; `public/index.php` line 150;
   `src/Controllers/SprintController.php::delete` line 1114.
 - **What**: After delete, the user is redirected to `/?deleted=<urlencoded
@@ -937,7 +963,26 @@ note. Do not delete entries — they're history.
 
 ### R01-N27 — Session lifetime / GC is filesystem-only on container restart
 - **Severity**: LOW.
-- **Status**: open.
+- **Status**: fixed-in-`32d03fc` — went with a backgrounded loop in
+  `bin/docker-entrypoint.sh` rather than installing cron (cron would
+  be a new external dependency for a 4-line shell loop). After
+  migrations succeed, the entrypoint spawns a child process that
+  sleeps `${SESSION_GC_INTERVAL_SECONDS}` (default 3600 = 1h), then
+  runs `find ${SESSION_PATH} -mindepth 1 -type f -mmin
+  +${SESSION_GC_MAX_AGE_MINUTES} -delete` (default 480 minutes = 8h,
+  matching `gc_maxlifetime` on `SessionGuard::start()`). The loop is
+  a child of the entrypoint's PID 1, so `docker stop` propagates
+  SIGTERM and reaps it together with Apache; no orphaned process on
+  container restart. `find` is a coreutils binary already in the
+  `php:8.3-apache` base image, so no new package install. Both knobs
+  are env-var overridable
+  (`SESSION_GC_MAX_AGE_MINUTES`,`SESSION_GC_INTERVAL_SECONDS`) for
+  operators who want a tighter sweep on a public deployment.
+  `doc/admin-manual.md` §5.7 documents the loop, the env vars, and a
+  copy-pasteable system-cron snippet for bare-metal deployments that
+  don't run the container's entrypoint. No new tests — the change is
+  shell-script + docs, exercised by `docker compose up`. Existing
+  PHPUnit suite still 333 / 868.
 - **Where**: `Dockerfile` line 57; `src/Auth/SessionGuard.php` lines 33-39.
 - **What**: Sessions are stored on a mounted volume. PHP's GC depends on
   request traffic to fire — a low-traffic deployment may keep stale session
@@ -1102,7 +1147,13 @@ A reasonable cadence (do not treat as binding):
     `f1aa924`.
 20. ~~**R01-N21** (Twig autoescape pin + static |raw guard)~~ —
     fixed in `00bcf73`.
-21. The rest as time permits.
+21. ~~**R01-N25** (X-Permitted-Cross-Domain-Policies header)~~ —
+    fixed in `f6ce13f`.
+22. ~~**R01-N26** (one-shot session flash for the post-delete chip)~~ —
+    fixed in `1f11117`.
+23. ~~**R01-N27** (entrypoint session-file GC loop)~~ —
+    fixed in `32d03fc`.
+24. 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.