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.
This commit is contained in:
Jakub Zych
@@ -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 '^<Name>$'`) 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) |
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user