docs(06-05): map every T-06-xx threat to a passing test or restated accept
- 19-row register T-06-01 through T-06-18 plus T-06-SC - mitigate rows name file:TestName; accept rows copy originating rationale - grep: no bearer/hash logging; inv.must-change-password only in HouseMiddlewares
This commit is contained in:
Jakub Zych
@@ -0,0 +1,177 @@
|
||||
---
|
||||
phase: 06
|
||||
slug: http-routing-auth-groups-and-rate-limiting
|
||||
status: verified
|
||||
threats_open: 0
|
||||
asvs_level: 1
|
||||
created: 2026-09-19
|
||||
verified: 2026-09-19
|
||||
---
|
||||
|
||||
# Phase 6 — Security Review
|
||||
|
||||
> Guard registry, dual-group auth, rate limiting, raw-group house-middleware refusal, CORS/body-limit scoping, and the SSRF fetch helper. Every `T-06-01` through `T-06-18` plus `T-06-SC` from plans 06-01 through 06-04 is mapped below to a named passing test or a restated accept rationale. Unmapped IDs are a review gap, not an accepted risk.
|
||||
|
||||
**Date:** 2026-09-19
|
||||
**Scope:** Plans 06-01 through 06-04 (implementation) and 06-05 (coverage + this review).
|
||||
**Repos grepped:** `summercms.go` and `fonoteka.go` (excluding `.planning/` and `vendor/`).
|
||||
|
||||
---
|
||||
|
||||
## Trust Boundaries
|
||||
|
||||
| Boundary | Description | Data Crossing |
|
||||
|----------|-------------|---------------|
|
||||
| client → Authorization header | untrusted JWT or `inv_` bearer parsed on every request | raw token, token hash, `users.id` |
|
||||
| guard registry → plugin Boot | plugin-declared guard names become live auth middleware | `jwt`, `inv_token` |
|
||||
| inv_token guard → `golem15_fonoteka_api_tokens` | hash-indexed lookup of an untrusted bearer | `token_hash`, scopes, expiry, revocation |
|
||||
| client → X-Forwarded-For / limiter keys | untrusted IP / token id / route param feeds the Store | `RemoteAddr`, XFF, `tok:<id>` |
|
||||
| public-share group → anonymous caller | zero-credential surface; 429 bodies must not leak internals | Retry-After, JSON error body |
|
||||
| raw group → house middleware | RFC/OAuth surface must never inherit the house envelope | `inv.must-change-password` |
|
||||
| request body → handler | unbounded POST is a resource-exhaustion vector | `http.MaxBytesReader` |
|
||||
| caller-supplied URL → outbound fetch | user/third-party URL must never reach loopback, RFC1918, CGNAT, or metadata | dial-time IP, host allow-list |
|
||||
|
||||
---
|
||||
|
||||
## Threat Register
|
||||
|
||||
| Threat ID | Category | Plan of origin | Disposition | Proof |
|
||||
|-----------|----------|----------------|-------------|-------|
|
||||
| T-06-01 | Spoofing | 06-01 | mitigate | `bouncer/registry_test.go:TestDuplicateGuardNameFailsWithPluginAndName`; `bouncer/registry_test.go:TestUnknownGuardNameFails`; `bouncer/registry_test.go:TestRegisterNeitherInterfaceNamesPluginAndName` |
|
||||
| T-06-02 | Elevation of Privilege | 06-01 | mitigate | `plugins/golem15/fonoteka/routes_isolation_test.go:TestFullRouteTableAuthGroupMutualExclusivity`; `plugins/golem15/fonoteka/routes_group_test.go:TestGenresSharedHandler` |
|
||||
| T-06-03 | Information Disclosure | 06-01 | accept | Already hidden via json:"-" and Hidden() (Phase 5, verified by 05-06's hidden-marshal test); this plan adds no new serialization path for the hash |
|
||||
| T-06-04 | Repudiation | 06-01 | mitigate | `plugins/golem15/fonoteka/classes/auth/token_guard_test.go:TestTokenGuard` (`valid-stamps-once`); grep of the auth package finds no fmt/log of the raw bearer |
|
||||
| T-06-05 | Tampering | 06-01 | accept | Indexed equality lookup (not a byte-for-byte secret compare) is not a timing side-channel per RESEARCH.md's V6 Cryptography note; crypto/subtle is reserved for a future raw-compare path (e.g. OAuth client secrets, Phase 8), not needed here |
|
||||
| T-06-06 | Denial of Service | 06-02 | mitigate | `surf/clientip_test.go:TestClientIPRejectsSpoofedXFF` |
|
||||
| T-06-07 | Information Disclosure | 06-02 | mitigate | `plugins/golem15/fonoteka/middleware/public_share_headers_test.go:TestPublicShareHeadersRewrites429` |
|
||||
| T-06-08 | Denial of Service | 06-02 | accept | v1 ships an unbounded-until-swept map per CONTEXT D-03's explicit "no otter/cooler this phase" decision; the sweep goroutine bounds long-term growth to roughly one decay window's worth of distinct keys, acceptable for a single-instance v1 deployment |
|
||||
| T-06-09 | Repudiation | 06-02 | mitigate | `parity/php_debug_test.go:TestPHPParityPinsAppDebugFalse` |
|
||||
| T-06-10 | Elevation of Privilege | 06-03 | mitigate | `plugins/golem15/fonoteka/routes_isolation_test.go:TestFullRouteTableAuthGroupMutualExclusivity` (full assembled `Router.Routes()`, not a hand-built fixture) |
|
||||
| T-06-11 | Tampering | 06-03 | mitigate | `surf/routetable_test.go:TestRawGroupHouseMiddlewareRefusedAtBuild`; `plugins/golem15/fonoteka/routes_cors_test.go:TestRawGroupRefusesHouseMiddlewareOnRealPlugins`; `plugins/golem15/fonoteka/routes_cors_test.go:TestHouseMiddlewareCapabilityOnRealPlugins` |
|
||||
| T-06-12 | Denial of Service | 06-03 | mitigate | `surf/bodylimit_test.go:TestBodyLimitDefaultRejectsOversizedBody`; `surf/bodylimit_test.go:TestBodyLimitRawExempt` |
|
||||
| T-06-13 | Information Disclosure | 06-03 | mitigate | `http_config_test.go:TestProductionBodyLimitsOperatorConfirmed` (134217728 / 134217728; no INTERIM) |
|
||||
| T-06-14 | Elevation of Privilege | 06-04 | mitigate | `fetchguard/fetch_test.go:TestFetchPrivateIPBlockedInBothModes`; `fetchguard/ip_test.go:TestIsReservedOrPrivate` |
|
||||
| T-06-15 | Tampering | 06-04 | mitigate | `fetchguard/fetch_test.go:TestFetchPrivateIPBlockedInBothModes` (dial-time `net.Dialer.Control` on the address being connected, not a pre-resolved hostname) |
|
||||
| T-06-16 | Denial of Service | 06-04 | mitigate | `fetchguard/fetch_test.go:TestFetchTooLargeIsStreaming` |
|
||||
| T-06-17 | Elevation of Privilege | 06-04 | mitigate | `fetchguard/fetch_test.go:TestFetchDoesNotFollowRedirect` |
|
||||
| T-06-18 | Spoofing | 06-04 | mitigate | `fetchguard/fetch_test.go:TestFetchAllowHostsRejectsDottedSuffixBypass`; `fetchguard/fetch_coverage_test.go:TestHostAllowedExactAndDottedSuffix` |
|
||||
| T-06-SC | Tampering | 06-03 | accept | Both packages are STACK.md-named and pass 06-RESEARCH.md's Package Legitimacy Audit (Approved disposition, no [ASSUMED]/[SUS] verdicts) -- no additional human-verify checkpoint required beyond that prior audit |
|
||||
|
||||
*Status: closed. Disposition copied verbatim from the originating plan. Accept rationales copied verbatim.*
|
||||
|
||||
---
|
||||
|
||||
## Findings by Threat
|
||||
|
||||
### T-06-01 — duplicate or unknown guard names fail boot
|
||||
|
||||
- **Source:** `bouncer/registry.go` (`Register`, `Middleware`).
|
||||
- **Test evidence:** `TestDuplicateGuardNameFailsWithPluginAndName`, `TestUnknownGuardNameFails`, `TestRegisterNeitherInterfaceNamesPluginAndName`.
|
||||
- **Finding:** Empty name, nil guard, a type implementing neither `Guard` nor `CredentialGuard`, a duplicate name, and an unknown `Middleware` lookup all return a `bouncer: ...` error naming plugin and guard. No silent no-op auth.
|
||||
- **Disposition:** closed / mitigate.
|
||||
|
||||
### T-06-02 / T-06-10 — jwt and inv_token groups are mutually exclusive
|
||||
|
||||
- **Source:** `plugins/golem15/fonoteka/routes.go`; `surf/routetable.go` `Routes()`.
|
||||
- **Test evidence:** `TestFullRouteTableAuthGroupMutualExclusivity` walks the real `BuildRouter` table for `golem15.user` + `golem15.fonoteka`. Zero `/api/v1/fonoteka*` entries carry `jwt.auth`; zero `/_fonoteka/api/v1*` entries carry `inv_token` or `inv.scope:*`. `TestGenresSharedHandler` proves both groups reach the same handler through different guards.
|
||||
- **Finding:** Plan 06-01's partial coverage (two groups never sharing a middleware list literal) is completed over the whole assembled table, not the genres pair alone.
|
||||
- **Disposition:** closed / mitigate.
|
||||
|
||||
### T-06-03 — ApiToken.TokenHash serialization (accept)
|
||||
|
||||
- **Rationale (verbatim from 06-01):** Already hidden via json:"-" and Hidden() (Phase 5, verified by 05-06's hidden-marshal test); this plan adds no new serialization path for the hash.
|
||||
- **Supporting evidence:** `classes/hidden_marshal_test.go:TestHiddenNeverMarshals` / `TestSecretColumnNames` (`token_hash` is a secret column). Phase 6 added no marshal path.
|
||||
- **Disposition:** closed / accept.
|
||||
|
||||
### T-06-04 — last_used stamp without logging the bearer
|
||||
|
||||
- **Source:** `plugins/golem15/fonoteka/classes/auth/token_guard.go` (`UpdateColumns` of `last_used_at` / `last_used_ip` only).
|
||||
- **Test evidence:** `TestTokenGuard` / `valid-stamps-once` asserts one stamp per `AuthenticateCredential` call.
|
||||
- **Grep:** `rg -n 'fmt\.(Print\|Printf\|Println)\|log\.(Print\|Printf\|Println\|Fatal)\|slog\.'` over `fonoteka.go/plugins/golem15/fonoteka/classes/auth` and `summercms.go/bouncer` returns no matches. `LastUsedIP` appears only as the DB column write and test assertions. `bearerToken` is local; the raw bearer is hashed then discarded. `bouncer.Credential` call sites are InvScope (type-assert + HasScope) and the `fonoteka-api-token` bucket key (`tok:<id>`), never a log line.
|
||||
- **Disposition:** closed / mitigate.
|
||||
|
||||
### T-06-05 — SHA-256 hash lookup timing (accept)
|
||||
|
||||
- **Rationale (verbatim from 06-01):** Indexed equality lookup (not a byte-for-byte secret compare) is not a timing side-channel per RESEARCH.md's V6 Cryptography note; crypto/subtle is reserved for a future raw-compare path (e.g. OAuth client secrets, Phase 8), not needed here.
|
||||
- **Disposition:** closed / accept.
|
||||
|
||||
### T-06-06 — X-Forwarded-For spoofing
|
||||
|
||||
- **Source:** `surf/clientip.go`.
|
||||
- **Test evidence:** `TestClientIPRejectsSpoofedXFF` — untrusted `RemoteAddr` ignores XFF; `TestClientIPRightmostUntrustedHop` honors XFF only when RemoteAddr is inside `http.trusted_proxies`.
|
||||
- **Disposition:** closed / mitigate.
|
||||
|
||||
### T-06-07 — public-share 429 body
|
||||
|
||||
- **Source:** `plugins/golem15/fonoteka/middleware/public_share_headers.go`.
|
||||
- **Test evidence:** `TestPublicShareHeadersRewrites429` rewrites `{"message":"Too Many Attempts."}` to `{"error":"Too many requests"}` while preserving limiter headers and setting `X-Robots-Tag` / `Cache-Control`.
|
||||
- **Disposition:** closed / mitigate.
|
||||
|
||||
### T-06-08 — MemoryStore cardinality (accept)
|
||||
|
||||
- **Rationale (verbatim from 06-02):** v1 ships an unbounded-until-swept map per CONTEXT D-03's explicit "no otter/cooler this phase" decision; the sweep goroutine bounds long-term growth to roughly one decay window's worth of distinct keys, acceptable for a single-instance v1 deployment.
|
||||
- **Supporting evidence:** `surf/limiter_coverage_test.go:TestMemoryStoreSweepRemovesExpiredEntry` proves the sweep actually deletes expired entries (not only the lazy `TooManyAttempts` path).
|
||||
- **Disposition:** closed / accept.
|
||||
|
||||
### T-06-09 — APP_DEBUG on recorded fixtures
|
||||
|
||||
- **Source:** `parity/php_parity.sh` `export APP_DEBUG=false`.
|
||||
- **Test evidence:** `TestPHPParityPinsAppDebugFalse`.
|
||||
- **Finding:** 06-02 audited three existing HTML-exception fixtures recorded under debug; they remain flagged for re-record and are not 429s. Future recordings are production-shaped.
|
||||
- **Disposition:** closed / mitigate.
|
||||
|
||||
### T-06-11 — raw group cannot take house-envelope middleware
|
||||
|
||||
- **Source:** `surf/router.go` `wrap()`; `pact.HasHouseMiddleware`; `Plugin.HouseMiddlewares()`.
|
||||
- **Test evidence:** `TestRawGroupHouseMiddlewareRefusedAtBuild`, `TestRawGroupRefusesHouseMiddlewareOnRealPlugins`, `TestHouseMiddlewareCapabilityOnRealPlugins`.
|
||||
- **Grep:** `inv.must-change-password` appears in `plugin.go` only inside `HouseMiddlewares()` (line 75), never inside `Middlewares()`. Plugins do not call `RegisterHouseMiddleware` / `RegisterMiddleware`.
|
||||
- **Disposition:** closed / mitigate.
|
||||
|
||||
### T-06-12 / T-06-13 — body limits
|
||||
|
||||
- **Source:** `surf/bodylimit.go`; `fonoteka.go/config/http.yaml`.
|
||||
- **Test evidence:** `TestBodyLimitDefaultRejectsOversizedBody` (MaxBytesReader 413 on non-raw); `TestBodyLimitRawExempt`; `TestProductionBodyLimitsOperatorConfirmed` (both keys 134217728, no INTERIM). Operator-confirmed 2026-09-19 from nginx `client_max_body_size=128M` and php.ini `post_max_size=128M` / `upload_max_filesize=128M`.
|
||||
- **Disposition:** closed / mitigate.
|
||||
|
||||
### T-06-14 through T-06-18 — SSRF fetch helper
|
||||
|
||||
- **Source:** `fetchguard/ip.go`, `fetchguard/fetch.go`.
|
||||
- **Test evidence:** private/reserved/CGNAT/metadata table (`TestIsReservedOrPrivate`); always-on dial-time block in both modes (`TestFetchPrivateIPBlockedInBothModes`); streaming cap (`TestFetchTooLargeIsStreaming`); no automatic redirects (`TestFetchDoesNotFollowRedirect`); dotted-suffix allow-list (`TestFetchAllowHostsRejectsDottedSuffixBypass`, `TestHostAllowedExactAndDottedSuffix`).
|
||||
- **Disposition:** closed / mitigate.
|
||||
|
||||
### T-06-SC — OpenAPI toolchain packages (accept)
|
||||
|
||||
- **Rationale (verbatim from 06-03):** Both packages are STACK.md-named and pass 06-RESEARCH.md's Package Legitimacy Audit (Approved disposition, no [ASSUMED]/[SUS] verdicts) -- no additional human-verify checkpoint required beyond that prior audit.
|
||||
- **Disposition:** closed / accept.
|
||||
|
||||
---
|
||||
|
||||
## Credential / bearer logging grep
|
||||
|
||||
`rg -n 'raw|bearer|LastUsedIP' fonoteka.go/plugins/golem15/fonoteka/classes/auth` (excluding tests): `bearerToken` helper, `LastUsedIP` column write in `UpdateColumns`, no adjacent `fmt.Print*` / `log.*` / `slog`. `summercms.go/bouncer` has no Print/log of the token. `bouncer.Credential` is read by InvScope and the named bucket key only.
|
||||
|
||||
## House-middleware registration grep
|
||||
|
||||
```
|
||||
grep -n "inv.must-change-password" fonoteka.go/plugins/golem15/fonoteka/plugin.go
|
||||
```
|
||||
|
||||
```
|
||||
75: "inv.must-change-password": middleware.MustChangePassword,
|
||||
```
|
||||
|
||||
That line is inside `HouseMiddlewares()`. `Middlewares()` registers `public.share-headers` and `inv_token` only. Plan 06-03's move onto `pact.HasHouseMiddleware` is the only registration path.
|
||||
|
||||
---
|
||||
|
||||
## Accepted Risks Log
|
||||
|
||||
Four accepts (06-05's "three" list omitted T-06-05, which 06-01 already accepted). No new accepts in this review. Rationales are copied verbatim in the Threat Register `Proof` column for each accept row.
|
||||
|
||||
---
|
||||
|
||||
## Security Audit Trail
|
||||
|
||||
| Audit Date | Threats Total | Closed | Open | Run By |
|
||||
|------------|---------------|--------|------|--------|
|
||||
| 2026-09-19 | 19 | 19 | 0 | gsd-executor (06-05) |
|
||||
Reference in New Issue
Block a user