Browse Source

Docs: mark R01-N20 fixed in f1aa924

Seventeenth fix from doc/REVIEW_01.md.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chiappa 2 ngày trước cách đây
mục cha
commit
2ea4b0b7b8
1 tập tin đã thay đổi với 25 bổ sung2 xóa
  1. 25 2
      doc/REVIEW_01.md

+ 25 - 2
doc/REVIEW_01.md

@@ -695,7 +695,28 @@ note. Do not delete entries — they're history.
 
 ### R01-N20 — `Response::redirect()` accepts arbitrary location strings
 - **Severity**: MEDIUM (no current open redirect; pattern is fragile).
-- **Status**: open.
+- **Status**: fixed-in-`f1aa924` — went with both bullet points
+  from the original Suggested-fix list. `Response::redirect($location)`
+  now refuses anything that is not a single `/` followed by something
+  other than `/`, and also refuses CR/LF/NUL (header injection).
+  Protocol-relative URLs (`//evil.example.com/x`) — the dangerous
+  shape that *looks* like a path — are caught by the second test
+  (without it `str_starts_with($location, '/')` would happily wave
+  them through). The HTTP→HTTPS canonical redirect in
+  `public/index.php` (the one place that needs to leave the origin)
+  moves to the new `Response::external($url, $status)` helper, which
+  restricts the scheme to `http`/`https` and requires a non-empty
+  host so a stray `javascript:` / `data:` payload cannot wind up in
+  the `Location:` header. Both helpers throw
+  `\InvalidArgumentException` on misuse — loud in dev/test, harmless
+  under the existing `FatalErrorHandler` safety net in production.
+  Pure-static `isPathOnly()` and `isExternalUrl()` back the contract
+  for direct unit tests. All 56 existing callers were audited:
+  every one passes a `/`-prefixed literal or a `'/'`-prefixed
+  template, and dynamic suffixes are integer IDs / tokens / `urlencode()`
+  output, so none regress under the new guard. New
+  `tests/Http/ResponseTest.php` (15 cases, 36 assertions). Tests:
+  302 / 791 (was 266 / 721).
 - **Where**: `src/Http/Response.php` lines 54-57.
 - **What**: All current callers pass path-only or literal strings, so no
   open-redirect today. But the helper has no shape check: a future caller
@@ -967,7 +988,9 @@ A reasonable cadence (do not treat as binding):
     one-line UX nit, see Status block.
 18. ~~**R01-N19** (CSP `report-uri` + audit endpoint)~~ — fixed in
     `f59f368`.
-19. The rest as time permits.
+19. ~~**R01-N20** (Response::redirect contract)~~ — fixed in
+    `f1aa924`.
+20. 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.