diff --git a/.planning/phases/07-user-plugin-and-authentication/07-SECURITY.md b/.planning/phases/07-user-plugin-and-authentication/07-SECURITY.md new file mode 100644 index 0000000..24b0628 --- /dev/null +++ b/.planning/phases/07-user-plugin-and-authentication/07-SECURITY.md @@ -0,0 +1,102 @@ +--- +phase: 07 +slug: user-plugin-and-authentication +status: verified +threats_open: 0 +asvs_level: 1 +created: 2026-09-23 +--- + +# Phase 07 — Security + +> Per-phase security contract: threat register, accepted risks, and audit trail. + +Paths: `F/` = `summercms.go` (framework), `A/` = `../fonoteka.go` (application), `U/` = `A/plugins/golem15/user`, `FN/` = `A/plugins/golem15/fonoteka`. + +--- + +## Trust Boundaries + +| Boundary | Description | Data Crossing | +|----------|-------------|---------------| +| Client → bouncer.Verify / jwtGuard | Untrusted bearer/cookie token enters JWT parsing, blacklist and cutoff checks | JWT (sub, iat, exp, jti), cookie `token`/`auth_token` | +| golang.org/x/crypto supply chain | New direct dependency inside the password-hashing boundary | bcrypt hashing/verification of user passwords | +| Client → login / register | Untrusted email+password into credential verification and account creation | email, password, client IP (throttle key) | +| Client → refresh / logout | Untrusted bearer token into sliding-refresh and forever-blacklist paths | JWT jti, exp; blacklist rows | +| Client → reset / activation codes | Untrusted `{id}!{code}` strings into DB lookup and password/activation state change | user id, reset/activation code | +| Client → avatar upload | Untrusted multipart bytes into blob storage, later served as public URL | file bytes, sniffed MIME, size | +| Client → token mint / revoke | Authenticated but possibly malicious client into scope-ceiling and owner-scoped lookup | scopes[], name, expires_at, token id | +| Parity recorder → isolated PHP (persistent file cache) | Recorder drives a real PHP checkout including cache/session state under `$PHP_ROOT` | credentials, JWTs, cache files | +| Parity recorder → committed fixtures | Secret-bearing capture session output becomes a public git artifact | masked tokens, request/response bodies, well-known test password | +| Go production error page → HTTP client | Embedded Winter 500 page served verbatim to any caller of the already-activated path | static HTML (contains recorder-host asset URL) | +| Parity test seeding → Postgres | Direct-DB seed hooks write bcrypt-hashed test credentials into ephemeral testcontainers Postgres | bcrypt hashes, `must_change_password` flags | + +--- + +## Threat Register + +| Threat ID | Category | Component | Disposition | Mitigation | Status | +|-----------|----------|-----------|-------------|------------|--------| +| T-07-01 | Spoofing / Elevation of Privilege | bouncer.jwtGuard.Authenticate + Logout → jwt_blacklist | mitigate | `F/bouncer/jwt.go:110-118` checks `IsBlacklisted` on every authenticated request, blocked → generic 401 (`:266-268`); Logout `bl.Add(ctx, jti, exp, time.Now())` at `U/controllers/api_controller.go:148-149`; store wired at `U/plugin.go:82-96`, `jwt.auth` bound `:208` and applied to all `/_fonoteka/api/v1` groups (`FN/routes.go:12,28`); e2e `TestSessionSequence` (`U/sequence_test.go:38-43`) asserts 401 after logout | closed | +| T-07-02 | Denial of Service (self-inflicted) | bouncer.Refresh + BlacklistStore | mitigate | `F/bouncer/blacklist.go:16-19` store interface carries `validUntil`; `IsBlacklisted` returns `!now.Before(validUntil)` (`:58`, `:116`); `F/bouncer/refresh.go:64` adds with `time.Now().Add(grace)`; grace read from `golem15.user.jwt.blacklist_grace` (`api_controller.go:190,862-868`), app config sets 10s | closed | +| T-07-03 | Information Disclosure | ForgotPassword | mitigate | Single `forgotPasswordBody` const (`U/controllers/api_controller.go:212`); lookup-error (`:241`) and found/not-found (`:253`) paths return the identical 200 body | closed | +| T-07-04 | Spoofing (credential stuffing) | Login | mitigate | Outer `throttle:user-api` on `U/routes.go:10`, bucket 120/min keyed by sub or IP (`U/plugin.go:142-150`); inner `CheckAndRecordLogin` 5 attempts / 15 min (`U/classes/throttle.go:35,107-148`), `RejectIfThrottled` runs before the password check (`api_controller.go:68`), failures recorded (`:79`) | closed | +| T-07-05 | Spoofing | codes.go VerifyResetCode / VerifyActivationCode | mitigate | `subtle.ConstantTimeCompare` (`U/classes/codes.go:187-191`); TTLs 60 min (`:34`) and 72 h (`:43`) enforced via `codeFresh` in `VerifyResetCode` (`:99`) and `VerifyActivationCode` (`:122`) | closed | +| T-07-06 | Elevation of Privilege | token_api_controller.Store | mitigate | `MintableScopes = {read,write,ai}` (`FN/classes/auth/api_token_manager.go:18`); Store rejects any scope outside the list (`FN/controllers/api/token_api_controller.go:185-189`) and mints only the filtered set (`:42,66`) | closed | +| T-07-07 | Tampering | bouncer.Verify / NewJWTGuard (empty JWT secret) | accept (already mitigated, Phase 3 C-01) | `U/classes/user_lookup.go:66-74` errors on empty/whitespace secret; called from Boot (`U/plugin.go:66-69`) so boot fails | closed | +| T-07-08 | Elevation of Privilege | 423 must-change-password lock route grouping | mitigate | `TestRequirePasswordChangeExemptSet` (`FN/routes_isolation_test.go:128-175`) asserts middleware sets over the real assembled `surf.BuildRouter(...).Routes()`; `TestMustChangePasswordLock` (`FN/phase07_coverage_test.go:16-40`) asserts 423 on genres and tokens; middleware `FN/middleware/must_change_password.go:11-17`; locked group `FN/routes.go:12` vs exempt `:28` | closed | +| T-07-09 | Denial of Service | UploadAvatar | mitigate | `http.DetectContentType` sniff + `avatarExt` allow-list (`U/controllers/api_controller.go:1131-1140`); `avatarMaxBytes = 4_096_000` (`:37`) checked at `:1142-1150`; transport cap `http.body_limits.default_bytes` (`F/surf/router.go:371,466`, `F/surf/bodylimit.go:29-33`) | closed | +| T-07-10 | Information Disclosure | Recorded parity fixtures (07-05) | mitigate | `A/parity/capture-rules.yaml:6,14` masks Authorization/Set-Cookie, `:20-51` masks `$.token` as `jwt:*`; audit grep of `A/parity/fixtures/` for JWT, `inv_`, bcrypt shapes = 0 hits; vars store 0600 (`F/tide/variables.go:74,85,122,131`); `A/.gitignore:6` ignores `parity/.vars/` | closed | +| T-07-11 | Tampering / Race | bouncer.PostgresBlacklist | mitigate | `ON CONFLICT (jti) DO UPDATE` upsert (`F/bouncer/blacklist.go:97-98`); single-statement `SELECT ... WHERE jti = $1` (`:107`) and `DELETE` (`:123`); table identifier regex-checked (`:72,86-90`) | closed | +| T-07-12 | Information Disclosure | register()'s disabled/throttled branches | mitigate | `writeSafe500` emits generic `Internal server error` unless `app.debug` (`U/controllers/api_controller.go:748-754`); used by disabled (`:611`) and throttled (`:615`) branches | closed | +| T-07-13 | Elevation of Privilege | ChangePassword `has_self_set_password=false` branch | mitigate | `if !user.HasSelfSetPassword` → log + `writeOpaque500` (`U/controllers/api_controller.go:490-494`) | closed | +| T-07-14 | Information Disclosure | parity/db_capture.go | mitigate | Column restricted by switch to two names (`A/parity/db_capture.go:53`); email regex-gated (`:32-35`); Postgres side parameterized `$1` (`:114-116`); PHP side fixed tinker one-liner (`:66-67`) | closed | +| T-07-SC | Tampering (supply chain) | golang.org/x/crypto direct dependency | mitigate | `F/go.mod:25` pins `golang.org/x/crypto v0.57.0`; 07-01 SUMMARY records the blocking human-verify checkpoint approved before promotion | closed | +| T-07-IDOR | Information Disclosure | token_api_controller.Destroy | mitigate | `Where("user_id = ?", user.ID).First(&token, id)` (`FN/controllers/api/token_api_controller.go:130`); bad id (`:126`) and not-found (`:132`) return byte-identical `{"error":"Token not found"}` 404 | closed | +| T-07-20 | Information Disclosure | Re-recorded fixtures (07-07) | mitigate | Same masking as T-07-10; audit grep over all committed fixtures incl. `nuxt-auth.yaml` and `nuxt-auth-lock.yaml` for JWT / `inv_` / bcrypt shapes = 0 hits; all `token` fields are `{{jwt:*}}` placeholders | closed | +| T-07-21 | Tampering | `CACHE_DRIVER=file` writing into shared PHP checkout | mitigate | `A/parity/php_parity.sh:39` sets `CACHE_DRIVER=file`, `:62` runs `php artisan cache:clear` on reset; PHP checkout `storage/framework/cache/.gitignore` is `*` + `!.gitignore`; `git status --porcelain storage/framework/cache` verified clean at audit time. Follow-up: the pre/post git status check is not scripted in `php_parity.sh` | closed | +| T-07-22 | Information Disclosure | winter_error_page.html (embedded 500 body) | mitigate | `U/controllers/winter_error_page.html` (496 bytes) contains no stack trace, exception text, or filesystem path; served with fixed headers (`api_controller.go:1094-1099`). Low-severity note: stylesheet href embeds the recorder host `127.0.0.1:8423`; a relative URL would be cleaner | closed | +| T-07-23 | Repudiation | Logout blacklist enforcement (unchanged by 07-07) | accept (no new risk) | `U/sequence_test.go:43` asserts 401 after logout; `F/bouncer/phase07_coverage_test.go:15-38` asserts a blacklisted jti cannot refresh; nuxt flow replay asserts 401 after logout | closed | +| T-07-24 | Tampering | Manifest status flips pending → ported | mitigate | All 15 `/_user/api/v1` routes `status: ported` (`A/parity/manifest.yaml:2983-3240`); `ported-mismatch` / `ported-mutated-response` guard subtests (`A/parity/parity_test.go:125,140`, `parity_contract_test.go:130,134`); 07-07 SUMMARY records green replay (169 recorded / 22 ported / 0 failing) before the flips | closed | +| T-07-25 | Information Disclosure | Test-only credentials from seedUserAPI / lock-user insert | accept | See AR-07-01. `A/parity/user_api_seed_test.go:16` constant; allow-listed as known test credential in `A/parity/check_corpus.go:56`; no bcrypt hashes in fixtures | closed | + +*Status: open · closed* +*Disposition: mitigate (implementation required) · accept (documented risk) · transfer (third-party)* + +--- + +## Accepted Risks Log + +| Risk ID | Threat Ref | Rationale | Accepted By | Date | +|---------|------------|-----------|-------------|------| +| AR-07-01 | T-07-07 | Empty JWT secret already fails boot since Phase 3 (C-01); this phase layers extraction, blacklist and cutoff checks on top and does not touch secret loading. Re-asserted, not a new risk. | Jakub Zych (via secure-phase) | 2026-09-23 | +| AR-07-02 | T-07-23 | 07-07 changed no bouncer blacklist logic; the sequence test, the bouncer coverage test and the nuxt flow replay continue to assert 401 after logout. The PHP "reused token → 200" case was dropped from the ported corpus; Go intentionally keeps 401. | Jakub Zych (via secure-phase) | 2026-09-23 | +| AR-07-03 | T-07-25 | The well-known throwaway test password `parity-alice-pass` appears in plaintext in committed fixture request bodies (`nuxt-auth.yaml:12,68`, `nuxt-auth-lock.yaml:12,64`, `seed/bootstrap.yaml:26`), the same as the pre-existing Phase 2 corpus. It exists only inside an ephemeral testcontainers Postgres instance, is allow-listed as a known test credential in `check_corpus.go`, and is never committed as a hash. The plan's original wording ("never in a committed fixture body verbatim") was inaccurate and is corrected here. | Jakub Zych (via secure-phase) | 2026-09-23 | + +*Accepted risks do not resurface in future audit runs.* + +--- + +## Non-blocking Follow-ups + +1. Add the declared `git status --porcelain storage/framework/cache` pre/post check to `A/parity/php_parity.sh` (T-07-21). +2. Make the stylesheet link in `U/controllers/winter_error_page.html` relative to drop the `127.0.0.1:8423` recorder host (T-07-22). + +--- + +## Security Audit Trail + +| Audit Date | Threats Total | Closed | Open | Run By | +|------------|---------------|--------|------|--------| +| 2026-09-23 | 22 | 22 | 0 | gsd-security-auditor (verify mitigations, plan-time register) | + +--- + +## Sign-Off + +- [x] All threats have a disposition (mitigate / accept / transfer) +- [x] Accepted risks documented in Accepted Risks Log +- [x] `threats_open: 0` confirmed +- [x] `status: verified` set in frontmatter + +**Approval:** verified 2026-09-23