docs(phase-08): add security threat verification
This commit is contained in:
@@ -0,0 +1,96 @@
|
||||
---
|
||||
phase: "08"
|
||||
slug: "oauth2-1-authorization-server"
|
||||
status: verified
|
||||
# threats_open = count of OPEN threats at or above workflow.security_block_on severity (the blocking gate)
|
||||
threats_open: 0
|
||||
threats_total: 11
|
||||
threats_closed: 11
|
||||
accepted_risks: 0
|
||||
asvs_level: 1
|
||||
block_on: high
|
||||
register_authored_at_plan_time: true
|
||||
created: "2026-10-12"
|
||||
---
|
||||
|
||||
# Phase 08 — Security
|
||||
|
||||
> Per-phase security contract: threat register, accepted risks, and audit trail.
|
||||
|
||||
Detailed per-threat findings and named test evidence live in `08-SECURITY-REVIEW.md` (2026-09-24, performed by the 08-10 executor, not an independent auditor; see its Reviewer Note). This file is the gate contract derived from all ten `<threat_model>` blocks. The plans carry no severity column; severities below are assigned at this audit. Framework paths are relative to `summercms.go/modules/`; app paths are relative to `fonoteka.go/plugins/golem15/fonoteka/`.
|
||||
|
||||
---
|
||||
|
||||
## Trust Boundaries
|
||||
|
||||
| Boundary | Description | Data Crossing |
|
||||
|----------|-------------|---------------|
|
||||
| connector → raw `/.well-known` and `/oauth/mcp/*` routes | untrusted discovery, authorize, register and token traffic on raw (unguarded) routes | client metadata, redirect URIs, PKCE challenge/verifier, client secret (high) |
|
||||
| validated redirect → `Location` | client-controlled redirect trusted only after exact allow-list match | code, state, error (high) |
|
||||
| wristband records → GORM | app-agnostic code/refresh/client state persisted with row locks | code hash, refresh hash, lineage (high) |
|
||||
| browser JWT principal → consent / connected-app API | authenticated user ids cross ownership, scope and collection boundaries | request handle, scopes, token ids (high) |
|
||||
| refresh credential → token endpoint | long-lived bearer-like credential requests a new grant branch | refresh token (high) |
|
||||
| transaction outcome → OAuth error | lineage revocation must commit although the response is `invalid_grant` | revocation state |
|
||||
| operator CLI → client store | trusted input creates a recoverable-once confidential client | client secret (high) |
|
||||
| bearer header → personal-token `/me` | untrusted bearer crosses existing token and scope guards | user profile fields (medium) |
|
||||
| PHP capture → committed fixtures; gate → child processes | live credentials must become placeholders before git; secrets cross shell/Node/Go | credentials (high) |
|
||||
|
||||
---
|
||||
|
||||
## Threat Register
|
||||
|
||||
| Threat ID | Category | Component | Severity | Disposition | Mitigation | Status |
|
||||
|-----------|----------|-----------|----------|-------------|------------|--------|
|
||||
| T-08-PKCE | Spoofing/Elevation | authorize / token exchange | high | mitigate | `wristband/authorize.go:77` requires `code_challenge_method=S256`, rejects missing challenge (`:84`); verifier compared through `wristband/crypto.go:34` `subtle.ConstantTimeCompare` | closed |
|
||||
| T-08-CODE-REPLAY | Spoofing | auth-code store / exchange | high | mitigate | `classes/auth/oauth_store.go:130` `ByCodeHashForUpdate` (`clause.Locking UPDATE`) inside the single transaction at `:45`; concurrent one-winner tests per 08-SECURITY-REVIEW | closed |
|
||||
| T-08-REFRESH-REPLAY | Spoofing/Elevation | refresh state machine | high | mitigate | `oauth_store.go:199` `ByTokenHashForUpdate`, `:243` `RevokeLineage` (locked walk); lineage kill committed before `invalid_grant`; manual-token-delete cascade fixed in 08-10 (`controllers/api/token_api_controller.go` `Destroy`) | closed |
|
||||
| T-08-OPEN-REDIRECT | Spoofing/Disclosure | authorize | high | mitigate | `wristband/authorize.go:62` exact `stringSliceContains(client.RedirectURIs, …)` check precedes every `authorizeErrorRedirect` | closed |
|
||||
| T-08-SCOPE-CEILING | Elevation | authorize / consent / refresh / CLI | high | mitigate | requested ∩ `ScopeCeiling` (`wristband/stores.go:29`) before persistence; consent four-way intersection app-owned (`wristband/consent.go:8`, D-08); refresh copies stored scopes only | closed |
|
||||
| T-08-CROSS-USER | Elevation | consent / connected apps | high | mitigate | `controllers/api/connected_app_controller.go:28,75` scope reads/deletes to `user_id = ?`; missing/foreign/manual ids return identical 404 `Token not found` (`:70,:78`) | closed |
|
||||
| T-08-SURFACE | Elevation | route groups / MCP boundary | high | mitigate | raw OAuth routes vs JWT management routes proven by `routes_isolation_test.go:TestFullRouteTableAuthGroupMutualExclusivity`, `TestOAuthTokenSurfaceIsolationCoverage`, `TestOAuthMetadataRouteIsolation` | closed |
|
||||
| T-08-REQUEST-LEAK | Information Disclosure | metadata / logs / fixtures / output | high | mitigate | opaque pending-request handle (`wristband/authorize.go:29`); positive allow-list serializers (`TestOAuthConsentShowExactKeySet`); no `fmt.Print*`/`log.*`/`slog.*` on secrets; `check-phase8.sh` `redact_phase8` + `stage_secret_scan` | closed |
|
||||
| T-08-SECRET-TIMING | Information Disclosure | client auth / issued client | medium | mitigate | single hash/compare path through `wristband/crypto.go:34` `subtle.ConstantTimeCompare`; secret shown once by the client command | closed |
|
||||
| T-08-DCR-FLOOD | Denial of Service | register / client store | medium | mitigate | `wristband/register.go:67` `http.MaxBytesReader` at 65536 (`server.go:99`); `DCRClientCap` (`server.go:50`, `ErrClientCapReached`) under transactional cap/sweep/create; `throttle:fonoteka-oauth-register` 30/min per IP | closed |
|
||||
| T-08-SC | Tampering | dependencies | low | mitigate | no `go.mod`/`go.sum` change in any phase 08 commit; stdlib port, no zitadel/oidc (D-01); Playwright/Node reused from existing lockfile | closed |
|
||||
|
||||
*Status: open · closed · open — below high threshold (non-blocking)*
|
||||
*Severity: critical > high > medium > low — only open threats at or above workflow.security_block_on count toward threats_open*
|
||||
*Disposition: mitigate (implementation required) · accept (documented risk) · transfer (third-party)*
|
||||
|
||||
---
|
||||
|
||||
## Accepted Risks Log
|
||||
|
||||
No accepted risks.
|
||||
|
||||
*Accepted risks do not resurface in future audit runs.*
|
||||
|
||||
---
|
||||
|
||||
## Security Audit Trail
|
||||
|
||||
| Audit Date | Threats Total | Closed | Open | Run By |
|
||||
|------------|---------------|--------|------|--------|
|
||||
| 2026-09-24 | 11 | 11 | 0 | 08-10 executor (`08-SECURITY-REVIEW.md`, self-performed) |
|
||||
| 2026-10-12 | 11 | 11 | 0 | /msd-secure-phase orchestrator (L1 grep re-check; `go test -short` green for `modules/wristband`, fonoteka `classes/auth`, `controllers/api`) |
|
||||
|
||||
## Security Audit 2026-10-12
|
||||
|
||||
| Metric | Count |
|
||||
|--------|-------|
|
||||
| Threats found | 11 |
|
||||
| Closed | 11 |
|
||||
| Open | 0 |
|
||||
|
||||
Short-circuited at L1 (threats_open 0, register authored at plan time, ASVS 1); no auditor subagent spawned. Every mitigation above was re-located in current source rather than inherited from the 2026-09-24 review.
|
||||
|
||||
---
|
||||
|
||||
## 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
|
||||
|
||||
**Approval:** verified 2026-10-12
|
||||
Reference in New Issue
Block a user