golem15/summercms
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
8818d7b0236115df98af629643bfcba59b516997
summercms/.planning/phases/08-oauth2-1-authorization-server/08-10-SUMMARY.md
Jakub Zych 482944b160 docs(08-10): add the coverage, security-review and final-gate summary 
Documents the 103-method audit closure, the self-performed 11/11-closed
security review (with disclosure), the real defects check-phase8.sh's first
end-to-end run found and fixed, and the checkpoint decision to close Phase 8
with the Playwright UI matrix gap carried forward.
2026-09-24 01:02:48 +02:00

22 KiB
Raw Blame History

phase: 08-oauth2-1-authorization-server plan: 10 subsystem: auth tags: [oauth2, security-review, coverage, wristband, mcp, gate, playwright-gap] # Dependency graph requires: - phase: 08-oauth2-1-authorization-server plan: 09 provides: "The complete, self-validated (never end-to-end executed) scripts/check-phase8.sh final gate, the recorded mcp-lifecycle fixture, and all nine OAuth manifest routes flipped to ported" provides: - "08-PHP-TEST-MAP.md: all 103 PHP OAuth functional/security methods mapped one-to-one to named, passing Go tests/subtests, with an executable audit (parity/oauth_audit_test.go) that fails closed on any missing/duplicate/renamed mapping" - "08-SECURITY-REVIEW.md: 11/11 T-08 threats closed, 0 open, status: verified, one real HIGH-relevant gap found and fixed during the review (manual token-delete route not cascading an OAuth refresh-token revoke)" - "scripts/check-phase8.sh's first and sole real end-to-end execution (2026-09-24): every stage green -- docker preflight, Postgres, app boot, the real unchanged fonoteka-mcp lifecycle (discovery, DCR, PKCE authorize, JWT consent, token, tool call, refresh, replay, revoke, post-revoke failure), both repos' vet/test/race, 169/169 parity corpus, secret scan, return-path 6/6, i18n 74 keys, unchanged-client diff, and the security-review gate -- except stage_ui_harness's Playwright browser matrix, a deliberate fatal() never authored by 08-05" - "The user's checkpoint decision closing Phase 8 with the Playwright UI matrix gap explicitly carried forward, recorded in deferred-items.md, 08-VALIDATION.md, and STATE.md" - "AUTH-05, AUTH-06 and AUTH-07 marked complete in REQUIREMENTS.md -- their full text is satisfied by the delivered backend/gate evidence; none of the three requirement texts mandate a Playwright-verified browser regression suite" - "A real Boot-order bug fix (plugins/golem15/fonoteka/plugin.go) that resolves the inv_token guard and the OAuth backend lazily instead of at Boot -- affects the real binary beyond Phase 8" affects: [09-admin-schema-pipeline, any-future-plan-authoring-the-deferred-playwright-spec] # Tech tracking tech-stack: added: [] patterns: - "Self-performed security review with an explicit Reviewer Note when no Task/Agent spawning tool is available, per 08-CONTEXT.md D-04's own documented fallback: every threat is closed with source citations and individually re-run named tests, not inherited unverified from prior plans' claims." - "A checkpoint gate stage that is a deliberate, self-documenting fatal() (not a stub, not a skip) is the correct shape for a scope boundary an earlier plan intentionally left to a later plan -- it fails closed forever until the real implementation lands, and the failure message names exactly which plan owns the gap." key-files: created: - .planning/phases/08-oauth2-1-authorization-server/08-PHP-TEST-MAP.md - .planning/phases/08-oauth2-1-authorization-server/08-SECURITY-REVIEW.md - .planning/phases/08-oauth2-1-authorization-server/08-10-SUMMARY.md modified: - wristband/phase08_coverage_test.go - scripts/check-phase8.sh - scripts/check-phase8-mcp-client.mjs - ../fonoteka.go/parity/oauth_audit_test.go - ../fonoteka.go/plugins/golem15/fonoteka/phase08_coverage_test.go - ../fonoteka.go/plugins/golem15/fonoteka/classes/auth/phase08_coverage_test.go - ../fonoteka.go/plugins/golem15/fonoteka/console/phase08_coverage_test.go - ../fonoteka.go/plugins/golem15/fonoteka/updates/phase08_coverage_test.go - ../fonoteka.go/plugins/golem15/fonoteka/controllers/api/token_api_controller.go - ../fonoteka.go/plugins/golem15/fonoteka/plugin.go - ../fonoteka.go/plugins/golem15/fonoteka/routes_isolation_test.go - .planning/phases/08-oauth2-1-authorization-server/08-VALIDATION.md - .planning/phases/08-oauth2-1-authorization-server/deferred-items.md - .planning/REQUIREMENTS.md - .planning/STATE.md key-decisions: - "The security-review agent (gsd-security-auditor) could not be spawned in this executor's tool set, so per 08-CONTEXT.md D-04 and 08-10-PLAN.md Task 2's own documented fallback, the review was performed directly by the 08-10 executor. 08-SECURITY-REVIEW.md's frontmatter and Reviewer Note say so explicitly; every cited file:TestName was individually re-run, not taken on faith." - "The review's own pass found one real, previously-undetected gap: controllers/api/token_api_controller.go's generic Destroy route (the manual, non-OAuth-aware token-delete path) revoked only the ApiToken row, leaving its linked OAuthRefreshToken alive and rotatable -- a real divergence from PHP's single canonical revoke path. Fixed in the same review pass by routing Destroy through the same wristband.Server.Revoke cascade ConnectedAppsDestroy already uses (fonoteka.go b936ce6)." - "scripts/check-phase8.sh's first real end-to-end run surfaced two further genuine defects, both fixed under Rule 1/Rule 3: (1) check-phase8-mcp-client.mjs read the wrong connected-apps JSON field name and revoked in the wrong order relative to the scripted lifecycle (summercms.go e562bf6); (2) plugins/golem15/fonoteka/plugin.go resolved the inv_token auth guard and the wristband OAuth backend eagerly at Boot time, before the *gorm.DB and other boot-order-dependent services were guaranteed available under the gate's real disposable-Postgres boot sequence -- both are now resolved lazily (fonoteka.go 55ed600). This second fix is a real correctness fix to the production Boot path, not gate-only scaffolding, and affects the real binary beyond Phase 8." - "Checkpoint decision (user, 2026-09-24): 'Approve, carry gap forward.' Every scripts/check-phase8.sh stage ran green in the sole full gate execution except stage_ui_harness's Playwright browser matrix (32 UI-SPEC scenarios), a deliberate fatal() at check-phase8-ui.mjs:458 that 08-05 intentionally left unauthored as 08-10's seam. The user approved closing Phase 8 now, with that gap recorded as a named follow-up rather than fixed in this plan -- the plan's own action text forbids weakening, skipping, or stubbing the stage to close it." - "AUTH-05, AUTH-06 and AUTH-07 are marked complete: each requirement's exact REQUIREMENTS.md text was checked against the delivered evidence and none of the three mandates a Playwright-verified browser regression suite -- AUTH-05's 'consent screen' clause and AUTH-07's 'fonoteka-mcp completes its install and auth flow unchanged' clause are both about the backend/API-level OAuth flow, which the real scripted-SDK MCP client proved end to end (discovery, DCR, PKCE authorize, JWT consent, token, tool call, refresh, replay, revoke, post-revoke failure). The Playwright gap is Wave 0 test-coverage machinery (08-W0-08), not requirement text." - "08-VALIDATION.md's nyquist_compliant flips to false (from the pre-gate true) because the file's own definition of Nyquist compliance requires every task-to-gate mapping to be actually implemented, and 08-W0-08's Playwright matrix is not. status flips to complete because the phase itself is closed by user approval; the two flags are deliberately allowed to diverge here (a phase can be complete and closed while a documented, carried-forward test-coverage gap keeps nyquist_compliant honestly false)." patterns-established: - "A checkpoint's carried-forward gap gets three synchronized records: deferred-items.md (what's missing, in implementation-ready detail, plus the exact failing identifier), the owning VALIDATION.md row (flipped to a qualified partial-verified status, never silently green), and STATE.md's Deferred Items table plus a state.add-blocker entry (so /gsd:progress surfaces it). All three must name the same failing identifier so a future plan can grep straight to the seam." requirements-completed: [AUTH-05, AUTH-06, AUTH-07] # Metrics duration: ~55min (Tasks 1-2 plus Task 3's automated fixes: 2026-09-24T00:05 - 00:52 local; plus the single check-phase8.sh gate run and this continuation's bookkeeping) completed: 2026-09-24

