Docs: mark R01-N28 fixed-in-216c15d; R01-N29/N30 accepted-by-design

R01-N29 (Migrator file_get_contents): took the audit's own
"None required; document the convention" disposition. The trust model
is now spelled out in a phpdoc on Migrator::apply() — migration files
are committed code reviewed in PRs and share the trust boundary with
the rest of src/. Post-R01-N22 the read happens at deploy time inside
bin/migrate.php, so an oversized file aborts the container start
rather than OOM-ing a live request.

R01-N30 (import_upload colour-mapping): the only emitter of the view
is GET /sprints/import which gates on SessionGuard::requireAdmin
before render. Accepted-by-design as the audit's own Suggested fix
already prescribed.

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

doc/REVIEW_01.md

@@ -994,7 +994,16 @@ note. Do not delete entries — they're history.
 
 ### R01-N28 — `SettingsController::update` builds `$changed[]` but never uses it
 - **Severity**: LOW (dead code).
-- **Status**: open.
+- **Status**: fixed-in-`216c15d` — went with the "delete the variable"
+  branch of the Suggested-fix list. The `$changed = []` declaration and
+  the trailing `$changed[] = $key` accumulator were both removed; per-key
+  audit rows already capture what changed via
+  `AuditLogger::recordForRequest()` (and `AppSettingsRepository::set()`'s
+  no-op short-circuit ensures no audit row fires when before/after match),
+  so the redirect doesn't need a count. The `?updated=N`-style alternative
+  was rejected as scope creep — `?updated=1` plus the audit log is
+  sufficient signal for an operator. No new tests: the change is a pure
+  dead-code removal with no behavioural delta. Suite still 333 / 868.
 - **Where**: `src/Controllers/SettingsController.php` lines 95-127.
 - **What**: `$changed[]` is appended to but the variable is never read.
   Audit happens on a per-key basis already; the local accumulator is dead.
@@ -1003,7 +1012,19 @@ note. Do not delete entries — they're history.
 
 ### R01-N29 — `Migrator::apply` reads file via `file_get_contents` without size cap
 - **Severity**: LOW.
-- **Status**: open.
+- **Status**: accepted-by-design — took the audit's own
+  "None required; document the convention" disposition. The trust model
+  is now spelled out in a phpdoc on `Migrator::apply()`: migration files
+  live under `migrations/`, are committed code reviewed in PRs, and
+  therefore share the trust boundary with the rest of `src/`. After
+  R01-N22 the `apply()` path runs at deploy time inside
+  `bin/migrate.php` (Docker entrypoint), not at request time, so an
+  oversized file aborts the container start with a loud `RuntimeException`
+  rather than OOM-ing a live request. If `migrations/` ever accepts
+  non-reviewed input (e.g. a future `gh-pr-deploys`-style workflow that
+  lands files there from outside the review pipeline) the comment calls
+  out the revisit point. No SPEC change. No test change — the contract
+  pinned by `MigratorTest` (R01-N22) covers the read path end-to-end.
 - **Where**: `src/Db/Migrator.php` lines 101-125.
 - **What**: A migration file copied accidentally as a multi-GB blob would
   OOM the request. Minor — migrations are committed code, not user-supplied.
@@ -1011,7 +1032,16 @@ note. Do not delete entries — they're history.
 
 ### R01-N30 — `import_upload.twig` reveals colour-mapping rules to anyone with the link
 - **Severity**: LOW (info disclosure; admin-gated).
-- **Status**: open.
+- **Status**: accepted-by-design — confirmed with `grep` that the only
+  route emitting `import_upload.twig` is `GET /sprints/import` in
+  `public/index.php`, which gates on `SessionGuard::requireAdmin($users)`
+  before the view renders. An anonymous request 302s to `/auth/login`
+  with a flash, never reaching the colour-mapping block. The mapping
+  itself documents the *Tool_Sprint Planning* workbook's own colour
+  convention — common knowledge inside the team that owns the workbook
+  — so even if the gate were ever weakened, the disclosure cost is
+  near zero. The audit's own Suggested fix is "None"; recording that
+  here as the closing disposition. No code change.
 - **Where**: `views/sprints/import_upload.twig` lines 42-52.
 - **What**: Page is admin-gated (`SessionGuard::requireAdmin`) so this is
   not exposed publicly. Listed for completeness.
@@ -1153,7 +1183,13 @@ A reasonable cadence (do not treat as binding):
     fixed in `1f11117`.
 23. ~~**R01-N27** (entrypoint session-file GC loop)~~ —
     fixed in `32d03fc`.
-24. The rest as time permits.
+24. ~~**R01-N28** (drop dead `$changed[]` in SettingsController)~~ —
+    fixed in `216c15d`.
+25. ~~**R01-N29** (Migrator file size — document trust model)~~ —
+    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.
 
 Each fix should ship as its own commit per SPEC.md §14, with a follow-up
 SPEC update if behaviour or config surface changes.

src/Db/Migrator.php

@@ -124,6 +124,12 @@ final class Migrator
         return $out;
     }
 
+    /**
+     * R01-N29: `file_get_contents` is unbounded by design. Migration files are
+     * committed code reviewed in PRs — not user-supplied — so the trust model
+     * matches the rest of the source tree. If `migrations/` ever accepts
+     * non-reviewed input, revisit.
+     */
     private function apply(int $version, string $filename, string $fullPath): void
     {
         $sql = file_get_contents($fullPath);