golem15/summercms
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

docs(06-06): extend security review through gap closure

Browse Source
This commit is contained in:
Jakub Zych
2026-09-20 13:30:41 +02:00
parent da8f247065
commit 3423da2222
1 changed files with 24 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

29
.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-SECURITY-REVIEW.md
View File

@@ -5,15 +5,15 @@ status: verified
threats_open: 0
asvs_level: 1
created: 2026-09-19
verified: 2026-09-19
verified: 2026-09-20
---
# 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.
> 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`, `T-06-21`, `T-06-22`, and `T-06-SC` from Plans 06-01 through 06-06 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).
**Date:** 2026-09-20
**Scope:** Plans 06-01 through 06-06 (implementation, coverage, gap closure, and this review).
**Repos grepped:** `summercms.go` and `fonoteka.go` (excluding `.planning/` and `vendor/`).
---
@@ -26,6 +26,8 @@ verified: 2026-09-19
| 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>` |
| unauthenticated client → personal-token route | missing or invalid bearer traffic must consume a bounded per-IP budget before scope denial returns | bearer status, client IP, limiter counter |
| inv_token context → limiter key resolver | valid credentials must be resolved before rate limiting to retain independent token budgets | `bouncer.Credential`, `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` |
@@ -55,6 +57,8 @@ verified: 2026-09-19
| 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-21 | Denial of Service | 06-06 | mitigate | `plugins/golem15/fonoteka/routes_isolation_test.go:TestPersonalTokenGenresUnauthenticatedRequestsAreRateLimited` drives 61 same-IP requests through the real handler returned by `surf.Assemble`: requests 1-60 retain 401 `Invalid token`, while request 61 receives the exact 429 response. The source declaration and runtime-order invariant is `inv_token` -> `throttle:fonoteka-api-token` -> `inv.scope:read`. |
| T-06-22 | Denial of Service | 06-06 | mitigate | The live route keeps `inv_token` before `throttle:fonoteka-api-token`, so `bouncer.Credential` is populated before the bucket key closure and valid credentials retain `tok:<id>` keying instead of collapsing onto the IP fallback. The exact ordering is covered by `TestPersonalTokenGenresUnauthenticatedRequestsAreRateLimited` plus the route-source invariant. |
| 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.*
@@ -139,6 +143,20 @@ verified: 2026-09-19
- **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-21 — unauthenticated personal-token traffic cannot bypass the limiter
- **Source:** `plugins/golem15/fonoteka/routes.go`, whose exact declaration is `inv_token` -> `throttle:fonoteka-api-token` -> `inv.scope:read`; `surf/router.go` applies that declaration last-to-first so the same sequence is the runtime onion.
- **Test evidence:** `TestPersonalTokenGenresUnauthenticatedRequestsAreRateLimited` creates one fresh handler through `surf.Assemble` for the real `golem15.user` + `golem15.fonoteka` plugin set and drives 61 same-IP requests through `GET /api/v1/fonoteka/genres`.
- **Finding:** Requests 1-60 retain the PHP-compatible 401 `{"error":"Invalid token"}` response, proving `InvScope` still owns denial before exhaustion. Request 61 receives exactly `{"message":"Too Many Attempts."}` with exhausted `X-RateLimit-*` headers, proving missing credentials consume the 60/minute IP-fallback budget.
- **Disposition:** closed / mitigate.
### T-06-22 — valid credentials retain isolated token buckets
- **Source:** `plugins/golem15/fonoteka/routes.go`; `plugins/golem15/fonoteka/plugin.go` `fonoteka-api-token` key closure.
- **Test and invariant evidence:** The executed route keeps `inv_token` before `throttle:fonoteka-api-token`, while `TestPersonalTokenGenresUnauthenticatedRequestsAreRateLimited` exercises the same assembled production middleware chain. The exact invariant is `inv_token` -> `throttle:fonoteka-api-token` -> `inv.scope:read`.
- **Finding:** A valid personal token populates `bouncer.Credential` before the limiter resolves its key, preserving `tok:<id>` keying. Only missing or invalid credentials fall back to `surf.ClientIP`; valid credentials do not collapse onto a shared IP budget.
- **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.
@@ -166,7 +184,7 @@ That line is inside `HouseMiddlewares()`. `Middlewares()` registers `public.shar
## 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.
Four accepts (06-05's "three" list omitted T-06-05, which 06-01 already accepted). Plan 06-06 adds two mitigated threats and no new accepts. Rationales are copied verbatim in the Threat Register `Proof` column for each accept row.
---
@@ -175,3 +193,4 @@ Four accepts (06-05's "three" list omitted T-06-05, which 06-01 already accepted
| Audit Date | Threats Total | Closed | Open | Run By |
|------------|---------------|--------|------|--------|
| 2026-09-19 | 19 | 19 | 0 | gsd-executor (06-05) |
| 2026-09-20 | 21 | 21 | 0 | gsd-executor (06-06) |