13887ee0b1
Map T-03-01 through T-03-SC to passing tests, record the check-phase3.sh gate, and mark nyquist_compliant after that gate exited 0. Co-authored-by: Cursor <cursoragent@cursor.com>
133 lines
9.2 KiB
Markdown
133 lines
9.2 KiB
Markdown
---
|
|
phase: 03
|
|
slug: first-vertical-slice-genres-end-to-end
|
|
status: verified
|
|
threats_open: 0
|
|
asvs_level: 1
|
|
created: 2026-09-17
|
|
verified: 2026-09-17
|
|
---
|
|
|
|
# Phase 3 — Security Review
|
|
|
|
> Token verification, cross-plugin guard lookup, migration isolation, query tenant boundaries, and secret handling for `GET /_fonoteka/api/v1/genres`.
|
|
|
|
---
|
|
|
|
## Trust Boundaries
|
|
|
|
| Boundary | Description | Data Crossing |
|
|
|----------|-------------|---------------|
|
|
| Config and Go module resolution → process | DSN, JWT secret, and dependency metadata enter boot | `SUMMER_DATABASE__DSN`, `SUMMER_GOLEM15__USER__JWT__SECRET`, `go.mod` |
|
|
| Bearer token → user lookup → handler | Untrusted JWT claims become authenticated context | Authorization header, `users.id`, `MustChangePassword` |
|
|
| Plugin declarations → ServeMux | Named middleware and group routes compile to stdlib mux | `jwt.auth`, `inv.must-change-password`, path params |
|
|
| Authenticated user and context → SQL | Active collection and album counts must stay tenant-scoped | `owner_id`, editor rows, `collection_id`, `genre_id` |
|
|
| CLI plugin argument → migration history | Rollback must not select another plugin's gormigrate table | `--plugin`, history table name |
|
|
| Recorded fixture → test request | JWT and colliding IDs come from trusted seed state | `jwt:alice`, `id:genre` / `id:token` / `id:wishlist-album` |
|
|
|
|
---
|
|
|
|
## Threat Register
|
|
|
|
| Threat ID | Category | Severity | Component | Disposition | Mitigation | Status |
|
|
|-----------|----------|----------|-----------|-------------|------------|--------|
|
|
| T-03-01 | Tampering | high | migrations / locale / rollback | mitigate | Explicit DDL, per-plugin history tables, ICU `pl-PL` check before migrate, isolated `RollbackLast` | closed |
|
|
| T-03-02 | Elevation of privilege | high | named middleware | mitigate | Missing names fail boot with plugin+name; unauthenticated requests never reach the handler; seven-stage order | closed |
|
|
| T-03-03 | Spoofing | high | JWT guard | mitigate | HS256 pinned, `exp`+`sub` required, persisted user lookup, empty secret fails boot, 401 bodies omit secrets | closed |
|
|
| T-03-04 | Information disclosure | high | aggregate query | mitigate | Grouped owner OR editor, AND active collection; foreign albums stay at count 0 | closed |
|
|
| T-03-05 | Tampering | medium | `lagoon.OrderBy` / `non_empty` | mitigate | Allow-listed identifiers/direction, no COLLATE, `non_empty` in `0\|1` else 422 | closed |
|
|
| T-03-06 | Tampering | high | parity acceptance | mitigate | Unmodified fixture, passing increments only after replay, mutated body fails, 153 pending never count as pass | closed |
|
|
| T-03-07 | Information disclosure | medium | typed route IDs | mitigate | Malformed and unknown IDs both 404; constraints compiled at registration | closed |
|
|
| T-03-SC | Tampering | medium | Go module resolution | mitigate | Official module paths already in `go.mod`; testcontainers added only as the named STACK test dependency | closed |
|
|
|
|
*Status: open · closed*
|
|
*Disposition: mitigate (implementation required) · accept (documented risk) · transfer (third-party)*
|
|
|
|
---
|
|
|
|
## Findings by Threat
|
|
|
|
### T-03-01 — schema drift / locale / rollback isolation
|
|
|
|
- **Source:** `lagoon/connection.go` (`Open`/`Use`/`CheckLocale`), `lagoon/migrations.go` (`HistoryTableName`, `Migrate`, `RollbackLast`), Fonoteka plugin `updates` (explicit `CREATE TABLE`, no `AutoMigrate`).
|
|
- **Test evidence:** `TestWrongICULocaleFailsOpen`, `TestTwoPluginMigrationSetsIsolated`, `TestNoAutoMigrate`, app `TestMigrateSeedsCanonicalGenres`, `TestRollbackLastIsolatesFonoteka`, `TestPluginMigrationsDoNotUseAutoMigrate`, `TestGenreQueryFailsOnWrongLocale`.
|
|
- **Finding:** ICU `pl-PL` is required before GORM is used. Two plugins keep separate `summer_migrations_*` tables; rolling back `demo.beta` (framework) or `golem15.fonoteka` (app) leaves the other plugin's history and rows intact. Production code and plugin sources contain no `AutoMigrate`.
|
|
- **Disposition:** closed / mitigate.
|
|
|
|
### T-03-02 — missing or bypassed named guard
|
|
|
|
- **Source:** `surf/router.go` (`wrap` fails on unknown names; `compile` wraps recover → CORS around the mux; named auth runs after locale and before org/rate/handler), `plugins/golem15/fonoteka/plugin.go` (`Use("jwt.auth", "inv.must-change-password")`).
|
|
- **Test evidence:** `TestAssembleMissingMiddlewareFailsBoot`, `TestUnauthenticatedNamedGuardDoesNotReachHandler`, `TestPipelineOrderRecoverCORSLocaleAuthPasswordOrgRateHandler`, `TestCORSPreflightBypassesNamedAuth`, app `TestGenreSecurityBoundaries/unauthenticated` and `/cors-preflight`.
|
|
- **Finding:** A missing `jwt.auth` name fails Assemble with plugin ID and name. OPTIONS preflight returns 204 without running named auth or the genre handler (`Cache-Control` stays unset). GET without a token returns 401 `Token not provided` and does not set the handler's `Cache-Control: no-cache, private`.
|
|
- **Disposition:** closed / mitigate.
|
|
|
|
### T-03-03 — JWT forgery and secret handling
|
|
|
|
- **Source:** `bouncer/jwt.go` (`WithValidMethods([]string{"HS256"})`, `WithExpirationRequired()`, nonempty `sub`, `UserProvider.FindByID`), `plugins/golem15/user/user.go` (`jwtSecret` fails closed).
|
|
- **Test evidence:** `TestVerifyRejectsBadTokens`, `TestVerifyRejectsAlgNoneEmptySecretAndAbsentExp`, `TestVerifyAndMiddlewareOmitTokenAndSecret`, `TestMiddlewareStatusBodies`, app `TestGenreSecurityBoundaries` (malformed / expired / wrong HMAC / alg:none / bad signature / absent exp / absent sub / unknown user), `TestEmptyJWTSecretFailsBoot`, `TestGenreListBehindJWT`.
|
|
- **Finding:** `alg:none`, HS384, bad signatures, missing/expired `exp`, missing `sub`, and unknown subjects never reach the handler. Empty `golem15.user.jwt.secret` fails `Boot`. 401 JSON bodies are PHP-shaped and do not echo the token or secret.
|
|
- **Disposition:** closed / mitigate.
|
|
|
|
### T-03-04 — cross-tenant album counts
|
|
|
|
- **Source:** `plugins/golem15/fonoteka/active_collection.go` (`AccessibleByMembership` grouped OR, then AND `collection_id`), `genre_handler.go` left-join count.
|
|
- **Test evidence:** `TestGenreCountsScopedToActiveCollection`, `TestGenreSecurityBoundaries/alice-counts-ignore-foreign`, `/bob-counts-ignore-alice`.
|
|
- **Finding:** Alice's rock albums do not appear in Bob's jazz counts and vice versa. Invalid stored context falls back to the lowest-ID accessible real collection. Wishlist and foreign collections are not counted.
|
|
- **Disposition:** closed / mitigate.
|
|
|
|
### T-03-05 — ORDER BY / `non_empty` injection
|
|
|
|
- **Source:** `lagoon/order.go` allow-list, `genre_handler.go` `parseNonEmpty`.
|
|
- **Test evidence:** `TestOrderClauseAllowList`, integration `non_empty=0|1|invalid` and duplicate-key last-wins, 422 envelope.
|
|
- **Finding:** Unknown columns and directions are rejected. No COLLATE is emitted. Invalid `non_empty` returns the PHP 422 envelope.
|
|
- **Disposition:** closed / mitigate.
|
|
|
|
### T-03-06 — false-green parity
|
|
|
|
- **Source:** `parity/parity_test.go` (`runCorpusRoute` increments passing only after ported replay), `parity/manifest.yaml` (one `ported` route with `seed_hook: genres`), unmodified `fixtures/routes/get_genres_jwt.yaml`.
|
|
- **Test evidence:** `TestParityCorpus` coverage subtest (154 recorded, 1 passing, 153 pending, 0 failing, 0 unrecorded), `TestParityContract/honest-counts`, `ported-mismatch`, `ported-mutated-response`.
|
|
- **Finding:** Pending routes are not sent to the Go handler. A mutated `album_count` fails `ReplayFlow`. The recorded PHP fixture still contains `Bearer {{jwt:alice}}` and `"album_count":0`.
|
|
- **Disposition:** closed / mitigate.
|
|
|
|
### T-03-07 — typed ID oracle
|
|
|
|
- **Source:** `surf/params.go` (`IntParam`, `constrain`), `examples/hello/plugins/greeter/plugin.go`.
|
|
- **Test evidence:** `TestTypedIDRouteReturns404`, `TestWhereInRejectsOutsideEnum`, `TestTypedItemRoute`.
|
|
- **Finding:** `/items/nope` and `/items/99` both 404; `/kinds/other` 404. Request text never builds a regex or SQL fragment.
|
|
- **Disposition:** closed / mitigate.
|
|
|
|
### T-03-SC — module path legitimacy
|
|
|
|
- **Source:** framework `go.mod` (GORM, pgx, gormigrate, golang-jwt, testcontainers at STACK versions).
|
|
- **Test evidence:** `go vet ./...` / `go test ./...` in both modules; slopcheck false positives on recent versions documented in 03-RESEARCH.md.
|
|
- **Finding:** No unofficial module path was added. testcontainers is the STACK-named test dependency used by `lagoon` isolation tests and the existing app `TestMain`.
|
|
- **Disposition:** closed / mitigate.
|
|
|
|
---
|
|
|
|
## Accepted Risks Log
|
|
|
|
No accepted risks.
|
|
|
|
High-severity JWT, missing-guard, cross-tenant, migration-isolation, and false-green parity issues are mitigated with failing-when-broken tests. Token *issuing* remains test-only (Phase 7). Full CORS/locale/rate-limit depth remains Phase 6. Accented/punctuation Polish collation vs MariaDB is documented as a later fixture risk (T-03-05 residual, medium, not open).
|
|
|
|
---
|
|
|
|
## Security Audit Trail
|
|
|
|
| Audit Date | Threats Total | Closed | Open | Run By |
|
|
|------------|---------------|--------|------|--------|
|
|
| 2026-09-17 | 8 | 8 | 0 | gsd-executor (03-04) |
|
|
|
|
---
|
|
|
|
## 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
|
|
- [x] No open high-severity JWT or cross-tenant issue remains
|
|
|
|
**Approval:** verified 2026-09-17
|