Przeglądaj źródła

Docs: mark R01-N01 fixed, refresh SPEC §9 / §11 / §13

- doc/REVIEW_01.md: flip R01-N01 (CRITICAL, plaintext local-admin
  password) to fixed-in-857df15. Records the operator's explicit
  decision to take the hash-only break (no plaintext fallback) rather
  than the deprecation-window option REVIEW_01 originally suggested.
  Strike out R01-N01 in the suggested ordering; next open finding in
  the cadence is R01-N06 (login throttling).
- SPEC §9: insert a new shipped entry above R01-N04 explaining the
  break and the hash-recipe surface (README + admin-manual).
- SPEC §11: 150 / 430 → 159 / 443.
- SPEC §13: append 857df15 plus the previously-missing f075e12 so
  every commit since the last doc-meta update is in the history block.

Pure docs follow-up to 857df15.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chiappa 2 dni temu
rodzic
commit
48a351c1f6
2 zmienionych plików z 39 dodań i 4 usunięć
  1. 25 1
      SPEC.md
  2. 14 3
      doc/REVIEW_01.md

+ 25 - 1
SPEC.md

@@ -1048,6 +1048,28 @@ with a `BOOTSTRAP_ADMIN` audit row.
       unchanged. *New sprint* stays in the header as the one
       quick-action admins reach for from any page.
 
+- [x] **R01-N01 — Local-admin password is hash-only (no plaintext fallback)**
+      (`857df15`). `src/Auth/LocalAdmin.php` previously read the password
+      verbatim from `LOCAL_ADMIN_PASSWORD` and compared it with
+      `hash_equals()`; anyone with read access to `.env` had immediate
+      admin. Now the env var is `LOCAL_ADMIN_PASSWORD_HASH` (a
+      `password_hash()` output, typically `$2y$...` bcrypt) and
+      verification is `password_verify()`. Operator's explicit choice
+      was the clean break — there is *no* plaintext fallback. Existing
+      deployments must regenerate the hash before the next sign-in;
+      until they do, `LocalAdmin::isEnabled()` returns false and
+      `/auth/local` 404s rather than silently re-using stale config.
+      Email comparison stays `hash_equals()` (timing-safe) on the
+      trimmed input. Hash recipe lives in README §Quick setup step 3
+      and admin-manual §3.5 — both ship the host-PHP one-liner *and*
+      the `docker run --rm php:8.3-cli php -r '...'` form for
+      hash-without-host-PHP, with the single-quotes-in-`.env` warning
+      so the `$` segments aren't eaten by the shell that runs
+      `docker compose up`. New `tests/Auth/LocalAdminTest.php` (9
+      cases) locks the contract in, including a regression guard
+      asserting that `LOCAL_ADMIN_PASSWORD` alone does *not* enable
+      the fallback. Tests: 159 / 443 (was 150 / 430).
+
 - [x] **R01-N04 — `SESSION_SECRET` removed from env template + docs**
       (`296883c`). The env var was documented as the salt for the
       session cookie name / CSRF tokens but nothing in the code reads
@@ -1148,7 +1170,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 (150 tests, 430 assertions)
+# → OK (159 tests, 443 assertions)
 ```
 
 The Phase 20 parser tests need `ext-dom`, `ext-zip`, `ext-xmlreader`,
@@ -1195,6 +1217,8 @@ before acting — nothing here is load-bearing once it grows stale.
 ## 13. Git history (as of this writing)
 
 ```
+857df15 Fix R01-N01: hash-only LOCAL_ADMIN_PASSWORD_HASH (no plaintext fallback)
+f075e12 Docs: mark R01-N04 fixed, refresh SPEC §9 / §13
 296883c Fix R01-N04: drop unused SESSION_SECRET from env template + docs
 18389bb Docs: mark R01-N02 / R01-N31 fixed, refresh SPEC §9 / §11 / §13
 7fd849b Fix R01-N02 / R01-N31: gate runtime panel on home page to admins

+ 14 - 3
doc/REVIEW_01.md

@@ -38,7 +38,18 @@ note. Do not delete entries — they're history.
 ### R01-N01 — Local-admin password compared in plaintext from `.env`
 - **Severity**: CRITICAL (within the local-admin code path; effectively HIGH
   for the app overall because the path is opt-in).
-- **Status**: open — partially documented as by-design in SPEC §8 / .env.example.
+- **Status**: fixed-in-`857df15` — `src/Auth/LocalAdmin.php` now reads
+  `LOCAL_ADMIN_PASSWORD_HASH` and verifies with `password_verify()`.
+  Per the operator's explicit choice this is a clean break: NO plaintext
+  `LOCAL_ADMIN_PASSWORD` fallback. `LocalAdmin::isEnabled()` returns false
+  until the hash is provided, so `/auth/local` 404s instead of silently
+  re-using stale plaintext config. Email comparison stays timing-safe
+  (`hash_equals`, trimmed). Hash recipe documented in `README.md` Quick
+  setup step 3 and `doc/admin-manual.md` §3.5 (host-PHP and
+  `docker run php:8.3-cli` one-liners). New `tests/Auth/LocalAdminTest.php`
+  (9 cases, +13 assertions) covers the happy path, wrong-password /
+  wrong-email rejection, and an explicit regression guard that the legacy
+  `LOCAL_ADMIN_PASSWORD` env var alone does NOT enable the fallback.
 - **Where**:
   - `src/Auth/LocalAdmin.php` lines 51-66 (`verify()`, `password()`)
   - `.env.example` lines 25-33 (the comment acknowledging the trade-off)
@@ -638,8 +649,8 @@ A reasonable cadence (do not treat as binding):
 2. ~~**R01-N04** (`SESSION_SECRET` doc cleanup)~~ — fixed in `296883c`.
 3. **R01-N15** (`rel="noopener noreferrer"`) — one-character fix.
 4. **R01-N11** (audit column whitelist) — defensive, trivial.
-5. **R01-N01** (local-admin password hashing) — biggest single security
-   improvement; touches `LocalAdmin` + admin manual.
+5. ~~**R01-N01** (local-admin password hashing)~~ — fixed in `857df15`
+   (hash-only, no plaintext fallback per operator decision).
 6. **R01-N06** (login throttling) — tiny new repo + integration.
 7. **R01-N03** (first-user bootstrap hardening).
 8. **R01-N07** + **R01-N05** (proxy / HTTPS handling) — paired.