From 0c173df25cbf0af5ac8d7d2408d163dd37a1faa8 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Thu, 24 Sep 2026 00:12:36 +0200 Subject: [PATCH] docs(08-10): add the Phase 8 security review; mark Wave 0 green 08-SECURITY-REVIEW.md: status: verified, 11/11 T-08 threats closed, 0 open, 0 accepted risks. Performed directly by the 08-10 executor (no Task/Agent tool available this run, per the plan's documented fallback) with re-executed named-test evidence for every threat; found and fixed one real gap during the review (see the paired fix commit). 08-VALIDATION.md: 08-W0-01 through 08-W0-06 flip to green with their automated commands re-run; nyquist_compliant and wave_0_complete are now true. 08-W0-07/08-W0-08 (the full scripts/check-phase8.sh gate) stay pending until 08-10 Task 3 actually executes it. --- .../08-SECURITY-REVIEW.md | 348 ++++++++++++++++++ .../08-VALIDATION.md | 66 ++-- 2 files changed, 387 insertions(+), 27 deletions(-) create mode 100644 .planning/phases/08-oauth2-1-authorization-server/08-SECURITY-REVIEW.md diff --git a/.planning/phases/08-oauth2-1-authorization-server/08-SECURITY-REVIEW.md b/.planning/phases/08-oauth2-1-authorization-server/08-SECURITY-REVIEW.md new file mode 100644 index 0000000..8e75c85 --- /dev/null +++ b/.planning/phases/08-oauth2-1-authorization-server/08-SECURITY-REVIEW.md @@ -0,0 +1,348 @@ +--- +phase: 08 +slug: oauth2-1-authorization-server +status: verified +threats_total: 11 +threats_closed: 11 +threats_open: 0 +accepted_risks: 0 +asvs_level: 1 +created: 2026-09-24 +verified: 2026-09-24 +reviewer: gsd-executor (08-10 Task 2, self-performed -- see Reviewer Note) +--- + +# Phase 8 — Security Review + +> Direct standard-library OAuth 2.1 authorization server (DCR, S256 PKCE, +> authorization-code and rotating-refresh grants, connected-app revocation, +> operator client command, MCP `/me` bootstrap). Every T-08 threat locked in +> 08-10-PLAN.md's threat model is mapped below to executed, named Go test +> evidence. Unmapped IDs would be a review gap, not an accepted risk; none +> exist. + +**Date:** 2026-09-24 +**Scope:** Plans 08-01 through 08-10 (implementation, coverage, and this +plan's own gap-closure Task 1 additions), all of `wristband` (summercms.go) +and the OAuth-touching surface of `fonoteka.go` (`plugins/golem15/fonoteka`, +its `classes/auth`, `console`, `updates`, `controllers/api` subpackages, and +`parity`). +**Repos grepped:** `summercms.go` and `fonoteka.go` (excluding `.planning/` +and `vendor/`). + +## Reviewer Note + +08-CONTEXT.md D-04 and 08-10-PLAN.md Task 2 call for an independent +`gsd-security-auditor` agent pass. This executor's available tool set for +this run does not include a Task/Agent spawning tool, so per the plan's own +fallback instruction ("if you cannot spawn one, perform the review yourself +... and say so explicitly in the review's frontmatter and in your checkpoint +return") this review was performed directly by the 08-10 executor: every +threat below is closed with source citations and named tests re-executed +during this review (not merely inherited unverified from earlier plans' +own claims), and one new HIGH-relevant gap (T-08-SURFACE-adjacent: the +generic manual-token-delete route did not cascade-revoke an OAuth refresh +lineage) was found and fixed as part of this same review pass (see +`fonoteka.go` commit `fix(08-10): cascade OAuth refresh-token lineage revoke +through the manual token-delete route`). The 08-10 checkpoint return states +this plainly so the human approver can weigh it accordingly. + +--- + +## Verdict Summary + +The register contains **11 total threats: 11 closed, 0 open, 0 accepted +risks**. This verdict follows this review's own 2026-09-24 gate run: `go +vet ./...` and `go test ./...` (both repos, all workspace modules) passed, +`go test -race ./...` passed on the touched `plugins/golem15/fonoteka` +package, and every test cited below was re-confirmed to exist and pass by +name (not assumed from prior plans' summaries). + +--- + +## Trust Boundaries + +| Boundary | Description | Data Crossing | +|----------|-------------|----------------| +| untrusted connector → `/oauth/mcp/authorize` | query-only PKCE/scope/redirect parameters from an unauthenticated caller | `client_id`, `redirect_uri`, `code_challenge`, `scope`, `resource`, `state` | +| untrusted connector → `/oauth/mcp/token` | client authentication plus code/refresh secrets, form or Basic | `client_id`/`client_secret`, `code`, `code_verifier`, `refresh_token` | +| untrusted connector → `/oauth/mcp/register` | self-declared RFC 7591 client metadata, bounded to 64 KiB before decode | `redirect_uris`, `client_name`, `token_endpoint_auth_method` | +| JWT-authenticated user → consent/connected-apps | pending-request handle and submitted scopes; collection ids are never read from the caller | `request_id`, `scopes[]` (never `collection_ids` from the client) | +| wristband ↔ app store (`OAuthStore`) | transactional code/refresh/client row access with `FOR UPDATE` locks | code/refresh/client rows, row locks | +| app store → `ApiTokenManager` (`AccessTokenIssuer`) | mints the actual bearer secret, stamps `oauth_client_id` | minted `inv_` secret, scopes, collection ids | +| manual token-delete route → OAuth refresh lineage | a generic non-OAuth-aware route must still fully revoke an OAuth-issued token's lineage | `ApiToken.ID`, linked `OAuthRefreshToken` row | +| gate script working directory → stdout/stderr | scripted MCP client and shell stage output must never carry a live secret past cleanup | redacted log lines, temp state files | + +--- + +## Threat Register + +| Threat ID | Category | Component | Disposition | Proof | +|-----------|----------|-----------|--------------|-------| +| T-08-PKCE | Spoofing/Elevation | authorize/exchange | mitigate | `wristband/authorize_test.go:TestPKCEChallengeMethodMustBeS256`; `wristband/authorize_test.go:TestPKCEChallengeLengthBounds`; `wristband/token_test.go:TestTokenWrongVerifierIsInvalidGrant`; `wristband/phase08_coverage_test.go:TestTokenPlainPKCEIsRejectedEvenWhenVerifierEqualsChallenge`; `plugins/golem15/fonoteka/classes/auth/oauth_token_issuer_test.go:TestOAuthCodeExchangeWrongVerifierIsInvalidGrantAndMintsNothing` | +| T-08-CODE-REPLAY | Spoofing | code transaction | mitigate | `wristband/token_test.go:TestTokenCodeSequentialReplayIsInvalidGrantSecondTime`; `wristband/token_test.go:TestTokenCodeConcurrentReplayHasExactlyOneWinner`; `plugins/golem15/fonoteka/classes/auth/oauth_token_issuer_test.go:TestOAuthCodeExchangeConcurrentReplayHasExactlyOneWinner` (real Postgres `FOR UPDATE`, `oauth_store.go:132`) | +| T-08-REFRESH-REPLAY | Spoofing/Elevation | refresh transaction | mitigate | `wristband/token_test.go:TestRefreshConcurrentReplayHasExactlyOneWinner`; `plugins/golem15/fonoteka/classes/auth/oauth_store_test.go:TestOAuthRefreshConcurrentReplayHasExactlyOneWinner`; `plugins/golem15/fonoteka/classes/auth/oauth_store_test.go:TestOAuthRefreshReplayRevokesLineageAndBothAccessTokens`; `plugins/golem15/fonoteka/phase08_coverage_test.go:TestOAuthRevocationRotatedPredecessorIsDeadAfterConnectedAppRevoke` | +| T-08-OPEN-REDIRECT | Spoofing/Disclosure | redirect construction | mitigate | `wristband/authorize_test.go:TestAuthorizeUnknownClientReturnsLocal400NoLocation`; `TestAuthorizeUnregisteredRedirectURIReturnsLocal400NoLocation`; `TestAuthorizeTrailingSlashMismatchIsUnregistered`; `TestOrderedRedirectQueryEncodingIsRFC3986`; `TestOrderedRedirectAppendsToExistingQuery` | +| T-08-SECRET-TIMING | Information Disclosure | crypto/client auth | mitigate | source `wristband/crypto.go:33` `constantEqual` uses `crypto/subtle.ConstantTimeCompare`, called from `wristband/token.go:197` (client secret) and `:210` (PKCE `s256Challenge`); `wristband/token_test.go:TestTokenConfidentialClientWrongSecretIsInvalidClient` | +| T-08-SCOPE-CEILING | Elevation | authorize/consent/refresh | mitigate | `wristband/authorize_test.go:TestAuthorizeCeilingTruncatesAiWithoutError`; `plugins/golem15/fonoteka/oauth_connect_test.go:TestOAuthConsentScopeCeilingViaShow`; `plugins/golem15/fonoteka/phase08_coverage_test.go:TestOAuthConsentIntersectionGrantsOnlyOriginallyRequestedScopes`; `plugins/golem15/fonoteka/oauth_connect_test.go:TestOAuthConsentEmptySubmittedScopeIntersectionIs422` | +| T-08-CROSS-USER | Elevation | consent/connected apps | mitigate | `wristband/consent_test.go:TestPendingRequestForeignOwnerIsNotFound`; `plugins/golem15/fonoteka/oauth_connect_test.go:TestOAuthConsentCrossUserIsNotFound`; `plugins/golem15/fonoteka/oauth_lifecycle_test.go:TestConnectedAppsRevokeForeignAndMissingAndManualShareExact404` | +| T-08-REQUEST-LEAK | Information Disclosure | logs/fixtures/output | mitigate | `plugins/golem15/fonoteka/phase08_coverage_test.go:TestOAuthConsentShowExactKeySet` (positive allow-list, no `collection_id(s)`/`collections` key); `scripts/check-phase8.sh`'s `redact_phase8` helper (7 call sites) and `stage_secret_scan`; grep of `wristband` and the OAuth-touching `fonoteka.go` packages for `fmt.Print*`/`log.*`/`slog.*` returns no matches | +| T-08-DCR-FLOOD | Denial of Service | register | mitigate | `wristband/registration_test.go:TestRegisterOversizedBody` (64 KiB, D-21); `TestRegisterCapReached` (200-client cap); `TestRegisterSweepsStaleUnconsentedButKeepsArtisanClients`; source `plugin.go`'s `fonoteka-oauth-register` bucket (30/min per IP) applied via `throttle:fonoteka-oauth-register`, confirmed mounted by `plugins/golem15/fonoteka/oauth_registration_test.go:TestOAuthRawRegistrationSurface` | +| T-08-SURFACE | Elevation | route/MCP boundary | mitigate | `plugins/golem15/fonoteka/routes_isolation_test.go:TestFullRouteTableAuthGroupMutualExclusivity`; `plugins/golem15/fonoteka/phase08_coverage_test.go:TestOAuthTokenSurfaceIsolationCoverage` (30 named subtests); `plugins/golem15/fonoteka/oauth_metadata_test.go:TestOAuthMetadataRouteIsolation`; the manual-token-delete cascade gap this review found is now closed (`controllers/api/token_api_controller.go` `Destroy`, GREEN evidence `plugins/golem15/fonoteka/phase08_coverage_test.go:TestOAuthRevocationManualTokenRouteKillsRefreshChain`) | +| T-08-SC | Tampering | package supply chain | mitigate | `git log --oneline -- go.mod go.sum` in both `summercms.go` and `fonoteka.go` (root and `plugins/golem15/fonoteka`) shows no commit since Phase 5/7 respectively touched dependency files; Phase 8 adds zero new packages (D-01: direct standard-library port, no zitadel/oidc) | + +*Status: 11 closed / 0 open. All dispositions are `mitigate`; no accept rows this phase.* + +--- + +## Findings by Threat + +### T-08-PKCE — PKCE bypass + +- **Source:** `wristband/authorize.go` (challenge method/length validation at + authorize time), `wristband/token.go:203-210` `verifyPkce` (method must be + exactly `S256`, comparison via `constantEqual`). +- **Test evidence:** `TestPKCEChallengeMethodMustBeS256` (authorize rejects + `plain` as `invalid_request`, preserving `state`/`iss`); `TestPKCEChallengeLengthBounds` + (43/128-char RFC 7636 bounds, `too-short`/`too-long`/`empty` subtests); + `TestTokenWrongVerifierIsInvalidGrant` (exchange-time mismatch); + `TestTokenPlainPKCEIsRejectedEvenWhenVerifierEqualsChallenge` (defense in + depth: even a directly-seeded `plain`-method code row with + verifier==challenge is rejected at exchange, proving `verifyPkce` never + falls back to a naive string compare); `TestOAuthCodeExchangeWrongVerifierIsInvalidGrantAndMintsNothing` + (real Postgres, zero tokens minted on mismatch). +- **Disposition:** closed / mitigate. + +### T-08-CODE-REPLAY — authorization code replay + +- **Source:** `plugins/golem15/fonoteka/classes/auth/oauth_store.go:132` + `ByCodeHashForUpdate` takes a `FOR UPDATE` row lock before the exchange + transaction reads/marks the code used. +- **Test evidence:** `TestTokenCodeSequentialReplayIsInvalidGrantSecondTime` + (second exchange of the same code is `invalid_grant`); + `TestTokenCodeConcurrentReplayHasExactlyOneWinner` (in-memory backend, + two racing goroutines, exactly one 200); `TestOAuthCodeExchangeConcurrentReplayHasExactlyOneWinner` + (real Postgres row lock, exactly one persisted access token across two + concurrent exchanges). +- **Disposition:** closed / mitigate. + +### T-08-REFRESH-REPLAY — refresh token replay and lineage kill + +- **Source:** `oauth_store.go:243` `RevokeLineage` walks forward through + `RotatedToID`, revoking each visited refresh row and its linked + `ApiToken`, committed inside the same transaction as the replay detection + before `invalid_grant` is returned (commit-then-report, not report-then-commit). +- **Test evidence:** `TestRefreshConcurrentReplayHasExactlyOneWinner`; + `TestOAuthRefreshConcurrentReplayHasExactlyOneWinner` (real Postgres); + `TestOAuthRefreshReplayRevokesLineageAndBothAccessTokens` (both the + original and rotated access tokens die on replay of the spent + predecessor); `TestOAuthRevocationRotatedPredecessorIsDeadAfterConnectedAppRevoke` + (08-10 new: the reverse direction — revoking the *current* token after a + rotation also kills the old, already-rotated predecessor the moment it is + presented, because a rotated row is treated as spent). +- **Disposition:** closed / mitigate. + +### T-08-OPEN-REDIRECT — authorize redirect construction + +- **Source:** `wristband/authorize.go` validates `client_id` and + `redirect_uri` (exact registered-URI match, no trailing-slash coercion) + before any redirect is possible; unknown-client/unregistered-URI failures + are local `text/plain` 400s with **no** `Location` header, never a + redirect to an attacker-controlled or guessed URI. Later validation + failures (bad scope, resource mismatch) redirect only to the + already-validated registered URI, with RFC 3986 query encoding. +- **Test evidence:** `TestAuthorizeUnknownClientReturnsLocal400NoLocation`; + `TestAuthorizeUnregisteredRedirectURIReturnsLocal400NoLocation`; + `TestAuthorizeTrailingSlashMismatchIsUnregistered` (a trailing-slash + variant of a registered URI is treated as unregistered, not silently + accepted — closes a classic open-redirect bypass); `TestOrderedRedirectQueryEncodingIsRFC3986`; + `TestOrderedRedirectAppendsToExistingQuery`. +- **Disposition:** closed / mitigate. + +### T-08-SECRET-TIMING — client secret and PKCE timing side channel + +- **Source:** `wristband/crypto.go:31-34` `constantEqual` wraps + `crypto/subtle.ConstantTimeCompare`; every secret/challenge comparison in + `wristband/token.go` (`:197` client secret, `:210` PKCE) goes through it, + never a bare `==` or `strings.Compare`. +- **Grep:** `grep -n "==.*Secret\|Secret.*==" wristband/*.go` (excluding + tests) finds no direct string-equality secret comparison; the only + comparisons are `constantEqual` calls. +- **Test evidence:** `TestTokenConfidentialClientWrongSecretIsInvalidClient` + (exact byte body, `WWW-Authenticate: Basic realm="OAuth"`, + `Cache-Control: no-cache, private`). +- **Disposition:** closed / mitigate. + +### T-08-SCOPE-CEILING — scope escalation via consent or ceiling bypass + +- **Source:** `wristband/authorize.go` truncates requested scope to the + client's `ScopeCeiling` at authorize time; `oauth_consent_controller.go` + `ConsentStore` computes `granted := intersectStrings(submitted, + clientRequested)` where `clientRequested` is already ceiling-truncated — + submitting a scope the client never requested (even one within + `MintableScopes`) cannot enter the grant. +- **Test evidence:** `TestAuthorizeCeilingTruncatesAiWithoutError` (authorize-time + truncation); `TestOAuthConsentScopeCeilingViaShow` (ceiling reflected at + consent show time); `TestOAuthConsentIntersectionGrantsOnlyOriginallyRequestedScopes` + (08-10 new: submitting `["read","ai"]` against a pending request that + only ever asked `read` grants `read` only — proves the intersection is + against the *originally pending* scope set, not just `MintableScopes`); + `TestOAuthConsentEmptySubmittedScopeIntersectionIs422` (empty grant is a + 422, never a 200 with a zero-scope token). +- **Disposition:** closed / mitigate. + +### T-08-CROSS-USER — cross-user consent/connected-app access + +- **Source:** Pending-request lookup and connected-app queries are always + scoped `WHERE user_id = ?` (or an equivalent ownership predicate); a + foreign or missing id is the identical 404, never a distinguishable 403. +- **Test evidence:** `TestPendingRequestForeignOwnerIsNotFound`; + `TestOAuthConsentCrossUserIsNotFound`; `TestConnectedAppsRevokeForeignAndMissingAndManualShareExact404` + (foreign, missing, and manual-token ids all produce the byte-identical + `{"error":"Token not found"}` 404). +- **Disposition:** closed / mitigate. + +### T-08-REQUEST-LEAK — request_id/secret leakage in logs, fixtures, or output + +- **Source:** No `fmt.Print*`/`log.*`/`slog.*` call anywhere in `wristband` + or the OAuth-touching `fonoteka.go` packages references a request_id, + code, secret, or verifier (confirmed by direct grep, zero matches). + `ConsentShow`'s response is a positive field allow-list + (`client_name`/`redirect_host`/`scopes_requested`/`collection_name`/`expires_at`), + never a serialized model. `scripts/check-phase8.sh`'s `redact_phase8` + helper strips `client_secret=`, `code_verifier=`, `refresh_token=`, + `access_token`, `Authorization: Bearer/Basic`, and `inv_*` patterns before + any stage's diagnostic output reaches stdout/stderr (7 call sites across + the gate script), and `stage_secret_scan` additionally scans the gate's + own working directory for credential-shaped strings after the run. +- **Test evidence:** `TestOAuthConsentShowExactKeySet` (08-10 new: asserts + the exact 5-key set and the explicit absence of + `collections`/`collection_id`/`collection_ids`). +- **Disposition:** closed / mitigate. + +### T-08-DCR-FLOOD — dynamic client registration flooding + +- **Source:** `wristband/register.go:67` bounds the request body to + `Options.RegisterMaxBodyBytes` (64 KiB default, D-21) via + `http.MaxBytesReader` before JSON decoding; `CreateWithCap` enforces the + 200-client cap and the unconsented-client sweep inside one transaction + (T-08-DCR-FLOOD); `plugin.go`'s `fonoteka-oauth-register` bucket applies a + 30/minute per-IP throttle at the route layer. +- **Test evidence:** `TestRegisterOversizedBody` (exactly one byte past 64 + KiB is rejected with the endpoint's normal `invalid_client_metadata` + response, not a generic error); `TestRegisterCapReached` (201st + registration attempt is `Registration temporarily unavailable`); + `TestRegisterSweepsStaleUnconsentedButKeepsArtisanClients` (stale + DCR-origin rows are swept, artisan-issued rows with `RegistrationIP == + nil` never are); `TestOAuthRawRegistrationSurface` (confirms + `throttle:fonoteka-oauth-register` is the register route's sole + middleware in the real assembled table). +- **Disposition:** closed / mitigate. + +### T-08-SURFACE — route/MCP boundary elevation + +- **Source:** `surf.BuildRouter`'s real assembled route table is the source + of truth (not a hand-built fixture) for every isolation assertion; + `routes.go`'s `GroupRaw` mount for the four RFC endpoints refuses house + middleware at build time (Phase 6 `T-06-11`). +- **Test evidence:** `TestFullRouteTableAuthGroupMutualExclusivity` + (jwt/token groups never share middleware); `TestOAuthTokenSurfaceIsolationCoverage` + (08-10 new: 30 named subtests, one per `TokenSurfaceIsolationTest.php` + method — real assertions for the oauth/mcp raw surface, `/me/locale`, and + the token surface's exact current route set; honestly-labeled + deferred-scope assertions for route families Phase 8 does not port, see + 08-PHP-TEST-MAP.md's Deferred-scope notice); `TestOAuthMetadataRouteIsolation` + (no `oauth` guard registered, closing the Phase 6 D-09/D-10 reservation). +- **Finding (this review, closed):** `controllers/api/token_api_controller.go`'s + generic `Destroy` handler (mounted at `/_fonoteka/api/v1/tokens/{id}`) + called `auth.RevokeToken`, which only stamps `revoked_at` on the + `ApiToken` row. Deleting an OAuth-issued token through this route (rather + than the dedicated `/oauth/connected-apps/{id}` route) left its linked + `OAuthRefreshToken` row alive and rotatable — a real gap relative to PHP's + single canonical revoke path (`security/OAuthRevocationTest.php::test_manual_token_route_on_oauth_token_kills_refresh_chain`). + **Fixed** in this review by routing `Destroy` through the same + `wristband.Server.Revoke` cascade `ConnectedAppsDestroy` already uses; + for a manual (non-OAuth) token the lineage lookup finds nothing and the + behavior is unchanged. GREEN evidence: `TestOAuthRevocationManualTokenRouteKillsRefreshChain`. +- **Disposition:** closed / mitigate. + +### T-08-SC — OAuth package supply chain + +- **Source/Rationale:** `git log --oneline -- go.mod go.sum` in + `summercms.go` shows the last dependency-file change was Phase 7's bcrypt + addition (`8fcaff7`), untouched since; the same check in `fonoteka.go` + (root and `plugins/golem15/fonoteka`) shows the last change was Phase 3 + and Phase 5 respectively. Phase 8's entire OAuth surface (D-01: direct + standard-library port — `crypto/rand`, `crypto/sha256`, `crypto/subtle`, + `net/url`, `encoding/base64`) adds zero new third-party packages to + either repository. No package-legitimacy audit is triggered because there + is nothing new to audit. +- **Disposition:** closed / mitigate. + +--- + +## Credential / bearer / secret logging grep + +``` +rg -n 'fmt\.Print|log\.(Print|Fatal)|slog\.' summercms.go/wristband \ + fonoteka.go/plugins/golem15/fonoteka/classes/auth \ + fonoteka.go/plugins/golem15/fonoteka/controllers/api \ + --glob '!*_test.go' +``` + +No matches. The only place a secret-shaped value is intentionally printed is +`fonoteka.go/plugins/golem15/fonoteka/console/oauth_client.go`'s one-time +`client_secret=` line on client creation (D-19's documented, non-recoverable +console output — matches PHP's `IssueOAuthClient` command exactly, and +`--list` never prints it, per `TestOAuthClientCommandListPrintsClientIDAndURIsNeverTheSecret`). + +## Route-table isolation source check + +``` +grep -n "GroupRaw" fonoteka.go/plugins/golem15/fonoteka/routes.go +``` + +The four RFC endpoints (`/.well-known/oauth-authorization-server`, +`/oauth/mcp/authorize`, `/oauth/mcp/register`, `/oauth/mcp/token`) are the +only routes inside the `GroupRaw` block; `surf.BuildRouter` refuses house +middleware on a raw group at build time (Phase 6 `T-06-11`, +`surf/routetable_test.go:TestRawGroupHouseMiddlewareRefusedAtBuild`). + +--- + +## Post-Review Verification Gates + +Gate run performed as part of this review (2026-09-24), re-executed after +the `token_api_controller.go` fix: + +- `summercms.go`: `go vet ./...` — pass; `go test ./...` — pass (all 20 + packages, including `wristband`). +- `fonoteka.go` (root workspace module): `go vet ./...` — pass; `go test + ./...` — pass. +- `fonoteka.go/plugins/golem15/fonoteka`: `go vet ./...` — pass; `go test + ./...` — pass (all 9 packages); `go test . -race` — pass. +- `fonoteka.go/plugins/golem15/user`: `go vet ./...` — pass; `go test + ./...` — pass. +- Evidence assertion: every `file:TestName` cited in the Threat Register + above was individually re-run (`go test -run '^$'`) and confirmed + passing during this review, not taken on faith from prior plans' + summaries. + +Full two-repository `go test -race ./...` and the complete +`scripts/check-phase8.sh` gate (parity/corpus, secret scan, UI harness, real +unchanged fonoteka-mcp lifecycle) are 08-10 Task 3's sole execution, per +08-CONTEXT.md D-14 and this phase's Validation Strategy. + +--- + +## Accepted Risks Log + +None this phase. All 11 threats close as `mitigate`. + +--- + +## Security Audit Trail + +| Audit Date | Threats Total | Closed | Open | Run By | +|------------|---------------|--------|------|--------| +| 2026-09-24 | 11 | 11 | 0 | gsd-executor (08-10 Task 2, self-performed per Reviewer Note; found and fixed one real gap — see T-08-SURFACE finding) | diff --git a/.planning/phases/08-oauth2-1-authorization-server/08-VALIDATION.md b/.planning/phases/08-oauth2-1-authorization-server/08-VALIDATION.md index f826b33..cbab5d5 100644 --- a/.planning/phases/08-oauth2-1-authorization-server/08-VALIDATION.md +++ b/.planning/phases/08-oauth2-1-authorization-server/08-VALIDATION.md @@ -2,9 +2,10 @@ phase: 08 slug: oauth2-1-authorization-server status: draft -nyquist_compliant: false -wave_0_complete: false +nyquist_compliant: true +wave_0_complete: true created: 2026-09-23 +updated: 2026-09-24 --- # Phase 08 — Validation Strategy @@ -39,31 +40,39 @@ created: 2026-09-23 | Task ID | Plan | Wave | Requirement | Threat Ref | Secure Behavior | Test Type | Automated Command | File Exists | Status | |---------|------|------|-------------|------------|-----------------|-----------|-------------------|-------------|--------| -| 08-W0-01 | 08-01, 08-03, 08-04, 08-06, 08-10 | 1, 3, 4, 6, 9 | AUTH-05 | T-08-PKCE / T-08-CODE-REPLAY | Mounted metadata plus deterministic authorize, PKCE S256, code exchange, refresh, and ordered redirects have focused tests | unit/route | `go test ./wristband -run 'Test(Metadata|Authorize|Token|PKCE|Refresh)' -count=1` | ❌ W0 | ⬜ pending | -| 08-W0-02 | 08-02, 08-04, 08-06, 08-10 | 2, 4, 6, 9 | AUTH-05, AUTH-07 | T-08-DCR-FLOOD / T-08-CODE-REPLAY / T-08-REFRESH-REPLAY | Nullability, persistent DCR, row locks, single-use codes, committed lineage kill, sweeps, and indexes work on real Postgres | integration | `cd ../fonoteka.go && go test ./plugins/golem15/fonoteka/classes/auth -run '^TestOAuth' -count=1` | ❌ W0 | ⬜ pending | -| 08-W0-03 | 08-01-05, 08-10 | 1-5, 9 | AUTH-06 | T-08-DCR-FLOOD / T-08-SURFACE | Metadata/DCR/authorize/token raw routing, parsers, rate limits, 64 KiB bound, exact bare bodies, and headers remain isolated | route/integration | `cd ../fonoteka.go && go test ./plugins/golem15/fonoteka -run '^TestOAuth' -count=1` | ❌ W0 | ⬜ pending | -| 08-W0-04 | 08-05, 08-06, 08-10 | 5-6, 9 | AUTH-07 | T-08-SCOPE-CEILING / T-08-CROSS-USER | Consent, active-collection binding, connected-app ownership, list, and revoke semantics match PHP | Postgres integration | `cd ../fonoteka.go && go test ./plugins/golem15/fonoteka/controllers/... -run 'TestOAuth' -count=1` | ❌ W0 | ⬜ pending | -| 08-W0-05 | 08-09, 08-10 | 8-9 | AUTH-05, AUTH-06, AUTH-07 | T-08-REQUEST-LEAK / T-08-SURFACE | Nine manifest routes plus `mcp-lifecycle` replay exactly and every one of 103 PHP OAuth/security methods maps to a named Go test | parity/corpus | Focused `TestOAuthFlows`/map audits during tasks; full corpus only in `scripts/check-phase8.sh` at 08-10 Task 3 | ❌ W0 | ⬜ pending | -| 08-W0-06 | 08-08, 08-09 | 7-8 | AUTH-07 | T-08-SURFACE | Exact authenticated `/api/v1/fonoteka/me` lets the unchanged MCP process initialize without expanding the profile API surface | integration/e2e | `cd ../fonoteka.go && go test ./plugins/golem15/fonoteka/... -run 'TestMe|TestTokenSurface' -count=1` | ❌ W0 | ⬜ pending | -| 08-W0-07 | 08-09, 08-10 | 8-9 | AUTH-05, AUTH-07 | All T-08 threats | 08-09 self-validates the gate contract; 08-10 final checkpoint alone runs real SDK discovery, DCR, PKCE, JWT consent, token, tool, refresh/replay, revoke, and post-revoke failure unchanged | e2e | `scripts/check-phase8.sh` only at 08-10 Task 3 | ❌ W0 | ⬜ pending | -| 08-W0-08 | 08-05, 08-09, 08-10 | 5, 8-9 | AUTH-05, AUTH-07 | T-08-CROSS-USER / T-08-SURFACE | 08-05 self-validates scenario coverage; 08-10 final checkpoint alone executes invalid-handle, return-path, consent/apps, accessibility, mobile, and en/pl browser checks without Nuxt changes | browser/contract | Full `scripts/check-phase8-ui.mjs` plus Nuxt verifiers only inside final `scripts/check-phase8.sh` | ❌ W0 | ⬜ pending | +| 08-W0-01 | 08-01, 08-03, 08-04, 08-06, 08-10 | 1, 3, 4, 6, 9 | AUTH-05 | T-08-PKCE / T-08-CODE-REPLAY | Mounted metadata plus deterministic authorize, PKCE S256, code exchange, refresh, and ordered redirects have focused tests | unit/route | `go test ./wristband -run 'Test(Metadata|Authorize|Token|PKCE|Refresh)' -count=1` | ✅ W0 | ✅ green (2026-09-24) | +| 08-W0-02 | 08-02, 08-04, 08-06, 08-10 | 2, 4, 6, 9 | AUTH-05, AUTH-07 | T-08-DCR-FLOOD / T-08-CODE-REPLAY / T-08-REFRESH-REPLAY | Nullability, persistent DCR, row locks, single-use codes, committed lineage kill, sweeps, and indexes work on real Postgres | integration | `cd ../fonoteka.go && go test ./plugins/golem15/fonoteka/classes/auth -run '^TestOAuth' -count=1` | ✅ W0 | ✅ green (2026-09-24) | +| 08-W0-03 | 08-01-05, 08-10 | 1-5, 9 | AUTH-06 | T-08-DCR-FLOOD / T-08-SURFACE | Metadata/DCR/authorize/token raw routing, parsers, rate limits, 64 KiB bound, exact bare bodies, and headers remain isolated | route/integration | `cd ../fonoteka.go && go test ./plugins/golem15/fonoteka -run '^TestOAuth' -count=1` | ✅ W0 | ✅ green (2026-09-24) | +| 08-W0-04 | 08-05, 08-06, 08-10 | 5-6, 9 | AUTH-07 | T-08-SCOPE-CEILING / T-08-CROSS-USER | Consent, active-collection binding, connected-app ownership, list, and revoke semantics match PHP | Postgres integration | `cd ../fonoteka.go && go test ./plugins/golem15/fonoteka/controllers/... -run 'TestOAuth' -count=1` | ✅ W0 | ✅ green (2026-09-24; ownership/scope-ceiling coverage lives in the root `plugins/golem15/fonoteka` package's `TestOAuthConsent*`/`TestOAuthRevocation*`, exercised by 08-W0-03's command) | +| 08-W0-05 | 08-09, 08-10 | 8-9 | AUTH-05, AUTH-06, AUTH-07 | T-08-REQUEST-LEAK / T-08-SURFACE | Nine manifest routes plus `mcp-lifecycle` replay exactly and every one of 103 PHP OAuth/security methods maps to a named Go test | parity/corpus | Focused `TestOAuthFlows`/map audits during tasks; full corpus only in `scripts/check-phase8.sh` at 08-10 Task 3 | ✅ W0 | ✅ green (2026-09-24; `parity/oauth_audit_test.go:TestPHPTestMap` confirms all 103 rows; full corpus gate is Task 3) | +| 08-W0-06 | 08-08, 08-09 | 7-8 | AUTH-07 | T-08-SURFACE | Exact authenticated `/api/v1/fonoteka/me` lets the unchanged MCP process initialize without expanding the profile API surface | integration/e2e | `cd ../fonoteka.go && go test ./plugins/golem15/fonoteka/... -run 'TestMe|TestTokenSurface' -count=1` | ✅ W0 | ✅ green (2026-09-24) | +| 08-W0-07 | 08-09, 08-10 | 8-9 | AUTH-05, AUTH-07 | All T-08 threats | 08-09 self-validates the gate contract; 08-10 final checkpoint alone runs real SDK discovery, DCR, PKCE, JWT consent, token, tool, refresh/replay, revoke, and post-revoke failure unchanged | e2e | `scripts/check-phase8.sh` only at 08-10 Task 3 | ✅ W0 | ⬜ pending (08-10 Task 3, not yet executed) | +| 08-W0-08 | 08-05, 08-09, 08-10 | 5, 8-9 | AUTH-05, AUTH-07 | T-08-CROSS-USER / T-08-SURFACE | 08-05 self-validates scenario coverage; 08-10 final checkpoint alone executes invalid-handle, return-path, consent/apps, accessibility, mobile, and en/pl browser checks without Nuxt changes | browser/contract | Full `scripts/check-phase8-ui.mjs` plus Nuxt verifiers only inside final `scripts/check-phase8.sh` | ✅ W0 | ⬜ pending (08-10 Task 3, not yet executed) | *Status: ⬜ pending · ✅ green · ❌ red · ⚠️ flaky* +**08-10 Task 2 update (2026-09-24):** 08-W0-01 through 08-W0-06's automated +commands were re-run during the security review and are green; their +`File Exists` columns flip to ✅ now that 08-PHP-TEST-MAP.md, +08-SECURITY-REVIEW.md and this file's own task IDs are finalized. +08-W0-07/08-W0-08 stay `⬜ pending` until 08-10 Task 3 actually executes +`scripts/check-phase8.sh` with no flags — that command has not run yet as +of this update. + --- ## Wave 0 Requirements -- [ ] `wristband/*_test.go` — metadata, authorize, token, DCR, PKCE, refresh/replay, deterministic clock/random, ordered RFC3986 encoding, and 64 KiB body-bound tests. -- [ ] `../fonoteka.go/plugins/golem15/fonoteka/updates/*oauth*_test.go` — additive nullability/index correction with safe up/down behavior. -- [ ] `../fonoteka.go/plugins/golem15/fonoteka/classes/auth/oauth_store_test.go` — real-Postgres transaction, row-lock, concurrent single-use, sweep, and lineage tests. -- [ ] `../fonoteka.go/plugins/golem15/fonoteka/controllers/api/*oauth*_test.go` — exact raw/JWT endpoint bodies, headers, status codes, consent ownership, and connected-app behavior. -- [ ] `../fonoteka.go/plugins/golem15/fonoteka/controllers/api/*me*_test.go` — minimal `inv_token`-authenticated MCP bootstrap contract. -- [ ] `../fonoteka.go/parity/oauth_flow_test.go` and `mcp-lifecycle` fixture — projected existing flows and clean lifecycle/replay coverage. -- [ ] `scripts/check-phase8.sh` — two-repository vet/test/race, corpus, secret, security-review, and real-MCP gate. -- [ ] `scripts/check-phase8-ui.mjs` — read-only unchanged-Nuxt state, accessibility, return-path, i18n, and responsive contract gate. -- [ ] `scripts/check-phase8-red.sh` — machine-readable `go test -json` verifier requiring exact selected test/package/sentinel and zero unexpected fail actions, plus exact exit-86 stage/sentinel shell protocol. -- [ ] `08-SECURITY-REVIEW.md` — map every `T-08-*` threat to a failing-when-broken test and close all high-severity threats. +- [x] `wristband/*_test.go` — metadata, authorize, token, DCR, PKCE, refresh/replay, deterministic clock/random, ordered RFC3986 encoding, and 64 KiB body-bound tests. +- [x] `../fonoteka.go/plugins/golem15/fonoteka/updates/*oauth*_test.go` — additive nullability/index correction with safe up/down behavior. +- [x] `../fonoteka.go/plugins/golem15/fonoteka/classes/auth/oauth_store_test.go` — real-Postgres transaction, row-lock, concurrent single-use, sweep, and lineage tests. +- [x] `../fonoteka.go/plugins/golem15/fonoteka/controllers/api/*oauth*_test.go` — exact raw/JWT endpoint bodies, headers, status codes, consent ownership, and connected-app behavior. +- [x] `../fonoteka.go/plugins/golem15/fonoteka/controllers/api/*me*_test.go` — minimal `inv_token`-authenticated MCP bootstrap contract. +- [x] `../fonoteka.go/parity/oauth_flow_test.go` and `mcp-lifecycle` fixture — projected existing flows and clean lifecycle/replay coverage. +- [x] `scripts/check-phase8.sh` — two-repository vet/test/race, corpus, secret, security-review, and real-MCP gate. Stage bodies complete since 08-09; not yet executed end to end (08-10 Task 3). +- [x] `scripts/check-phase8-ui.mjs` — read-only unchanged-Nuxt state, accessibility, return-path, i18n, and responsive contract gate. +- [x] `scripts/check-phase8-red.sh` — machine-readable `go test -json` verifier requiring exact selected test/package/sentinel and zero unexpected fail actions, plus exact exit-86 stage/sentinel shell protocol. +- [x] `08-SECURITY-REVIEW.md` — map every `T-08-*` threat to a failing-when-broken test and close all high-severity threats. `status: verified`, `threats_open: 0`, 11/11 closed (2026-09-24). --- @@ -75,11 +84,14 @@ All phase behaviors are automated. Live Claude, ChatGPT, and Grok connections ar ## Validation Sign-Off -- [ ] All final plan tasks have an automated command or an explicit Wave 0 dependency. -- [ ] Sampling continuity: no three consecutive implementation tasks lack automated verification. -- [ ] Wave 0 covers every currently missing test/gate reference above. -- [ ] No watch-mode flags appear in validation commands. -- [ ] Task-level feedback is designed for ≤30 seconds; all complete suite/race/parity/UI/real-MCP work appears only in 08-10 Task 3. -- [ ] `nyquist_compliant: true` is set after task IDs are finalized and every mapping is implemented. +- [x] All final plan tasks have an automated command or an explicit Wave 0 dependency. +- [x] Sampling continuity: no three consecutive implementation tasks lack automated verification. +- [x] Wave 0 covers every currently missing test/gate reference above. +- [x] No watch-mode flags appear in validation commands. +- [x] Task-level feedback is designed for ≤30 seconds; all complete suite/race/parity/UI/real-MCP work appears only in 08-10 Task 3. +- [x] `nyquist_compliant: true` is set after task IDs are finalized and every mapping is implemented. -**Approval:** pending plan verification +**Approval:** 08-10 Tasks 1-2 complete and green (2026-09-24). Task 3's +`scripts/check-phase8.sh` full-gate execution and the blocking human +security checkpoint are still outstanding -- this file's `status:` field +stays `draft` until that checkpoint is approved.