From b759669f2398377496a781380b78854296113fb8 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Sat, 19 Sep 2026 21:21:42 +0200 Subject: [PATCH] docs(06-05): complete unit coverage and security-review plan --- .planning/REQUIREMENTS.md | 4 +- .planning/ROADMAP.md | 6 +- .planning/STATE.md | 26 +-- .../06-05-SUMMARY.md | 170 ++++++++++++++++++ 4 files changed, 190 insertions(+), 16 deletions(-) create mode 100644 .planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-05-SUMMARY.md diff --git a/.planning/REQUIREMENTS.md b/.planning/REQUIREMENTS.md index 3c1f53b..1dbc5e2 100644 --- a/.planning/REQUIREMENTS.md +++ b/.planning/REQUIREMENTS.md @@ -56,7 +56,7 @@ Requirements for v1 (the Płytarium port). Each maps to roadmap phases. "User" b - [x] **HTTP-04**: A rate limiter supports named buckets keyed by a resolver (token id, IP, route param), stacking two limiters on one route, and ports Płytarium's five named buckets and inline throttles 1:1 - [x] **HTTP-05**: An auth guard registry lets plugins add guards (JWT, personal token, OAuth bearer) that all resolve to the same current-user accessor - [x] **HTTP-06**: Response conventions are preserved: empty arrays serialize as [], timestamps as +00:00, tri-state booleans keep null, conditional keys are omitted not nulled, and no blanket envelope or error middleware wraps OAuth routes -- [ ] **HTTP-07**: A guarded outbound fetch helper enforces host allow-lists, byte caps and timeouts for user-supplied URLs (manual cover URL, Discogs cover) +- [x] **HTTP-07**: A guarded outbound fetch helper enforces host allow-lists, byte caps and timeouts for user-supplied URLs (manual cover URL, Discogs cover) - [x] **HTTP-08**: OpenAPI is generated from swaggo/swag annotations on handlers and openapi-typescript produces the admin SPA's types - [x] **HTTP-09**: CORS and JSON body size limits match the PHP deployment @@ -189,7 +189,7 @@ Which phases cover which requirements. Updated during roadmap creation. | HTTP-04 | Phase 6 | Complete | | HTTP-05 | Phase 6 | Complete | | HTTP-06 | Phase 6 | Complete | -| HTTP-07 | Phase 6 | Pending | +| HTTP-07 | Phase 6 | Complete | | HTTP-08 | Phase 6 | Complete | | HTTP-09 | Phase 6 | Complete | | AUTH-01 | Phase 7 | Pending | diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index 847ec58..b8d51f5 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -18,7 +18,7 @@ Decimal phases appear between their surrounding integers in numeric order. - [x] **Phase 3: First vertical slice — genres end to end** - `GET /_fonoteka/api/v1/genres` passes the parity diff through every layer (completed 2026-09-17) - [x] **Phase 4: CLI scaffolding, i18n and mail** - Scaffolding commands, translated/pluralized strings, mail templates (completed 2026-09-18) - [x] **Phase 5: Data layer full fidelity** - All 25 models and their squashed migrations with fillable/hidden/cast/soft-delete discipline (completed 2026-09-18) -- [ ] **Phase 6: HTTP routing, auth groups and rate limiting** - Three auth groups, named rate buckets, OAuth-safe middleware structure +- [x] **Phase 6: HTTP routing, auth groups and rate limiting** - Three auth groups, named rate buckets, OAuth-safe middleware structure (completed 2026-09-19) - [ ] **Phase 7: User plugin and authentication** - Registration, login, JWT, organizations, personal tokens, must-change-password - [ ] **Phase 8: OAuth2.1 authorization server** - zitadel/oidc server for fonoteka-mcp and the ChatGPT connector - [ ] **Phase 9: Backend admin authentication and schema pipeline** - Admin roles, fields.yaml/columns.yaml, relation manager @@ -241,7 +241,7 @@ Plans: **Wave 4** *(blocked on 06-01..06-04)* -- [ ] 06-05-PLAN.md — Full unit coverage across both repos, full route-table mutual-exclusivity test, 06-SECURITY-REVIEW.md +- [x] 06-05-PLAN.md — Full unit coverage across both repos, full route-table mutual-exclusivity test, 06-SECURITY-REVIEW.md ### Phase 7: User plugin and authentication @@ -410,7 +410,7 @@ Phases execute in numeric order: 1 → 2 → 3 → 4 → 5 → 6 → 7 → 8 → | 3. First vertical slice — genres end to end | 4/4 | Complete | 2026-09-17 | | 4. CLI scaffolding, i18n and mail | 4/4 | Complete | 2026-09-18 | | 5. Data layer full fidelity | 6/6 | Complete | 2026-09-18 | -| 6. HTTP routing, auth groups and rate limiting | 4/5 | In Progress| | +| 6. HTTP routing, auth groups and rate limiting | 5/5 | Complete | 2026-09-19 | | 7. User plugin and authentication | 0/TBD | Not started | - | | 8. OAuth2.1 authorization server | 0/TBD | Not started | - | | 9. Backend admin authentication and schema pipeline | 0/TBD | Not started | - | diff --git a/.planning/STATE.md b/.planning/STATE.md index 912d10a..7656926 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -2,16 +2,16 @@ gsd_state_version: 1.0 milestone: v1.0 milestone_name: milestone -status: executing -stopped_at: Completed 06-03-PLAN.md -last_updated: "2026-09-19T19:00:29.025Z" +status: verifying +stopped_at: Completed 06-05-PLAN.md +last_updated: "2026-09-19T19:21:17.969Z" last_activity: 2026-09-19 progress: total_phases: 15 - completed_phases: 5 + completed_phases: 6 total_plans: 28 - completed_plans: 27 - percent: 33 + completed_plans: 28 + percent: 40 --- # Project State @@ -26,11 +26,11 @@ See: .planning/PROJECT.md (updated 2026-09-16) ## Current Position Phase: 06 (http-routing-auth-groups-and-rate-limiting) — EXECUTING -Plan: 4 of 5 -Status: Ready to execute +Plan: 5 of 5 +Status: Phase complete — ready for verification Last activity: 2026-09-19 -Progress: [██████████] 96% +Progress: [██████████] 100% ## Performance Metrics @@ -73,6 +73,7 @@ Progress: [██████████] 96% | Phase 06 P01 | 25 min | 3 tasks | 26 files | | Phase 06 P02 | 14 min | 3 tasks | 16 files | | Phase 06 P03 | 20 min | 3 tasks | 24 files | +| Phase 06 P05 | 13 min | 3 tasks | 13 files | ## Accumulated Context @@ -167,6 +168,9 @@ Recent decisions affecting current work: - [Phase 06]: Production body limits are 134217728/134217728 (128MiB), operator-confirmed 2026-09-19 from nginx client_max_body_size=128M and php.ini post_max_size=128M/upload_max_filesize=128M (D-18, T-06-13) - [Phase 06]: CORS path globs compile as Laravel nested * because Go path.Match would miss /api/v1/fonoteka/genres (Pitfall 10) - [Phase 06]: swag v1 Swagger 2 is converted by a local swagger2openapi helper to OpenAPI 3 for openapi-typescript 7; Phase 10 wires types into the admin SPA +- [Phase 06]: Full route-table isolation uses surf.BuildRouter of the real plugins; app.Handler returns http.Handler and cannot call Routes() — app.Handler assembles an http.Handler; Routes() is on *surf.Router +- [Phase 06]: T-06-05 remains accept as originating 06-01 (the 06-05 plan three-accepts list omitted it) — Originating plan disposition is copied verbatim into 06-SECURITY-REVIEW.md +- [Phase 06]: PublicOnlyMode any-host-when-public is proven via skipReservedCheck httptest, not a live public IP dial — Unit tests must not require outbound network ### Pending Todos @@ -188,6 +192,6 @@ Items acknowledged and carried forward from previous milestone close: ## Session Continuity -Last session: 2026-09-19T19:00:29.002Z -Stopped at: Completed 06-03-PLAN.md +Last session: 2026-09-19T19:21:17.950Z +Stopped at: Completed 06-05-PLAN.md Resume file: None diff --git a/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-05-SUMMARY.md b/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-05-SUMMARY.md new file mode 100644 index 0000000..135404a --- /dev/null +++ b/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-05-SUMMARY.md @@ -0,0 +1,170 @@ +--- +phase: 06-http-routing-auth-groups-and-rate-limiting +plan: 05 +subsystem: testing +tags: [coverage, security-review, route-table, isolation, parity, tdd-close] + +requires: + - phase: 06-http-routing-auth-groups-and-rate-limiting + provides: bouncer.Registry, FixedWindowLimiter, GroupRaw, wire helpers, path-scoped CORS, body limits, fetchguard.Fetch +provides: + - coverage-gap tests for bouncer, surf, wire, fetchguard and fonoteka auth/middleware + - full assembled route-table mutual-exclusivity test + - 06-SECURITY-REVIEW.md mapping T-06-01 through T-06-18 plus T-06-SC +affects: [phase-07-user-plugin, phase-08-oauth, phase-12-collections-albums] + +tech-stack: + added: [] + patterns: + - phase-ending *_coverage_test.go files close named gap categories with a test or a one-line skip comment + - 06-SECURITY-REVIEW.md is the phase-close threat-to-test map (file:TestName or verbatim accept) + +key-files: + created: + - bouncer/registry_coverage_test.go + - surf/limiter_coverage_test.go + - surf/routetable_coverage_test.go + - surf/cors_coverage_test.go + - wire/response_coverage_test.go + - fetchguard/fetch_coverage_test.go + - plugins/golem15/fonoteka/classes/auth/token_guard_coverage_test.go + - plugins/golem15/fonoteka/middleware/token_scope_coverage_test.go + - plugins/golem15/fonoteka/routes_isolation_test.go + - http_config_test.go + - parity/php_debug_test.go + - .planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-SECURITY-REVIEW.md + modified: + - parity/parity_test.go + +key-decisions: + - "Full route-table isolation uses surf.BuildRouter of the real plugins; app.Handler returns http.Handler and cannot call Routes()" + - "T-06-05 remains accept as originating 06-01 (the 06-05 plan's 'three accepts' list omitted it)" + - "PublicOnlyMode any-host-when-public is proven via skipReservedCheck httptest, not a live public IP dial" + +patterns-established: + - "Gap categories (a)-(g) each have a named test or an explicit one-line comment in the coverage file" + - "Phase-close security review table has exactly 19 T-06-xx rows; accept rationales are copied verbatim" + +requirements-completed: [HTTP-03, HTTP-04, HTTP-05, HTTP-06, HTTP-07, HTTP-08, HTTP-09] + +duration: 13 min +completed: 2026-09-19 +--- + +# Phase 6 Plan 05: Unit coverage, route-table isolation, and security review Summary + +**Phase-ending coverage of bouncer/surf/wire/fetchguard and fonoteka auth/middleware, a full assembled Routes() mutual-exclusivity test, and 06-SECURITY-REVIEW.md mapping every T-06-01..18 plus T-06-SC to a named passing test or a restated accept** + +## Performance + +- **Duration:** 13 min +- **Started:** 2026-09-19T19:02:51Z +- **Completed:** 2026-09-19T19:16:44Z +- **Tasks:** 3 +- **Files modified:** 13 + +## Accomplishments + +- Closed framework coverage gaps (a)-(g): neither-interface Register, MemoryStore sweep (purge 0% → 100%), RegisterHouseMiddlewareFactory duplicate-name, unmatched CORS path, Time +00:00/Z round-trip, PublicOnlyMode private-IP and any-host, empty-router Routes() plus empty-raw-group introspection on Group.raw +- App-side: TokenGuard future/past ExpiresAt and RevokedAt against real Postgres; InvScope fail-closed on a non-*ApiToken credential; PublicShareHeaders late headers after a partial body write; `TestFullRouteTableAuthGroupMutualExclusivity` walks the real assembled table +- Parity corpus: `recorded 154/154 passing 2 failing 0 unrecorded 0 pending 152` — both genres routes (jwt and personal_token) passing, no regression from 06-01 +- `06-SECURITY-REVIEW.md` maps 19 threat IDs; grep confirms no bearer/hash logging and `inv.must-change-password` only inside `HouseMiddlewares()` +- `go vet ./...` and `go test ./... -race` green in both repos (testcontainers included) + +## Task Commits + +Each task was committed atomically: + +1. **Task 1: Framework coverage gaps** - `98dd098` (test, summercms.go) +2. **Task 2: App coverage, isolation, short-safe corpus** - `760c1ff` (test, fonoteka.go) +3. **Task 2 follow-up: named tests for T-06-09 / T-06-13** - `e160e81` (test, fonoteka.go) +4. **Task 3: Security review** - `74d1eb3` (docs, summercms.go) + +**Plan metadata:** (this commit) + +## Files Created/Modified + +- `bouncer/registry_coverage_test.go` — neither-interface, nil registry, authenticate default +- `surf/limiter_coverage_test.go` — MemoryStore sweep, TrustedProxies, ClientIP empty XFF +- `surf/routetable_coverage_test.go` — house factory duplicate, empty router, empty raw group +- `surf/cors_coverage_test.go` — unmatched path independent of fonoteka; exact/pattern origins +- `wire/response_coverage_test.go` — Time null/invalid/Z/+00:00; WriteJSON encode error +- `fetchguard/fetch_coverage_test.go` — PublicOnlyMode both halves; hostAllowed; mapTransportError +- `plugins/golem15/fonoteka/classes/auth/token_guard_coverage_test.go` — future/past/revoked rows +- `plugins/golem15/fonoteka/middleware/token_scope_coverage_test.go` — wrong credential type; late headers +- `plugins/golem15/fonoteka/routes_isolation_test.go` — full Routes() exclusivity +- `parity/parity_test.go` — skip coverage counts under `-short` +- `parity/php_debug_test.go` — APP_DEBUG=false pin (T-06-09) +- `http_config_test.go` — 134217728/134217728, no INTERIM (T-06-13) +- `06-SECURITY-REVIEW.md` — 19-row threat-to-test map + +## Decisions Made + +- Isolation test calls `surf.BuildRouter` on activated real plugins (`golem15.user`, `golem15.fonoteka`) because `app.Handler` returns `http.Handler` and cannot expose `Routes()`. Importing `app` from package `fonoteka` would cycle. +- T-06-05 stays **accept**, copied verbatim from 06-01. The 06-05 plan's "three accepts" list (T-06-03, T-06-08, T-06-SC) omitted it; the originating disposition wins. +- PublicOnlyMode "any host accepted when public" uses `skipReservedCheck` + httptest TLS. A live public-IP dial is not asserted in unit tests. + +## Deviations from Plan + +### Auto-fixed Issues + +**1. [Rule 3 - Blocking] TestParityCorpus coverage subtest failed under `-short`** +- **Found during:** Task 2 verify (`go test ./... -race -short`) +- **Issue:** Ported replay skips via `parityDB` under `-short`, so `cov.Passing` stayed 0 while the coverage subtest required `passing == ported`. Pre-existing (flagged in 06-02 SUMMARY); this plan's verify command includes `-short`. +- **Fix:** Skip the coverage counts subtest when `testing.Short()`; live replay remains the non-short gate. +- **Files modified:** `parity/parity_test.go` +- **Verification:** `go test ./... -race -short` green; `go test ./parity/... -run TestParityCorpus` → passing 2 +- **Committed in:** `760c1ff` (Task 2) + +**2. [Rule 2 - Missing Critical] T-06-09 and T-06-13 had no named test for the security-review map** +- **Found during:** Task 3 (threat-to-test mapping) +- **Issue:** APP_DEBUG=false lived only in `php_parity.sh`; production 134217728 lived only in yaml. Mitigate dispositions require `file:TestName`. +- **Fix:** `TestPHPParityPinsAppDebugFalse` and `TestProductionBodyLimitsOperatorConfirmed`. +- **Files modified:** `parity/php_debug_test.go`, `http_config_test.go` +- **Verification:** both tests pass +- **Committed in:** `e160e81` + +--- + +**Total deviations:** 2 auto-fixed (1 Rule 3, 1 Rule 2) +**Impact on plan:** Both required for the plan's own verify command and the 19-row security review. No production-code change. No scope creep. + +## Issues Encountered + +None beyond the documented deviations. + +`scripts/check-phase2.sh --fresh-php` is present (PHP artisan at `/media/nvme/dev/golem15/fonoteka`, port 8423 free, docker up). It was not run as a completion gate, matching Phase 3's precedent that `--fresh-php` is a sign-off convenience, not a plan gate. + +## User Setup Required + +None - no external service configuration required. + +## Next Phase Readiness + +- Phase 6 is complete: guard registry, limiter, raw groups, CORS/body limits, SSRF helper, full coverage, and the security review. +- Ready for `/gsd:verify-work 6` and `/gsd:plan-phase 7`. +- No blockers. OAuth client-credentials/token-exchange remains a Phase 8 pre-planning check. + +## Self-Check: PASSED + +- FOUND: `bouncer/registry_coverage_test.go` +- FOUND: `surf/limiter_coverage_test.go` +- FOUND: `surf/routetable_coverage_test.go` +- FOUND: `surf/cors_coverage_test.go` +- FOUND: `wire/response_coverage_test.go` +- FOUND: `fetchguard/fetch_coverage_test.go` +- FOUND: `../fonoteka.go/plugins/golem15/fonoteka/classes/auth/token_guard_coverage_test.go` +- FOUND: `../fonoteka.go/plugins/golem15/fonoteka/middleware/token_scope_coverage_test.go` +- FOUND: `../fonoteka.go/plugins/golem15/fonoteka/routes_isolation_test.go` +- FOUND: `.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-SECURITY-REVIEW.md` +- FOUND: `98dd098` +- FOUND: `760c1ff` +- FOUND: `e160e81` +- FOUND: `74d1eb3` +- FOUND: threat table 19 rows (`grep -c '^| T-06-'` = 19) +- FOUND: parity `recorded 154/154 passing 2` +- FOUND: `go vet ./...` and `go test ./... -race` green in both repos + +--- +*Phase: 06-http-routing-auth-groups-and-rate-limiting* +*Completed: 2026-09-19*