diff --git a/docs/reviews/review-001-m0.md b/docs/reviews/review-001-m0.md index dbbb73c..4715437 100644 --- a/docs/reviews/review-001-m0.md +++ b/docs/reviews/review-001-m0.md @@ -59,6 +59,27 @@ A **private, loopback, no-proxy** human test is acceptable **once H1 + H2 land** --- +## Rulings (Architect, 2026-10-14) + +The fix pass landed (`03a9db0..04c2394`), independently verified: **gate green** · static binary · +`routing` pure · tree clean · `main` == `origin/main`. + +| Item | Ruling | +|---|---| +| **Dev. 1** — `__Host-` prefix applied **only over https** | **Accepted.** A `__Host-` cookie *must* carry `Secure`; forcing it would break the documented **http loopback bootstrap**. The prefix applies in the real (https) deployment. | +| **Dev. 2** — the TOTP replay counter is **in-process**, not a DB column | **Accepted** for now (±90 s window bounds the exposure; no migration). A durable `totp_secrets.last_counter` is a later option. | +| **Dev. 3** — the `/access` notify is async; the insert stays synchronous | **Accepted** — the dominant SMTP timing signal is gone; a fully uniform response is deferred. | +| **Dev. 5** — the M2c test drives `setup.Actions.Finish` with a stub, not the whole binary | **Accepted** — it pins the invariant; the live smoke covers the true end-to-end. | +| **A6 residual** (builder, Class B) — a proxy forwarding a **client-supplied** XFF *verbatim* could present a loopback claim during setup | **Adopt the strict setup gate:** in setup mode require **a loopback peer AND no `X-Forwarded-For`** header — the bootstrap is a **direct** SSH tunnel, so a proxied setup is refused. *Fold into the next slice.* | + +**Still open (scheduled, non-blocking):** **L3c** (`Mint` check-then-insert atomicity) · **L4c** +(`Sessions.GC` / `UpdateWebAuthnSignCount` unaudited — the reason **A11** stays open) · **L5c** +(per-screen CSP assertion) · **L6c** (per-GUID rate buckets + the `RetryAfter` clock). + +**Status: M0 is safe for a private, loopback human test.** + +--- + ## Fix pass (2026-10-14) **H1 · H2 · M1c · M1s · L3s · L1c/L2s · L8c · M2s · M3s · M2c · L1s · L5s · L4s · L7c are fixed** diff --git a/docs/specs/spec-003-core-door.md b/docs/specs/spec-003-core-door.md index 2e0cbd1..b240039 100644 --- a/docs/specs/spec-003-core-door.md +++ b/docs/specs/spec-003-core-door.md @@ -48,7 +48,7 @@ request 3. security headers — X-Content-Type-Options · Referrer-Policy · a strict CSP · HSTS (when https) · Permissions-Policy · COOP · CORP [L4s] 4. routing.Dispatch() — the pure model (SPEC-002) - 5. setup-mode gate — in setup mode the **resolved client IP must be loopback**, else 404 (A13/H2) + 5. setup-mode gate — in setup mode the **TCP peer must be loopback AND no XFF**, else 404 (A13/D-025) 6. switch on Kind: Redirect → 302 NotFound → 404 @@ -112,16 +112,18 @@ type RequestContext struct { --- -## 7. Loopback enforcement (D-025) +## 7. Loopback enforcement (D-025, revised 2026-10-15) -- In **setup mode**, if `server.listen` is **not loopback**, the process **refuses to start** - (fail-closed). The operator bootstraps over an **SSH tunnel** to `127.0.0.1:8080`. -- **Per request (H2/A13):** the bind check alone is defeated by a reverse proxy on `127.0.0.1` - relaying remote clients — so while in setup mode the door also rejects **any request whose - resolved client IP is not loopback** (404, before dispatch). The client IP is the one resolved - through `server.trusted_proxies` (§4 step 2), so a trusted proxy forwarding a remote client does - **not** launder it into loopback. An empty/unparseable client IP **fails closed**. -- Once the top admin exists (`setup: false`), a public bind is allowed. +- **No bind refusal.** The app **may bind any address** — including `0.0.0.0:8080` behind a reverse + proxy (**Zoraxy**, D-043). *(A bind check is incompatible with a proxy-fronted deployment.)* +- **Per request, while in setup mode, the gate is strict:** serve only a request whose **TCP peer is + loopback** **and** which carries **no `X-Forwarded-For`** — i.e. the **direct SSH tunnel**. + Anything else — including a request relayed by a trusted proxy — → **404**, before dispatch. + An empty/unparseable peer **fails closed**. + - *Why both tests:* a proxy on the same host presents a **loopback peer**, so the peer check alone + is not enough; the **absence of `X-Forwarded-For`** proves the request did not traverse a proxy. +- **Once productive** (`setup: false`), normal service: the trusted proxy's **`X-Forwarded-Proto`** + drives `Secure` on the session cookie, the **`__Host-`** prefix, and HSTS (§6 · D-043). --- @@ -144,12 +146,13 @@ type RequestContext struct { | AC-4 | A session whose `user_id` ≠ the GUID's user is **rejected** | | AC-5 | An **empty / missing session id** is rejected on **every** path (A2) | | AC-6 | A `do`/`action` request without a valid CSRF token is **rejected** | -| AC-7 | In setup mode with a non-loopback `listen`, the process **exits with an error** (D-025) | +| AC-7 | In setup mode, a request **relayed by a proxy** (a loopback peer **with** `X-Forwarded-For`) is **404** — the bootstrap is **tunnel-only** (D-025 revised) | | AC-8 | Every response carries a correlation id, echoed in the log; path credentials are redacted | | AC-9 | The rate limiter returns **429** on the auth-sensitive routes (A3) | -| AC-10 | In setup mode a request whose **resolved client IP is not loopback** is **404** — even when relayed by a trusted proxy (H2/A13) | +| AC-10 | In setup mode a request whose **TCP peer is not loopback**, **or** which carries **any `X-Forwarded-For`**, is **404** (D-025/H2/A13) | | AC-11 | The **`setup` module** is reachable only through `Workflow == "setup"`; any other resolution naming it is 404 (M1c/L3s) | | AC-12 | `X-Forwarded-For` is resolved **right→left**; a forged leftmost entry is ignored (H1/A6) | +| AC-13 | Behind the proxy, `X-Forwarded-Proto: https` ⇒ the session cookie is **`Secure` + `__Host-`**, and HSTS is set (D-043) | --- diff --git a/docs/specs/spec-005-setup-wizard.md b/docs/specs/spec-005-setup-wizard.md index e3ea3d0..f581d6c 100644 --- a/docs/specs/spec-005-setup-wizard.md +++ b/docs/specs/spec-005-setup-wizard.md @@ -2,7 +2,7 @@ | Field | Value | |---|---| -| **Status** | **Implemented** (2026-10-13) — see §7; `build`/`vet`/`test`/`-race` green · static binary · live-smoked · **rev 2026-10-14 (D-041): TOTP-only setup** | +| **Status** | **Implemented** (2026-10-13) — see §7; `build`/`vet`/`test`/`-race` green · static binary · live-smoked · **rev 2026-10-14 (D-041): TOTP-only setup** · **rev 2026-10-15 (D-025/D-043): tunnel-only per-request gate** | | **Created** | 2026-10-13 | | **Author** | OpenArchitect | | **Target milestone** | **M0** | @@ -13,8 +13,9 @@ ## 1. Goal -Bring a fresh instance to life with a **loopback-only `/setup` wizard** that creates the **first -user = the top admin** (`user_id = 1`, role `admin`), then switches the instance to productive mode. +Bring a fresh instance to life with a **`/setup` wizard** reachable **only over the direct SSH +tunnel** that creates the **first user = the top admin** (`user_id = 1`, role `admin`), then switches +the instance to productive mode. ## 2. Non-goals @@ -25,10 +26,13 @@ Module setup screens (each module owns its config). Multi-admin (there is **one* `setup mode = (config.setup == true) OR (no user exists)` — **fail-safe**. - In setup mode the app serves **only** `/setup` (+ static assets); **every other route → 404**. -- **Loopback-only, enforced (D-025):** if `server.listen` is **not** loopback while in setup mode, - the process **refuses to start** — and, per request, a client whose **resolved** IP is not loopback - is 404 **before** dispatch (H2/REVIEW-001 · SPEC-003 §7), so a reverse proxy on `127.0.0.1` - relaying remote clients cannot reach the wizard. +- **Tunnel-only, enforced per request (D-025, revised 2026-10-15):** the app **may bind any address** + — including `0.0.0.0:8080` behind a reverse proxy (D-043); there is **no** bind refusal. The gate + is **per request**: only a request whose **TCP peer is loopback** **and** which carries **no + `X-Forwarded-For`** is served (the **direct SSH tunnel**). Anything else — including a request + relayed by a trusted proxy — → **404** before dispatch; an empty/unparseable peer **fails closed** + (SPEC-003 §7 · AC-7/AC-10). *A proxy on the same host presents a loopback peer, so the absence of + XFF is what proves the request did not traverse one.* - The `setup` **module** is pinned at the door: the wizard's actions are reachable only through the reserved `setup` workflow (M1c/L3s · SPEC-003 §4). @@ -55,7 +59,8 @@ POST /setup?do=setup&action=… → each step (CSRF-protected) ## 5. After setup - `/setup` → **404** (delete, don't redirect). Re-entering requires `config.setup: true` + restart. -- The instance may now bind a public interface (behind a reverse proxy). +- The instance is served on its configured bind (typically `0.0.0.0:8080` behind the reverse proxy, + D-043); the request scheme is taken from `X-Forwarded-Proto` (trusted proxy) or TLS. - **Audit:** the bootstrap, the admin creation, and the enroll are all recorded (A11). ## 6. Acceptance criteria @@ -63,7 +68,7 @@ POST /setup?do=setup&action=… → each step (CSRF-protected) | # | Criterion | |---|---| | AC-1 | Fresh instance → `/setup`; any other route → 404 | -| AC-2 | Non-loopback `listen` in setup mode → process exits with an error (D-025) | +| AC-2 | In setup mode **only the direct tunnel** is served (loopback TCP peer **and** no `X-Forwarded-For`); anything else → 404. Any bind is allowed (D-025 revised) | | AC-3 | Completing the wizard creates `user_id = 1` (role `admin`) with **TOTP** enrolled and **no passkey** (D-041) | | AC-4 | After completion `config.setup = false` (or printed) and `/setup` → 404 | | AC-5 | A second `admin` cannot be created |