docs(06-05): complete unit coverage and security-review plan
This commit is contained in:
Jakub Zych
@@ -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*
|
||||
Reference in New Issue
Block a user