Phase 08 Plan 10: Coverage, Security Review and Final Gate Summary

103/103 PHP-to-Go OAuth method audit, a self-performed 11/11-closed security review that found and fixed a real revoke-cascade gap, and scripts/check-phase8.sh's first-ever full execution (every stage green except the Playwright UI matrix, a deliberate, never-authored seam) -- Phase 8 closed on user approval with that one gap carried forward.

Performance

  • Duration: ~55 min of active task execution (2026-09-24T00:05:41+02:00 through 00:52:10+02:00 across Tasks 1-3's automated commits), plus the single scripts/check-phase8.sh full gate run and this continuation's bookkeeping (deferred-items.md, 08-VALIDATION.md, REQUIREMENTS.md, STATE.md, this SUMMARY)
  • Started: 2026-09-24T00:05:41+02:00 (first Task 1 commit)
  • Completed: 2026-09-24 (this continuation)
  • Tasks: 3/3 (Task 1 auto/tdd, Task 2 auto, Task 3 checkpoint:human-verify -- approved with a carried-forward gap)
  • Files modified: 17 across both repos (this plan's own commits) plus 4 planning docs in this continuation

Accomplishments

  • 08-PHP-TEST-MAP.md enumerates all 103 PHP OAuth functional/security methods (11 authorize, 8 client-command, 2 metadata, 7 migration, 10 register, 10 token, 7 consent/scope, 10 refresh rotation, 8 revocation, 30 surface isolation) mapped one-to-one to named, passing Go tests/subtests. parity/oauth_audit_test.go's executable audit parses the map and the real Go test list so counts alone cannot hide a missing, renamed, or duplicated mapping. New focused coverage tests closed the remaining gaps: framework handler/store error branches, app boot/config/route/controller/command branches, and exact response/header paths not already covered by earlier Phase 8 plans (wristband/phase08_coverage_test.go; plugins/golem15/fonoteka/{phase08_coverage_test.go, classes/auth/phase08_coverage_test.go, console/phase08_coverage_test.go, updates/phase08_coverage_test.go}).
  • 08-SECURITY-REVIEW.md: 11 total T-08 threats, 11 closed, 0 open, status: verified, asvs_level: 1. The review's tool set had no Task/Agent spawner available, so per 08-CONTEXT.md D-04's documented fallback the executor performed the review directly and says so explicitly in the frontmatter (reviewer: gsd-executor (08-10 Task 2, self-performed -- see Reviewer Note)) and a dedicated Reviewer Note section. Every cited file:TestName was individually re-run during the review, not inherited unverified from earlier plans' own claims.
  • The review found one real, previously-undetected HIGH-relevant gap and fixed it in the same pass: the generic manual token-delete route (controllers/api/token_api_controller.go's Destroy) only stamped revoked_at on the ApiToken row, leaving an OAuth-issued token's linked OAuthRefreshToken row alive and rotatable -- a real divergence from PHP's single canonical revoke path. Fixed by routing Destroy through the same wristband.Server.Revoke cascade ConnectedAppsDestroy already used; GREEN evidence is TestOAuthRevocationManualTokenRouteKillsRefreshChain.
  • scripts/check-phase8.sh ran end to end for the first time ever (it was built and self-validated by 08-09 but never executed). The run surfaced two more genuine defects, both fixed: check-phase8-mcp-client.mjs read the wrong JSON field name for connected apps and revoked in the wrong order relative to the scripted lifecycle; and plugins/golem15/fonoteka/plugin.go resolved the inv_token auth guard and the wristband OAuth backend eagerly at Boot time rather than lazily, which broke under the gate's real disposable-Postgres boot sequence. The second fix touches the real production Boot path, not gate-only scaffolding.
  • After those fixes, the gate's sole full run was green on every stage except one: stage_ui_harness's real Playwright browser matrix (32 08-UI-SPEC.md scenarios) is a deliberate fatal() at check-phase8-ui.mjs:458 that 08-05 intentionally left as 08-10's seam and this plan did not author. Presented with that evidence -- real unchanged fonoteka-mcp discovery/DCR/PKCE-authorize/JWT-consent/token/tool-call/refresh/replay/revoke/post-revoke-failure, both repositories' vet/test/race, 169/169 parity corpus, secret scan, 6/6 return-path checks, 74-key i18n check, unchanged-client diff, and the security-review gate all green -- the user approved closing Phase 8 with the Playwright gap explicitly carried forward.
  • AUTH-05, AUTH-06 and AUTH-07 are marked complete in .planning/REQUIREMENTS.md. Each requirement's exact text was checked against the delivered evidence; none mandates a Playwright-verified browser regression suite, so the carried-forward gap does not block completion.

Task Commits

  1. Task 1: Close the 103-method PHP audit and Phase 8 coverage gaps
    • f425956 (test, summercms.go): wristband/phase08_coverage_test.go -- focused coverage additions
    • eb40d1b (docs, summercms.go): 08-PHP-TEST-MAP.md -- the 103-method map
    • 39974f8 (test, fonoteka.go): the executable audit (parity/oauth_audit_test.go) plus coverage tests across phase08_coverage_test.go, classes/auth, console, updates
  2. Task 2: Run the mandated security-review agent and close every high-severity finding
    • 034f639 (feat, summercms.go): fail-closed 08-SECURITY-REVIEW.md checks added to scripts/check-phase8.sh
    • 0c173df (docs, summercms.go): 08-SECURITY-REVIEW.md (11/11 closed) and 08-VALIDATION.md updates
    • b936ce6 (fix, fonoteka.go): the manual-token-delete revoke-cascade gap the review found, fixed in the same pass
  3. Task 3 (automated part): First real end-to-end run of scripts/check-phase8.sh; real defects fixed
    • fef037e (fix, summercms.go): scripts/check-phase8.sh defects found on first real run
    • e562bf6 (fix, summercms.go): check-phase8-mcp-client.mjs's connected-apps field name and revoke ordering
    • 55ed600 (fix, fonoteka.go): plugin.go -- inv_token guard and the OAuth backend now resolve lazily, not at Boot
  4. Task 3 (checkpoint): Approve the OAuth security and unchanged-client evidence
    • 2d551c0 (docs, summercms.go): this continuation's deferred-items.md Playwright follow-up and 08-VALIDATION.md checkpoint-decision update
    • REQUIREMENTS.md AUTH-05/06/07 marked complete via gsd-sdk query requirements.mark-complete
    • STATE.md updated (Deferred Items table plus state.add-blocker)

Plan metadata: committed as part of this summary/state-update commit.

Files Created/Modified

  • .planning/phases/08-oauth2-1-authorization-server/08-PHP-TEST-MAP.md -- the 103-method audit map
  • .planning/phases/08-oauth2-1-authorization-server/08-SECURITY-REVIEW.md -- 11/11 closed, status: verified
  • wristband/phase08_coverage_test.go -- framework-side coverage additions
  • scripts/check-phase8.sh -- fail-closed security-review gate stage plus real-run defect fixes
  • scripts/check-phase8-mcp-client.mjs -- corrected connected-apps field name and revoke ordering
  • ../fonoteka.go/parity/oauth_audit_test.go -- the executable 103-method audit
  • ../fonoteka.go/plugins/golem15/fonoteka/{phase08_coverage_test.go, classes/auth/phase08_coverage_test.go, console/phase08_coverage_test.go, updates/phase08_coverage_test.go} -- app-side coverage additions
  • ../fonoteka.go/plugins/golem15/fonoteka/controllers/api/token_api_controller.go -- manual-delete revoke cascade fix
  • ../fonoteka.go/plugins/golem15/fonoteka/plugin.go + routes_isolation_test.go -- lazy Boot-order fix
  • .planning/phases/08-oauth2-1-authorization-server/08-VALIDATION.md -- 08-W0-07 green, 08-W0-08 partially verified, nyquist_compliant: false, status: complete
  • .planning/phases/08-oauth2-1-authorization-server/deferred-items.md -- the named Playwright follow-up
  • .planning/REQUIREMENTS.md -- AUTH-05, AUTH-06, AUTH-07 marked complete
  • .planning/STATE.md -- Deferred Items table row and blocker entry for the Playwright gap

Decisions Made

See frontmatter key-decisions. Most load-bearing: the security review was self-performed with explicit disclosure (no Task/Agent spawner available, per the plan's own documented fallback); the user approved Phase 8 closure with the Playwright UI matrix gap carried forward rather than fixed in this plan; and AUTH-05/06/07 are marked complete because none of their exact requirement texts require a Playwright-verified browser suite -- that gap is Wave 0 test-coverage machinery, not unmet requirement text.

Deviations from Plan

Auto-fixed Issues

1. [Rule 1 - Bug] Manual token-delete route did not cascade-revoke an OAuth refresh-token lineage

  • Found during: Task 2, security-review pass
  • Issue: controllers/api/token_api_controller.go's Destroy handler called auth.RevokeToken, which only stamps revoked_at on the ApiToken row; deleting an OAuth-issued token through this generic route left its linked OAuthRefreshToken row alive and rotatable, diverging from PHP's single canonical revoke path.
  • Fix: Destroy now routes through the same wristband.Server.Revoke cascade ConnectedAppsDestroy already uses; a manual (non-OAuth) token's lineage lookup finds nothing and behavior is unchanged.
  • Files modified: ../fonoteka.go/plugins/golem15/fonoteka/controllers/api/token_api_controller.go
  • Verification: TestOAuthRevocationManualTokenRouteKillsRefreshChain
  • Committed in: b936ce6

2. [Rule 3 - Blocking] check-phase8-mcp-client.mjs read the wrong connected-apps field and revoked out of order

  • Found during: Task 3, first real scripts/check-phase8.sh run
  • Issue: The scripted SDK driver's connected-apps stage referenced the wrong JSON field name and performed the revoke step before a prerequisite lifecycle step had actually completed.
  • Fix: Corrected field name and reordered the revoke step to match the real lifecycle.
  • Files modified: scripts/check-phase8-mcp-client.mjs
  • Verification: full scripts/check-phase8.sh run
  • Committed in: e562bf6

3. [Rule 1 - Bug] plugin.go resolved the inv_token guard and OAuth backend at Boot instead of lazily

  • Found during: Task 3, first real scripts/check-phase8.sh run (real disposable-Postgres app-boot stage)
  • Issue: plugin.go resolved the inv_token auth guard and the wristband OAuth backend eagerly during Boot, before other boot-order-dependent services (notably *gorm.DB) were guaranteed available under the gate's real service-startup sequence -- a real correctness bug in the production Boot path, not gate scaffolding.
  • Fix: Both are now resolved lazily.
  • Files modified: ../fonoteka.go/plugins/golem15/fonoteka/plugin.go, routes_isolation_test.go
  • Verification: full scripts/check-phase8.sh app-boot and real-MCP stages; go test ./... both repos
  • Committed in: 55ed600

Total deviations: 3 auto-fixed (1 Rule 1 security-review finding, 1 Rule 3 blocking script fix, 1 Rule 1 production Boot-order bug) Impact on plan: All three were necessary for the plan's own stated goal (a fail-closed final gate and a genuinely closed security review) to hold. The Boot-order fix is the most consequential: it is a real defect in the production binary's startup sequence, only surfaced because this plan is the first to boot the assembled app against a real disposable Postgres end to end. No scope creep -- each fix stayed inside its triggering file's declared scope.

Issues Encountered

The Playwright UI matrix gap (not auto-fixed; carried forward by user decision). scripts/check-phase8-ui.mjs's --final-gate mode runs verify:oauth-return-path and verify:oauth-i18n for real (both green: 6/6 and 74 keys respectively), then reaches a deliberate fatal('--final-gate Playwright matrix wiring is 08-10's responsibility; not implemented in 08-05.') at check-phase8-ui.mjs:458. The 32-scenario SCENARIOS catalog (08-UI-SPEC.md's complete state/accessibility/responsive/i18n matrix) is fully authored and self-tested for completeness by --contract-self-test, but no Playwright spec file or config exists to actually run the browser matrix against it. This is Task 3's type="checkpoint:human-verify" decision point, not a Rule 1-3 auto-fixable defect: the plan's own action text says "Block completion if any displayed result is missing or non-green," and per the checkpoint protocol this required the user's explicit call rather than an automatic fix. The user's decision ("Approve, carry gap forward") is documented in deferred-items.md, 08-VALIDATION.md, and STATE.md, all naming the same failing identifier (check-phase8-ui.mjs:458) so a future plan can locate the seam directly.

User Setup Required

None -- no external service configuration required.

Next Phase Readiness

  • Phase 8 is closed. AUTH-05, AUTH-06 and AUTH-07 are complete in .planning/REQUIREMENTS.md.
  • A named, well-specified follow-up remains open: author a Playwright config and spec (outside the Nuxt checkout, consuming check-phase8-ui.mjs's existing SCENARIOS catalog, wired via NUXT_DEV_BACKEND_ORIGIN or equivalent to the gate's ephemeral Go app, asserting against connect.vue / ConsentScopePicker.vue / ConnectedAppsManager.vue via their data-testid selectors) so stage_ui_harness can run the real 32-scenario matrix instead of failing closed. See deferred-items.md's "Follow-up: Playwright UI matrix" entry for the full specification. scripts/check-phase8.sh must keep failing closed on this stage until that spec exists -- do not weaken, skip, or stub it.
  • 08-VALIDATION.md's nyquist_compliant: false is intentional and honest; it should flip back to true only once the Playwright follow-up lands.
  • The lazy Boot-order fix in plugins/golem15/fonoteka/plugin.go (55ed600) is a real production fix that future phases inherit for free -- no further action needed, but worth knowing about if a later phase's own Boot-order assumptions are ever questioned.
  • No other blockers.

Self-Check: PASSED

  • FOUND: .planning/phases/08-oauth2-1-authorization-server/08-PHP-TEST-MAP.md
  • FOUND: .planning/phases/08-oauth2-1-authorization-server/08-SECURITY-REVIEW.md
  • FOUND: .planning/phases/08-oauth2-1-authorization-server/08-VALIDATION.md
  • FOUND: .planning/phases/08-oauth2-1-authorization-server/deferred-items.md
  • FOUND commits (summercms.go): f425956, eb40d1b, 034f639, 0c173df, fef037e, e562bf6, 2d551c0
  • FOUND commits (fonoteka.go): 39974f8, b936ce6, 55ed600
  • FOUND: .planning/REQUIREMENTS.md shows AUTH-05/06/07 as [x] and Complete

Phase: 08-oauth2-1-authorization-server Completed: 2026-09-24

Reference in New Issue View Git Blame Copy Permalink