From 37fbcdc6de88b8bafac0fa18c9070fd552991221 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Wed, 23 Sep 2026 12:32:31 +0200 Subject: [PATCH] docs(phase-08): add validation strategy and resolve research decisions --- .../08-CONTEXT.md | 4 +- .../08-RESEARCH.md | 14 ++-- .../08-VALIDATION.md | 82 +++++++++++++++++++ 3 files changed, 92 insertions(+), 8 deletions(-) create mode 100644 .planning/phases/08-oauth2-1-authorization-server/08-VALIDATION.md diff --git a/.planning/phases/08-oauth2-1-authorization-server/08-CONTEXT.md b/.planning/phases/08-oauth2-1-authorization-server/08-CONTEXT.md index ebfde4a..e9706f1 100644 --- a/.planning/phases/08-oauth2-1-authorization-server/08-CONTEXT.md +++ b/.planning/phases/08-oauth2-1-authorization-server/08-CONTEXT.md @@ -6,7 +6,7 @@ ## Phase Boundary -Port Płytarium's MCP OAuth 2.1 authorization server so fonoteka-mcp, Claude, ChatGPT and Grok connect to the Go backend unchanged. The surface is the unauthenticated RFC group already declared raw in Phase 6 (`GET /.well-known/oauth-authorization-server`, `GET /oauth/mcp/authorize`, `POST /oauth/mcp/token`, `POST /oauth/mcp/register`) plus the five JWT-group endpoints the Nuxt `/connect` page and settings page call (`GET oauth/request/{request_id}`, `POST oauth/consent`, `POST oauth/deny`, `GET oauth/connected-apps`, `DELETE oauth/connected-apps/{id}`), the `fonoteka:oauth-client` console command, and the `scope_ceiling` logic Phase 7 C-03 deferred here. +Port Płytarium's MCP OAuth 2.1 authorization server so fonoteka-mcp, Claude, ChatGPT and Grok connect to the Go backend unchanged. The surface is the unauthenticated RFC group already declared raw in Phase 6 (`GET /.well-known/oauth-authorization-server`, `GET /oauth/mcp/authorize`, `POST /oauth/mcp/token`, `POST /oauth/mcp/register`) plus the five JWT-group endpoints the Nuxt `/connect` page and settings page call (`GET oauth/request/{request_id}`, `POST oauth/consent`, `POST oauth/deny`, `GET oauth/connected-apps`, `DELETE oauth/connected-apps/{id}`), the `fonoteka:oauth-client` console command, and the `scope_ceiling` logic Phase 7 C-03 deferred here. Phase 8 also includes the minimal token-authenticated `GET /api/v1/fonoteka/me` prerequisite required for the unchanged `fonoteka-mcp` process to start and complete D-14's real MCP tool-call acceptance gate; this is an explicit exception to the otherwise OAuth-only endpoint boundary. The PHP server is hand-rolled inside the `golem15/fonoteka` plugin (not a separate `oauthserver` plugin and not league/oauth2-server): `OAuthCodeManager` plus four API controllers, about 1,000 lines with exact bodies. Every OAuth access token is an ordinary `inv_` personal token minted by `ApiTokenManager` and verified by the existing `inv_token` guard. Grant types are `authorization_code` and `refresh_token` only; there is no client-credentials or token-exchange grant. This closes the STATE.md blocker about `ClientCredentialsStorage`/`TokenExchangeStorage`: neither is needed. @@ -43,6 +43,8 @@ Out of scope: social login (`/oauth/{provider}`, `oauth-identities` routes, Phas - **D-17 (sweep):** Wristband adds an expiry sweep that PHP lacks, run where PHP runs its client sweep (`/register`) and also on `/token`, deleting only rows past `expires_at`: pending requests, codes (used or not) and refresh rows. Revoked or rotated rows that have not expired stay, so replay detection and connected-apps semantics match PHP. No timer, no goroutine; Phase 11 may move it into a River job. It is unobservable in recorded replays because nothing expires within a run; the schema-diff and db-capture harness must not be affected. - **D-18 (tests):** All PHP OAuth tests are ported (functional: OAuthAuthorizeTest, OAuthClientCommandTest, OAuthMetadataTest, OAuthMigrationTest, OAuthRegisterTest, OAuthTokenTest; security: OAuthConsentScopeCeilingTest, OAuthRefreshRotationTest, OAuthRevocationTest, TokenSurfaceIsolationTest). Framework behaviour tests run on wristband with the in-memory store; app tests run on real Postgres through the existing `classes` TestMain harness, and route-surface isolation tests inspect the surf route table as Phase 6 did. Each PHP test method maps to a named Go test so coverage can be audited. - **D-19 (command):** `fonoteka:oauth-client` is ported in `fonoteka.go` as a bonfire command scaffolded the Phase 4 way with the same signature (`name`, `--redirect-uri=*`, `--scope=*`, `--auth-method`, `--client-id`, `--list`) and the same output lines (`client_id=`, `client_secret=` printed once, the non-recoverable warning, `--list` never printing a secret). It is thin over wristband's client issuing helper and the app `ClientStore`. +- **D-20 (MCP prerequisite):** Phase 8 ports the minimal exact `GET /api/v1/fonoteka/me` personal-token endpoint required by `fonoteka-mcp/src/http.ts` before it constructs the MCP server. The endpoint authenticates through the existing `inv_token` surface and implements only the response contract needed by the unchanged MCP client. This prerequisite is in scope solely to preserve D-14's real MCP tool-call gate; broader user/profile API work remains deferred. +- **D-21 (DCR body bound):** `POST /oauth/mcp/register` accepts at most 64 KiB of JSON request body. An oversized document returns the endpoint's normal `invalid_client_metadata` response rather than a house envelope or generic HTML error. The bound is enforced before unbounded JSON decoding and is covered by the `T-08-DCR-FLOOD` failing-when-broken test. ### Claude's Discretion - Interface names and signatures in wristband, the transaction seam, in-memory store design, and where shared helpers (base64url, sha256 hex, constant-time compare) live. diff --git a/.planning/phases/08-oauth2-1-authorization-server/08-RESEARCH.md b/.planning/phases/08-oauth2-1-authorization-server/08-RESEARCH.md index 832ec2c..a8aaaec 100644 --- a/.planning/phases/08-oauth2-1-authorization-server/08-RESEARCH.md +++ b/.planning/phases/08-oauth2-1-authorization-server/08-RESEARCH.md @@ -2,7 +2,7 @@ **Researched:** 2026-09-23 **Domain:** OAuth 2.1-style authorization-code server, PKCE, dynamic registration, refresh rotation, and exact PHP/client wire parity -**Confidence:** HIGH for the PHP/client contract and RFC requirements; MEDIUM for the final end-to-end gate until the `/api/v1/fonoteka/me` scope conflict is resolved +**Confidence:** HIGH for the PHP/client contract, RFC requirements, and final end-to-end gate after the `/api/v1/fonoteka/me` scope decision was resolved ## User Constraints (from CONTEXT.md) @@ -465,19 +465,19 @@ The locked source suite contains 103 named methods: 11 authorize, 8 client-comma | # | Claim | Section | Risk if Wrong | |---|-------|---------|---------------| | A1 | Additive schema-correction `down` behavior may need to refuse when null lifecycle rows exist rather than destructively coercing them. | Required Schema Correction | Planner must choose a safe rollback contract; careless down migration can destroy pending/client data. | -| A2 | A small explicit `/register` request-body bound should be added despite raw-group exemption; exact byte value is not locked. | Threat map / Security | Without a decision, DCR rate/cap controls do not bound per-request memory; an arbitrary cap could diverge on oversized inputs no real client sends. | +| A2 | Resolved by CONTEXT D-21: `/register` has a 64 KiB request-body bound and oversized input returns `invalid_client_metadata`. | Threat map / Security | Closed by the user decision; plans must retain the exact bound and endpoint-native error contract. | -## Open Questions +## Resolved Planning Questions -1. **How is D-14's real MCP tool call reconciled with the Phase Boundary?** +1. **How is D-14's real MCP tool call reconciled with the Phase Boundary? — Resolved: add the prerequisite route.** - What we know: `fonoteka-mcp` calls `/api/v1/fonoteka/me` before it constructs the MCP server; that route is absent from current Go routes and from the Phase 8 endpoint list. [VERIFIED: MCP source and Go routes] - What's unclear: whether Phase 8 may port this one prerequisite route or D-14 should stop at token + direct bearer proof until the API phase. [VERIFIED: context conflict] - - Recommendation: add the minimal exact `/api/v1/fonoteka/me` personal-token route to Phase 8 as an explicitly approved prerequisite; this is the only choice that preserves the locked “real MCP tool call unchanged” acceptance. [ASSUMED] + - Decision: add the minimal exact `/api/v1/fonoteka/me` personal-token route to Phase 8 as an explicitly approved prerequisite; this preserves the locked “real MCP tool call unchanged” acceptance. [LOCKED: user decision 2026-09-23; CONTEXT D-20] -2. **What request-body limit should raw OAuth machine endpoints enforce?** +2. **What request-body limit should raw OAuth machine endpoints enforce? — Resolved: 64 KiB.** - What we know: the raw group intentionally bypasses the house body-limit middleware; `ParseForm` has an internal 10 MB cap, but the JSON register decoder is otherwise unbounded. Valid registration data is limited to five 512-character redirect URIs plus small metadata. [VERIFIED: surf/raw behavior; CITED: https://go.dev/pkg/net/http/; VERIFIED: PHP validation] - What's unclear: the desired explicit cap and error body for an oversized DCR document. [ASSUMED] - - Recommendation: configure a conservative `wristband` max request size (64 KiB is ample for the locked metadata shape) and return the endpoint's normal `invalid_client_metadata` response. [ASSUMED] + - Decision: configure a 64 KiB `wristband` maximum registration request size and return the endpoint's normal `invalid_client_metadata` response. [LOCKED: user decision 2026-09-23; CONTEXT D-21] ## Sources diff --git a/.planning/phases/08-oauth2-1-authorization-server/08-VALIDATION.md b/.planning/phases/08-oauth2-1-authorization-server/08-VALIDATION.md new file mode 100644 index 0000000..faa4763 --- /dev/null +++ b/.planning/phases/08-oauth2-1-authorization-server/08-VALIDATION.md @@ -0,0 +1,82 @@ +--- +phase: 08 +slug: oauth2-1-authorization-server +status: draft +nyquist_compliant: false +wave_0_complete: false +created: 2026-09-23 +--- + +# Phase 08 — Validation Strategy + +> Per-phase validation contract for feedback sampling during execution. + +--- + +## Test Infrastructure + +| Property | Value | +|----------|-------| +| **Framework** | Go 1.27 `testing`; existing testcontainers-backed Postgres harness; Node.js 22 plus the unchanged `fonoteka-mcp` only for end-to-end gates | +| **Config file** | Existing `go.work`, repository package tests, `../fonoteka.go/plugins/golem15/fonoteka/classes/TestMain`, and planned `scripts/check-phase8.sh` | +| **Quick run command** | `go test ./wristband -count=1` | +| **App-focused command** | `cd ../fonoteka.go && go test ./plugins/golem15/fonoteka/... -run 'TestOAuth|TestMe' -count=1` | +| **Full suite command** | `scripts/check-phase8.sh` | +| **Estimated runtime** | Quick package checks under 30 seconds; full two-repository parity/race/e2e gate may take several minutes | + +--- + +## Sampling Rate + +- **After every task commit:** Run the narrowest affected package test; `go test ./wristband -count=1` is the default framework check. +- **After every plan wave:** Run `go vet ./...` and `go test ./...` in each affected repository; storage waves also run focused real-Postgres tests. +- **Before `$gsd-verify-work`:** `scripts/check-phase8.sh` must pass, including both repositories' vet/test/race suites, parity corpus audit, secret scan, security review, and unchanged real-MCP lifecycle. +- **Max feedback latency:** 30 seconds for task-level sampling; slow Postgres, race, parity, and real-MCP gates run at wave/phase boundaries. + +--- + +## Per-Task Verification Map + +| Task ID | Plan | Wave | Requirement | Threat Ref | Secure Behavior | Test Type | Automated Command | File Exists | Status | +|---------|------|------|-------------|------------|-----------------|-----------|-------------------|-------------|--------| +| 08-W0-01 | TBD | 0 | AUTH-05 | T-08-PKCE / T-08-CODE-REPLAY | Metadata, authorize, PKCE S256, code exchange, refresh, DCR, and ordered redirects have deterministic framework tests | unit | `go test ./wristband -run 'Test(Metadata|Authorize|Token|Register|PKCE|Refresh)' -count=1` | ❌ W0 | ⬜ pending | +| 08-W0-02 | TBD | 0 | AUTH-05, AUTH-07 | T-08-CODE-REPLAY / T-08-REFRESH-REPLAY | Nullability, 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/... -run 'TestOAuth' -count=1` | ❌ W0 | ⬜ pending | +| 08-W0-03 | TBD | 0 | AUTH-06 | T-08-DCR-FLOOD / T-08-SURFACE | Raw routing, parser rules, rate limits, 64 KiB DCR bound, exact bare bodies, and headers remain isolated from house middleware | route/integration | `cd ../fonoteka.go && go test ./plugins/golem15/fonoteka/... -run 'TestOAuth' -count=1` | ❌ W0 | ⬜ pending | +| 08-W0-04 | TBD | 0 | 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 | TBD | 0 | 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 | `cd ../fonoteka.go && go test ./parity -run 'TestOAuthFlows|TestParityCorpus' -count=1` | ❌ W0 | ⬜ pending | +| 08-W0-06 | TBD | 0 | AUTH-07 | T-08-SURFACE | Minimal 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 | TBD | 0 | AUTH-05, AUTH-07 | All T-08 threats | Real SDK discovery, DCR, PKCE, JWT consent, token, MCP tool call, refresh/replay, connected-app revoke, and post-revoke failure complete unchanged | e2e | `scripts/check-phase8.sh` | ❌ W0 | ⬜ pending | + +*Status: ⬜ pending · ✅ green · ❌ red · ⚠️ flaky* + +--- + +## 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. +- [ ] `08-SECURITY-REVIEW.md` — map every `T-08-*` threat to a failing-when-broken test and close all high-severity threats. + +--- + +## Manual-Only Verifications + +All phase behaviors are automated. Live Claude, ChatGPT, and Grok connections are explicitly deferred to cutover UAT; they are not Phase 8 acceptance checks. + +--- + +## 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 remains under 30 seconds; slow suites are assigned to wave/phase gates. +- [ ] `nyquist_compliant: true` is set after task IDs are finalized and every mapping is implemented. + +**Approval:** pending plan verification