docs(19.7): gap-closure plans 06-09 for the review findings and the sm-servers-app escalations
This commit is contained in:
Jakub Zych
@@ -1454,7 +1454,7 @@ Plans:
|
||||
3. `tools/list` lists only the tools the session's principals may call (frontend permissions through the merged group/user permissions, backend permissions and token scope through cabana), and a call runs through the route's own middleware.
|
||||
4. An editor signs in once through `wristband` OAuth as a frontend user; with a verified link to a backend user, backend-guarded tools such as Journal's run as that admin, and removing the link or the grant removes them. The phase gets a security review; the MCP SDK dependency has a decision note before code lands; unit tests are the last plan.
|
||||
|
||||
**Plans:** 5/5 plans executed
|
||||
**Plans:** 5/9 plans executed
|
||||
|
||||
Plans:
|
||||
**Wave 1**
|
||||
@@ -1472,6 +1472,20 @@ Plans:
|
||||
**Wave 5** *(blocked on Wave 4 completion)*
|
||||
- [x] 19.7-05-PLAN.md — Unit tests across six repos, `scripts/check-phase19.7.sh`, the filled validation map and the security review
|
||||
|
||||
Gap closure (`19.7-REVIEW.md` findings, escalations E-1 and E-2; `19.7-GAP-INPUTS.md`). No tag, push or deploy until verification; then framework v0.5.0, the plugin tags, the app bumps.
|
||||
|
||||
**Gap wave 1**
|
||||
- [ ] 19.7-06-PLAN.md — pier hardening (lane A): session reaper, atomic per-binding reservation, no global lock across guard I/O, identity widening, Redis fan-out for stateless instances, listen re-check, path-value rules, fixed error text, prompt write ceiling
|
||||
|
||||
**Gap wave 2** *(blocked on 19.7-06: shared pier and pact files)*
|
||||
- [ ] 19.7-07-PLAN.md — Framework auth and admin (lane B): `auth_time` claim, `pact.HasFrontendTokenScopes` seam (E-2), token-specific revoke events, client address on the resolve path, My account URL and section isolation, hotkey fixes, OpenAPI and dist
|
||||
|
||||
**Gap wave 3** *(blocked on 19.7-06 and 19.7-07)*
|
||||
- [ ] 19.7-08-PLAN.md — Plugins and demo (lane C): link proof on `auth_time` with the code in the fragment, grant revocation on ban/reset and link changes, member token scopes (E-2), organisation logo (E-1), Journal prompt ceiling, nginx MCP body cap
|
||||
|
||||
**Gap wave 4** *(blocked on 19.7-06, 19.7-07 and 19.7-08)*
|
||||
- [ ] 19.7-09-PLAN.md — Unit tests for every fix and both escalations, the extended gate (workspace check, unreleased-pin policy), validation rows, the corrected security review, deferred items
|
||||
|
||||
### Phase 19.8: Console commands overhaul (INSERTED)
|
||||
|
||||
**Goal:** Console commands in `summer` and in application binaries reach the bar set by Laravel's Artisan. The command logic stays; what changes is logging, the shared console layer in `bonfire`, and every command's help and output. A user sees what a command did, can ask for more or less of it, and learns from `--help` how to use it without opening the docs. Example of today's gap: `serve` prints raw `slog` lines from plugins and River mixed with styled `ℹ` lines, says nothing about what it booted (plugins, migrations state, mail driver, cache, realtime, scheduler), and has no way to see more or less. Logging today is one hard-coded `slog.NewTextHandler` at INFO in `sunscreen.Install`, with no level, format or destination config.
|
||||
|
||||
407
.planning/phases/19.7-generic-mcp-module/19.7-06-PLAN.md
Normal file
407
.planning/phases/19.7-generic-mcp-module/19.7-06-PLAN.md
Normal file
@@ -0,0 +1,407 @@
|
||||
---
|
||||
phase: 19.7-generic-mcp-module
|
||||
plan: 06
|
||||
type: execute
|
||||
wave: 1
|
||||
depends_on: []
|
||||
gap_closure: true
|
||||
files_modified:
|
||||
- modules/pier/session.go
|
||||
- modules/pier/pier.go
|
||||
- modules/pier/audit.go
|
||||
- modules/pier/store.go
|
||||
- modules/pier/store_memory.go
|
||||
- modules/pier/store_redis.go
|
||||
- modules/pier/stateless.go
|
||||
- modules/pier/identity.go
|
||||
- modules/pier/dispatch.go
|
||||
- modules/pier/catalog.go
|
||||
- modules/pier/filter.go
|
||||
- modules/pier/export_test.go
|
||||
- modules/pier/reaper_smoke_test.go
|
||||
- modules/pier/README.md
|
||||
- modules/pact/mcp.go
|
||||
- modules/pact/README.md
|
||||
- modules/cooler/cooler.go
|
||||
- docs/services/mcp.md
|
||||
autonomous: true
|
||||
requirements: [SC-1, SC-2, SC-3, D-10, D-11, D-13, D-17, D-19, D-20, D-25, D-26, D-27, D-28, D-32]
|
||||
|
||||
estimate:
|
||||
tokens: 110000
|
||||
raw_tokens: 110000
|
||||
tasks: 4
|
||||
confidence: low
|
||||
|
||||
must_haves:
|
||||
truths:
|
||||
- "A stateful session whose store record has expired (idle past mcp.sessions.idle_ttl, or never DELETEd) is closed on every instance that holds a local copy: its SDK server session ends, it leaves the local map and the fan-out matching set, and an `expired` audit line is written (A-CR-01, A-IN-06, D-27)"
|
||||
- "N concurrent initialize POSTs from one binding open at most mcp.sessions.max_per_binding sessions on the memory store and on Redis; a failed connect or initialize releases its reservation (A-WR-03, D-25)"
|
||||
- "A slow guard or database check during one client's initialize, rehydration or stateless server build does not block other clients' requests on the instance (A-WR-07)"
|
||||
- "When the same binding comes back with a wider identity (more scopes, a backend subject added), its stateful session and its stateless server gain the newly allowed tools and prompts on that request (A-WR-04, D-20, D-23)"
|
||||
- "On the Redis store an instance subscribes to fan-out events at mount without waiting for a stateful request; a stateless subscriptions/listen stream is re-verified periodically and ends when its bearer stops resolving; an LRU-evicted stateless server's sessions are closed (A-WR-05, A-WR-06, D-26, D-32)"
|
||||
- "A personal-token subject without a TokenID is refused at /mcp, so no two users share a binding (A-WR-09, D-19)"
|
||||
- "pier never hands a route a path value the HTTP mux could not produce: empty, `.` or `..` segments are refused, a single segment never contains `/`, an omitted path field is refused at runtime and a path field that is not required in the schema fails boot (A-WR-01, A-WR-02, D-10, D-11)"
|
||||
- "A guard dispatch failure answers the MCP client with a fixed text and is logged server-side; integer arguments above 2^53 keep their digits; the dispatched request carries the outer Host and forwarding headers; an embedded struct on a GET or DELETE tool fails boot; a provider answer naming the backend guard for a frontend tool is refused (A-WR-08, A-IN-04, A-IN-05, A-IN-07, A-IN-08, D-13, D-17)"
|
||||
- "A prompt declared with Writes is hidden from an identity without write, as a non-read-only tool is (C-IN-01 framework part, D-20, D-28)"
|
||||
artifacts:
|
||||
- path: "modules/pier/session.go"
|
||||
provides: "the session reaper and the lock-free connect path"
|
||||
contains: "func (sr *sessionRouter) reap("
|
||||
- path: "modules/pier/store.go"
|
||||
provides: "the unexported reserver contract next to updater"
|
||||
contains: "type reserver interface"
|
||||
- path: "modules/pier/store_redis.go"
|
||||
provides: "the atomic Lua reservation"
|
||||
contains: "NewScript"
|
||||
- path: "modules/pier/reaper_smoke_test.go"
|
||||
provides: "the tracer's end-to-end smoke"
|
||||
contains: "func TestPierReaperSmoke("
|
||||
- path: "modules/pact/mcp.go"
|
||||
provides: "MCPPrompt.Writes and the corrected personal-token resolver contract"
|
||||
contains: "Writes"
|
||||
key_links:
|
||||
- from: "modules/pier/pier.go"
|
||||
to: "modules/pier/session.go"
|
||||
via: "Mount starts the reaper goroutine"
|
||||
pattern: "reap("
|
||||
- from: "modules/pier/session.go"
|
||||
to: "modules/pier/store.go"
|
||||
via: "serveNew reserves through the reserver interface instead of Count then Put"
|
||||
pattern: "Reserve("
|
||||
- from: "modules/pier/filter.go"
|
||||
to: "modules/pact/mcp.go"
|
||||
via: "allowsPrompt applies the read/write ceiling when decl.Writes is set"
|
||||
pattern: "Writes"
|
||||
---
|
||||
|
||||
<!-- planner-discipline-allow: TokenID == 0 -->
|
||||
|
||||
<objective>
|
||||
Close lane A of the phase code review (`19.7-REVIEW.md`) in the framework's `pier` module: the critical session leak A-CR-01, the nine warnings A-WR-01..A-WR-09, and every lane A info finding (A-IN-01..A-IN-08), all of which are cheap. It also lands the framework part of C-IN-01 (`pact.MCPPrompt.Writes`), because that field lives next to the prompt filter. This is gap closure for phase 19.7 before framework v0.5.0 (`19.7-GAP-INPUTS.md`). The code review found no identity or guard bypass. These are lifecycle, concurrency and edge-handling defects, and the fixes keep D-25's rule that every request is authorized fresh.
|
||||
|
||||
**Planner decisions recorded here (P-24..P-28 continue plan 05's P-21..P-23):**
|
||||
- **P-24 (reaper, A-CR-01):** a janitor goroutine started at `Mount` checks every local session against the store every `min(idle_ttl/2, 1m)`. It closes each local copy whose record is gone and logs `expired`. The store's TTL stays the single source of truth across instances: a session another instance keeps alive still has its record, so this instance keeps its copy. No new config key. Stop path: `backpack.App` has no shutdown hook today (its methods are `SetPlugins`, `HasPlugin`, `Publish` and `Lookup`). So `Mount` runs the reaper under a context whose cancel func is kept on the `Pier` (`p.stopReap`). In production it runs for the life of the process, like the store subscription and the user plugin's blacklist sweep. Tests stop it through the `StopReaper` seam, and when backpack gains a lifecycle hook, `Mount` registers `p.stopReap` there (one line, noted in the README). `export_test.go` gets the `ReapNow`, `LocalSessions` and `StopReaper` seams.
|
||||
- **P-25 (atomic reservation, A-WR-03):** an unexported `reserver` interface (`Reserve`, `Release`) sits next to plan 05's `updater`. It follows the same pattern, so the exported `pier.Store` interface does not change. The memory store reserves under its mutex. The Redis store reserves with one Lua script (prune dead members, `SCARD`, `SADD` plus a placeholder `SET ... PX`); miniredis supports `EVAL`. A store that does not implement it falls back to `Count` then `Put`.
|
||||
- **P-26 (widening, A-WR-04):** `Identity.fingerprint()` covers the authorization-relevant parts: sorted scopes, the frontend and backend subjects (guard, user, client, token), the ceiling and the token kind. A stateless hit or a stateful lookup whose fingerprint changed runs the same add/remove diff as a fan-out event.
|
||||
- **P-27 (path values, A-WR-01, A-WR-02):** these refusals mirror what `net/http.ServeMux` guarantees over HTTP:
|
||||
- every `path:` field must be listed as required in the reflected schema (boot error otherwise);
|
||||
- at runtime an omitted path value, an empty, `.` or `..` segment, and a `/` inside a single-segment value are refused before dispatch.
|
||||
- **P-28 (prompt ceiling, C-IN-01):** the new field `pact.MCPPrompt.Writes bool` (additive, zero value unchanged) puts a prompt under the same read/write ceiling as a non-read-only tool. Journal sets it in plan 08.
|
||||
|
||||
Purpose: the review's lane A blockers are gone before v0.5.0. Plan 07 builds its token-specific events on these files, plan 08 uses `MCPPrompt.Writes`, and plan 09 pins every fix with named tests.
|
||||
Output: framework commits in `modules/pier`, `modules/pact` and `modules/cooler` with their README and docs pages, and one tracer smoke test.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@~/.claude/msd-core/workflows/execute-plan.md
|
||||
@~/.claude/msd-core/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<context>
|
||||
@.planning/PROJECT.md
|
||||
@.planning/ROADMAP.md
|
||||
@.planning/STATE.md
|
||||
@.planning/phases/19.7-generic-mcp-module/19.7-GAP-INPUTS.md
|
||||
@.planning/phases/19.7-generic-mcp-module/19.7-REVIEW.md
|
||||
@.planning/phases/19.7-generic-mcp-module/19.7-CONTEXT.md
|
||||
@.planning/phases/19.7-generic-mcp-module/19.7-02-SUMMARY.md
|
||||
@.planning/phases/19.7-generic-mcp-module/19.7-05-SUMMARY.md
|
||||
@CLAUDE.md
|
||||
@modules/pier/README.md
|
||||
</context>
|
||||
|
||||
<project_rules>
|
||||
- Unit tests are plan 09's job (project rule 2). This plan carries one tracer smoke, `TestPierReaperSmoke`. Every existing pier, surf and pact test, and the gate's go stage, stays green after each task: the earlier named tests (`TestSessions`, `TestFanOut`, `TestToolsList`, `TestDispatch`, `TestCatalog`, `TestEndpointGuards` and the smokes) are regression pins, not something to rewrite. A test that encodes the old, wrong behaviour (for example one relying on a shared `pat:user:0` binding, or on a non-required path field compiling) is changed in the same commit as the fix, and the SUMMARY names it.
|
||||
- Documentation rule (CLAUDE.md): an exported API change, here `pact.MCPPrompt.Writes` and the resolver contract text, updates `modules/pact/README.md`, `modules/pier/README.md` and `docs/services/mcp.md` in the same commit. Every identifier in those files must exist (`summer docs:build --check`).
|
||||
- No new dependency: `go.mod`, `go.sum` and `admin/package-lock.json` stay unchanged.
|
||||
- Several sessions work in this repo: run `git status --short modules/pier modules/pact modules/cooler docs` before every task and leave other sessions' edits alone. Stage by explicit path, one logical change per commit, no co-author tags. Planning docs go in their own commit.
|
||||
- Every commit is self-contained: at each commit the framework builds, `go vet ./...` is clean and the `-short` tests pass. No commit relies on a later task's commit. A task with several commits orders them so this holds.
|
||||
- Release rule: no tag, push or deploy in this plan. Commits stay local on master.
|
||||
</project_rules>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="tracer">
|
||||
<name>Task 1: Tracer: an abandoned stateful session is reaped end to end (store expiry → local close → audit line → gone from fan-out)</name>
|
||||
<files>modules/pier/session.go, modules/pier/pier.go, modules/pier/audit.go, modules/pier/export_test.go, modules/pier/reaper_smoke_test.go, modules/pier/README.md</files>
|
||||
<read_first>
|
||||
- .planning/phases/19.7-generic-mcp-module/19.7-REVIEW.md, sections A-CR-01, A-IN-01 and A-IN-06 (failure scenario and suggested janitor)
|
||||
- modules/pier/session.go: `sessionRouter`, `localSession`, `serveNew`, `lookup`, `connect`, `persist`, `serveStream` (the GET re-check ticker), `closeLocal`, `matching`
|
||||
- modules/pier/pier.go: `Mount` (where `listenEvents` and the store are set up) and `newStore`
|
||||
- modules/pier/audit.go: `logSession` and the documented steps, including `expired`
|
||||
- modules/pier/store_memory.go (lazy expiry, `now` clock) and store_redis.go (`Get` on an expired key)
|
||||
- modules/pier/export_test.go (`AdvanceStoreClock`, `SetRecheck`) and modules/pier/helpers_test.go (`newApp`, `startInstance`, `sdkClient`, `fakeVerifier`)
|
||||
</read_first>
|
||||
<action>
|
||||
Implement P-24 for A-CR-01.
|
||||
|
||||
In session.go:
|
||||
- Add `func (sr *sessionRouter) reap(ctx context.Context, every time.Duration)`, a ticker loop that calls `reapOnce`.
|
||||
- Add `func (sr *sessionRouter) reapOnce(ctx context.Context) int`. It copies the session ids from `sr.local` under `sr.mu`, releases the lock, then calls `sr.p.store.Get` for each id without holding any router lock. When the answer is `ok == false` and `err == nil`, it calls `closeLocal(id)` and `logSession(logger(), "expired", id, binding)`. It returns how many sessions it closed. A store error keeps the local copy, logs one warning per pass, and never closes on doubt.
|
||||
- `closeLocal` already closes the SDK server session, which ends the `ss.Wait()` goroutine, and removes the id from `sr.local`. Confirm that `matching` reads only `sr.local`, so a reaped session drops out of fan-out.
|
||||
|
||||
In pier.go `Mount`, after `p.sessions` and `p.store` exist, create a cancellable context with `context.WithCancel(context.Background())`, store its cancel func in the new unexported `Pier` field `stopReap`, and start `go p.sessions.reap(ctx, reapInterval(cfg.IdleTTL))` with that context. The loop returns when the context is cancelled. `reapInterval` returns `min(idleTTL/2, time.Minute)`, floored at one second.
|
||||
|
||||
A-IN-01: in `serveStream`'s re-check ticker, after the bearer re-resolves, call `store.Get` for the session. A record that is gone (`ok == false`, no error) cancels the stream and calls `closeLocal`, so an `ended` event lost between instances no longer keeps the stream open.
|
||||
|
||||
A-IN-06:
|
||||
- Emit `expired` from the reaper. Update the audit.go doc so every documented step is emitted somewhere.
|
||||
- Log the tool calls `serveEphemeral` serves with the session label `ephemeral` instead of `stateless`. Change only that call site's label.
|
||||
|
||||
export_test.go gains three seams (the third is `func (p *Pier) StopReaper()`, which calls `p.stopReap`, so a test can end the goroutine; the smoke registers it with `t.Cleanup`):
|
||||
- `func (p *Pier) ReapNow(ctx context.Context) int`, which calls `p.sessions.reapOnce`;
|
||||
- `func (p *Pier) LocalSessions() int`, which counts `sr.local` under its lock.
|
||||
|
||||
New file `modules/pier/reaper_smoke_test.go` (package `pier_test`, built on helpers_test.go) holds `TestPierReaperSmoke` on the memory store:
|
||||
1. Open a session with an SDK client (`sdkClient`) and confirm `LocalSessions() == 1`.
|
||||
2. Advance the store clock past `mcp.sessions.idle_ttl` with `AdvanceStoreClock`, then call `ReapNow`; it returns 1 and `LocalSessions() == 0`.
|
||||
3. The next request with the old `Mcp-Session-Id` answers 404.
|
||||
4. A second session that stays active (touched by a request) survives a reap pass.
|
||||
|
||||
In modules/pier/README.md, add one paragraph under the session lifecycle: idle sessions are reaped at the store TTL, and `expired` joins the audit steps. No new configuration key.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>go vet ./modules/pier/... && go test -count=1 -short ./modules/pier/... && go test -count=1 -short -tags summercms_mcp ./modules/pier/... ./modules/surf/... && go test -count=1 -short -run '^TestPierReaperSmoke$' -v ./modules/pier/ 2>&1 | grep -q -- '^--- PASS: TestPierReaperSmoke'</automated>
|
||||
<fails_when>non-zero exit, "FAIL" in the output, or no `--- PASS: TestPierReaperSmoke` line (a SKIP counts as missing)</fails_when>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `grep -c "func (sr \*sessionRouter) reap(\|func (sr \*sessionRouter) reapOnce(" modules/pier/session.go` prints 2
|
||||
- `grep -v '^\s*//' modules/pier/pier.go | grep -c "reap("` prints at least 1
|
||||
- `grep -c '"expired"' modules/pier/session.go` prints at least 1 and `grep -c '"ephemeral"' modules/pier/session.go` prints at least 1
|
||||
- `grep -c "func (p \*Pier) ReapNow(\|func (p \*Pier) LocalSessions(\|func (p \*Pier) StopReaper(" modules/pier/export_test.go` prints 3 and `grep -c "stopReap" modules/pier/pier.go` prints at least 1
|
||||
- `grep -c "func TestPierReaperSmoke(" modules/pier/reaper_smoke_test.go` prints 1
|
||||
- `grep -ci "reap\|expired" modules/pier/README.md` prints at least 2
|
||||
</acceptance_criteria>
|
||||
<done>An idle, abandoned stateful session is closed and forgotten at the store's TTL on every instance holding a copy, with an `expired` audit line; the smoke proves it end to end through a real SDK client.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 2: Atomic per-binding reservation and no router-wide lock across guard or database I/O</name>
|
||||
<files>modules/pier/store.go, modules/pier/store_memory.go, modules/pier/store_redis.go, modules/pier/session.go, modules/pier/stateless.go</files>
|
||||
<read_first>
|
||||
- 19.7-REVIEW.md, sections A-WR-03 and A-WR-07
|
||||
- modules/pier/store.go (`Store`, `updater`), store_memory.go (`Count`, `Put`, `Update`), store_redis.go (`Count` with `SMEMBERS` and `EXISTS`, the `MULTI` in `Put`, key layout under cooler's prefix, the `lazyStore` wrapper and how it forwards `updater`)
|
||||
- modules/pier/session.go: `serveNew`, `lookup` (rehydration path calling `connect` under `sr.mu`), `connect`
|
||||
- modules/pier/stateless.go: `getServer` (builds the server under `sp.mu`)
|
||||
- the plan 05 commit 1111828 (`git show 1111828 --stat` and its diff of store.go): the `updater` pattern to mirror
|
||||
</read_first>
|
||||
<action>
|
||||
**P-25 (A-WR-03).** In store.go, declare an unexported `reserver` interface:
|
||||
- `Reserve(ctx context.Context, binding, id string, rec Record, max int, ttl time.Duration) (bool, error)`
|
||||
- `Release(ctx context.Context, binding, id string) error`
|
||||
|
||||
Document it as the atomic form of `Count` then `Put`.
|
||||
|
||||
Implementations:
|
||||
- **memoryStore:** `Reserve` runs under `m.mu`. It sweeps, counts the binding's live records, refuses at `max` and writes `rec` with the TTL. `Release` deletes the record.
|
||||
- **redisStore:** `Reserve` is one `redis.NewScript` Lua script over the binding set and the record key. The script drops set members whose record key no longer exists, refuses when `SCARD >= max`, and otherwise runs `SADD` plus `SET <record> <json> PX <ttl>`. Keep the existing key layout so `Count`, `Get`, `Delete` and the TTLs stay compatible. `Release` does `SREM` plus `DEL`.
|
||||
- **lazyStore:** forwards both methods, the same way it forwards `updater`.
|
||||
|
||||
In `serveNew`, replace the `Count` / `Put` pair:
|
||||
- Reserve through the `reserver` when the store implements it. A refusal is the existing 429 body. A store error is the existing `storeUnavailable` answer.
|
||||
- Otherwise fall back to the present `Count` then `Put` sequence.
|
||||
- On a connect error, or an initialize that leaves `InitializeParams() == nil`, call `Release` so the slot frees.
|
||||
|
||||
**A-WR-07.**
|
||||
- In `serveNew` and in `lookup`'s rehydration path, call `sr.connect(...)` with no router lock held. Then take `sr.mu`: if `sr.local[sid]` is already set by a concurrent request, close the newly built session and use the existing one; otherwise insert. Release the lock before serving.
|
||||
- In stateless.go `getServer`, look up under `sp.mu`; on a miss, release the lock, build the server with `newServer`, take the lock again, and either adopt an entry another request inserted (dropping the new server) or insert and run the capacity eviction.
|
||||
- `matching`, `closeLocal` and the fan-out paths keep their short critical sections.
|
||||
- Never hold `sr.mu` or `sp.mu` while calling a guard, a resolver or the store.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>go vet ./modules/pier/... && go test -count=1 -short -race ./modules/pier/... && go test -count=1 -short -tags summercms_mcp ./modules/pier/... && go test -count=1 -short -run '^(TestSessions|TestFanOut|TestPierSessionsSmoke|TestPierReaperSmoke)$' -v ./modules/pier/ 2>&1 | grep -c -- '^--- PASS: Test' | grep -qx 4</automated>
|
||||
<fails_when>non-zero exit, a data race report, "FAIL" in the output, or fewer than 4 top-level PASS lines</fails_when>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `grep -c "type reserver interface" modules/pier/store.go` prints 1
|
||||
- `grep -c "func (m \*memoryStore) Reserve(\|func (m \*memoryStore) Release(" modules/pier/store_memory.go` prints 2
|
||||
- `grep -c "NewScript" modules/pier/store_redis.go` prints at least 1 and `grep -c ") Reserve(" modules/pier/store_redis.go` prints at least 2 (redisStore and the lazyStore forwarder, both in store_redis.go)
|
||||
- `grep -v '^\s*//' modules/pier/session.go | grep -c "Reserve("` prints at least 1
|
||||
- `go test -count=1 -short -race ./modules/pier/...` reports no race
|
||||
</acceptance_criteria>
|
||||
<done>The max_per_binding cap holds under concurrency on both stores; no router-wide pier lock is held across guard, resolver or store I/O; the existing session, fan-out and reaper tests stay green under -race.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 3: Identity widening on return, fan-out for stateless-only Redis instances, stateless listen re-check and LRU close, personal-token binding</name>
|
||||
<reversibility rating="costly">The `pact.MCPPersonalTokenResolver` contract text now requires TokenID; a third-party resolver written against the old text must set it (published framework API, documented in pact README).</reversibility>
|
||||
<files>modules/pier/session.go, modules/pier/stateless.go, modules/pier/pier.go, modules/pier/identity.go, modules/pact/mcp.go, modules/pact/README.md, modules/pier/README.md</files>
|
||||
<read_first>
|
||||
- 19.7-REVIEW.md, sections A-WR-04, A-WR-05, A-WR-06, A-WR-09, A-IN-03
|
||||
- modules/pier/identity.go: `Identity`, `subjectFor`, `has`, `writes`, `resolvePersonalToken` (binding `pat:user:<TokenID>`)
|
||||
- modules/pier/events.go: `handleEvent`, `recomputeSession`, `recomputeStateless` (the diff to reuse); `applyDiff` or its equivalent in session.go
|
||||
- modules/pier/stateless.go: `getServer`, `evict`, `statelessEntry`, `ServeHTTP`; modules/pier/pier.go `route`, `newStore` (`lazyStore.open`), `Mount`
|
||||
- modules/pier/session.go: `serveStream` (the periodic bearer re-check to mirror), `peekMethods`, `lookup`
|
||||
- modules/pact/mcp.go: the `MCPPersonalTokenResolver` doc comment and `MCPSubject`
|
||||
</read_first>
|
||||
<action>
|
||||
**P-26 (A-WR-04).** Add `func (id Identity) fingerprint() string` in identity.go. It encodes the sorted scopes, both subjects (guard, user, client, token, and their sorted scopes), `ceiling` and `TokenKind`.
|
||||
- Store the fingerprint on `localSession` and on `statelessEntry` when each is built.
|
||||
- In `lookup` (stateful) and on a `getServer` hit (stateless), compare the request identity's fingerprint. When it differs, under the session's or entry's own mutex (never `sr.mu` or `sp.mu`), recompute `p.catalog.allowed(ctx, id)`, apply the same add/remove diff `recomputeSession` and `recomputeStateless` use, and store the new identity and fingerprint.
|
||||
- A narrower identity still relies on the receiving middleware filter, which stays unchanged.
|
||||
|
||||
**A-WR-05.** When the store is Redis, `Mount` starts a background warm-up. It calls the `lazyStore`'s open with backoff (1 s doubling to 30 s, logging each failure once per backoff step) until the store is open, and with it the `mcp:events` subscription.
|
||||
- `route` also triggers the lazy open on the first request on any path, without blocking it. Stateless requests never wait for Redis.
|
||||
- Stateful requests keep the 503 answer while the store is down.
|
||||
|
||||
**A-WR-06.**
|
||||
- In `getServer`'s capacity eviction, close the evicted entry's SDK sessions the way `evict` does, outside `sp.mu`.
|
||||
- In `statelessPath.ServeHTTP`, for a POST, read the body (it is already limited by `bodyLimit`) and restore it. When `peekMethods` contains `subscriptions/listen`, serve the request with a cancellable context and a ticker every `p.recheck` that re-resolves the request's bearer through `p.identities.resolve`. A resolve failure cancels the context, so the SDK ends the long-lived POST. This mirrors `serveStream`.
|
||||
|
||||
**A-WR-09 and A-IN-03.**
|
||||
- `resolvePersonalToken` refuses a subject whose `TokenID == 0`, with the same `ErrUnauthorized` path as `UserID == 0`.
|
||||
- Rewrite the `pact.MCPPersonalTokenResolver` doc in pact/mcp.go:
|
||||
- one resolver per application (published through `app.Lookup`);
|
||||
- false means "not this resolver's prefix" and pier answers unauthorized;
|
||||
- `TokenID` must identify the token row, because it keys the session binding.
|
||||
- Update the matching row in modules/pact/README.md and the personal-token paragraph in modules/pier/README.md in the same commit.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>go vet ./modules/pier/... ./modules/pact/... && go test -count=1 -short -race ./modules/pier/... ./modules/pact/... && go test -count=1 -short -tags summercms_mcp ./modules/pier/... ./modules/surf/... && go test -count=1 -short -run '^(TestToolsList|TestFanOut|TestPierStatelessSmoke|TestPierFanOutSmoke)$' -v ./modules/pier/ 2>&1 | grep -c -- '^--- PASS: Test' | grep -qx 4</automated>
|
||||
<fails_when>non-zero exit, a data race report, "FAIL" in the output, or fewer than 4 top-level PASS lines</fails_when>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `grep -c "func (id Identity) fingerprint() string" modules/pier/identity.go` prints 1
|
||||
- `grep -v '^\s*//' modules/pier/stateless.go | grep -c "fingerprint()"` prints at least 1 and `grep -v '^\s*//' modules/pier/session.go | grep -c "fingerprint()"` prints at least 1
|
||||
- `grep -c "subscriptions/listen\|methodListen" modules/pier/stateless.go` prints at least 1
|
||||
- `grep -v '^\s*//' modules/pier/identity.go | grep -c "TokenID == 0"` prints at least 1
|
||||
- `grep -c "TokenID" modules/pact/mcp.go` prints at least 2 and the resolver doc no longer says another resolver is tried (`grep -c "next resolver is tried" modules/pact/mcp.go` prints 0)
|
||||
</acceptance_criteria>
|
||||
<done>A returning binding with more reach sees its new tools at once; a Redis instance serving only stateless clients receives fan-out; stateless listen streams end when their bearer stops resolving or their server is evicted; personal-token sessions can never share a binding; the resolver contract says what pier actually does.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 4: Dispatch and catalog hardening (path values, fixed error text, big integers, Host and forwarding headers, embedded fields, guard list), the prompt write ceiling, and cooler's doc comments</name>
|
||||
<reversibility rating="costly">`pact.MCPPrompt.Writes` is a new field of a published capability (D-28 costly); its zero value keeps today's behaviour.</reversibility>
|
||||
<files>modules/pier/dispatch.go, modules/pier/catalog.go, modules/pier/filter.go, modules/pact/mcp.go, modules/pact/README.md, modules/cooler/cooler.go, modules/pier/README.md, docs/services/mcp.md</files>
|
||||
<read_first>
|
||||
- 19.7-REVIEW.md, sections A-WR-01, A-WR-02, A-WR-08, A-IN-02, A-IN-04, A-IN-05, A-IN-07, A-IN-08 and lane C's C-IN-01
|
||||
- modules/pier/dispatch.go: `callInfo` and its capture, `run` (argument split, `scalarString`, `queryStrings`, the guard dispatch error branch, `bouncer.WithDispatch`, the `httptest.NewRequestWithContext` build, `SetPathValue`)
|
||||
- modules/pier/catalog.go: `compileTool` (segment ↔ `path:` field check, the GET/DELETE body-field refusal), `inputFields` (the anonymous-field skip), the reflected `schema.Required`
|
||||
- modules/pier/filter.go `allowsPrompt`, `allowsTool`; modules/pact/mcp.go `MCPPrompt`, `MCPTool` (ReadOnly and Destructive overrides as the style model)
|
||||
- modules/cooler/cooler.go lines 345-370 (`KeyPrefix` and `keyPrefix` doc comments)
|
||||
- docs/services/mcp.md (prompts paragraph, tool argument rules) and modules/pier/README.md (catalog boot errors list)
|
||||
</read_first>
|
||||
<action>
|
||||
**A-WR-01 (P-27).** In `run`, for every `argPath` value:
|
||||
- split it on `/` and refuse any empty, `.` or `..` segment;
|
||||
- for a single-segment field, refuse any `/` (as today);
|
||||
- answer `pier: invalid arguments: "<name>" must not contain empty, . or .. segments` as a tool error before any dispatch.
|
||||
|
||||
**A-WR-02.**
|
||||
- In `compileTool`, after the schema is reflected, fail boot with `<prefix>: path field "<name>" must be required (drop omitempty/omitzero)` when a `path:` field is missing from `schema.Required`.
|
||||
- At runtime, refuse a missing path value before the request is built (defence in depth).
|
||||
|
||||
**A-WR-08.** Replace the branch that turns a `MCPDispatch` error into tool text built from the error's own message:
|
||||
- log `pier: guard dispatch failed` server-side with tool, guard and error;
|
||||
- return the fixed tool error `pier: the tool could not be run for this caller`;
|
||||
- keep a specific message only for `pact.ErrMCPTokenInvalid` and the provider sentinel errors dispatch.go already distinguishes.
|
||||
|
||||
**A-IN-04.** Decode path and query scalars with `json.NewDecoder(bytes.NewReader(raw))` and `UseNumber()`, and render a `json.Number` as its literal string.
|
||||
|
||||
**A-IN-05.** Capture the outer `r.Host`, `X-Forwarded-Proto` and `X-Forwarded-Host` in `callInfo`. Set `req.Host` and copy both headers onto the dispatched request, next to X-Forwarded-For and X-Real-IP. The inner request still carries no Authorization or Cookie.
|
||||
|
||||
**A-IN-07.** In `compileTool`, fail boot when a GET or DELETE tool's input struct has an anonymous (embedded) field without a `path`, `query` or `json:"-"` tag. Name the field in the error.
|
||||
|
||||
**A-IN-08.** After `MCPDispatch`, refuse the call with a fixed tool error and a warning log when the answer contradicts the tool's guard:
|
||||
- `disp.Guards` contains `BackendGuard` while the tool's guard is not backend;
|
||||
- or `principal.Backend != (t.guardName == BackendGuard)`.
|
||||
|
||||
**C-IN-01 framework part (P-28).**
|
||||
- Add `Writes bool` to `pact.MCPPrompt`, with a doc comment: the prompt directs write tools, so pier lists it only for an identity that may write.
|
||||
- In filter.go `allowsPrompt`, return false when `p.decl.Writes && !id.writes()`, before the guard check.
|
||||
|
||||
**A-IN-02.** In modules/cooler/cooler.go, move the original `keyPrefix` comment block back directly above `func keyPrefix`, so `go doc` shows each function's own comment.
|
||||
|
||||
**Docs, same commit as the code:**
|
||||
- modules/pact/README.md: the `pact.MCPPrompt` row names `Writes`.
|
||||
- modules/pier/README.md: the boot-error list gains required path fields and embedded fields on GET/DELETE; the dispatch section gains the path-value rule, Host and forwarding headers, and the fixed guard-failure text.
|
||||
- docs/services/mcp.md: the prompts paragraph explains `Writes`; the arguments section states the path rules.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>go vet ./... && go test -count=1 -short ./modules/pier/... ./modules/pact/... ./modules/cooler/... && go test -count=1 -short -tags summercms_mcp ./modules/pier/... ./modules/surf/... && go doc ./modules/cooler KeyPrefix | sed -n 4p | grep -q '^ KeyPrefix returns' && go doc -u ./modules/cooler keyPrefix | grep -A1 '^func keyPrefix' | grep -q 'keyPrefix returns' && bash scripts/check-phase19.7.sh --go && bash scripts/check-phase19.7.sh --docs && bash scripts/check-phase19.7.sh --marker</automated>
|
||||
<fails_when>non-zero exit, "FAIL" in the output, or a gate line printing FAIL, "refuse:" or "missing-named"</fails_when>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `grep -c "must be required" modules/pier/catalog.go` prints at least 1
|
||||
- `grep -c "UseNumber()" modules/pier/dispatch.go` prints at least 1 and `grep -c "X-Forwarded-Proto\|X-Forwarded-Host" modules/pier/dispatch.go` prints at least 2
|
||||
- `grep -c "the tool could not be run for this caller" modules/pier/dispatch.go` prints 1
|
||||
- `grep -c "Writes" modules/pact/mcp.go` prints at least 1 and `grep -v '^\s*//' modules/pier/filter.go | grep -c "decl.Writes"` prints 1
|
||||
- `go doc ./modules/cooler KeyPrefix | sed -n 4p` starts with ` KeyPrefix returns`, and `go doc -u ./modules/cooler keyPrefix` prints a `keyPrefix returns` comment directly under `func keyPrefix`
|
||||
- `grep -c "Writes" modules/pact/README.md docs/services/mcp.md` prints at least 1 per file
|
||||
- `git diff --quiet 2527498 -- go.mod go.sum admin/package-lock.json` (no dependency change since the gap-closure planning commit)
|
||||
</acceptance_criteria>
|
||||
<done>pier delivers only path values the mux could deliver, never leaks a provider or database error to an agent, keeps big integer ids exact, forwards Host and scheme, refuses embedded GET/DELETE fields and contradictory guard answers at the right time, and lets a plugin keep a write-directing prompt away from read-only grants; the gate's go, docs and marker stages are green.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<artifacts_this_phase_produces>
|
||||
## Artifacts this plan produces
|
||||
|
||||
**Framework code (summercms.go, local commits on master):**
|
||||
- `modules/pier`:
|
||||
- session reaper (`reap`, `reapOnce`), `expired` and `ephemeral` audit labels;
|
||||
- `reserver` with memory and Redis `Reserve`/`Release`;
|
||||
- lock-free connect and stateless build;
|
||||
- `Identity.fingerprint` and the widening diff;
|
||||
- Redis warm-up at mount;
|
||||
- stateless listen re-check and closing on LRU eviction;
|
||||
- refusal of a personal-token subject without a TokenID;
|
||||
- path-value and path-field rules, fixed guard-failure text, `UseNumber`, Host and forwarding headers, the embedded-field refusal, the guard-list check;
|
||||
- the prompt write ceiling.
|
||||
- `modules/pact`: `MCPPrompt.Writes`; the corrected `MCPPersonalTokenResolver` contract.
|
||||
- `modules/cooler`: corrected doc comments.
|
||||
|
||||
**Docs:** `modules/pier/README.md`, `modules/pact/README.md`, `docs/services/mcp.md`.
|
||||
|
||||
**Test:** `modules/pier/reaper_smoke_test.go` (`TestPierReaperSmoke`), plus the export_test seams `ReapNow` and `LocalSessions`.
|
||||
|
||||
**Decisions:** P-24 to P-28 (in the objective above).
|
||||
</artifacts_this_phase_produces>
|
||||
|
||||
<threat_model>
|
||||
## Trust Boundaries
|
||||
|
||||
| Boundary | Description |
|
||||
|----------|-------------|
|
||||
| MCP client to /mcp | any agent presents a bearer, a session id and JSON-RPC bodies (tool arguments become path, query and body of a plugin route) |
|
||||
| pier to plugin routes (in-process dispatch) | pier hands a compiled route a request it built, with path values set directly (no mux cleaning) |
|
||||
| instances to the shared session store | Redis holds session records, reservations and the fan-out channel |
|
||||
| guard providers to pier | providers answer dispatch principals and guard lists that pier turns into the marker |
|
||||
|
||||
## STRIDE Threat Register
|
||||
|
||||
| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan |
|
||||
|-----------|----------|-----------|----------|-------------|-----------------|
|
||||
| T-19.7-42 | Denial of service | abandoned stateful sessions (local SDK server, transport, reader goroutines) | high | mitigate | P-24 reaper closes every local copy whose record expired, at min(idle_ttl/2, 1m); `expired` audit line; TestPierReaperSmoke here, TestSessionReaper in plan 09 |
|
||||
| T-19.7-43 | Denial of service | concurrent initialize past mcp.sessions.max_per_binding | medium | mitigate | P-25 atomic `Reserve` (memory under lock, Redis Lua), `Release` on failure; plan 09 TestSessionReserve fires concurrent initializes |
|
||||
| T-19.7-44 | Tampering | path values the mux never delivers (`..`, `.`, empty, `/` in one segment, an omitted optional path field) | high | mitigate | P-27: refusal before dispatch; path fields must be required at boot; plan 09 TestDispatchHardening and TestCatalogPathFields |
|
||||
| T-19.7-45 | Information disclosure | raw provider or database errors (pgx address, database and user names) in tool results | medium | mitigate | fixed tool text, error logged server-side only |
|
||||
| T-19.7-46 | Spoofing | personal-token sessions sharing the binding `pat:user:0` (cap exhaustion, cross-user DELETE) | medium | mitigate | `resolvePersonalToken` refuses TokenID 0; contract text requires TokenID |
|
||||
| T-19.7-47 | Elevation of privilege | stateless listen streams and stateless-only Redis instances outliving a revocation | medium | mitigate | warm-up subscription at mount; periodic bearer re-check on subscriptions/listen; LRU eviction closes sessions |
|
||||
| T-19.7-48 | Denial of service | router-wide locks held across guard and database I/O | medium | mitigate | connect and server build outside `sr.mu` and `sp.mu`, insert with re-check; -race clean |
|
||||
| T-19.7-49 | Elevation of privilege | a provider naming the backend guard for a frontend tool's principal | low | mitigate | pier refuses a dispatch whose guard list or principal kind contradicts the tool's guard |
|
||||
| T-19.7-50 | Information disclosure | a write-directing prompt listed for a read-only grant | low | mitigate | `MCPPrompt.Writes` under the read/write ceiling (P-28) |
|
||||
| T-19.7-SC | Tampering | package installs | low | mitigate | no package is added: go.mod, go.sum and admin/package-lock.json unchanged (acceptance criterion of Task 4) |
|
||||
</threat_model>
|
||||
|
||||
<verification>
|
||||
- `go vet ./... && go test -count=1 -short ./...` in the framework, plus `go test -count=1 ./modules/pier/...` on a Docker host (no change expected; pier has no Postgres suite)
|
||||
- `go test -count=1 -short -race ./modules/pier/...` is clean
|
||||
- `bash scripts/check-phase19.7.sh --go`, `--docs` and `--marker` print PASS
|
||||
- `cd examples/hello && go test ./...` stays green (the separate workspace module)
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- A-CR-01 and A-WR-01..A-WR-09 are fixed, each in a commit whose subject starts with `fix(pier)` (or `fix(pact)` / `docs(cooler)` for the contract and doc-only parts); A-IN-01..A-IN-08 are fixed
|
||||
- `pact.MCPPrompt.Writes` exists with docs, ready for Journal in plan 08
|
||||
- No exported pier, pact or cooler identifier is removed or changes signature (the gate's docs and marker stages and `go vet` stay green; fonoteka.go and sm-bm-app still build against the local framework in plan 09's consumers stage)
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create `.planning/phases/19.7-generic-mcp-module/19.7-06-SUMMARY.md` when done, with every commit sha mapped to its finding ids. Commit the SUMMARY separately as a planning doc.
|
||||
|
||||
Release rule: this plan does not tag, push or deploy. Every repository works against the local framework (go.work, or `replace ../summercms.go`), and every plugin's go.mod `require` line stays at its current version. The plans may commit locally in every repository. The releases happen only after phase verification, in this order: framework v0.5.0, then the plugin tags that require it, then the application bumps.
|
||||
</output>
|
||||
522
.planning/phases/19.7-generic-mcp-module/19.7-07-PLAN.md
Normal file
522
.planning/phases/19.7-generic-mcp-module/19.7-07-PLAN.md
Normal file
@@ -0,0 +1,522 @@
|
||||
---
|
||||
phase: 19.7-generic-mcp-module
|
||||
plan: 07
|
||||
type: execute
|
||||
wave: 2
|
||||
depends_on: ["19.7-06"]
|
||||
gap_closure: true
|
||||
files_modified:
|
||||
- modules/bouncer/mint.go
|
||||
- modules/bouncer/refresh.go
|
||||
- modules/bouncer/jwt.go
|
||||
- modules/bouncer/auth_time_smoke_test.go
|
||||
- modules/bouncer/README.md
|
||||
- docs/services/authentication.md
|
||||
- modules/pact/capabilities.go
|
||||
- modules/pact/mcp.go
|
||||
- modules/pact/README.md
|
||||
- modules/cabana/token_scopes.go
|
||||
- docs/plugins/registration.md
|
||||
- docs/backend/users-and-permissions.md
|
||||
- .planning/notes/2026-10-23-frontend-token-scopes.md
|
||||
- modules/pier/store.go
|
||||
- modules/pier/events.go
|
||||
- modules/pier/session.go
|
||||
- modules/pier/stateless.go
|
||||
- modules/pier/pier.go
|
||||
- modules/pier/host.go
|
||||
- modules/pier/README.md
|
||||
- modules/surf/pier_on.go
|
||||
- modules/towel/context.go
|
||||
- modules/towel/README.md
|
||||
- modules/cabana/account_tokens.go
|
||||
- modules/cabana/subject_events.go
|
||||
- modules/cabana/token_resolver.go
|
||||
- modules/cabana/account_contributions.go
|
||||
- modules/cabana/actions.go
|
||||
- modules/cabana/admin_openapi.go
|
||||
- modules/cabana/README.md
|
||||
- modules/phrasebook/backend/lang/en/lang.yaml
|
||||
- modules/phrasebook/backend/lang/pl/lang.yaml
|
||||
- docs/services/mcp.md
|
||||
- admin/openapi/admin.json
|
||||
- admin/src/api/schema.d.ts
|
||||
- admin/src/app/hotkeys.ts
|
||||
- admin/src/components/account/AccountContributions.vue
|
||||
- admin/tests/fixtures/lang.json
|
||||
- modules/boardwalk/dist
|
||||
autonomous: true
|
||||
requirements: [SC-4, D-04, D-05, D-17, D-19, D-26, D-27, D-34, E-2]
|
||||
|
||||
estimate:
|
||||
tokens: 130000
|
||||
raw_tokens: 130000
|
||||
tasks: 5
|
||||
confidence: low
|
||||
|
||||
must_haves:
|
||||
truths:
|
||||
- "A frontend or backend token minted at sign-in carries an auth_time claim; POST /refresh (any audience) copies it unchanged into the rotated token, and a token that had none is refreshed without one; plugins read it through bouncer.VerifyAuthTime, which applies the same signature, audience and expiry checks as VerifyClaims (C-CR-02 framework part, D-05)"
|
||||
- "A plugin declares member (frontend) personal-token scopes through pact.HasFrontendTokenScopes, a separate capability that cabana's admin token catalog never reads; pact.ValidTokenScopeName is the one name rule both catalogs use, and cabana's g15_ catalog behaves exactly as before (E-2 framework seam, decision P-30)"
|
||||
- "Revoking one admin personal token ends only the MCP sessions opened with that token; an event naming a client ends only that client's OAuth sessions; an event naming neither ends every session of the subject as before (B-WR-04, D-26, D-27)"
|
||||
- "A failed g15_ token attempt at /mcp is logged with the client address, and a resolver plugin can read the address from the context (B-IN-04)"
|
||||
- "An account contribution action can only hand the admin an http(s) or same-origin relative URL; the server drops anything else and the SPA refuses to render or open it (B-WR-03)"
|
||||
- "One contributing plugin's Status or Applies error no longer removes the other My account sections: the failing section is sent with error true, and a failed GET shows a toast (B-WR-06)"
|
||||
- "A typed character or Esc inside a plugin field type's shadow DOM never triggers a plain-key shortcut, and Ctrl/Cmd+S or Ctrl/Cmd+Enter commits a pending taglist draft before saving (B-WR-01, B-WR-02)"
|
||||
- "The published admin API contract says a prompted record action accepts payload, and admin.json, schema.d.ts and the committed admin dist match their sources (B-WR-05)"
|
||||
artifacts:
|
||||
- path: "modules/bouncer/mint.go"
|
||||
provides: "auth_time stamped at sign-in"
|
||||
contains: "ClaimAuthTime"
|
||||
- path: "modules/bouncer/jwt.go"
|
||||
provides: "the plugin-facing accessor"
|
||||
contains: "func VerifyAuthTime("
|
||||
- path: "modules/pact/capabilities.go"
|
||||
provides: "the member token scope capability and the shared name rule"
|
||||
contains: "type HasFrontendTokenScopes interface"
|
||||
- path: ".planning/notes/2026-10-23-frontend-token-scopes.md"
|
||||
provides: "the recorded E-2 design decision"
|
||||
contains: "HasFrontendTokenScopes"
|
||||
- path: "modules/towel/context.go"
|
||||
provides: "client address on the context"
|
||||
contains: "func ClientIP("
|
||||
- path: "modules/cabana/account_contributions.go"
|
||||
provides: "URL scheme rule and per-section isolation"
|
||||
contains: "Error"
|
||||
key_links:
|
||||
- from: "modules/bouncer/refresh.go"
|
||||
to: "modules/bouncer/mint.go"
|
||||
via: "refreshAudience passes the presented token's auth_time to the internal mint"
|
||||
pattern: "auth_time"
|
||||
- from: "modules/cabana/account_tokens.go"
|
||||
to: "modules/pier/events.go"
|
||||
via: "the revoked event carries TokenID and pier closes only matching sessions"
|
||||
pattern: "TokenID"
|
||||
- from: "modules/pier/pier.go"
|
||||
to: "modules/cabana/token_resolver.go"
|
||||
via: "bearer puts towel.WithClientIP on the resolve context; ResolveBackendToken logs it"
|
||||
pattern: "ClientIP"
|
||||
---
|
||||
|
||||
<objective>
|
||||
Close lane B of the phase code review (B-WR-01..B-WR-06 and the cheap B-IN items) and add the two framework pieces the plugins need in plan 08:
|
||||
- the `auth_time` claim that makes the account link's fresh-sign-in proof real (C-CR-02, framework part);
|
||||
- the plugin-declared member token scope seam escalated by sm-servers-app (E-2, `19.7-GAP-INPUTS.md`).
|
||||
|
||||
**Serialization (planner decision, forced by file overlap):**
|
||||
- The user confirmed waves 06 = 1, 07 = 1, 08 = 2 and 09 = 3. This plan instead depends on 19.7-06, which makes the waves 1, 2, 3 and 4.
|
||||
- The overlap: both plans edit `modules/pier/{session,stateless,store,pier}.go`, `modules/pact/mcp.go` and `modules/pact/README.md`. The token-specific revoke event in Task 3 also builds on 06's session and stateless matching code.
|
||||
- Disjoint file sets would have meant moving B-WR-04 out of this plan, against the confirmed split.
|
||||
- The plan count (four) and the execution order 06 → 07 → 08 → 09 are unchanged. The orchestrator reports the change to the user.
|
||||
|
||||
**Planner decisions recorded here (P-29..P-33):**
|
||||
- **P-29 (auth_time, C-CR-02):** the claim is `auth_time` (OIDC Core's name, NumericDate seconds), exported as `bouncer.ClaimAuthTime`.
|
||||
- `MintAudience` (and so `Mint`) stamps it equal to `iat`, because every direct mint is a sign-in: password login, auto-activated registration, activation by code, social login, the cabana admin login and its re-mint after a password change.
|
||||
- `refreshAudience` copies the presented token's value unchanged. A token without one is refreshed without one; it is never stamped "now".
|
||||
- Plugins read it with `bouncer.VerifyAuthTime(tokenString, secret, audience string) (sub string, authTime time.Time, ok bool, err error)`, which runs exactly the checks `VerifyClaims` runs for `AudienceUser` and `VerifyClaimsAudience` runs for other audiences. `ok` is false when the claim is absent.
|
||||
- Settled by the review's suggested fix, so there is no decision checkpoint. Reversibility costly: once sessions carry the claim and plugins read it, renaming needs both names for a refresh window.
|
||||
- **P-30 (E-2 seam, the open design point in GAP-INPUTS):** **a separate capability, not a reuse of `pact.HasTokenScopes` and not an audience field.**
|
||||
- Shape: `pact.FrontendTokenScope{Name, Label, Description}` and `pact.HasFrontendTokenScopes{ FrontendTokenScopes() []FrontendTokenScope }`. This mirrors the existing `Permission`/`FrontendPermission` pair.
|
||||
- Reasons:
|
||||
1. The escalation asks for no change to the backend `g15_` catalog. Reusing the interface would show every member scope in the admin's token catalog, and every admin scope in the member list.
|
||||
2. `pact.TokenScope` carries `Permissions` (cabana permission patterns) and `Methods` (an HTTP method filter). Neither applies to a member token, whose route middleware `user.scope:<name>` decides. cabana's boot validation requires both fields, so a member scope could not even be declared through it.
|
||||
3. An audience field would leave two fields meaningless for one audience, and cabana would have to skip entries silently.
|
||||
- `pact.ValidTokenScopeName(name string) bool` carries cabana's existing pattern, so both catalogs validate names identically. cabana switches to it with no behaviour change.
|
||||
- The framework never reads `HasFrontendTokenScopes`. The user plugin compiles it in plan 08, as it does `HasFrontendPermissions`.
|
||||
- Written down as `.planning/notes/2026-10-23-frontend-token-scopes.md`. Reversibility costly: a published framework API.
|
||||
- **P-31 (token-specific revocation, B-WR-04 and the framework half of C-IN-02):** `pact.MCPSubjectChanged` gains `TokenID uint` and `ClientID string`, and `pier.Event` carries both as `omitempty` JSON. For `MCPSubjectRevoked`:
|
||||
- a non-zero `TokenID` ends only the sessions whose subject under `Guard` has that `TokenID` and no `ClientID` (personal-token sessions);
|
||||
- a non-empty `ClientID` ends only the sessions whose subject has that `ClientID`;
|
||||
- neither ends every session of the subject (today's behaviour);
|
||||
- other kinds ignore the fields.
|
||||
|
||||
cabana fires the admin token revoke with the token row id.
|
||||
- **P-32 (client address, B-IN-04 and the seam for C-IN-04):**
|
||||
- `towel.WithClientIP` and `towel.ClientIP` carry the client address on a context.
|
||||
- pier's `bearer` sets it before resolving, from the host's optional `ClientIP(*http.Request) string` (surf's adapter uses `surf.ClientIP` with `http.trusted_proxies`), falling back to the host part of `RemoteAddr`.
|
||||
- `cabana.ResolveBackendToken` logs it.
|
||||
- **P-33 (My account seam, B-WR-03 and B-WR-06):**
|
||||
- An action result URL is kept only when it is `https`/`http` absolute or same-origin relative (starts with one `/`, not `//`); anything else is dropped with an error log.
|
||||
- A section whose `Status` fails is sent with `Status` empty and `Error: true` (`json:"error,omitempty"`, additive). An action whose `Applies` fails is omitted with a log. The list never answers 500 for one plugin's failure.
|
||||
|
||||
Purpose: lane B is fixed before v0.5.0, and plan 08 has `bouncer.VerifyAuthTime`, `pact.HasFrontendTokenScopes`, the token-specific events and `towel.ClientIP` to build on.
|
||||
Output: framework commits in bouncer, pact, pier, surf, towel and cabana; the admin SPA fixes with a rebuilt committed dist; the regenerated OpenAPI; docs; one decision note; one tracer smoke test.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@~/.claude/msd-core/workflows/execute-plan.md
|
||||
@~/.claude/msd-core/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<context>
|
||||
@.planning/PROJECT.md
|
||||
@.planning/ROADMAP.md
|
||||
@.planning/STATE.md
|
||||
@.planning/phases/19.7-generic-mcp-module/19.7-GAP-INPUTS.md
|
||||
@.planning/phases/19.7-generic-mcp-module/19.7-REVIEW.md
|
||||
@.planning/phases/19.7-generic-mcp-module/19.7-CONTEXT.md
|
||||
@.planning/phases/19.7-generic-mcp-module/19.7-01-SUMMARY.md
|
||||
@.planning/phases/19.7-generic-mcp-module/19.7-06-SUMMARY.md
|
||||
@CLAUDE.md
|
||||
</context>
|
||||
|
||||
<project_rules>
|
||||
- Unit tests are plan 09's job. This plan carries one tracer smoke, `TestAuthTimeSmoke`, and keeps every existing test green. A test that pinned the old wrong behaviour (for example the per-admin session kill on token revoke) is changed in the fix commit and named in the SUMMARY.
|
||||
- Documentation rule: every exported API change updates the module README and the affected `docs/` page in the same commit; `summer docs:build --check` must stay green.
|
||||
- Admin rule (CLAUDE.md): swag annotations change → `scripts/check-admin-openapi.sh` regenerates `admin/openapi/admin.json` and `admin/src/api/schema.d.ts` in the same commit; `admin/` changes → `npm --prefix admin run build`, commit `modules/boardwalk/dist`, and `scripts/check-admin-dist.sh` passes. Run `npm --prefix admin ci` first if `admin/node_modules` is missing.
|
||||
- Response shapes stay as they are (core value): the only payload change is the additive `error` key on a My account section. Existing `/account*` and `/auth/me` payloads stay byte-identical (the gate checks `modules/cabana/account.go` against 51d088a; this plan must not touch that file).
|
||||
- Several sessions work in this repo, cabana especially: run `git status --short modules/cabana modules/pier modules/pact admin modules/boardwalk/dist` before every task and leave foreign edits alone. Stage by explicit path, one logical change per commit, no co-author tags; the decision note and the SUMMARY are planning docs in their own commits.
|
||||
- No new dependency (go.mod, go.sum, admin/package-lock.json unchanged).
|
||||
- Every commit is self-contained. At each commit:
|
||||
- the framework builds, `go vet ./...` is clean and the `-short` tests pass;
|
||||
- generated files (admin.json, schema.d.ts) match their annotations;
|
||||
- the committed admin dist matches the SPA source.
|
||||
|
||||
No commit relies on a later task's commit.
|
||||
- Release rule: no tag, push or deploy in this plan.
|
||||
</project_rules>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="tracer">
|
||||
<name>Task 1: Tracer: auth_time is minted at sign-in, survives refresh unchanged and is readable through bouncer.VerifyAuthTime</name>
|
||||
<reversibility rating="costly">A JWT claim that plugins read (the account-link proof in plan 08); renaming it later needs both names accepted for a whole refresh window.</reversibility>
|
||||
<files>modules/bouncer/mint.go, modules/bouncer/refresh.go, modules/bouncer/jwt.go, modules/bouncer/auth_time_smoke_test.go, modules/bouncer/README.md, docs/services/authentication.md</files>
|
||||
<read_first>
|
||||
- 19.7-REVIEW.md, section C-CR-02 (the failure scenario and the suggested fix)
|
||||
- modules/bouncer/mint.go (`registeredClaims`, `MintAudience`), refresh.go (`refreshAudience`, which calls `MintAudience` after its checks; `RefreshAudienceFor`), jwt.go (`VerifyClaims`, `VerifyClaimsAudience`, `verifyClaims`, `frontendAudienceOK`, `claimTime`)
|
||||
- modules/bouncer/mint_test.go and refresh_test.go (what the existing tests pin about the claim set)
|
||||
- modules/cabana/auth.go lines 200-270 and account.go around line 411 (the backend mints and refresh)
|
||||
- ../sm-demo-app/plugins/golem15/user/controllers/backend_link_controller.go (the consumer in plan 08: it re-reads the session token with `bouncer.VerifyClaims` today)
|
||||
- modules/bouncer/README.md (API reference table) and docs/services/authentication.md (token lifecycle section)
|
||||
</read_first>
|
||||
<action>
|
||||
Implement P-29 for C-CR-02.
|
||||
|
||||
In mint.go:
|
||||
- Add the exported constant `ClaimAuthTime = "auth_time"`.
|
||||
- Add `AuthTime *jwt.NumericDate \`json:"auth_time,omitempty"\`` to `registeredClaims`.
|
||||
- Move the body of `MintAudience` into an unexported `mintAudience(secret, sub, issuerURL string, ttl time.Duration, audience string, authTime time.Time)`. It sets `AuthTime` when `authTime` is non-zero.
|
||||
- `MintAudience` keeps its signature and calls `mintAudience` with `now`, so `auth_time` equals `iat` on every sign-in mint.
|
||||
|
||||
In refresh.go `refreshAudience`:
|
||||
- Read the presented token's `auth_time` with `claimTime(claims, ClaimAuthTime)`.
|
||||
- Mint the rotated token through `mintAudience` with that value. When the claim is absent, pass the zero time, so the rotated token carries none.
|
||||
- Every exported refresh function keeps its signature.
|
||||
|
||||
In jwt.go, add `func VerifyAuthTime(tokenString, secret, audience string) (sub string, authTime time.Time, ok bool, err error)`:
|
||||
- it runs exactly the verification `VerifyClaims` runs for `AudienceUser` (which accepts a frontend token without `aud`) and `VerifyClaimsAudience` runs for any other audience: signature, audience, `exp`, required claims;
|
||||
- it returns the subject and the `auth_time` claim;
|
||||
- `ok` is false when the claim is missing or not a number.
|
||||
|
||||
Update existing tests that compare the exact claim set of a minted token in the same commit.
|
||||
|
||||
New file `modules/bouncer/auth_time_smoke_test.go` holds `TestAuthTimeSmoke`:
|
||||
1. `Mint` a token, then `VerifyAuthTime` returns `ok` and a time equal to `iat` (seconds).
|
||||
2. Sleep past one second (or mint with a past clock through `mintAudience` from an in-package test), call `Refresh`, and `VerifyAuthTime` on the new token returns the original auth_time while its `iat` is later.
|
||||
3. A token minted by the test with `registeredClaims` and no `AuthTime` refreshes into a token where `ok == false`.
|
||||
4. A token with the wrong audience, a bad signature or past `exp` answers an error.
|
||||
|
||||
Docs in the same commit:
|
||||
- modules/bouncer/README.md: API rows for `ClaimAuthTime` and `VerifyAuthTime`, and a Features line saying refresh keeps the sign-in time.
|
||||
- docs/services/authentication.md: a short "Sign-in time (auth_time)" subsection explaining that a "fresh sign-in" check compares `auth_time`, never `iat`, because refresh renews `iat`.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>go vet ./modules/bouncer/... ./modules/cabana/... && go test -count=1 -short ./modules/bouncer/... ./modules/cabana/... && go test -count=1 -short -run '^TestAuthTimeSmoke$' -v ./modules/bouncer/ 2>&1 | grep -q -- '^--- PASS: TestAuthTimeSmoke' && bash scripts/check-phase19.7.sh --docs</automated>
|
||||
<fails_when>non-zero exit, "FAIL" in the output, the PASS line missing, or the docs stage printing FAIL or "refuse:"</fails_when>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `grep -c 'ClaimAuthTime = "auth_time"' modules/bouncer/mint.go` prints 1 and `grep -c "func mintAudience(" modules/bouncer/mint.go` prints 1
|
||||
- `grep -v '^\s*//' modules/bouncer/refresh.go | grep -c "mintAudience("` prints at least 1 and `grep -v '^\s*//' modules/bouncer/refresh.go | grep -c "ClaimAuthTime"` prints at least 1
|
||||
- `grep -c "func VerifyAuthTime(tokenString, secret, audience string) (sub string, authTime time.Time, ok bool, err error)" modules/bouncer/jwt.go` prints 1
|
||||
- `grep -c "VerifyAuthTime\|ClaimAuthTime" modules/bouncer/README.md` prints at least 2 and `grep -c "auth_time" docs/services/authentication.md` prints at least 1
|
||||
- `grep -c "func TestAuthTimeSmoke(" modules/bouncer/auth_time_smoke_test.go` prints 1
|
||||
</acceptance_criteria>
|
||||
<done>Every sign-in mint carries auth_time, refresh never renews it, a legacy token stays without it, and plugins have one documented accessor; the smoke proves mint → refresh → read end to end.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 2: The E-2 framework seam: pact.HasFrontendTokenScopes, the shared scope-name rule, and the recorded decision</name>
|
||||
<reversibility rating="costly">A published pact capability (decision P-30); plugins and the user plugin compile against it, so a later reshape needs a deprecation cycle.</reversibility>
|
||||
<files>modules/pact/capabilities.go, modules/cabana/token_scopes.go, modules/pact/README.md, docs/plugins/registration.md, docs/backend/users-and-permissions.md, .planning/notes/2026-10-23-frontend-token-scopes.md</files>
|
||||
<read_first>
|
||||
- .planning/phases/19.7-generic-mcp-module/19.7-GAP-INPUTS.md, section "E-2" (the ask, "Not asked for", acceptance and the open design point)
|
||||
- modules/pact/capabilities.go (`TokenScope`, `HasTokenScopes`; the doc at line ~367 on prompted record actions is Task 4's); modules/pact/mcp.go (`FrontendPermission`, `HasFrontendPermissions`, the style to mirror)
|
||||
- modules/cabana/token_scopes.go (`tokenScopeNameRE`, `validatePluginTokenScope`)
|
||||
- modules/pact/README.md (Features list and type table rows for TokenScope and FrontendPermission); docs/plugins/registration.md (capability table, lines ~80-95); docs/backend/users-and-permissions.md (the `pact.HasTokenScopes` section around line 92)
|
||||
- .planning/notes/ (an existing decision note's shape, for example 2026-10-22-mcp-go-sdk.md)
|
||||
</read_first>
|
||||
<action>
|
||||
Implement P-30.
|
||||
|
||||
In modules/pact/capabilities.go, next to `TokenScope`/`HasTokenScopes`:
|
||||
- `type FrontendTokenScope struct { Name, Label, Description string }`. Doc: one scope of the user plugin's member personal API tokens (`usr_`). Label and Description are phrase keys (containing "::") or literal text. The name is what a route's `user.scope:<name>` middleware checks.
|
||||
- `type HasFrontendTokenScopes interface { FrontendTokenScopes() []FrontendTokenScope }`. Doc: the frontend counterpart of `HasTokenScopes`. The framework's admin token catalog never reads it; the plugin that owns member tokens compiles it.
|
||||
- `func ValidTokenScopeName(name string) bool`, with the pattern `^[a-z][a-z0-9_-]*(:[a-z][a-z0-9_-]*)?$` compiled once in pact. Doc: the one name rule for both catalogs.
|
||||
|
||||
In modules/cabana/token_scopes.go, replace the body of the name check with `pact.ValidTokenScopeName`. Remove the local regexp only if nothing else uses it. Every error message stays word for word.
|
||||
|
||||
Docs in the same commit:
|
||||
- modules/pact/README.md: a Features line and two type rows plus the function row.
|
||||
- docs/plugins/registration.md: a capability table row.
|
||||
- docs/backend/users-and-permissions.md: one paragraph after the `pact.HasTokenScopes` section saying member token scopes are declared with `pact.HasFrontendTokenScopes`, appear only in the member token list, and never in the admin catalog.
|
||||
|
||||
Write `.planning/notes/2026-10-23-frontend-token-scopes.md` (title "Decision: member token scopes use a separate pact capability") with these sections:
|
||||
- Context: the E-2 escalation, with what was asked and what was not;
|
||||
- Options: reuse `HasTokenScopes`, an audience field, a separate interface;
|
||||
- Decision and reasons: the three reasons in P-30;
|
||||
- Consequences: the user plugin compiles and validates the scopes and fails boot on bad ones; the cabana catalog is unchanged; `pact.ValidTokenScopeName` is shared;
|
||||
- Related: GAP-INPUTS E-2, plan 08 Task 3.
|
||||
|
||||
Commit the note separately as a planning doc.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>go vet ./modules/pact/... ./modules/cabana/... && go test -count=1 -short ./modules/pact/... ./modules/cabana/... && go test -count=1 -short -run '^(TestPactStaysSDKFree|TestPhase197ContractsOnSamplePlugin)$' -v ./modules/pact/ 2>&1 | grep -c -- '^--- PASS: Test' | grep -qx 2 && bash scripts/check-phase19.7.sh --docs && bash scripts/check-phase19.7.sh --marker</automated>
|
||||
<fails_when>non-zero exit, "FAIL" in the output, fewer than 2 PASS lines, or a gate stage printing FAIL or "refuse:"</fails_when>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `grep -c "type FrontendTokenScope struct\|type HasFrontendTokenScopes interface\|func ValidTokenScopeName(" modules/pact/capabilities.go` prints 3
|
||||
- `grep -v '^\s*//' modules/cabana/token_scopes.go | grep -c "pact.ValidTokenScopeName("` prints at least 1
|
||||
- `grep -rn "HasFrontendTokenScopes" modules/cabana --include=*.go | grep -v _test | wc -l` prints 0 (cabana never reads the member capability)
|
||||
- `grep -c "HasFrontendTokenScopes" modules/pact/README.md docs/plugins/registration.md docs/backend/users-and-permissions.md` prints at least 1 per file
|
||||
- `test -f .planning/notes/2026-10-23-frontend-token-scopes.md && grep -c "HasTokenScopes\|audience" .planning/notes/2026-10-23-frontend-token-scopes.md` prints at least 2
|
||||
</acceptance_criteria>
|
||||
<done>The framework offers the member token scope capability and one shared name rule, cabana's admin catalog is untouched in behaviour, and the E-2 design decision is recorded as a note.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 3: Token- and client-specific revocation events, and the client address on pier's resolve path</name>
|
||||
<reversibility rating="costly">New fields on the published pact.MCPSubjectChanged event and on pier.Event's wire JSON between instances (additive and omitempty, so mixed-version instances ignore them).</reversibility>
|
||||
<files>modules/pact/mcp.go, modules/pier/store.go, modules/pier/events.go, modules/pier/session.go, modules/pier/stateless.go, modules/pier/pier.go, modules/pier/host.go, modules/surf/pier_on.go, modules/towel/context.go, modules/cabana/account_tokens.go, modules/cabana/subject_events.go, modules/cabana/token_resolver.go, modules/pact/README.md, modules/pier/README.md, modules/towel/README.md, modules/cabana/README.md, docs/services/mcp.md</files>
|
||||
<read_first>
|
||||
- 19.7-REVIEW.md, sections B-WR-04, B-IN-04 and C-IN-02
|
||||
- modules/pact/mcp.go: `MCPSubjectChanged`, `MCPSubject` (ClientID, TokenID)
|
||||
- modules/pier/events.go (`listenEvents`, `handleEvent`), store.go (`Event`), session.go (`matching`, and the matching method 06 left), stateless.go (`matching`, `evict`), identity.go (`Identity.has`, `subjectFor`; which subjects carry ClientID and TokenID for OAuth, personal frontend and admin `g15_` tokens)
|
||||
- modules/pier/pier.go `bearer` (where `identities.resolve` is called); host.go (`Host`); modules/surf/pier_on.go (`pierHost`, `mountPier`); modules/surf/clientip.go (`ClientIP`, `TrustedProxies`)
|
||||
- modules/towel/context.go (`withValue`, `stringValue`, `WithActor`/`Actor` as the model)
|
||||
- modules/cabana/account_tokens.go around line 155-170 (the revoke handler) and subject_events.go (`fireSubjectChanged`); token_resolver.go (`ResolveBackendToken`, its `logTokenAuthAt` calls) and token_guard.go (`logTokenAuthAt`)
|
||||
</read_first>
|
||||
<action>
|
||||
**P-31.**
|
||||
- In pact/mcp.go, add `TokenID uint` and `ClientID string` to `MCPSubjectChanged`. Rewrite its doc to state the narrowing rule for `MCPSubjectRevoked`; other kinds ignore both fields.
|
||||
- In pier/store.go, add `TokenID uint \`json:"token_id,omitempty"\`` and `ClientID string \`json:"client_id,omitempty"\`` to `Event`.
|
||||
- `listenEvents` copies both.
|
||||
- In `handleEvent`'s revoked/deleted branch, select sessions with a predicate:
|
||||
- with `TokenID != 0`: sessions whose subject under `ev.Guard` has that `TokenID` and an empty `ClientID`;
|
||||
- with `ClientID != ""`: sessions whose subject under `ev.Guard` has that `ClientID`;
|
||||
- otherwise: today's `has(guard, userID)`.
|
||||
- Deleted events always use the whole subject.
|
||||
- Add the predicate form to `sessionRouter.matching` and to `statelessPath.matching`/`evict`, keeping the existing signatures as wrappers.
|
||||
- In cabana/subject_events.go, add an unexported `fireTokenRevoked(ctx, app, adminID, tokenID uint)` that fires `&pact.MCPSubjectChanged{Guard: "backend", UserID: adminID, Kind: pact.MCPSubjectRevoked, TokenID: tokenID}` like `fireSubjectChanged`.
|
||||
- In account_tokens.go, the revoke handler calls it with `row.ID` instead of the per-admin event. Fix the misleading comment above the call so it says which sessions end.
|
||||
|
||||
**P-32.**
|
||||
- In towel/context.go, add `WithClientIP(ctx context.Context, ip string) context.Context` and `ClientIP(ctx context.Context) (string, bool)` on an unexported key, following `WithActor`/`Actor`.
|
||||
- In pier, declare an unexported interface in host.go, `clientIPHost interface{ ClientIP(*http.Request) string }`. Document it as optional for hosts.
|
||||
- In surf/pier_on.go, `pierHost` gains a `trusted []netip.Prefix` field, filled in `mountPier` from `TrustedProxies(app.Config)`, and a `ClientIP` method calling `ClientIP(req, h.trusted)`.
|
||||
- In pier.go `bearer`, before `p.identities.resolve`, wrap the context with `towel.WithClientIP`. Use the host's answer when it implements the interface; otherwise use the host part of `r.RemoteAddr` (`net.SplitHostPort`, or the raw value on error).
|
||||
- In cabana/token_resolver.go, pass `towel.ClientIP(ctx)` as the `remote` of every `logTokenAuthAt` call in `ResolveBackendToken`.
|
||||
|
||||
Docs in the same commit:
|
||||
- pact README: the `MCPSubjectChanged` row.
|
||||
- pier README: the fan-out section's narrowing rule.
|
||||
- towel README: the API rows.
|
||||
- cabana README: the token revoke event and resolver logging.
|
||||
- docs/services/mcp.md: the fan-out paragraph.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>go vet ./... && go vet -tags summercms_mcp ./modules/surf/... ./modules/pier/... && go test -count=1 -short ./modules/pier/... ./modules/pact/... ./modules/towel/... ./modules/cabana/... && go test -count=1 -short -tags summercms_mcp ./modules/pier/... ./modules/surf/... && go test -count=1 -short -run '^(TestFanOut|TestPierFanOutSmoke|TestFireSubjectChangedReachesPointerListener)$' -v ./modules/pier/ ./modules/cabana/ 2>&1 | grep -c -- '^--- PASS: Test' | grep -qx 3 && bash scripts/check-phase19.7.sh --docs && bash scripts/check-phase19.7.sh --marker</automated>
|
||||
<fails_when>non-zero exit, "FAIL" in the output, fewer than 3 PASS lines, or a gate stage printing FAIL or "refuse:"</fails_when>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `grep -A6 "type MCPSubjectChanged struct" modules/pact/mcp.go | grep -c "TokenID\|ClientID"` prints 2
|
||||
- `grep -c 'json:"token_id,omitempty"\|json:"client_id,omitempty"' modules/pier/store.go` prints 2
|
||||
- `grep -v '^\s*//' modules/pier/events.go | grep -c "TokenID\|ClientID"` prints at least 2
|
||||
- `grep -v '^\s*//' modules/cabana/account_tokens.go | grep -c "fireTokenRevoked("` prints 1
|
||||
- `grep -c "func WithClientIP(\|func ClientIP(" modules/towel/context.go` prints 2 and `grep -v '^\s*//' modules/cabana/token_resolver.go | grep -c "towel.ClientIP("` prints at least 1
|
||||
- `grep -v '^\s*//' modules/pier/pier.go | grep -c "towel.WithClientIP("` prints 1 and `grep -c "func (h pierHost) ClientIP(" modules/surf/pier_on.go` prints 1
|
||||
</acceptance_criteria>
|
||||
<done>Revoking one admin token ends only what that token opened; the event shape can also target one OAuth client (used by the plugins in plan 08); failed g15_ attempts at /mcp carry the client address and resolvers can read it.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 4: cabana admin API: account action URL rule, per-section isolation, localized texts, the prompted record action contract, regenerated OpenAPI</name>
|
||||
<files>modules/cabana/account_contributions.go, modules/cabana/actions.go, modules/cabana/admin_openapi.go, modules/pact/capabilities.go, modules/cabana/README.md, modules/phrasebook/backend/lang/en/lang.yaml, modules/phrasebook/backend/lang/pl/lang.yaml, admin/tests/fixtures/lang.json, admin/openapi/admin.json, admin/src/api/schema.d.ts</files>
|
||||
<read_first>
|
||||
- 19.7-REVIEW.md, sections B-WR-03 (server part), B-WR-05, B-WR-06 (server part), B-IN-02, B-IN-03, B-IN-05
|
||||
- modules/cabana/account_contributions.go (`accountContributionsList`, `AccountContributionView`, the action handler around line 200 that writes `AccountActionResultView{URL: result.URL}`); modules/cabana/crud.go `localizeDetail`
|
||||
- modules/cabana/actions.go lines 240-280 (`promptRequiredKey` and the three prompt refusals)
|
||||
- modules/cabana/admin_openapi.go lines 790-830 (`AdminRecordAction` description, the body param text, `AdminActionRequest.Payload`) and the My account contributions annotations (the view type's doc)
|
||||
- modules/pact/capabilities.go around line 367 (the prompted record action doc that promises a validation error on the input)
|
||||
- modules/phrasebook/backend/lang/en/lang.yaml `form:` and `myaccount:` sections, the pl twin, admin/tests/fixtures/lang.json
|
||||
- scripts/check-admin-openapi.sh (how to regenerate vs `--check`)
|
||||
</read_first>
|
||||
<action>
|
||||
**B-WR-03, server (P-33).** Before `WriteData` in the action handler, keep `result.URL` only when one of these holds:
|
||||
- `url.Parse` succeeds with scheme `https` or `http` and a host;
|
||||
- or it is a same-origin relative reference (starts with exactly one `/`, not `//`, and has no scheme).
|
||||
|
||||
Otherwise set it to "" and log `cabana: account contribution action answered a non-http URL` with section and action, never the URL itself.
|
||||
|
||||
**B-WR-06, server (P-33).**
|
||||
- Add `Error bool \`json:"error,omitempty"\`` to `AccountContributionView`, documented as "the section's status could not be loaded".
|
||||
- When `Status` errors, log as today, leave `Status` empty, set `Error`, and continue.
|
||||
- When an action's `Applies` errors, log and skip that action.
|
||||
- The handler never answers 500 for a plugin's error. A database or registry failure outside the plugin hooks still does.
|
||||
|
||||
**B-IN-03.** Translate a section's status with `localizeDetail(ctx, tr, status)` instead of `translateKey`.
|
||||
|
||||
**B-IN-02.** In actions.go, replace the hard-coded English refusal for a non-string payload with the phrase key `backend::lang.form.action_prompt_invalid` (en "The answer must be text." / pl "Odpowiedź musi być tekstem."), added to both lang files and to admin/tests/fixtures/lang.json.
|
||||
|
||||
**B-IN-05.** Rewrite the sentence in pact/capabilities.go that promises a validation error on the input. It should say what happens: the admin refuses the value with a 422 whose message is shown as a toast, and the prompt dialog closes. The dialog-stays-open UX is deferred (recorded by plan 09 in deferred-items).
|
||||
|
||||
**B-WR-05, in admin_openapi.go.** Replace the stale sentence in the `AdminRecordAction` description with: the body is `{}` for an action without a prompt; an action whose record response carries `prompt` takes `{"payload": "<text>"}`, trimmed, non-empty and at most `prompt.max_length` runes, with a 422 on `payload` otherwise. Make these match:
|
||||
- the body param text: "Empty object, or {"payload": text} for a prompted action";
|
||||
- the `AdminActionRequest.Payload` comment: toolbar routes refuse it; record routes accept a JSON string only for a prompted action.
|
||||
|
||||
Document the new `error` key wherever the contribution view type is annotated.
|
||||
|
||||
Run `scripts/check-admin-openapi.sh` to regenerate admin/openapi/admin.json and admin/src/api/schema.d.ts, then `scripts/check-admin-openapi.sh --check`. The cabana README's My account section gains the URL rule and the `error` flag. Commit the code, the generated files and the docs together.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>go vet ./modules/cabana/... ./modules/pact/... && go test -count=1 -short ./modules/cabana/... ./modules/pact/... && go test -count=1 -short -run '^(TestAccountContributions|TestAccountContributionsSmoke|TestRecordActionPrompt|TestRecordActionPromptSmoke)$' -v ./modules/cabana/ 2>&1 | grep -c -- '^--- PASS: Test' | grep -qx 4 && bash scripts/check-admin-openapi.sh --check && bash scripts/check-phase19.7.sh --docs</automated>
|
||||
<fails_when>non-zero exit, "FAIL" in the output, fewer than 4 PASS lines, an OpenAPI drift report, or the docs stage printing FAIL or "refuse:"</fails_when>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `grep -c 'json:"error,omitempty"' modules/cabana/account_contributions.go` prints 1 and `grep -c "non-http URL" modules/cabana/account_contributions.go` prints 1
|
||||
- `grep -v '^\s*//' modules/cabana/account_contributions.go | grep -c "localizeDetail("` prints at least 1
|
||||
- `grep -c "action_prompt_invalid" modules/cabana/actions.go modules/phrasebook/backend/lang/en/lang.yaml modules/phrasebook/backend/lang/pl/lang.yaml admin/tests/fixtures/lang.json` prints at least 1 per file
|
||||
- `grep -c "payload are all refused" modules/cabana/admin_openapi.go admin/openapi/admin.json` prints 0 for both files and `grep -c "prompt.max_length" admin/openapi/admin.json` prints at least 1
|
||||
- `bash scripts/check-admin-openapi.sh --check` exits 0
|
||||
- `git diff --quiet 51d088a -- modules/cabana/account.go` (My account's own handlers untouched)
|
||||
</acceptance_criteria>
|
||||
<done>The admin API only hands out safe URLs, one broken contributing plugin cannot blank My account, the remaining English text is localized, the prompted-action contract is published correctly and the generated API files match.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 5: Admin SPA: shadow-DOM-aware plain keys, commit-before-save shortcuts, safe account links, the section error state and My account texts; rebuilt dist</name>
|
||||
<files>admin/src/app/hotkeys.ts, admin/src/components/account/AccountContributions.vue, modules/phrasebook/backend/lang/en/lang.yaml, modules/phrasebook/backend/lang/pl/lang.yaml, admin/tests/fixtures/lang.json, modules/boardwalk/dist</files>
|
||||
<read_first>
|
||||
- 19.7-REVIEW.md, sections B-WR-01, B-WR-02, B-WR-03 (SPA part), B-WR-06 (SPA part), B-IN-01, B-IN-06
|
||||
- admin/src/app/hotkeys.ts (`isEditableTarget` at ~100-105, `onModifierKey` at ~187-206 with `target.click()`, `onPlainKey` at ~208-236)
|
||||
- admin/src/components/form/fields/TaglistField.vue (`addDraft` on blur and Enter, lines ~73 and ~167)
|
||||
- admin/src/components/account/AccountContributions.vue (`load`, the confirm texts at ~52 and ~78, `links` and `window.open` at ~43-46 and ~103-111)
|
||||
- admin/tests/app/hotkeys.test.ts and admin/tests/account/AccountContributions.test.ts (existing expectations to keep green)
|
||||
- modules/phrasebook/backend/lang/en/lang.yaml `myaccount:` and `form:` sections
|
||||
- scripts/check-admin-dist.sh
|
||||
</read_first>
|
||||
<action>
|
||||
**B-WR-01.**
|
||||
- In hotkeys.ts, add `originOf(event)`: the first entry of `event.composedPath()` when available, else `event.target`.
|
||||
- `onPlainKey` tests `isEditableTarget(originOf(event))`.
|
||||
- `isEditableTarget` also treats any element whose tag name contains `-` (a custom element) and that sits inside a `form` or `[role=form]` as editable. That covers closed shadow roots, where `composedPath()` stops at the host.
|
||||
|
||||
**B-WR-02.** In `onModifierKey`, before `target.click()`:
|
||||
- when `document.activeElement` is an element other than the target and has `blur`, call `blur()`, so TaglistField's `addDraft` and native `change` run as a pointer click would trigger them;
|
||||
- restore focus to the element after the click only when it is still in the document and the view did not navigate (keep it simple: refocus when `isConnected`).
|
||||
|
||||
**B-WR-03, SPA.** In AccountContributions.vue, add `safeUrl(u: string): string`, returning the value only for `/^https?:\/\//i` or a single-slash relative path, else ''. Use it for both the rendered `href` and `window.open`. An unsafe URL is never bound and never opened. B-IN-06 stays as is: the visible link remains the fallback for a blocked popup, with no code change.
|
||||
|
||||
**B-WR-06, SPA.**
|
||||
- On a non-2xx GET of the contributions, show a toast with `backend::lang.form.error_generic` and keep the previous sections.
|
||||
- A section with `error: true` renders a muted line `backend::lang.myaccount.section_error` (en "This section could not be loaded." / pl "Nie udało się wczytać tej sekcji.").
|
||||
|
||||
**B-IN-01.** Use the new keys `backend::lang.myaccount.action_confirm` (en "Run “:action”?" / pl "Wykonać „:action”?") and `backend::lang.myaccount.action_stale` (en "This action no longer applies. The page has been refreshed." / pl "Ta akcja nie ma już zastosowania. Strona została odświeżona.") in place of the record-oriented form texts.
|
||||
|
||||
Add every new key to the en and pl lang files and to admin/tests/fixtures/lang.json. Adjust existing vitest expectations that asserted the record-oriented texts in the same commit (plan 09 adds the new tests).
|
||||
|
||||
Run `npm --prefix admin run typecheck`, `npm --prefix admin run test`, `npm --prefix admin run build`, then `scripts/check-admin-dist.sh`. Commit the SPA source, the lang changes and the rebuilt `modules/boardwalk/dist` in one commit, so `check-admin-dist.sh` passes at that commit (self-contained rule).
|
||||
</action>
|
||||
<verify>
|
||||
<automated>npm --prefix admin run typecheck && npm --prefix admin run test 2>&1 | grep -E 'Test Files +[0-9]+ passed' | grep -qv failed && bash scripts/check-admin-dist.sh && bash scripts/check-phase19.7.sh --admin</automated>
|
||||
<fails_when>non-zero exit, a failed or skipped vitest file, a dist drift report from check-admin-dist.sh, or the admin stage printing FAIL, "refuse:" or "missing-named"</fails_when>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `grep -c "composedPath" admin/src/app/hotkeys.ts` prints at least 1 and `grep -v '^\s*//' admin/src/app/hotkeys.ts | grep -c "\.blur()"` prints at least 1
|
||||
- `grep -c "safeUrl" admin/src/components/account/AccountContributions.vue` prints at least 3 (definition, href binding, window.open)
|
||||
- `grep -c "myaccount.action_confirm\|myaccount.action_stale\|myaccount.section_error" admin/src/components/account/AccountContributions.vue` prints at least 3
|
||||
- `grep -c "section_error\|action_confirm\|action_stale" modules/phrasebook/backend/lang/pl/lang.yaml` prints at least 3 and the same keys are in admin/tests/fixtures/lang.json
|
||||
- `bash scripts/check-admin-dist.sh` exits 0 at the task's commit (source and dist in the same commit)
|
||||
</acceptance_criteria>
|
||||
<done>Typing inside plugin field types is safe from plain shortcuts, keyboard saves include pending drafts, My account never renders or opens a script URL, a broken section shows an error instead of vanishing, the texts fit My account, and the embedded SPA matches its source.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<artifacts_this_phase_produces>
|
||||
## Artifacts this plan produces
|
||||
|
||||
**Framework code (summercms.go, local commits on master):**
|
||||
- `bouncer`: `ClaimAuthTime`, `VerifyAuthTime`, the auth_time mint and refresh carry-over.
|
||||
- `pact`:
|
||||
- `FrontendTokenScope`, `HasFrontendTokenScopes`, `ValidTokenScopeName`;
|
||||
- `MCPSubjectChanged.TokenID` and `ClientID`;
|
||||
- the corrected prompted-action doc.
|
||||
- `pier`: event narrowing, `Event.TokenID`/`ClientID`, the client address on the resolve context, and the optional host capability.
|
||||
- `surf`: `pierHost.ClientIP`.
|
||||
- `towel`: `WithClientIP` and `ClientIP`.
|
||||
- `cabana`:
|
||||
- the token-specific revoke event;
|
||||
- the resolver's remote address;
|
||||
- the account action URL rule, the section `error` flag and status localization;
|
||||
- the localized payload refusal;
|
||||
- the corrected record action annotations.
|
||||
|
||||
**Admin SPA:**
|
||||
- `hotkeys.ts` (composedPath, blur before click);
|
||||
- `AccountContributions.vue` (`safeUrl`, the error state, the My account texts);
|
||||
- the rebuilt `modules/boardwalk/dist`;
|
||||
- the regenerated `admin/openapi/admin.json` and `admin/src/api/schema.d.ts`.
|
||||
|
||||
**Docs:**
|
||||
- READMEs: bouncer, pact, pier, towel, cabana;
|
||||
- `docs/services/authentication.md`, `docs/services/mcp.md`, `docs/plugins/registration.md`, `docs/backend/users-and-permissions.md`.
|
||||
|
||||
**Decision note:** `.planning/notes/2026-10-23-frontend-token-scopes.md` (E-2, P-30).
|
||||
|
||||
**Test:** `modules/bouncer/auth_time_smoke_test.go` (`TestAuthTimeSmoke`).
|
||||
</artifacts_this_phase_produces>
|
||||
|
||||
<threat_model>
|
||||
## Trust Boundaries
|
||||
|
||||
| Boundary | Description |
|
||||
|----------|-------------|
|
||||
| browser session to the token refresh endpoints | any holder of a token inside its refresh window can rotate it without a password |
|
||||
| plugins to the admin token catalog and the member token catalog | plugins declare scope names that end up on stored tokens |
|
||||
| plugin account actions to the admin SPA | a plugin answers a URL the admin's browser renders and opens |
|
||||
| admin and plugins to pier's fan-out | revocation events decide which live MCP sessions end |
|
||||
| MCP clients to the token resolvers | failed attempts must be traceable to an address |
|
||||
|
||||
## STRIDE Threat Register
|
||||
|
||||
| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan |
|
||||
|-----------|----------|-----------|----------|-------------|-----------------|
|
||||
| T-19.7-51 | Spoofing | the fresh-sign-in proof (D-05) | high | mitigate | auth_time stamped only at sign-in, copied unchanged by refresh, absent stays absent; `VerifyAuthTime` checks signature, audience and expiry; plan 08 makes the link confirm compare auth_time; TestAuthTimeSmoke here, TestAuthTime and TestBackendLinkAuthTime in plan 09 |
|
||||
| T-19.7-52 | Elevation of privilege | member scopes crossing into the admin token catalog, or the reverse | medium | mitigate | separate capability that cabana never reads (acceptance grep); one shared name rule `pact.ValidTokenScopeName` |
|
||||
| T-19.7-53 | Tampering | a `javascript:`/`data:` URL from an account action rendered into href or window.open | medium | mitigate | server keeps only http(s) and same-origin relative URLs; SPA `safeUrl` on both sinks |
|
||||
| T-19.7-54 | Denial of service | revoking one token ends every MCP session of the admin, including OAuth sessions of the linked user | low | mitigate | token-specific revoked event; client-specific variant for plan 08 |
|
||||
| T-19.7-55 | Repudiation | failed g15_ attempts at /mcp logged without a client address | low | mitigate | towel client address on the resolve context, logged by ResolveBackendToken |
|
||||
| T-19.7-56 | Information disclosure | one plugin's error silently hides other plugins' My account sections (including the Unlink action) | low | mitigate | per-section isolation with the `error` flag and a toast on a failed GET |
|
||||
| T-19.7-SC | Tampering | package installs | low | mitigate | no package added; `npm --prefix admin ci` installs the committed lock only; go.mod, go.sum and admin/package-lock.json unchanged |
|
||||
</threat_model>
|
||||
|
||||
<verification>
|
||||
- `go vet ./... && go test -count=1 -short ./...` and `go test -count=1 ./modules/cabana/...` on a Docker host (the cabana Postgres suites, through `sudo -u golem` if the socket is denied)
|
||||
- `bash scripts/check-phase19.7.sh --go`, `--admin`, `--docs` and `--marker` print PASS
|
||||
- `cd examples/hello && go test ./...` stays green
|
||||
- `git diff --quiet 51d088a -- modules/cabana/account.go`
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- B-WR-01..B-WR-06 fixed; B-IN-01, B-IN-02, B-IN-03 and B-IN-04 fixed; B-IN-05's contract text corrected (the UX deferral is recorded by plan 09); B-IN-06 needs no change
|
||||
- `bouncer.VerifyAuthTime`, `pact.HasFrontendTokenScopes`, `pact.ValidTokenScopeName`, `pact.MCPSubjectChanged.TokenID/ClientID` and `towel.ClientIP` exist with docs, ready for plan 08
|
||||
- The E-2 design decision is recorded (P-30 and the note)
|
||||
- No exported identifier removed; every response shape unchanged except the additive `error` key
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create `.planning/phases/19.7-generic-mcp-module/19.7-07-SUMMARY.md` when done, with every commit sha mapped to its finding ids. Commit the SUMMARY separately as a planning doc.
|
||||
|
||||
Release rule: this plan does not tag, push or deploy. Every repository works against the local framework (go.work, or `replace ../summercms.go`), and every plugin's go.mod `require` line stays at its current version. The plans may commit locally in every repository. The releases happen only after phase verification, in this order: framework v0.5.0, then the plugin tags that require it, then the application bumps.
|
||||
</output>
|
||||
601
.planning/phases/19.7-generic-mcp-module/19.7-08-PLAN.md
Normal file
601
.planning/phases/19.7-generic-mcp-module/19.7-08-PLAN.md
Normal file
@@ -0,0 +1,601 @@
|
||||
---
|
||||
phase: 19.7-generic-mcp-module
|
||||
plan: 08
|
||||
type: execute
|
||||
wave: 3
|
||||
depends_on: ["19.7-06", "19.7-07"]
|
||||
gap_closure: true
|
||||
files_modified:
|
||||
- ../sm-demo-app/plugins/golem15/user/controllers/backend_link_controller.go
|
||||
- ../sm-demo-app/plugins/golem15/user/classes/backend_links.go
|
||||
- ../sm-demo-app/plugins/golem15/user/account_contributions.go
|
||||
- ../sm-demo-app/plugins/golem15/user/routes.go
|
||||
- ../sm-demo-app/plugins/golem15/user/config/config.yaml
|
||||
- ../sm-demo-app/plugins/golem15/user/backend_link_auth_time_smoke_test.go
|
||||
- ../sm-demo-app/plugins/golem15/user/classes/events.go
|
||||
- ../sm-demo-app/plugins/golem15/user/classes/access_revoked.go
|
||||
- ../sm-demo-app/plugins/golem15/user/classes/subject_events.go
|
||||
- ../sm-demo-app/plugins/golem15/user/classes/mcp_guard.go
|
||||
- ../sm-demo-app/plugins/golem15/user/controllers/api_controller.go
|
||||
- ../sm-demo-app/plugins/golem15/user/controllers/users_admin_controller.go
|
||||
- ../sm-demo-app/plugins/golem15/user/controllers/api_tokens.go
|
||||
- ../sm-demo-app/plugins/golem15/user/classes/api_token_scopes.go
|
||||
- ../sm-demo-app/plugins/golem15/user/classes/api_tokens.go
|
||||
- ../sm-demo-app/plugins/golem15/user/console/api_tokens.go
|
||||
- ../sm-demo-app/plugins/golem15/user/plugin.go
|
||||
- ../sm-demo-app/plugins/golem15/user/member_token_scopes_smoke_test.go
|
||||
- ../sm-demo-app/plugins/golem15/user/models/organisation.go
|
||||
- ../sm-demo-app/plugins/golem15/user/models/organisation/fields.yaml
|
||||
- ../sm-demo-app/plugins/golem15/user/classes/organisation_logo.go
|
||||
- ../sm-demo-app/plugins/golem15/user/lang/en/lang.yaml
|
||||
- ../sm-demo-app/plugins/golem15/user/lang/pl/lang.yaml
|
||||
- ../sm-demo-app/plugins/golem15/user/README.md
|
||||
- ../sm-oauthserver-plugin/classes/mcp_verifier.go
|
||||
- ../sm-oauthserver-plugin/classes/mcp_store.go
|
||||
- ../sm-oauthserver-plugin/classes/cascade.go
|
||||
- ../sm-oauthserver-plugin/classes/subject_events.go
|
||||
- ../sm-oauthserver-plugin/controllers/authorizations.go
|
||||
- ../sm-oauthserver-plugin/plugin.go
|
||||
- ../sm-oauthserver-plugin/mcp_revocation_smoke_test.go
|
||||
- ../sm-oauthserver-plugin/README.md
|
||||
- ../sm-journal-plugin/mcp.go
|
||||
- ../sm-journal-plugin/README.md
|
||||
- ../sm-demo-app/vue-demo-app/app/pages/link-admin.vue
|
||||
- ../sm-demo-app/vue-demo-app/app/lib/linkCode.ts
|
||||
- ../sm-demo-app/vue-demo-app/i18n/locales/en.json
|
||||
- ../sm-demo-app/vue-demo-app/i18n/locales/pl.json
|
||||
- ../sm-demo-app/vue-demo-app
|
||||
- ../sm-demo-app/deploy/nginx/demo.summercms.io.conf
|
||||
- ../sm-demo-app/scripts/check-deploy.sh
|
||||
- ../sm-demo-app/plugins/golem15/oauthserver
|
||||
- ../sm-demo-app/plugins/golem15/journal
|
||||
autonomous: true
|
||||
requirements: [SC-4, D-03, D-05, D-07, D-08, D-09, D-19, D-20, D-21, D-26, D-27, D-28, E-1, E-2]
|
||||
|
||||
estimate:
|
||||
tokens: 140000
|
||||
raw_tokens: 140000
|
||||
tasks: 4
|
||||
confidence: low
|
||||
|
||||
must_haves:
|
||||
truths:
|
||||
- "The account link is confirmed only by a session whose auth_time is at or after the link request: a token refreshed after the request is refused with 409 stale_session, a token without auth_time is refused the same way, and a fresh sign-in after the request links (C-CR-02 plugin side, D-05)"
|
||||
- "The link code never reaches a server log: My account answers <frontend_url><page_path>#code=<code>, the site page reads the fragment, keeps the code in sessionStorage across the sign-in, shows the request through POST /_user/api/v1/backend-link/show with the code in the body, and shows which frontend account is signed in before the confirm button (C-WR-01, C-IN-06, D-05)"
|
||||
- "Banning a user, resetting their password, an admin setting their password, or their own password change revokes their MCP access and refresh tokens created before the cut-off and ends their MCP sessions; the MCP verifier and the MCP refresh rotation refuse a banned user and any token created before tokens_valid_after plus one second (the stored cutoff is whole seconds, so a token from earlier in the cutoff's second is refused too), and the revoke uses the untruncated revocation instant; a banned user's link answers no backend user (C-CR-01, D-08, D-27)"
|
||||
- "Creating or removing a frontend user's backend link revokes that user's MCP grants that carry the backend scope, so a backend grant never runs with the reach of an administrator the user did not consent to (C-WR-02, D-21)"
|
||||
- "A plugin declares member token scopes with pact.HasFrontendTokenScopes; POST /_user/api/v1/tokens and MintAPITokenFor accept them for the application that declares them (a second application in the same process refuses them), the legacy MintAPIToken accepts them through a process-wide fallback, middleware.Scope and APIToken.HasScope check them unchanged, GET /tokens lists them, GET /_user/api/v1/tokens/scopes lists the built-ins and the declared scopes with translated labels, and a duplicate, a built-in redeclared or a bad name fails boot (E-2)"
|
||||
- "Organisations have a logo image (attachOne `logo`) editable in the backend Organisations form, and applications get the logo for their organisation payloads through exported helpers using the same keys the user payload uses for avatars (E-1)"
|
||||
- "Revoking one personal token or one OAuth authorization ends only the MCP sessions it opened; a personal token used at /mcp records last_used_ip; Journal's draft_post prompt is hidden from read-only grants; nginx accepts MCP bodies up to pier's 4 MiB cap (C-IN-01..C-IN-05)"
|
||||
- "Every plugin change is additive: no exported identifier, route, column, payload field or migration removed or changed; plugin go.mod require lines are unchanged"
|
||||
artifacts:
|
||||
- path: "../sm-demo-app/plugins/golem15/user/controllers/backend_link_controller.go"
|
||||
provides: "auth_time-based confirm and the POST show route"
|
||||
contains: "VerifyAuthTime"
|
||||
- path: "../sm-demo-app/plugins/golem15/user/classes/access_revoked.go"
|
||||
provides: "AccessRevokedEvent firing helper"
|
||||
contains: "func FireAccessRevoked("
|
||||
- path: "../sm-demo-app/plugins/golem15/user/classes/api_token_scopes.go"
|
||||
provides: "the member token scope catalog"
|
||||
contains: "HasFrontendTokenScopes"
|
||||
- path: "../sm-demo-app/plugins/golem15/user/classes/organisation_logo.go"
|
||||
provides: "OrganisationLogoURL and OrganisationLogoFields"
|
||||
contains: "func OrganisationLogoFields("
|
||||
- path: "../sm-oauthserver-plugin/classes/cascade.go"
|
||||
provides: "revocation on access-revoked and on link changes"
|
||||
contains: "AccessRevokedEvent"
|
||||
- path: "../sm-demo-app/vue-demo-app/app/lib/linkCode.ts"
|
||||
provides: "fragment parsing and sessionStorage key for the link page"
|
||||
contains: "readLinkCode"
|
||||
key_links:
|
||||
- from: "../sm-demo-app/plugins/golem15/user/controllers/backend_link_controller.go"
|
||||
to: "summercms.go modules/bouncer/jwt.go"
|
||||
via: "bouncer.VerifyAuthTime replaces the iat read"
|
||||
pattern: "VerifyAuthTime"
|
||||
- from: "../sm-oauthserver-plugin/plugin.go"
|
||||
to: "../sm-demo-app/plugins/golem15/user/classes/events.go"
|
||||
via: "Listen on *userclasses.AccessRevokedEvent, BackendLinkedEvent and BackendUnlinkedEvent"
|
||||
pattern: "AccessRevokedEvent"
|
||||
- from: "../sm-demo-app/plugins/golem15/user/plugin.go"
|
||||
to: "../sm-demo-app/plugins/golem15/user/classes/api_token_scopes.go"
|
||||
via: "Boot compiles the catalog from party.Activated and publishes it; a bad declaration fails boot"
|
||||
pattern: "CompileAPITokenScopes"
|
||||
---
|
||||
|
||||
<objective>
|
||||
Close lane C of the phase code review and both sm-servers-app escalations, on the plugin side, as additive changes against the local framework from plans 06 and 07.
|
||||
|
||||
| Repository | Changes |
|
||||
|------------|---------|
|
||||
| sm-user-plugin (checkout `../sm-demo-app/plugins/golem15/user`) | C-CR-01 user side; C-CR-02 plugin side (compare against `auth_time`); C-WR-01; the C-WR-02 link events; E-1 organisation logo; E-2 member token scopes; C-IN-02, C-IN-04, C-IN-05, C-IN-06 |
|
||||
| `../sm-oauthserver-plugin` | C-CR-01 (verifier and refresh refusals, revoke on ban, reset or password change); C-WR-02 (revoke backend grants on link and unlink); C-IN-02 (client-specific revoke event); C-IN-05 |
|
||||
| `../sm-journal-plugin` | C-IN-01 (`draft_post` marked `Writes`) |
|
||||
| `../sm-demo-app` | nginx body cap (C-IN-03); the vue-demo-app `/link-admin` page (code in the fragment, POST show, signed-in account shown); plugin submodule checkouts synced to the local commits for verification |
|
||||
|
||||
**Planner decisions recorded here (P-34..P-39):**
|
||||
- **P-34 (link page and proof, C-CR-02 and C-WR-01):**
|
||||
- My account answers `golem15.user.frontend_url` (else `app.url`) + `golem15.user.backend_links.page_path` (new key, default `/link-admin`, C-IN-06) + `#code=<code>`.
|
||||
- New route `POST /_user/api/v1/backend-link/show` with body `{"code"}` and the same answers as the GET route. The GET route stays for compatibility (core-plugin additive rule); the README marks it deprecated because the code in its path reaches access logs.
|
||||
- The confirm reads the session token with `bouncer.VerifyAuthTime(..., bouncer.AudienceUser)` and passes `auth_time` to `ConfirmLinkRequest`. The signature is unchanged; the parameter is renamed `sessionAuthTime` in the doc. A token without `auth_time` answers 409 `stale_session`.
|
||||
- **P-35 (access revocation, C-CR-01):**
|
||||
- New `classes.AccessRevokedEvent{UserIDs []uint, Before time.Time, Reason string, Tx *gorm.DB}`.
|
||||
- New `classes.FireAccessRevoked(ctx, app, tx, userIDs, before, reason) error`. It fires the event in `tx` (a listener error rolls the write back) and one `pact.MCPSubjectRevoked` per user after commit.
|
||||
- Called by the plugin's ban action, password reset, an admin setting a password, and the self-service password change.
|
||||
- The ban action is a wrapper around `BanUsers`, whose exported signature is unchanged. Inside the action's transaction it reads `ThrottleStates` for the selected ids before and after `BanUsers`. The newly banned ids are those banned after but not before, and only they are passed to `FireAccessRevoked`.
|
||||
- `Before` is the untruncated instant of the revocation (`time.Now()` taken at the write). The stored `tokens_valid_after` keeps its whole-second shape (for example `now.Truncate(time.Second)` in `ResetPassword`). The self-service change keeps its cutoff of iat minus one second and passes `cutoff.Add(time.Second)`, the current session's sign-in second, so tokens issued since that sign-in survive.
|
||||
- `RevokeUserAccess`, `BanUsers` and `DeactivateUsers` keep their signatures. The README tells applications that call them directly to call `FireAccessRevoked` too. Deactivation keeps its `DeactivatingEvent` cascade.
|
||||
- oauthserver revokes MCP-client access and refresh tokens with `created_at < Before`. Both the verifier and the MCP refresh rotation also refuse a banned user, and refuse a token with `created_at` before `tokens_valid_after + 1s` (`CreatedAt.Before(cutoff.Add(time.Second))`).
|
||||
- Why a second is added: the stored cutoff is truncated to whole seconds while `created_at` is not. A token created at 12:00:05.300, before a reset at 12:00:05.700 that stores 12:00:05, is therefore refused by the verifier even without the revoke listener.
|
||||
- Why `!CreatedAt.After(cutoff)` alone is not enough: it lets that token through.
|
||||
- The cost is a token legitimately minted in the same second right after a reset. An OAuth consent round trip cannot complete that fast, so the cost is accepted.
|
||||
- The revocation sits behind one oauthserver function, `revokeUserGrants(ctx, tx, userIDs []uint, before time.Time, kinds []string) error`. `OnAccessRevoked` calls it with `kinds = []string{models.ClientKindMCP}`. If the user decides that a ban or reset also ends OIDC grants (an open question to the user), the change is one line: add the OIDC kind at that call site. Plan 09 records the question in deferred-items.
|
||||
- **P-36 (backend grants follow the link, C-WR-02):** oauthserver listens to `BackendLinkedEvent` and `BackendUnlinkedEvent`. In the event's transaction it revokes the frontend user's unrevoked MCP access and refresh tokens whose scopes contain `backend`; after commit it fires `MCPSubjectRevoked` with each affected `ClientID`.
|
||||
- `BackendUnlinkedEvent` already fires on every unlink the plugin performs: My account, the Administrators screen, and the deletion or deactivation of the frontend user (`dropUserLink` → `RemoveLink`, documented in `classes/events.go`).
|
||||
- The link event is still handled for one silent path: deleting the backend administrator in cabana removes the link row through the foreign key's `ON DELETE CASCADE`, and no event fires. Revoking on every new link means a grant left over from that path cannot run with the reach of a later admin, so a backend grant only ever runs with the reach of the admin shown at its consent.
|
||||
- This is the review's first option. The second option, storing the consented admin on the grant, needs a framework field and an oauthserver migration and is not taken.
|
||||
- **P-37 (member token scopes, E-2):**
|
||||
- `classes.APITokenScope{Name, Label, Description, PluginID string; BuiltIn bool}` and `classes.APITokenScopeCatalog` (`Scopes()`, `Allowed(name)`).
|
||||
- `classes.CompileAPITokenScopes(plugins []party.Plugin) (*APITokenScopeCatalog, error)` validates with `pact.ValidTokenScopeName`. It refuses a built-in name (`read`, `write`, `ai`), a duplicate across plugins, an empty label or description, and a `::` label or description whose phrase key is missing (checked in Boot against the translator).
|
||||
- Boot compiles from `party.Activated`, publishes the catalog, and fails boot on an error.
|
||||
- `classes.APITokenScopesFor(app)` returns the published catalog or the built-ins.
|
||||
- **Validation is scoped to the application.** New `classes.MintAPITokenFor(app *backpack.App, prefix string, userID uint, name string, scopes []string, expiresAt *time.Time, collectionIDs []uint) (string, *models.APIToken, error)` checks scopes against `APITokenScopesFor(app)`. The HTTP mint route and the plugin's console command use it, and the README tells applications to use it.
|
||||
- The legacy `MintAPIToken` keeps its signature and keeps working for existing callers (for example an application's own token manager). It accepts the built-ins plus a process-wide fallback set: every name a booted catalog in this process declared. Production runs one application per process, so the set equals that application's catalog. The README documents the difference.
|
||||
- `classes.ResetDeclaredAPITokenScopes()` clears the fallback set. It is exported as a test seam (documented "tests only"), because the plugin's tests live outside the classes package.
|
||||
- The HTTP mint route checks the application's own catalog, so an undeclared scope answers the existing 422 text "The selected scope is invalid.". In a test binary that boots two applications, app B refuses app A's scope.
|
||||
- Discovery is `GET /_user/api/v1/tokens/scopes` behind `jwt.auth`, answering `{"data":[{"name","label","description","built_in"}]}` with built-ins first. Labels are translated with the plugin's `appTranslator` for the request locale.
|
||||
- `AllowedAPITokenScopes` stays the built-in list.
|
||||
- **P-38 (organisation logo, E-1):** a second public attachOne `logo` on `Organisation` (`OrganisationLogoField`); the existing `avatar` stays, for PHP parity and additivity.
|
||||
- Attachments live in `system_files`, so there is no migration.
|
||||
- The Organisations form gains an image `fileupload` field `logo`.
|
||||
- The plugin serves no frontend organisation payload today, so it ships two helpers for the applications that build one:
|
||||
- `classes.OrganisationLogoURL(ctx, app, org, size) (string, bool, error)`;
|
||||
- `classes.OrganisationLogoFields(ctx, app, org) (map[string]any, error)`, answering `has_logo`, `logo` (disk name or nil) and `logo_url` (a 256 px thumbnail or nil), the keys the user payload uses for avatars.
|
||||
- Reversibility costly: the relation name is stored in `system_files.field`.
|
||||
- **P-39 (local verification wiring):** oauthserver's git-ignored `go.work` points `sm-user-plugin` at `../sm-demo-app/plugins/golem15/user`. Today it points at another application's checkout, which lacks the new exports. The demo's `plugins/golem15/oauthserver` and `plugins/golem15/journal` submodule checkouts are moved, detached, to this plan's local plugin commits so the demo's end-to-end test runs them.
|
||||
- None of this is committed: the pointer bumps in sm-demo-app happen after the plugin tags, per the release order.
|
||||
- The vue-demo-app pointer is a normal local commit in sm-demo-app, as in plan 04. vue-demo-app is pushed before sm-demo-app when the user releases.
|
||||
|
||||
**Commits are self-contained:** each task's commits build and pass the repository's `go vet` and `-short` tests on their own. No commit leaves a repository relying on a later task's commit, and a cross-repository dependency is always committed in the order user plugin → oauthserver → demo.
|
||||
|
||||
Purpose: SC-4's identity chain holds under ban, password reset, refresh and re-linking; sm-servers-app gets E-1 and E-2; plan 09 can pin all of it.
|
||||
Output: local commits in sm-user-plugin, sm-oauthserver-plugin, sm-journal-plugin, vue-demo-app and sm-demo-app; three smoke tests.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@~/.claude/msd-core/workflows/execute-plan.md
|
||||
@~/.claude/msd-core/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<context>
|
||||
@.planning/PROJECT.md
|
||||
@.planning/STATE.md
|
||||
@.planning/phases/19.7-generic-mcp-module/19.7-GAP-INPUTS.md
|
||||
@.planning/phases/19.7-generic-mcp-module/19.7-REVIEW.md
|
||||
@.planning/phases/19.7-generic-mcp-module/19.7-CONTEXT.md
|
||||
@.planning/phases/19.7-generic-mcp-module/19.7-03-SUMMARY.md
|
||||
@.planning/phases/19.7-generic-mcp-module/19.7-04-SUMMARY.md
|
||||
@.planning/phases/19.7-generic-mcp-module/19.7-07-SUMMARY.md
|
||||
@.planning/notes/2026-10-23-frontend-token-scopes.md
|
||||
@.planning/notes/core-plugins-own-repos.md
|
||||
@CLAUDE.md
|
||||
</context>
|
||||
|
||||
<project_rules>
|
||||
- Core-plugin rule: sm-user-plugin is used by many applications, and oauthserver and Journal are shared. Every change is additive:
|
||||
- no exported func, method or type removed or re-signed (the gate's additive check against 930b158 and v0.1.9 for the user plugin, d31f8e7 for oauthserver, ffa2c8f for Journal);
|
||||
- no route, column, migration id, config key, permission code or payload field removed or renamed;
|
||||
- existing error bodies stay word for word.
|
||||
- Plugin go.mod `require` lines stay as they are (summercms v0.4.4, sm-user-plugin v0.1.10 in oauthserver). The local framework comes from each repository's `replace ../summercms.go` or go.work. Never run `go get` for the framework or a plugin.
|
||||
- The user plugin is built and tested with `GOWORK=off`; its go.mod replace points at `../../../../summercms.go`. oauthserver uses its git-ignored go.work (Task 2 retargets it). Journal uses its tracked go.work, which is left unchanged.
|
||||
- Docker-backed tests: if the socket is denied although the user is in the docker group, run them through `sudo -u golem` (CLAUDE.md). Postgres tests never use `t.Parallel`; they seed unique rows and clean up.
|
||||
- Unit tests are plan 09's job. This plan carries three smokes (`TestBackendLinkAuthTimeSmoke`, `TestMCPRevocationSmoke`, `TestMemberTokenScopesSmoke`) and keeps every existing suite green: `TestBackendLink`, `TestUserMCPGuard`, `TestMCPTokenVerifier`, `TestMCPConsent`, `TestJournalMCP`, `TestMCPEndToEnd`. A test pinning the old behaviour (for example the iat-based confirm) changes in the fix commit and is named in the SUMMARY.
|
||||
- Each plugin's README changes in the same commit as its exported API, config keys or routes.
|
||||
- sm-demo-app has uncommitted changes from other sessions (`main.go`, the `plugins/golem15/user` pointer from plan 05): run `git -C ../sm-demo-app status --short` first and never stage either. Stage by explicit path in every repository. One logical change per commit, no co-author tags. Order: plugin repo commits first, then sm-demo-app commits.
|
||||
- Release rule: no tag, push or deploy in this plan.
|
||||
</project_rules>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="tracer">
|
||||
<name>Task 1: Tracer: the account link proof end to end — My account answers a fragment link, the site page shows the request through POST and the signed-in account, and only a session signed in after the request (auth_time) confirms it</name>
|
||||
<precondition>Plan 07's `bouncer.VerifyAuthTime` exists in the local framework (`grep -c "func VerifyAuthTime(" modules/bouncer/jwt.go` prints 1), and a Docker daemon answers (`docker info`, directly or through `sudo -u golem`).</precondition>
|
||||
<files>../sm-demo-app/plugins/golem15/user/controllers/backend_link_controller.go, ../sm-demo-app/plugins/golem15/user/classes/backend_links.go, ../sm-demo-app/plugins/golem15/user/account_contributions.go, ../sm-demo-app/plugins/golem15/user/routes.go, ../sm-demo-app/plugins/golem15/user/config/config.yaml, ../sm-demo-app/plugins/golem15/user/README.md, ../sm-demo-app/plugins/golem15/user/backend_link_auth_time_smoke_test.go, ../sm-demo-app/vue-demo-app/app/pages/link-admin.vue, ../sm-demo-app/vue-demo-app/app/lib/linkCode.ts, ../sm-demo-app/vue-demo-app/i18n/locales/en.json, ../sm-demo-app/vue-demo-app/i18n/locales/pl.json, ../sm-demo-app/vue-demo-app</files>
|
||||
<read_first>
|
||||
- 19.7-REVIEW.md, sections C-CR-02, C-WR-01 and C-IN-06; 19.7-07-SUMMARY.md (`VerifyAuthTime`'s final signature)
|
||||
- U controllers/backend_link_controller.go (`BackendLinkShow`, `BackendLinkConfirm`, `backendLinks`, `isLinkGone`); U classes/backend_links.go (`LinkRequestByCode`, `ConfirmLinkRequest` and its doc, `requestUsable`); U account_contributions.go (`startFrontendLink`); U routes.go (the `/backend-link/*` routes and `g.Where("code", ...)`); U config/config.yaml (`frontend_url`, `backend_links`); U classes `FrontendURL`
|
||||
- U backend_link_smoke_test.go and backend_link_test.go (the Postgres harness, `startLink`/`confirmLink`, how a session token is minted for a user)
|
||||
- V app/pages/link-admin.vue, app/composables/useDemoAuth.ts (token cookie, logout), app/pages/login.vue (how `next` is followed), i18n/locales/en.json and pl.json (`linkAdmin.*`)
|
||||
- U README.md, the account link section
|
||||
</read_first>
|
||||
<action>
|
||||
Implement P-34.
|
||||
|
||||
**U backend_link_controller.go:**
|
||||
- `BackendLinkConfirm` replaces the `VerifyClaims` iat read with `bouncer.VerifyAuthTime(classes.SessionToken(r), secret, bouncer.AudienceUser)`. A subject mismatch or a verify error stays 401. `ok == false` answers the existing 409 `stale_session` body. Pass `authTime` to `ConfirmLinkRequest`.
|
||||
- Add `BackendLinkShowPost(app)`: it reads `{"code"}` with `readFields` and answers exactly like `BackendLinkShow`: the same 404 for an unknown, used or expired code, and the same data shape. Both show handlers set `Cache-Control: no-store, private`.
|
||||
|
||||
**U classes/backend_links.go:** rename the `ConfirmLinkRequest` parameter to `sessionAuthTime` and rewrite its doc. The proof is the sign-in time (`auth_time`), which a refresh never renews; the comparison stays in whole seconds against `created_at`. Keep the signature and types.
|
||||
|
||||
**U routes.go:** add `g.Post("/backend-link/show", controllers.BackendLinkShowPost(p.app))` in the same group. The GET route stays.
|
||||
|
||||
**U account_contributions.go:**
|
||||
- `startFrontendLink` answers `classes.FrontendURL(p.app) + pagePath + "#code=" + plain`. Rewrite its doc comment to describe the fragment link as well; the comment must not show the old query-string form.
|
||||
- `pagePath` comes from the new config key `golem15.user.backend_links.page_path` (default `/link-admin`; a value without a leading `/` gets one) through a small helper in classes next to `FrontendURL`.
|
||||
- Add the key to config/config.yaml under `backend_links`.
|
||||
|
||||
**U README.md:** the link section describes:
|
||||
- the fragment URL;
|
||||
- the POST show route;
|
||||
- the GET show route, kept for compatibility (its path puts the code in access logs, so pages use POST);
|
||||
- `page_path`;
|
||||
- the `auth_time` rule, with an upgrade note: session tokens issued before this release carry no `auth_time`, so a frontend user signed in before the upgrade gets `stale_session` and must sign in again before linking. A refresh does not help, because it carries the absence forward.
|
||||
|
||||
**New smoke `backend_link_auth_time_smoke_test.go`** (Postgres, created only when not `-short`) holds `TestBackendLinkAuthTimeSmoke`:
|
||||
1. A user signs in (minted token).
|
||||
2. The admin creates a request.
|
||||
3. Refreshing the old token through `POST /_user/api/v1/refresh` and confirming with the refreshed token answers 409 `stale_session`.
|
||||
4. A login after the request confirms with 200.
|
||||
5. `POST /backend-link/show` with the code in the body answers the admin's login.
|
||||
6. The My account action's URL contains `/link-admin#code=` and no query string.
|
||||
|
||||
**V app/lib/linkCode.ts** (pure, unit-testable in plan 09):
|
||||
- `LINK_CODE_KEY = 'link-admin-code'`;
|
||||
- `readLinkCode(hash: string): string`, returning the 64-hex value of a `#code=` fragment or '';
|
||||
- `isLinkCode(s: string): boolean`.
|
||||
|
||||
**V app/pages/link-admin.vue:**
|
||||
- In `onMounted` (client only), read the fragment. If it holds a code, store it under `LINK_CODE_KEY` in `sessionStorage` and drop the fragment with `history.replaceState`. Otherwise take the stored code.
|
||||
- Load the request with `$fetch('/_user/api/v1/backend-link/show', { method: 'POST', body: { code } })`.
|
||||
- When signed in, load `GET /_user/api/v1/fetch` with the bearer and show `linkAdmin.signedInAs` (the email) above the confirm button, with a `linkAdmin.notYou` action that calls `logout()` and shows the sign-in link.
|
||||
- The sign-in link is `/login?next=/link-admin`; the code stays in sessionStorage, never in a URL.
|
||||
- Remove the stored code on success and when the request is gone.
|
||||
- The route's query parameter is no longer read, and the page's header comment describes the fragment flow.
|
||||
- Add `linkAdmin.signedInAs` and `linkAdmin.notYou` to en.json and pl.json.
|
||||
- Reword the existing `linkAdmin.signInAgain` text (shown after a 409 `stale_session` or a 401) in both locales. It says the link needs a sign-in made after the request, and that a session started before the site's last update, or before the link was requested, must sign in again. For example, en: "Linking needs a fresh sign-in. Sessions started before the link was requested, or before the site was last updated, have to sign in again." Add pl to match.
|
||||
|
||||
Run `pnpm --dir ../sm-demo-app/vue-demo-app run test` (the existing lib tests stay green).
|
||||
|
||||
**Commits:**
|
||||
1. The user plugin commit.
|
||||
2. The vue-demo-app commit in its submodule checkout.
|
||||
3. In sm-demo-app, a separate local commit `chore: bump vue-demo-app (link page: code in the fragment, POST show)` staging only `vue-demo-app`.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>GOWORK=off go -C ../sm-demo-app/plugins/golem15/user vet ./... && GOWORK=off go -C ../sm-demo-app/plugins/golem15/user test -count=1 -short ./... && GOWORK=off go -C ../sm-demo-app/plugins/golem15/user test -count=1 -run '^(TestBackendLinkAuthTimeSmoke|TestBackendLink|TestBackendLinkSmoke)$' -v . 2>&1 | grep -c -- '^--- PASS: Test' | grep -qx 3 && pnpm --dir ../sm-demo-app/vue-demo-app run test</automated>
|
||||
<fails_when>non-zero exit, "FAIL" in the output, or fewer than 3 top-level PASS lines (a SKIP because Docker is unreachable counts as not passing: run on a Docker host, through sudo -u golem if needed)</fails_when>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `grep -v '^\s*//' ../sm-demo-app/plugins/golem15/user/controllers/backend_link_controller.go | grep -c "bouncer.VerifyAuthTime("` prints 1 and the same filter for `bouncer.VerifyClaims(` prints 0
|
||||
- `grep -c 'g.Post("/backend-link/show"' ../sm-demo-app/plugins/golem15/user/routes.go` prints 1 and `grep -c 'g.Get("/backend-link/{code}"' ../sm-demo-app/plugins/golem15/user/routes.go` prints 1 (kept)
|
||||
- `grep -c '#code=' ../sm-demo-app/plugins/golem15/user/account_contributions.go` prints 1 and `grep -c '?code=' ../sm-demo-app/plugins/golem15/user/account_contributions.go` prints 0
|
||||
- `grep -c "page_path" ../sm-demo-app/plugins/golem15/user/config/config.yaml ../sm-demo-app/plugins/golem15/user/README.md` prints at least 1 per file
|
||||
- `grep -c "route.query" ../sm-demo-app/vue-demo-app/app/pages/link-admin.vue` prints 0, `grep -c "sessionStorage" ../sm-demo-app/vue-demo-app/app/pages/link-admin.vue` prints at least 1, and `grep -c "signedInAs" ../sm-demo-app/vue-demo-app/i18n/locales/en.json ../sm-demo-app/vue-demo-app/i18n/locales/pl.json` prints 1 per file
|
||||
- `git -C ../sm-demo-app log -1 --name-only --format=%s | grep -c "^vue-demo-app$"` prints 1 and the same commit lists neither `main.go` nor `plugins/golem15/user`
|
||||
</acceptance_criteria>
|
||||
<done>The link proof rests on the sign-in time, the code never appears in a URL the server sees, and the person confirming sees which account they are linking; the smoke proves refresh no longer passes as a fresh sign-in.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 2: Grant revocation chain: ban, password reset or change revoke MCP grants and end sessions; the verifier and refresh refuse banned and pre-cutoff tokens; backend grants follow the link; token- and client-specific revoke events; last_used_ip on /mcp; deduplicated helpers</name>
|
||||
<reversibility rating="reversible">New events and listeners plus stricter refusals; nothing stored changes shape.</reversibility>
|
||||
<files>../sm-demo-app/plugins/golem15/user/classes/events.go, ../sm-demo-app/plugins/golem15/user/classes/access_revoked.go, ../sm-demo-app/plugins/golem15/user/classes/backend_links.go, ../sm-demo-app/plugins/golem15/user/classes/subject_events.go, ../sm-demo-app/plugins/golem15/user/classes/mcp_guard.go, ../sm-demo-app/plugins/golem15/user/controllers/api_controller.go, ../sm-demo-app/plugins/golem15/user/controllers/users_admin_controller.go, ../sm-demo-app/plugins/golem15/user/controllers/api_tokens.go, ../sm-demo-app/plugins/golem15/user/controllers/backend_link_controller.go, ../sm-demo-app/plugins/golem15/user/account_contributions.go, ../sm-demo-app/plugins/golem15/user/README.md, ../sm-oauthserver-plugin/classes/mcp_verifier.go, ../sm-oauthserver-plugin/classes/mcp_store.go, ../sm-oauthserver-plugin/classes/cascade.go, ../sm-oauthserver-plugin/classes/subject_events.go, ../sm-oauthserver-plugin/controllers/authorizations.go, ../sm-oauthserver-plugin/plugin.go, ../sm-oauthserver-plugin/mcp_revocation_smoke_test.go, ../sm-oauthserver-plugin/README.md</files>
|
||||
<read_first>
|
||||
- 19.7-REVIEW.md, sections C-CR-01, C-WR-02, C-IN-02, C-IN-04 and C-IN-05
|
||||
- U classes/admin_actions.go (`ThrottleStates`, `RevokeUserAccess`, `BanUsers`, `DeactivateUsers`); U classes/events.go (event structs, the `*EventName` constants, `BackendLinkedEvent`, `BackendUnlinkedEvent`); U classes/subject_events.go; U classes/backend_links.go (`LinkedBackendUser` SQL at ~385-410, `insertLink`, `RemoveLink`, `requestUsable`); U models/backend_link_request.go (`IsUsableAt`)
|
||||
- U controllers/api_controller.go (`ResetPassword` at ~440-460, `ChangePassword` at ~700-725); U controllers/users_admin_controller.go (password store at ~236-252, the bulk actions at ~625-660); U controllers/api_tokens.go (`APITokenDestroy`); U classes/mcp_guard.go (`MCPResolvePersonalToken`); U middleware/api_token.go (how `last_used_ip` is stamped)
|
||||
- O classes/mcp_verifier.go (`MCPVerifyToken`), classes/mcp_store.go (`mcpTx.refresh` at ~286, `revokeRefreshLineage`), classes/cascade.go (`Cascade`, `purge`), classes/subject_events.go, controllers/authorizations.go (revoke handler at ~110-125), plugin.go (listener registration at ~99-103), models/access_token.go and refresh_token.go (Scopes, CreatedAt, RevokedAt), models/client.go (`ClientKindMCP`), mcp_verifier_smoke_test.go and harness_test.go (test shapes)
|
||||
- summercms.go modules/towel/context.go (`ClientIP`) and modules/pact/mcp.go (`MCPSubjectChanged.TokenID`, `ClientID`) from plan 07
|
||||
- `cat ../sm-oauthserver-plugin/go.work` (the current `sm-user-plugin` replace target)
|
||||
</read_first>
|
||||
<action>
|
||||
**Step 0, local environment (P-39).** Edit the git-ignored `../sm-oauthserver-plugin/go.work` so its `sm-user-plugin` replace points at `../sm-demo-app/plugins/golem15/user`. Record the previous target in the SUMMARY. Never commit go.work.
|
||||
|
||||
**U, P-35:**
|
||||
- In classes/events.go, add `AccessRevokedEvent{UserIDs []uint; Before time.Time; Reason string; Tx *gorm.DB}` and `AccessRevokedEventName = "user.AccessRevoked"`, documented like the other events.
|
||||
- New file classes/access_revoked.go holds `FireAccessRevoked(ctx context.Context, app *backpack.App, tx *gorm.DB, userIDs []uint, before time.Time, reason string) error`. It fires the event (a listener error is returned, so the caller's transaction rolls back), then for each id calls `FireSubjectChangedAfterCommit` with `MCPSubjectRevoked`. A nil app or bus, or an empty id list, is a no-op.
|
||||
- Callers:
|
||||
- **Ban.** `usersAdminController` gets a `banUsers` wrapper used by the `ban` bulk action. Inside the action's transaction `tx`:
|
||||
1. read `classes.ThrottleStates(ctx, tx, ids)` for the selected ids;
|
||||
2. call `classes.BanUsers(ctx, tx, users)`;
|
||||
3. read `ThrottleStates` again;
|
||||
4. take the newly banned ids (banned after, not before), and only when that list is non-empty call `FireAccessRevoked(ctx, app, tx, newly, time.Now(), "ban")`.
|
||||
- **Password reset.** `ResetPassword` keeps storing `tokens_valid_after = now.Truncate(time.Second)`, so the column keeps its shape. It passes the untruncated `now` as `before`, reason `password_reset`. The handler runs outside a transaction today: wrap the update and the event in `lagoon.Transaction`, so a listener failure leaves the password unchanged.
|
||||
- **Self-service change.** `ChangePassword` keeps `cutoff = iat - 1s` in the column and passes `cutoff.Add(time.Second)` (the current session's sign-in second) as `before`, reason `password_changed`. Tokens issued since that sign-in survive, as the session JWT does.
|
||||
- **Admin-set password.** The admin password store keeps the truncated value in the column and passes the untruncated `time.Now()` as `before`, reason `password_set_by_admin`, in the save's transaction (`requestDB` or the hook's tx).
|
||||
- `LinkedBackendUser` adds `AND NOT EXISTS (SELECT 1 FROM user_throttle t WHERE t.user_id = l.user_id AND t.is_banned)`. The columns are `user_throttle.user_id` and `user_throttle.is_banned` (models/throttle.go).
|
||||
|
||||
**U, C-IN-02, C-IN-04, C-IN-05:**
|
||||
- `APITokenDestroy` fires `MCPSubjectRevoked` with `TokenID: token.ID` through a new exported helper pair in classes/subject_events.go: `FireSubjectEvent(ctx, app, ev pact.MCPSubjectChanged)` and `FireSubjectEventAfterCommit(ctx, app, tx, ev)`. The existing helpers become wrappers with their signatures kept.
|
||||
- `MCPResolvePersonalToken` also stamps `last_used_ip` from `towel.ClientIP(ctx)` when present, in the same `UpdateColumns` call as `last_used_at`.
|
||||
- `requestUsable` keeps its error split but uses `req.IsUsableAt` for the combined check.
|
||||
- Add an exported `classes.BackendLinksFor(app) *BackendLinks`, used by both account_contributions.go and the controller instead of their duplicate lookups.
|
||||
- Confirm, in code and README, that `BackendLinkedEvent` and `BackendUnlinkedEvent` fire on every link and unlink path, with `Link.UserID` set. These are the C-WR-02 events oauthserver listens to.
|
||||
- The README gains the access-revocation section: when it fires, the event, and what applications calling `RevokeUserAccess` or `BanUsers` directly must add.
|
||||
|
||||
**O:**
|
||||
- Add one unexported helper, `issuedBeforeCutoff(createdAt *time.Time, cutoff time.Time) bool`. It answers true when the cutoff is set and `createdAt` is nil or `createdAt.Before(cutoff.Add(time.Second))`. Reason: the stored cutoff is truncated to whole seconds, so a token created earlier in the cutoff's second must be refused too (P-35). Both checks below use it.
|
||||
- classes/mcp_verifier.go: after loading the principal, refuse with `pact.ErrMCPTokenInvalid` when `issuedBeforeCutoff(token.CreatedAt, principal.TokensValidAfter)`, or when `userclasses.ThrottleStates` reports the user banned. A database error is returned as an error.
|
||||
- classes/mcp_store.go `mcpTx.refresh`: apply the same three refusals (user gone through `PrincipalByID`, banned, `issuedBeforeCutoff` on the refresh token) by answering the store's existing not-found result, which wristband maps to `invalid_grant`.
|
||||
- classes/cascade.go:
|
||||
- Add the single revocation function `revokeUserGrants(ctx context.Context, tx *gorm.DB, userIDs []uint, before time.Time, kinds []string) error`. It sets `revoked_at` on the users' unrevoked access and refresh tokens created before `before`, whose client kind is in `kinds`, and returns the distinct client ids it touched.
|
||||
- `OnAccessRevoked(ctx, *userclasses.AccessRevokedEvent)` calls it in `ev.Tx` with `ev.Before` and `kinds = []string{models.ClientKindMCP}`. A comment there says that extending the revocation to OIDC grants, if the user decides so, is adding the OIDC kind to this list.
|
||||
- `OnBackendLinkChanged(ctx, userID uint, tx *gorm.DB)` sets `revoked_at` on the user's unrevoked MCP access and refresh tokens whose scopes contain `backend`, then fires `MCPSubjectRevoked` with each distinct `ClientID` after commit.
|
||||
- Two adapters: `OnBackendLinked(ctx, *userclasses.BackendLinkedEvent)` and `OnBackendUnlinked(ctx, *userclasses.BackendUnlinkedEvent)`.
|
||||
- plugin.go: register the three listeners next to the existing two.
|
||||
- controllers/authorizations.go: the revoke fires with `ClientID: clientID`.
|
||||
- classes/subject_events.go: delegate to the user plugin's helpers. Keep oauthserver's exported names as thin wrappers (additive rule).
|
||||
- README: the revocation behaviour.
|
||||
|
||||
**New smoke `../sm-oauthserver-plugin/mcp_revocation_smoke_test.go`** (Postgres) holds `TestMCPRevocationSmoke`:
|
||||
1. Mint an MCP access and refresh token for a user.
|
||||
2. `BanUsers` plus `FireAccessRevoked` in one transaction.
|
||||
3. The verifier answers `ErrMCPTokenInvalid`.
|
||||
4. Both rows have `revoked_at`.
|
||||
5. A refresh is refused.
|
||||
6. A second user linked to an admin with a `backend` grant loses it on `RemoveLink`, and keeps a grant without `backend`.
|
||||
|
||||
**Commits:** the user plugin first, then oauthserver.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>GOWORK=off go -C ../sm-demo-app/plugins/golem15/user vet ./... && GOWORK=off go -C ../sm-demo-app/plugins/golem15/user test -count=1 -short ./... && GOWORK=off go -C ../sm-demo-app/plugins/golem15/user test -count=1 -run '^(TestBackendLink|TestUserMCPGuard|TestBackendLinkAuthTimeSmoke)$' -v . 2>&1 | grep -c -- '^--- PASS: Test' | grep -qx 3 && go -C ../sm-oauthserver-plugin vet ./... && go -C ../sm-oauthserver-plugin test -count=1 -short ./... && go -C ../sm-oauthserver-plugin test -count=1 -run '^(TestMCPRevocationSmoke|TestMCPTokenVerifier|TestMCPConsent)$' -v . 2>&1 | grep -c -- '^--- PASS: Test' | grep -qx 3</automated>
|
||||
<fails_when>non-zero exit, "FAIL" in the output, or a PASS count other than 3 for either repository (a SKIP without Docker counts as not passing)</fails_when>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `grep -c "type AccessRevokedEvent struct" ../sm-demo-app/plugins/golem15/user/classes/events.go` prints 1 and `grep -c "func FireAccessRevoked(" ../sm-demo-app/plugins/golem15/user/classes/access_revoked.go` prints 1
|
||||
- `grep -v '^\s*//' ../sm-demo-app/plugins/golem15/user/controllers/api_controller.go | grep -c "FireAccessRevoked("` prints at least 2 and `grep -v '^\s*//' ../sm-demo-app/plugins/golem15/user/controllers/users_admin_controller.go | grep -c "FireAccessRevoked("` prints at least 2
|
||||
- `grep -c "is_banned" ../sm-demo-app/plugins/golem15/user/classes/backend_links.go` prints at least 1 (`user_throttle.is_banned`, models/throttle.go)
|
||||
- `grep -v '^\s*//' ../sm-oauthserver-plugin/classes/mcp_verifier.go | grep -c "ThrottleStates(\|TokensValidAfter"` prints at least 2
|
||||
- `grep -c "AccessRevokedEvent\|BackendLinkedEvent\|BackendUnlinkedEvent" ../sm-oauthserver-plugin/plugin.go` prints at least 3
|
||||
- `grep -v '^\s*//' ../sm-oauthserver-plugin/controllers/authorizations.go | grep -c "ClientID"` prints at least 1 and `grep -v '^\s*//' ../sm-demo-app/plugins/golem15/user/classes/mcp_guard.go | grep -c "last_used_ip"` prints at least 1
|
||||
- `git -C ../sm-oauthserver-plugin status --short go.work | wc -l` prints 0 (go.work stays ignored and uncommitted)
|
||||
- `grep -c "func revokeUserGrants(\|func issuedBeforeCutoff(" ../sm-oauthserver-plugin/classes/*.go | awk -F: '{s+=$2} END {print s}'` prints 2, and `grep -v '^\s*//' ../sm-oauthserver-plugin/classes/mcp_verifier.go ../sm-oauthserver-plugin/classes/mcp_store.go | grep -c "issuedBeforeCutoff("` prints at least 2
|
||||
- `grep -v '^\s*//' ../sm-demo-app/plugins/golem15/user/controllers/users_admin_controller.go | grep -c "ThrottleStates("` prints at least 2 (before and after BanUsers)
|
||||
</acceptance_criteria>
|
||||
<done>A banned or password-reset user's MCP grants stop working at once and their sessions end, refresh cannot revive them, a banned user's link stops resolving, a backend grant cannot outlive or be re-targeted by a link change, and single-token or single-client revocations end only what they opened.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 3: E-2 member token scopes: catalog from pact.HasFrontendTokenScopes, boot validation, mint, check, list and discovery</name>
|
||||
<reversibility rating="costly">Declared scope names are stored on `user_api_tokens` rows and the discovery route is a published contract of a core plugin.</reversibility>
|
||||
<files>../sm-demo-app/plugins/golem15/user/classes/api_token_scopes.go, ../sm-demo-app/plugins/golem15/user/classes/api_tokens.go, ../sm-demo-app/plugins/golem15/user/console/api_tokens.go, ../sm-demo-app/plugins/golem15/user/controllers/api_tokens.go, ../sm-demo-app/plugins/golem15/user/routes.go, ../sm-demo-app/plugins/golem15/user/plugin.go, ../sm-demo-app/plugins/golem15/user/lang/en/lang.yaml, ../sm-demo-app/plugins/golem15/user/lang/pl/lang.yaml, ../sm-demo-app/plugins/golem15/user/README.md, ../sm-demo-app/plugins/golem15/user/member_token_scopes_smoke_test.go</files>
|
||||
<read_first>
|
||||
- 19.7-GAP-INPUTS.md, section E-2 (the ask, "Not asked for", acceptance); .planning/notes/2026-10-23-frontend-token-scopes.md (decision P-30)
|
||||
- summercms.go modules/pact/capabilities.go (`FrontendTokenScope`, `HasFrontendTokenScopes`, `ValidTokenScopeName`) and modules/cabana/token_scopes.go (`validatePluginTokenScope`, `validateTokenScopeMessageKeys`: the validation style to mirror)
|
||||
- U classes/api_tokens.go (`AllowedAPITokenScopes`, `DefaultAPITokenScopes`, `MintAPIToken`), controllers/api_tokens.go (`APITokenStore`, `APITokenIndex`, `apiTokenScopes`, `apiTokenJSON`), middleware/api_token.go (`Scope`), models/api_token.go (`HasScope`), routes.go (the `/tokens` routes), plugin.go (`Boot`, `bootMCPGuard` as the publish pattern), classes/frontend_permission_catalog.go (`party.Activated` lookup), controllers/app_access.go (`appTranslator`)
|
||||
- U api_tokens_test.go and api_tokens_edge_test.go (harness for token routes)
|
||||
- ../fonoteka.go/plugins/golem15/fonoteka/classes/auth/api_token_manager.go (an application calling `MintAPIToken` directly; its call must keep compiling unchanged) and ../fonoteka.go/go.work (its workspace `use`s its own user checkout; the verify builds it through a scratch copy that swaps in this checkout; fonoteka's own go.work is never edited)
|
||||
</read_first>
|
||||
<action>
|
||||
Implement P-37 in a new file classes/api_token_scopes.go:
|
||||
- `APITokenScope` (Name, Label, Description, PluginID, BuiltIn).
|
||||
- `APITokenScopeCatalog` with `Scopes() []APITokenScope` (a copy, built-ins first, then declarations in activation order) and `Allowed(name string) bool`.
|
||||
- `CompileAPITokenScopes(plugins []party.Plugin) (*APITokenScopeCatalog, error)`. The built-ins `read`, `write` and `ai` carry the labels `golem15.user::lang.api_tokens.scope_<name>` and `scope_<name>_description`. Each `pact.HasFrontendTokenScopes` declaration is checked:
|
||||
- the name is trimmed and must pass `pact.ValidTokenScopeName`;
|
||||
- it must not be a built-in;
|
||||
- it must not repeat another plugin's name; the error names both plugins, like cabana's;
|
||||
- label and description must be non-empty.
|
||||
- Every accepted declared name is added to an unexported process-wide set guarded by a `sync.RWMutex`.
|
||||
- `APITokenScopesFor(app *backpack.App) *APITokenScopeCatalog` returns the published catalog or a built-ins-only one.
|
||||
- Add `MintAPITokenFor(app *backpack.App, prefix string, userID uint, name string, scopes []string, expiresAt *time.Time, collectionIDs []uint) (string, *models.APIToken, error)`. It checks scopes with `APITokenScopesFor(app).Allowed` and shares the record-building body with `MintAPIToken` through an unexported helper that takes the allow-check as a func.
|
||||
- `MintAPIToken` keeps its signature, dedupe and default. Its check becomes "built-in, or in the process-wide declared set" (the legacy fallback; its doc points new code to `MintAPITokenFor`).
|
||||
- Add `ResetDeclaredAPITokenScopes()`, which clears the process-wide set. Document it as a test seam that production never calls.
|
||||
- The plugin's console token command (console/api_tokens.go) switches to `MintAPITokenFor` with its app.
|
||||
- `AllowedAPITokenScopes` stays, documented as the built-ins.
|
||||
|
||||
In plugin.go `Boot`:
|
||||
- compile from `app.Lookup[party.Activated]()`;
|
||||
- check every `::` label and description against the translator (`appTranslator` or `app.Lookup[*phrasebook.Translator]`), failing boot on a missing key;
|
||||
- publish the catalog;
|
||||
- return any error, so boot fails.
|
||||
|
||||
In controllers/api_tokens.go:
|
||||
- `apiTokenScopes(raw any)` becomes `apiTokenScopes(catalog, raw)` (unexported, free to change) and checks `catalog.Allowed`. The 422 texts stay word for word.
|
||||
- `APITokenStore` mints with `classes.MintAPITokenFor(app, ...)`.
|
||||
- New handler `APITokenScopes(app)` answers `{"data":[{"name","label","description","built_in"}]}`, translating labels and descriptions with `appTranslator(app)` for the request locale.
|
||||
|
||||
In routes.go, add `g.Get("/tokens/scopes", controllers.APITokenScopes(p.app), "jwt.auth")` next to the token routes.
|
||||
|
||||
Lang en and pl gain the six built-in label and description keys:
|
||||
- read: "Read" / "Odczyt";
|
||||
- write: "Write" / "Zapis";
|
||||
- ai: "AI" / "AI";
|
||||
- each with a one-line description.
|
||||
|
||||
The README gets a "Member token scopes" section covering:
|
||||
- declaring with `pact.HasFrontendTokenScopes`, with a short example declaring `inventory:read` and `inventory:write` (neutral names: a shared plugin README names no consuming application);
|
||||
- the boot errors;
|
||||
- mint, check and list behaviour, with `MintAPITokenFor` as the call applications should use and `MintAPIToken`'s process-wide fallback explained;
|
||||
- the discovery route and its answer;
|
||||
- that the admin `g15_` catalog never shows these scopes.
|
||||
|
||||
**New smoke `member_token_scopes_smoke_test.go`:**
|
||||
- `-short` part: `CompileAPITokenScopes` with two test plugins declaring the same name fails; a plugin declaring `write` fails; a bad name fails. Call `ResetDeclaredAPITokenScopes()` in a `t.Cleanup`.
|
||||
- Postgres part (created only when not `-short`):
|
||||
1. A test plugin declaring `servers:write` is activated.
|
||||
2. `POST /_user/api/v1/tokens` with `["servers:write"]` answers 201.
|
||||
3. A test route behind `APITokenGuard` plus `middleware.Scope("servers:write")` accepts the token.
|
||||
4. A `read`-only token gets 403 `Missing required scope: servers:write`.
|
||||
5. `["servers:nope"]` answers 422 `The selected scope is invalid.`.
|
||||
6. `GET /tokens/scopes` lists `read`, `write`, `ai` and `servers:write` with labels.
|
||||
7. A second application booted in the same test without the declaring plugin refuses `["servers:write"]` with the same 422, and `MintAPITokenFor` with that application refuses it.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>GOWORK=off go -C ../sm-demo-app/plugins/golem15/user vet ./... && GOWORK=off go -C ../sm-demo-app/plugins/golem15/user test -count=1 -short ./... && GOWORK=off go -C ../sm-demo-app/plugins/golem15/user test -count=1 -run '^TestMemberTokenScopesSmoke$' -v . 2>&1 | grep -q -- '^--- PASS: TestMemberTokenScopesSmoke' && W="$(mktemp -d)" && trap 'rm -rf "$W"' EXIT && F="$(realpath ../fonoteka.go)" && U="$(realpath ../sm-demo-app/plugins/golem15/user)" && printf 'go 1.27.0
|
||||
|
||||
toolchain go1.27.0
|
||||
|
||||
use (
|
||||
%s
|
||||
%s/plugins/golem15/feedback
|
||||
%s/plugins/golem15/fonoteka
|
||||
%s/plugins/golem15/golem
|
||||
%s
|
||||
)
|
||||
|
||||
replace git.golem15.com/golem15/summercms => %s
|
||||
' "$F" "$F" "$F" "$F" "$U" "$(pwd)" > "$W/go.work" && test "$(GOWORK="$W/go.work" go -C ../fonoteka.go list -m -f '{{.Dir}}' git.golem15.com/golem15/sm-user-plugin)" = "$U" && GOWORK="$W/go.work" go -C ../fonoteka.go build ./... && rm -rf "$W"</automated>
|
||||
<fails_when>non-zero exit, "FAIL" in the output, the PASS line missing (SKIP without Docker counts as missing), the scratch workspace not resolving sm-user-plugin to the demo's user checkout, or fonoteka.go failing to build against the modified user plugin</fails_when>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `grep -c "func CompileAPITokenScopes(\|func APITokenScopesFor(\|type APITokenScopeCatalog struct\|func ResetDeclaredAPITokenScopes(" ../sm-demo-app/plugins/golem15/user/classes/api_token_scopes.go` prints 4
|
||||
- `grep -c "func MintAPITokenFor(" ../sm-demo-app/plugins/golem15/user/classes/api_tokens.go` prints 1 and `grep -v '^\s*//' ../sm-demo-app/plugins/golem15/user/controllers/api_tokens.go | grep -c "MintAPITokenFor("` prints 1
|
||||
- `git -C ../fonoteka.go status --short go.work | wc -l` prints 0 (fonoteka's workspace untouched)
|
||||
- `grep -c "pact.HasFrontendTokenScopes\|pact.ValidTokenScopeName" ../sm-demo-app/plugins/golem15/user/classes/api_token_scopes.go` prints at least 2
|
||||
- `grep -c 'g.Get("/tokens/scopes"' ../sm-demo-app/plugins/golem15/user/routes.go` prints 1
|
||||
- `grep -c "AllowedAPITokenScopes = \[\]string{\"read\", \"write\", \"ai\"}" ../sm-demo-app/plugins/golem15/user/classes/api_tokens.go` prints 1 (the built-in list keeps its exported form)
|
||||
- `grep -c "The selected scope is invalid." ../sm-demo-app/plugins/golem15/user/controllers/api_tokens.go` prints at least 1
|
||||
- `grep -c "HasFrontendTokenScopes\|tokens/scopes" ../sm-demo-app/plugins/golem15/user/README.md` prints at least 2
|
||||
</acceptance_criteria>
|
||||
<done>A plugin's declared member scopes can be minted, checked, listed and discovered; undeclared ones are refused at mint time; bad declarations fail boot; the admin token catalog is untouched; MintAPIToken's callers in applications compile unchanged.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 4: E-1 organisation logo; Journal's draft_post under the write ceiling; the demo's nginx MCP body cap; demo submodule checkouts on the local plugin commits</name>
|
||||
<reversibility rating="costly">The attachment relation name `logo` is stored in system_files.field and read by applications' payloads; renaming it needs a data update.</reversibility>
|
||||
<files>../sm-demo-app/plugins/golem15/user/models/organisation.go, ../sm-demo-app/plugins/golem15/user/models/organisation/fields.yaml, ../sm-demo-app/plugins/golem15/user/classes/organisation_logo.go, ../sm-demo-app/plugins/golem15/user/lang/en/lang.yaml, ../sm-demo-app/plugins/golem15/user/lang/pl/lang.yaml, ../sm-demo-app/plugins/golem15/user/README.md, ../sm-journal-plugin/mcp.go, ../sm-journal-plugin/README.md, ../sm-demo-app/deploy/nginx/demo.summercms.io.conf, ../sm-demo-app/scripts/check-deploy.sh, ../sm-demo-app/plugins/golem15/oauthserver, ../sm-demo-app/plugins/golem15/journal</files>
|
||||
<read_first>
|
||||
- 19.7-GAP-INPUTS.md, section E-1; 19.7-REVIEW.md, sections C-IN-01 and C-IN-03
|
||||
- U models/organisation.go (`OrganisationAvatarField`, `AttachRelations`, `MorphName`), models/organisation/fields.yaml (the `avatar` fileupload), classes/avatar.go (`AvatarURL`: the attachment query, the bucket lookup, `f.Thumb`), controllers/api_controller.go `applyAvatar` (the `avatar`, `avatar_url`, `has_avatar` keys), admin_organisations_test.go (how the organisation form is exercised)
|
||||
- J mcp.go (the `draft_post` prompt at ~154-166) and README.md (the MCP section); summercms.go modules/pact/mcp.go (`MCPPrompt.Writes` from plan 06)
|
||||
- D deploy/nginx/demo.summercms.io.conf (`location = /mcp` at ~213-226, the server-level `client_max_body_size 2m`), scripts/check-deploy.sh (`mcp_locations` and the `/mcp` directive loop at ~224-256); summercms.go modules/pier/pier.go (`maxRequestBody = 4 << 20`)
|
||||
- `git -C ../sm-demo-app submodule status plugins/golem15/oauthserver plugins/golem15/journal` and `cat ../sm-demo-app/go.work` (the demo uses those checkouts)
|
||||
</read_first>
|
||||
<action>
|
||||
**E-1 (P-38), in U:**
|
||||
- In models/organisation.go, add the constant `OrganisationLogoField = "logo"` and a second relation `{Name: OrganisationLogoField, Public: true}` in `AttachRelations`. Keep `avatar` first and unchanged.
|
||||
- In fields.yaml, add a `logo` field after `avatar`: `type: fileupload`, `mode: image`, `label: golem15.user::lang.organisation.logo`, `comment: golem15.user::lang.organisation.logo_comment`, `imageWidth: 240`, `imageHeight: 120`.
|
||||
- New file classes/organisation_logo.go with:
|
||||
- `OrganisationLogoURL(ctx context.Context, app *backpack.App, org *models.Organisation, size int) (string, bool, error)`. Same lookup as `AvatarURL` for the organisation's morph name and the `logo` field: a thumbnail of `size` by `size` in mode `auto`; the file path when the thumbnail fails (logged); false when there is no logo or no bucket. Only a database failure is an error.
|
||||
- `OrganisationLogoFields(ctx context.Context, app *backpack.App, org *models.Organisation) (map[string]any, error)`, answering `has_logo` (bool), `logo` (disk name, or nil) and `logo_url` (the 256 px URL, or nil): the avatar keys of the user payload, renamed.
|
||||
- Lang en and pl gain `organisation.logo` ("Logo" / "Logo") and `organisation.logo_comment` (en "Shown wherever the site displays the organisation." / pl "Wyświetlane wszędzie, gdzie strona pokazuje organizację.").
|
||||
- The README's Organisations feature and Organisations screen paragraphs name the logo and the two helpers, with the keys.
|
||||
- No migration and no change to any existing payload.
|
||||
|
||||
**C-IN-01, in J:** set `Writes: true` on the `draft_post` prompt. The README's MCP section says the prompt is offered only to grants that may write. Commit in sm-journal-plugin.
|
||||
|
||||
**C-IN-03, in D:**
|
||||
- Add `client_max_body_size 4m;` inside `location = /mcp`, with a comment naming pier's 4 MiB request cap.
|
||||
- In scripts/check-deploy.sh, add `client_max_body_size 4m;` to the directives the `/mcp` location must carry, with the message "(pier refuses bodies over 4 MiB itself; nginx must not cut earlier)".
|
||||
- Run `bash scripts/check-deploy.sh` in sm-demo-app.
|
||||
- Commit those two files only.
|
||||
|
||||
**P-39, demo checkouts (working tree only, never committed):**
|
||||
- `git -C ../sm-demo-app/plugins/golem15/oauthserver fetch "$(realpath ../sm-oauthserver-plugin)" master && git -C ../sm-demo-app/plugins/golem15/oauthserver checkout --detach FETCH_HEAD`.
|
||||
- The same for `plugins/golem15/journal` from `../sm-journal-plugin`.
|
||||
- Then `go -C ../sm-demo-app build -tags summercms_mcp ./...` and `go -C ../sm-demo-app test -count=1 -short ./...`.
|
||||
- Record both new submodule HEADs in the SUMMARY and the deferred pointer bumps.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>GOWORK=off go -C ../sm-demo-app/plugins/golem15/user vet ./... && GOWORK=off go -C ../sm-demo-app/plugins/golem15/user test -count=1 -short ./... && go -C ../sm-journal-plugin vet ./... && go -C ../sm-journal-plugin test -count=1 -short ./... && (cd ../sm-demo-app && bash scripts/check-deploy.sh && go build -tags summercms_mcp ./... && go test -count=1 -short ./...) && test "$(git -C ../sm-demo-app/plugins/golem15/oauthserver rev-parse HEAD)" = "$(git -C ../sm-oauthserver-plugin rev-parse HEAD)" && test "$(git -C ../sm-demo-app/plugins/golem15/journal rev-parse HEAD)" = "$(git -C ../sm-journal-plugin rev-parse HEAD)"</automated>
|
||||
<fails_when>non-zero exit, "FAIL" in the output, check-deploy.sh printing an error, or a demo submodule checkout not on its plugin repository's HEAD</fails_when>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `grep -c 'OrganisationLogoField = "logo"' ../sm-demo-app/plugins/golem15/user/models/organisation.go` prints 1 and `grep -c "OrganisationAvatarField" ../sm-demo-app/plugins/golem15/user/models/organisation.go` still prints at least 2
|
||||
- `grep -A3 "^ logo:" ../sm-demo-app/plugins/golem15/user/models/organisation/fields.yaml | grep -c "fileupload"` prints 1
|
||||
- `grep -c "func OrganisationLogoURL(\|func OrganisationLogoFields(" ../sm-demo-app/plugins/golem15/user/classes/organisation_logo.go` prints 2 and `grep -c '"has_logo"\|"logo_url"' ../sm-demo-app/plugins/golem15/user/classes/organisation_logo.go` prints at least 2
|
||||
- `git -C ../sm-demo-app/plugins/golem15/user log --oneline 72de822..HEAD -- updates/ | wc -l` prints 0 (no migration added since the plugin HEAD before the gap closure)
|
||||
- `grep -A12 "Name: \"draft_post\"" ../sm-journal-plugin/mcp.go | grep -c "Writes: *true"` prints 1
|
||||
- `grep -A16 "location = /mcp {" ../sm-demo-app/deploy/nginx/demo.summercms.io.conf | grep -c "client_max_body_size 4m;"` prints 1 and `grep -c "client_max_body_size 4m;" ../sm-demo-app/scripts/check-deploy.sh` prints 1
|
||||
- `git -C ../sm-demo-app diff --cached --name-only | wc -l` prints 0 after the task's commits (the submodule moves stay unstaged)
|
||||
</acceptance_criteria>
|
||||
<done>Organisations carry an editable logo that applications can expose under the avatar-style keys; Journal's drafting prompt follows the write ceiling; the demo's nginx no longer cuts MCP bodies below pier's cap; the demo runs this plan's plugin code for verification without committing unreleased pointers.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<artifacts_this_phase_produces>
|
||||
## Artifacts this plan produces
|
||||
|
||||
**sm-user-plugin (local commits, untagged):**
|
||||
- The account link:
|
||||
- auth_time-based confirm;
|
||||
- `POST /_user/api/v1/backend-link/show` (GET kept);
|
||||
- the `#code=` link with `golem15.user.backend_links.page_path`;
|
||||
- `BackendLinksFor`.
|
||||
- Access revocation:
|
||||
- `AccessRevokedEvent` and `FireAccessRevoked` (ban, password reset, admin-set password, self password change);
|
||||
- banned users excluded from `LinkedBackendUser`;
|
||||
- `FireSubjectEvent` and `FireSubjectEventAfterCommit` (token-specific revoke on `DELETE /tokens/{id}`);
|
||||
- `last_used_ip` on the MCP personal-token path.
|
||||
- Member token scopes (E-2): `APITokenScope`, `APITokenScopeCatalog`, `CompileAPITokenScopes`, `APITokenScopesFor`, `MintAPITokenFor`, the test seam `ResetDeclaredAPITokenScopes`, `GET /_user/api/v1/tokens/scopes`, and boot validation.
|
||||
- Organisation logo (E-1): the attachOne `logo` and `OrganisationLogoField`, the form field, `OrganisationLogoURL` and `OrganisationLogoFields`.
|
||||
- README and lang updates.
|
||||
|
||||
**sm-oauthserver-plugin (local commits, untagged):**
|
||||
- The verifier and MCP refresh refuse banned and pre-cutoff tokens.
|
||||
- `Cascade.OnAccessRevoked`, `OnBackendLinked`, `OnBackendUnlinked` and `OnBackendLinkChanged`, all built on the one revocation function `revokeUserGrants` (MCP kind only; adding the OIDC kind is the one-line change if the user decides so).
|
||||
- `issuedBeforeCutoff`, the whole-second-aware `tokens_valid_after` rule.
|
||||
- The client-specific revoke event.
|
||||
- Subject-event helpers delegate to the user plugin.
|
||||
- README.
|
||||
|
||||
**sm-journal-plugin (local commit, untagged):** `draft_post` with `Writes: true`; README.
|
||||
|
||||
**vue-demo-app (local commit):** `/link-admin` reads the fragment, uses POST show, shows the signed-in account, and keeps the code in sessionStorage; `app/lib/linkCode.ts`; i18n keys.
|
||||
|
||||
**sm-demo-app (local commits):**
|
||||
- the vue-demo-app pointer bump;
|
||||
- nginx `client_max_body_size 4m` on `/mcp`, enforced by `check-deploy.sh`;
|
||||
- the oauthserver and journal submodule checkouts moved to the local plugin commits (working tree only).
|
||||
|
||||
**Smokes:** `TestBackendLinkAuthTimeSmoke`, `TestMemberTokenScopesSmoke` (user plugin); `TestMCPRevocationSmoke` (oauthserver).
|
||||
|
||||
**Decisions:** P-34 to P-39.
|
||||
</artifacts_this_phase_produces>
|
||||
|
||||
<threat_model>
|
||||
## Trust Boundaries
|
||||
|
||||
| Boundary | Description |
|
||||
|----------|-------------|
|
||||
| site browser to the link pages | a signed-in browser (possibly shared) confirms a link code |
|
||||
| web server logs and analytics to the link code | anything that records request lines or query strings |
|
||||
| user plugin to oauthserver (events) | user state changes must reach the OAuth grants that act for the user |
|
||||
| MCP verifier and refresh to the user record | a grant is only as good as the account behind it |
|
||||
| plugins to the member token catalog | plugin-declared scope names are stored on member tokens and checked by route middleware |
|
||||
| admins to organisation records | backend editors upload organisation images that the frontend shows |
|
||||
| nginx to pier | request size limits in front of the MCP endpoint |
|
||||
|
||||
## STRIDE Threat Register
|
||||
|
||||
| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan |
|
||||
|-----------|----------|-----------|----------|-------------|-----------------|
|
||||
| T-19.7-57 | Elevation of privilege | banned or password-reset users keeping MCP grants and the linked admin's reach | high | mitigate | verifier and refresh refuse banned users and tokens created before `tokens_valid_after + 1s` (whole-second cutoff, so the same second is covered); the revoke uses the untruncated instant; `AccessRevokedEvent` revokes MCP tokens and ends sessions; `LinkedBackendUser` excludes banned users; TestMCPRevocationSmoke here, TestMCPVerifierRevocation, TestMCPGrantRevocation and TestAccessRevocation in plan 09 |
|
||||
| T-19.7-58 | Spoofing | the one-shot link code exposed in access logs, request logs and browser history | high | mitigate | `#code=` fragment; POST show; the code kept in sessionStorage across sign-in; the confirming account shown before the button; GET show kept only for compatibility and documented as deprecated |
|
||||
| T-19.7-59 | Elevation of privilege | a backend grant re-targeted to an administrator the user never consented to | high | mitigate | backend-scoped MCP tokens revoked on every link and unlink (P-36), sessions ended by client |
|
||||
| T-19.7-60 | Elevation of privilege | a member token minted with an undeclared scope, or a declared scope shadowing a built-in or another plugin's | medium | mitigate | catalog validation at boot (pattern, built-ins reserved, duplicates); the HTTP mint and `MintAPITokenFor` check the application's own catalog, so another application's declaration is refused; 422 unchanged; the legacy `MintAPIToken` fallback is process-wide and documented |
|
||||
| T-19.7-61 | Tampering | organisation logo uploads | low | mitigate | image-mode fileupload through lagoon/attach (the avatar's validation), behind the Organisations screen permission `golem15.users.access_users` |
|
||||
| T-19.7-62 | Denial of service | nginx's 2 MiB cap answering HTML 413 before pier's 4 MiB JSON-RPC error | low | mitigate | `client_max_body_size 4m` on `location = /mcp`, enforced by check-deploy.sh |
|
||||
| T-19.7-63 | Repudiation | personal tokens used at /mcp showing a stale last_used_ip | low | mitigate | `MCPResolvePersonalToken` stamps the client address from `towel.ClientIP` |
|
||||
| T-19.7-SC | Tampering | package installs | low | mitigate | no Go module or npm package added (go.mod and go.sum unchanged in the three plugins; vue-demo-app's pnpm-lock.yaml unchanged; only the existing lock is used for its test run) |
|
||||
</threat_model>
|
||||
|
||||
<verification>
|
||||
- User plugin: `GOWORK=off go vet ./... && GOWORK=off go test -count=1 ./...` on a Docker host
|
||||
- oauthserver: `go vet ./... && go test -count=1 ./...` on a Docker host, with the retargeted local go.work
|
||||
- Journal: `go vet ./... && go test -count=1 -short ./...`
|
||||
- Demo: `bash scripts/check-deploy.sh`, `go build -tags summercms_mcp ./...` and `go test -count=1 -tags summercms_mcp -run '^TestMCPEndToEnd$' .` on a Docker host
|
||||
- `bash scripts/check-phase19.7.sh --plugins` (the framework repository) prints `additive:` lines for all three plugins and PASS
|
||||
- fonoteka.go and sm-bm-app still build against their own pinned user checkouts: `(cd ../fonoteka.go && go build ./...) && (cd ../sm-bm-app && go build ./...)`. fonoteka.go also builds against this plan's user plugin through the scratch workspace of Task 3's verify.
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- C-CR-01 (user and oauthserver parts), C-CR-02 (plugin part), C-WR-01 and C-WR-02 are fixed; C-IN-01..C-IN-06 are fixed
|
||||
- E-1 and E-2 are delivered as the GAP-INPUTS ask states, including every E-2 acceptance item (proven by the smoke now and the named tests of plan 09)
|
||||
- Every change in the three plugins passes the gate's additive check; plugin go.mod require lines are unchanged
|
||||
- Nothing is tagged, pushed or deployed; the demo's plugin pointer bumps wait for the release
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create `.planning/phases/19.7-generic-mcp-module/19.7-08-SUMMARY.md` when done. Commit it in the framework repository as a planning doc. It contains:
|
||||
- a commit table per repository, mapping each sha to its finding or escalation ids;
|
||||
- the previous and new go.work target in sm-oauthserver-plugin;
|
||||
- the detached submodule HEADs in sm-demo-app;
|
||||
- a "Delivered differently" note for E-1. The plugin serves no frontend organisation payload, so the escalation's "frontend organisation payloads gain the logo" is delivered as `OrganisationLogoURL` and `OrganisationLogoFields`. They give applications the `has_logo`, `logo` and `logo_url` keys for their own organisation payloads. Name this for the sm-servers-app hand-off;
|
||||
- the open OIDC question, with the one-line change that would answer it (the kinds list in `OnAccessRevoked`).
|
||||
|
||||
Release rule: this plan does not tag, push or deploy. Every repository works against the local framework (go.work, or `replace ../summercms.go`), and every plugin's go.mod `require` line stays at its current version. The plans may commit locally in every repository. The releases happen only after phase verification, in this order:
|
||||
1. framework v0.5.0;
|
||||
2. the plugin tags that require it (sm-user-plugin, sm-oauthserver-plugin, sm-journal-plugin);
|
||||
3. the application bumps: the demo's plugin pointers, and pushing vue-demo-app before sm-demo-app.
|
||||
</output>
|
||||
566
.planning/phases/19.7-generic-mcp-module/19.7-09-PLAN.md
Normal file
566
.planning/phases/19.7-generic-mcp-module/19.7-09-PLAN.md
Normal file
@@ -0,0 +1,566 @@
|
||||
---
|
||||
phase: 19.7-generic-mcp-module
|
||||
plan: 09
|
||||
type: execute
|
||||
wave: 4
|
||||
depends_on: ["19.7-06", "19.7-07", "19.7-08"]
|
||||
gap_closure: true
|
||||
files_modified:
|
||||
- scripts/check-phase19.7.sh
|
||||
- modules/pier/reaper_test.go
|
||||
- modules/pier/reserve_test.go
|
||||
- modules/pier/lock_scope_test.go
|
||||
- modules/pier/widening_test.go
|
||||
- modules/pier/stateless_fanout_test.go
|
||||
- modules/pier/personal_binding_test.go
|
||||
- modules/pier/dispatch_hardening_test.go
|
||||
- modules/pier/catalog_path_fields_test.go
|
||||
- modules/pier/prompt_ceiling_test.go
|
||||
- modules/pier/token_events_test.go
|
||||
- modules/pier/export_test.go
|
||||
- modules/bouncer/auth_time_test.go
|
||||
- modules/pact/frontend_token_scopes_test.go
|
||||
- modules/towel/client_ip_test.go
|
||||
- modules/cabana/account_contributions_hardening_test.go
|
||||
- modules/cabana/record_action_prompt_localized_test.go
|
||||
- modules/cabana/token_scope_name_test.go
|
||||
- modules/cabana/account_token_events_test.go
|
||||
- modules/cabana/token_resolver_remote_test.go
|
||||
- admin/tests/app/hotkeysEditing.test.ts
|
||||
- admin/tests/account/AccountContributions.test.ts
|
||||
- ../sm-demo-app/plugins/golem15/user/backend_link_auth_time_test.go
|
||||
- ../sm-demo-app/plugins/golem15/user/backend_link_show_test.go
|
||||
- ../sm-demo-app/plugins/golem15/user/access_revocation_test.go
|
||||
- ../sm-demo-app/plugins/golem15/user/member_token_scopes_test.go
|
||||
- ../sm-demo-app/plugins/golem15/user/organisation_logo_test.go
|
||||
- ../sm-demo-app/plugins/golem15/user/mcp_guard_client_ip_test.go
|
||||
- ../sm-demo-app/plugins/golem15/user/api_token_events_test.go
|
||||
- ../sm-oauthserver-plugin/mcp_revocation_test.go
|
||||
- ../sm-journal-plugin/mcp_prompt_writes_test.go
|
||||
- ../sm-demo-app/mcp_revocation_e2e_test.go
|
||||
- ../sm-demo-app/vue-demo-app/app/lib/linkCode.test.ts
|
||||
- ../sm-demo-app/vue-demo-app
|
||||
- .planning/phases/19.7-generic-mcp-module/19.7-VALIDATION.md
|
||||
- .planning/phases/19.7-generic-mcp-module/19.7-SECURITY.md
|
||||
- .planning/phases/19.7-generic-mcp-module/19.7-SECURITY-REVIEW.md
|
||||
- .planning/phases/19.7-generic-mcp-module/deferred-items.md
|
||||
autonomous: true
|
||||
requirements: [SC-1, SC-2, SC-3, SC-4, D-05, D-08, D-17, D-19, D-20, D-21, D-25, D-26, D-27, D-31, E-1, E-2]
|
||||
|
||||
estimate:
|
||||
tokens: 130000
|
||||
raw_tokens: 130000
|
||||
tasks: 4
|
||||
confidence: low
|
||||
|
||||
must_haves:
|
||||
truths:
|
||||
- "Every fix of plans 06-08 (A-CR-01, A-WR-01..09, A-IN-01..08, B-WR-01..06, B-IN-01..04, C-CR-01, C-CR-02, C-WR-01, C-WR-02, C-IN-01..06) and both escalations have a named test that the phase gate requires by name, and the gate refuses a skipped, zero-match or missing run (D-31: unit tests last)"
|
||||
- "E-2's acceptance list is a test, item by item and verbatim: a plugin declaring servers:write mints a usr_ token with that scope through POST /tokens and a middleware.Scope(\"servers:write\") route accepts it; a token without the scope gets 403 \"Missing required scope: servers:write\"; an undeclared scope answers 422 at mint time; two plugins declaring one name, or a plugin redeclaring read, write or ai, fails boot; the discovery answer lists the built-ins and the declared scopes with labels"
|
||||
- "The gate cannot pass on a local accident: every repository must resolve the framework to this checkout and the OAuth server plugin must resolve the user plugin to USER_PLUGIN, a demo plugin submodule may be ahead of its pin only by commits its canonical checkout holds (unreleased, reported), and PHASE197_RELEASED=1 refuses any drift after the release"
|
||||
- "bash scripts/check-phase19.7.sh --all passes on a Docker host with every stage PASS"
|
||||
- "19.7-SECURITY-REVIEW.md corrects the verdicts the code review disproved (T-19.7-22 first, plus T-19.7-03, -08, -17, -18, -23, -31, -35 and cross-cutting findings 1 and 6 on link proof and logs), records T-19.7-42..T-19.7-65 with implementation, test and status, and shows for the three critical findings that the pinning test fails with the fix reverted; 19.7-SECURITY.md reports threats_open 0"
|
||||
- "19.7-VALIDATION.md has a row for every new behaviour with its final command, and deferred-items.md records every finding not fixed (B-IN-05, B-IN-06) with its reason plus the release order and the local-only wiring"
|
||||
artifacts:
|
||||
- path: "scripts/check-phase19.7.sh"
|
||||
provides: "the extended fail-closed gate with the unreleased-pin policy and workspace checks"
|
||||
contains: "PHASE197_RELEASED"
|
||||
- path: "../sm-demo-app/plugins/golem15/user/member_token_scopes_test.go"
|
||||
provides: "E-2's acceptance list as tests"
|
||||
contains: "Missing required scope: servers:write"
|
||||
- path: "../sm-demo-app/mcp_revocation_e2e_test.go"
|
||||
provides: "the end-to-end revocation matrix"
|
||||
contains: "func TestMCPRevocationEndToEnd("
|
||||
- path: ".planning/phases/19.7-generic-mcp-module/19.7-SECURITY-REVIEW.md"
|
||||
provides: "the corrected review and the gap-closure evidence"
|
||||
contains: "T-19.7-65"
|
||||
key_links:
|
||||
- from: "scripts/check-phase19.7.sh"
|
||||
to: "../sm-demo-app/plugins/golem15/user/member_token_scopes_test.go"
|
||||
via: "USER_TESTS requires TestMemberTokenScopes; USER_SHORT_TESTS requires TestMemberTokenScopesBoot"
|
||||
pattern: "TestMemberTokenScopes"
|
||||
- from: "scripts/check-phase19.7.sh"
|
||||
to: "modules/pier/reaper_test.go"
|
||||
via: "GO_TESTS and GO_TAGGED_TESTS require TestSessionReaper"
|
||||
pattern: "TestSessionReaper"
|
||||
- from: ".planning/phases/19.7-generic-mcp-module/19.7-SECURITY-REVIEW.md"
|
||||
to: "19.7-0{6,7,8}-PLAN.md threat registers"
|
||||
via: "one row per T-19.7-42..65 with implementation and test"
|
||||
pattern: "T-19.7-4"
|
||||
---
|
||||
|
||||
<objective>
|
||||
This is the gap closure's last plan; by project rule, unit tests come last. It:
|
||||
- adds a named test for every fix of plans 06-08 and for both escalations, including E-2's acceptance list verbatim;
|
||||
- extends `scripts/check-phase19.7.sh` so the new tests are required and the gate cannot pass on unreleased plugin drift or a stray local workspace;
|
||||
- adds the `19.7-VALIDATION.md` rows;
|
||||
- corrects `19.7-SECURITY.md` and `19.7-SECURITY-REVIEW.md`, where the code review proved two verdicts wrong: T-19.7-22's link proof was marked verified, and the review's claim that the link code never reaches a log was false. It also records the new threats T-19.7-42..T-19.7-65, among them the E-2 scopes and the `auth_time` claim;
|
||||
- updates `deferred-items.md`.
|
||||
|
||||
**Planner decisions recorded here (P-40, P-41):**
|
||||
- **P-40 (gate policy for the gap closure):**
|
||||
- **Workspace check.** Before running anything in a repository, the gate checks how it resolves the framework (`go list -m -f '{{if .Replace}}{{.Replace.Dir}}{{end}}' git.golem15.com/golem15/summercms`). Every repository must resolve it to this checkout. sm-oauthserver-plugin must also resolve `sm-user-plugin` to `$USER_PLUGIN`. A mismatch is a `refuse:` line naming both paths. This replaces trust in git-ignored go.work files.
|
||||
- **Unreleased pins.** `demo_pins` keeps accepting test-only drift, and also accepts a plugin submodule ahead of its pin when its HEAD descends from the pin and is the HEAD of, or an ancestor of, the canonical checkout: `$USER_PLUGIN`, `$OAUTHSERVER_PLUGIN` or `$JOURNAL_PLUGIN` by submodule name. That case prints `pins: <name> is ahead of its pin by unreleased commits (release after verification)`.
|
||||
- **After the release.** With `PHASE197_RELEASED=1` any drift fails, so the gate proves the bumped pins after the user's release.
|
||||
- **vue-demo-app.** The consumers stage runs the demo site's vitest run with `app/lib/linkCode.test.ts` required (the vitest detector).
|
||||
- **Consumers on the unreleased user plugin.** fonoteka.go and sm-bm-app are also built through a scratch copy of their own go.work that swaps their user checkout for `$USER_PLUGIN` (`consumer_with_user_plugin`), proving that the plugin's changes are additive for its consumers before any tag. Their own go.work files are never edited.
|
||||
- **Docs.** The docs stage checks cooler's two doc comments (A-IN-02).
|
||||
- **P-41 (revert-check, the lesson of T-19.7-22):** for each of the three critical findings, the security review records that the pinning test fails with the fix reverted and passes at HEAD:
|
||||
- A-CR-01 → `TestSessionReaper`;
|
||||
- C-CR-01 → `TestMCPVerifierRevocation`;
|
||||
- C-CR-02 → `TestBackendLinkAuthTime`.
|
||||
|
||||
Method: a temporary `git worktree add` under the session scratch directory at HEAD, then `git revert --no-commit <fix sha>`, run the single test, and record FAIL. Remove the worktree afterwards. Nothing is committed from the worktree.
|
||||
|
||||
Purpose: `/msd-verify-work` has one green gate covering the gap closure, and the security documents state only what a test proves.
|
||||
Output: named tests in six repositories (framework, user plugin, oauthserver, Journal, the demo and vue-demo-app), the extended gate, and the corrected and extended planning documents.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@~/.claude/msd-core/workflows/execute-plan.md
|
||||
@~/.claude/msd-core/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<context>
|
||||
@.planning/PROJECT.md
|
||||
@.planning/STATE.md
|
||||
@.planning/phases/19.7-generic-mcp-module/19.7-GAP-INPUTS.md
|
||||
@.planning/phases/19.7-generic-mcp-module/19.7-REVIEW.md
|
||||
@.planning/phases/19.7-generic-mcp-module/19.7-VALIDATION.md
|
||||
@.planning/phases/19.7-generic-mcp-module/19.7-SECURITY.md
|
||||
@.planning/phases/19.7-generic-mcp-module/19.7-SECURITY-REVIEW.md
|
||||
@.planning/phases/19.7-generic-mcp-module/deferred-items.md
|
||||
@.planning/phases/19.7-generic-mcp-module/19.7-05-SUMMARY.md
|
||||
@.planning/phases/19.7-generic-mcp-module/19.7-06-SUMMARY.md
|
||||
@.planning/phases/19.7-generic-mcp-module/19.7-07-SUMMARY.md
|
||||
@.planning/phases/19.7-generic-mcp-module/19.7-08-SUMMARY.md
|
||||
@CLAUDE.md
|
||||
</context>
|
||||
|
||||
<project_rules>
|
||||
- Tests, the gate and the planning documents only. A production defect found here gets its own `fix(<module or plugin>)` commit in the owning repository, before the test commit, and is named in the SUMMARY (as in 19.6.1-03 and 19.7-05). A test commit never touches a non-test .go file.
|
||||
- Postgres tests:
|
||||
- no `t.Parallel`; unique seeded rows and cleanup;
|
||||
- a Postgres subtest is created only when not `-short` (no `t.Skip`), so the gate's skip refusal stays meaningful;
|
||||
- the user plugin runs with `GOWORK=off`;
|
||||
- Docker through `sudo -u golem` if the socket is denied.
|
||||
- Gate entries are `<package>:<TestName>` and list every required test by name. Detectors refuse skipped, zero-match, "no tests to run" and missing names. Every new detector or policy gets a planted `--self-test` case.
|
||||
- Tests and docs in framework files name no consuming application (the docs stage scans the phase's test files: append every new framework test file to `PHASE_TEST_FILES`). Plugin and demo tests may use `servers:write` and `servers:read`, because those are E-2's acceptance values.
|
||||
- Stage by explicit path in every repository. One logical change per commit, no co-author tags. Planning documents (VALIDATION, SECURITY, SECURITY-REVIEW, deferred-items, SUMMARY) go in their own commits. In sm-demo-app, never stage `main.go` or a plugin submodule pointer.
|
||||
- Every commit is self-contained: the repository builds, vets and passes its `-short` tests at each commit, and the gate script works at each of its commits (the arrays name only tests that exist in that commit or in an earlier one).
|
||||
- Release rule: no tag, push or deploy in this plan.
|
||||
</project_rules>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="tracer">
|
||||
<name>Task 1: Tracer: the gate's workspace check and unreleased-pin policy run end to end, proven by --self-test, with the first gap-closure tests (TestSessionReaper, TestAuthTime) required in the go stage</name>
|
||||
<files>scripts/check-phase19.7.sh, modules/pier/reaper_test.go, modules/pier/export_test.go, modules/bouncer/auth_time_test.go</files>
|
||||
<read_first>
|
||||
- scripts/check-phase19.7.sh in full: arrays, `detect_go`, `detect_vitest`, `go_named`, `go_pkg_named`, `run_self_test`, `run_plugins`, `run_consumers`, `demo_pins`, `run_docker`, `run_docs`, `run_all`
|
||||
- 19.7-08-SUMMARY.md (the oauthserver go.work target, the detached demo submodule HEADs, the new commit shas)
|
||||
- modules/pier/reaper_smoke_test.go and export_test.go (plan 06 seams `ReapNow`, `LocalSessions`, `AdvanceStoreClock`); modules/pier/helpers_test.go (`twoInstances`, `sdkClient`)
|
||||
- modules/bouncer/auth_time_smoke_test.go (plan 07) and mint.go, refresh.go, jwt.go
|
||||
</read_first>
|
||||
<action>
|
||||
**Gate (P-40).**
|
||||
- Add `workspace_check DIR [EXPECT_USER_PLUGIN]`. It runs `go list -m -f '{{if .Replace}}{{.Replace.Dir}}{{end}}' git.golem15.com/golem15/summercms` in DIR (with `GOWORK=off` for the user plugin, as the plugins stage runs it), resolves both sides with `realpath`, and refuses when the answer is not `$ROOT`. For sm-oauthserver-plugin it does the same for `git.golem15.com/golem15/sm-user-plugin` against `$USER_PLUGIN`.
|
||||
- Call it at the top of `run_plugins` for all three plugins, of `run_consumers` for the demo, and of `run_docker` for the user plugin, oauthserver and the demo.
|
||||
- Add `consumer_with_user_plugin APP`. It writes a scratch go.work (under `mktemp -d`, removed on exit) that copies the application's own go.work and rewrites:
|
||||
- every `use` and `replace` path to absolute;
|
||||
- the application's `./plugins/golem15/user` `use` entry to `$USER_PLUGIN`;
|
||||
- a `replace git.golem15.com/golem15/summercms => $ROOT`.
|
||||
|
||||
It checks that `go list -m -f '{{.Dir}}' git.golem15.com/golem15/sm-user-plugin` answers `$USER_PLUGIN`, then runs `go build ./...` with that `GOWORK`.
|
||||
- `run_consumers` runs it for `$FONOTEKA` and `$BM_APP` after their plain builds, proving that the unreleased user plugin compiles with its consumers. The applications' own go.work files are never edited.
|
||||
- Add a self-test case: a planted go.work rewritten as expected.
|
||||
- Extend `demo_pins`:
|
||||
- After the test-only check fails, map the submodule name to its canonical checkout (`plugins/golem15/user` → `$USER_PLUGIN`, `plugins/golem15/oauthserver` → `$OAUTHSERVER_PLUGIN`, `plugins/golem15/journal` → `$JOURNAL_PLUGIN`).
|
||||
- Accept when the submodule HEAD descends from its pin and the canonical checkout's HEAD equals it or descends from it (`git -C <canonical> merge-base --is-ancestor <submodule HEAD> HEAD`). Print the `unreleased commits` line.
|
||||
- With `PHASE197_RELEASED=1`, any drift fails before these checks.
|
||||
- Document `PHASE197_RELEASED` in the header's environment list.
|
||||
- Add `--self-test` cases:
|
||||
- planted `go list` answers for the workspace check (match, mismatch);
|
||||
- planted `check-pins.sh` lines for the three pin outcomes (test-only, unreleased accepted, refused), plus the released-mode refusal.
|
||||
|
||||
**First required tests.**
|
||||
- `modules/pier/reaper_test.go`, `TestSessionReaper` (A-CR-01, A-IN-01, A-IN-06), with subtests:
|
||||
- memory store: idle past TTL → reaped, `ss.Wait` returns, the 404 on the next call, `expired` audit line captured through a test slog handler;
|
||||
- Redis (`twoInstances`, `miniredis.FastForward`): the instance that did not serve the last request keeps its copy only while the record lives;
|
||||
- a store error keeps the copy;
|
||||
- a GET stream whose record vanished is cancelled by the re-check (`SetRecheck`), with no `ended` event delivered;
|
||||
- `serveEphemeral` calls are logged as `ephemeral`.
|
||||
|
||||
Add any seam it needs to export_test.go.
|
||||
- `modules/bouncer/auth_time_test.go`, `TestAuthTime`:
|
||||
- mint carries `auth_time == iat` for both audiences;
|
||||
- `Refresh`, `RefreshAudience` and `RefreshAudienceFor` carry it unchanged across two rotations;
|
||||
- a legacy token without the claim refreshes without one;
|
||||
- `VerifyAuthTime` refuses a bad signature, the wrong audience and an expired token, and accepts a frontend token without `aud` as `VerifyClaims` does;
|
||||
- `ClaimAuthTime == "auth_time"`.
|
||||
- Append `./modules/pier:TestSessionReaper` to `GO_TESTS` and `GO_TAGGED_TESTS`, `./modules/bouncer:TestAuthTime` and the plan 06 and 07 smokes (`./modules/pier:TestPierReaperSmoke`, `./modules/bouncer:TestAuthTimeSmoke`) to `GO_TESTS`, and the new files to `PHASE_TEST_FILES`.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>bash scripts/check-phase19.7.sh --self-test && bash scripts/check-phase19.7.sh --go && bash scripts/check-phase19.7.sh --plugins && bash scripts/check-phase19.7.sh --consumers</automated>
|
||||
<fails_when>non-zero exit, or a stage line printing FAIL, "refuse:" or "missing-named"</fails_when>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `grep -c "PHASE197_RELEASED" scripts/check-phase19.7.sh` prints at least 2 and `grep -c "workspace_check" scripts/check-phase19.7.sh` prints at least 4
|
||||
- `grep -c "unreleased commits" scripts/check-phase19.7.sh` prints at least 1 and `grep -c "consumer_with_user_plugin" scripts/check-phase19.7.sh` prints at least 3 (definition and two calls)
|
||||
- `git -C ../fonoteka.go status --short go.work | wc -l` and `git -C ../sm-bm-app status --short go.work | wc -l` print 0 after the run
|
||||
- `grep -c "TestSessionReaper\|./modules/bouncer:TestAuthTime" scripts/check-phase19.7.sh` prints at least 3
|
||||
- `grep -c "func TestSessionReaper(" modules/pier/reaper_test.go` prints 1 and `grep -c "func TestAuthTime(" modules/bouncer/auth_time_test.go` prints 1
|
||||
- `bash scripts/check-phase19.7.sh --self-test` prints PASS for every planted case, including the workspace and pin policy cases, and exits 0
|
||||
- `PHASE197_RELEASED=1 bash scripts/check-phase19.7.sh --consumers` exits non-zero while the demo's submodules are ahead of their pins (proves strict mode; run once, not part of the verify chain)
|
||||
</acceptance_criteria>
|
||||
<done>The gate verifies where each repository resolves its dependencies, reports unreleased plugin commits honestly and turns strict after the release; the first two gap-closure tests are required by name and green.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 2: Framework and admin SPA tests for every fix of plans 06 and 07</name>
|
||||
<files>modules/pier/reserve_test.go, modules/pier/lock_scope_test.go, modules/pier/widening_test.go, modules/pier/stateless_fanout_test.go, modules/pier/personal_binding_test.go, modules/pier/dispatch_hardening_test.go, modules/pier/catalog_path_fields_test.go, modules/pier/prompt_ceiling_test.go, modules/pier/token_events_test.go, modules/pier/export_test.go, modules/pact/frontend_token_scopes_test.go, modules/towel/client_ip_test.go, modules/cabana/account_contributions_hardening_test.go, modules/cabana/record_action_prompt_localized_test.go, modules/cabana/token_scope_name_test.go, modules/cabana/account_token_events_test.go, modules/cabana/token_resolver_remote_test.go, admin/tests/app/hotkeysEditing.test.ts, admin/tests/account/AccountContributions.test.ts, scripts/check-phase19.7.sh</files>
|
||||
<behavior>
|
||||
- TestSessionReserve (A-WR-03):
|
||||
- 50 concurrent initialize POSTs from one binding open exactly `max_per_binding` sessions on the memory store and on miniredis;
|
||||
- the rest answer 429;
|
||||
- a forced connect failure releases its slot;
|
||||
- a store without `reserver` falls back to Count and Put.
|
||||
- TestLockScope (A-WR-07): a guard provider that blocks in `MCPAllows` during one client's initialize (a channel) does not delay another client's call on an existing session, a DELETE, or a stateless request (each finishes within a short deadline while the first is blocked).
|
||||
- TestScopeWidening (A-WR-04):
|
||||
- a binding first seen with `read` and then with `read write` lists the write tool on the next request on the stateless path and on the stateful path;
|
||||
- adding a backend subject (link) adds the backend tools without any fan-out event;
|
||||
- narrowing still hides the tools.
|
||||
- TestStatelessFanOut (A-WR-05, A-WR-06):
|
||||
- on miniredis an instance that has served only stateless requests receives a revoked event published by the other instance and evicts its entry;
|
||||
- a `subscriptions/listen` stream ends within the re-check interval once its bearer stops resolving;
|
||||
- an LRU eviction at capacity closes the evicted server's sessions.
|
||||
- TestPersonalTokenBinding (A-WR-09): a resolver answering TokenID 0 gives 401 at /mcp; two users' tokens get distinct bindings, and one cannot DELETE the other's session.
|
||||
- TestDispatchHardening (A-WR-01, A-WR-02 runtime, A-WR-08, A-IN-04, A-IN-05, A-IN-08):
|
||||
- `..`, `.`, `a/../b`, an empty segment and `a/b` in a single segment are refused and never reach the handler (counter);
|
||||
- a missing path value is refused;
|
||||
- a provider error with a pgx-like message yields the fixed text, and the message appears only in the captured log;
|
||||
- `9007199254740993` reaches the route unchanged in the path and the query;
|
||||
- the inner request has the outer Host, X-Forwarded-Proto and X-Forwarded-Host and still no Authorization or Cookie;
|
||||
- a frontend tool whose provider answers `Guards: ["backend"]` or a Backend principal is refused.
|
||||
- TestCatalogPathFields (A-WR-02 boot, A-IN-07):
|
||||
- a `path:"id"` field with `omitempty` fails boot naming the field;
|
||||
- the same field without it compiles;
|
||||
- an embedded struct on a GET tool fails boot, and on a POST tool compiles.
|
||||
- TestPromptWriteCeiling (C-IN-01): a `Writes: true` prompt is listed for a read+write grant and hidden from a read-only grant; a prompt without Writes is listed for both.
|
||||
- TestTokenScopedRevoke (B-WR-04, C-IN-02 framework):
|
||||
- a revoked event with TokenID closes only the session opened with that personal token, while another token's session and an OAuth session of the same user stay;
|
||||
- with ClientID only that client's OAuth sessions close;
|
||||
- with neither all close;
|
||||
- the fields survive the Redis round trip between two instances.
|
||||
- TestResolveClientAddress (B-IN-04): a resolver sees `towel.ClientIP` equal to the client address (RemoteAddr host; the trusted-proxy answer through a host that implements ClientIP).
|
||||
- TestFrontendTokenScopes (pact): `ValidTokenScopeName` table (accepts `read`, `servers:write`, `a_b-c:d`; refuses ``, `Read`, `servers:`, `:x`, `a:b:c`, `*`, `with space`); a sample plugin is discovered as `HasFrontendTokenScopes`.
|
||||
- TestTokenScopeNameParity (cabana): cabana's plugin scope validation accepts and refuses exactly the pact table's names.
|
||||
- TestClientIP (towel): round trip, absent value, empty string.
|
||||
- TestAccountContributionURL (cabana, -short, dispatch-marker harness):
|
||||
- `javascript:alert(1)`, `data:text/html,x`, `//evil.example/x` and `vbscript:` are dropped (`url` empty) and logged without the URL;
|
||||
- `https://site/link-admin#code=x` and `/relative` pass.
|
||||
- TestAccountContributionsIsolation (B-WR-06, B-IN-03):
|
||||
- plugin A's Status error gives A `error: true` and empty status while plugin B's section and actions are intact, with status 200;
|
||||
- an Applies error drops only that action;
|
||||
- a dynamic status text is not looked up as a phrase key (no missing-key warning).
|
||||
- TestRecordActionPromptLocalized (B-IN-02): a non-string payload answers 422 with the `action_prompt_invalid` text in en and pl.
|
||||
- TestAccountTokenRevokeEvent (cabana, Postgres): revoking one `g15_` token fires one revoked event whose TokenID is the row id; nothing per-admin.
|
||||
- TestResolveBackendTokenRemote (cabana, Postgres): a failed and a successful resolve log `remote` from `towel.ClientIP`.
|
||||
- Vitest hotkeysEditing.test.ts (B-WR-01, B-WR-02):
|
||||
- a keydown of `?` or Escape from an input inside an open shadow root, and from a custom element in a form (closed root), triggers no shortcut and is not prevented;
|
||||
- Ctrl+S with a TaglistField draft saves the record with the draft tag (the posted body contains it);
|
||||
- Ctrl+Enter does the same before closing.
|
||||
- Vitest AccountContributions.test.ts (extend) (B-WR-03, B-WR-06, B-IN-01):
|
||||
- a `javascript:` URL answer renders no link and never calls `window.open`;
|
||||
- a section with `error: true` shows the section_error text;
|
||||
- a 500 on GET shows the error toast and keeps the earlier sections;
|
||||
- the confirm and stale texts are the myaccount ones.
|
||||
</behavior>
|
||||
<read_first>
|
||||
- The plan 06 and 07 SUMMARYs (final identifiers, seams and any test they adjusted)
|
||||
- modules/pier/helpers_test.go, export_test.go, sessions_test.go, fanout_test.go, dispatch_test.go, catalog_test.go (extend the fakes; never duplicate them)
|
||||
- modules/cabana/account_contributions_test.go, dispatch_marker_test.go, administrators_harness_test.go (the -short marker harness and the Postgres `newScreensEnv`)
|
||||
- admin/tests/app/hotkeys.test.ts, admin/tests/account/AccountContributions.test.ts, admin/tests/form/RecordActions.test.ts, admin/tests/helpers.ts
|
||||
- scripts/check-phase19.7.sh (GO_TESTS, GO_TAGGED_TESTS, DOCKER_TESTS, ADMIN_FILES, PHASE_TEST_FILES)
|
||||
</read_first>
|
||||
<action>
|
||||
Write the tests in the behavior block, under exactly those names, in the files listed. Subtests carry the case names.
|
||||
|
||||
Work red-first where the behaviour is not yet pinned: most pass at once because plans 06 and 07 built the behaviour. A test that fails because the code is wrong gets a `fix(<module>)` commit before the test commit.
|
||||
|
||||
Gate arrays:
|
||||
- `GO_TESTS`: every -short name, as `<package>:<Name>`.
|
||||
- `GO_TAGGED_TESTS`: the pier names.
|
||||
- `DOCKER_TESTS`: `./modules/cabana:TestAccountTokenRevokeEvent` and `./modules/cabana:TestResolveBackendTokenRemote` (their -short part returns before creating Postgres subtests).
|
||||
- `ADMIN_FILES`: `tests/app/hotkeysEditing.test.ts`.
|
||||
- `PHASE_TEST_FILES`: every new framework test file.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>go vet ./... && go test -count=1 -short -race ./modules/pier/... && go test -count=1 -short ./modules/pier/... ./modules/pact/... ./modules/towel/... ./modules/cabana/... ./modules/bouncer/... && bash scripts/check-phase19.7.sh --go && bash scripts/check-phase19.7.sh --admin && bash scripts/check-phase19.7.sh --docs</automated>
|
||||
<fails_when>non-zero exit, a race report, "FAIL" in the output, or a gate line printing FAIL, "refuse:" or "missing-named"</fails_when>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `grep -h "^func Test" modules/pier/reserve_test.go modules/pier/lock_scope_test.go modules/pier/widening_test.go modules/pier/stateless_fanout_test.go modules/pier/personal_binding_test.go modules/pier/dispatch_hardening_test.go modules/pier/catalog_path_fields_test.go modules/pier/prompt_ceiling_test.go modules/pier/token_events_test.go | grep -c "TestSessionReserve\|TestLockScope\|TestScopeWidening\|TestStatelessFanOut\|TestPersonalTokenBinding\|TestDispatchHardening\|TestCatalogPathFields\|TestPromptWriteCeiling\|TestTokenScopedRevoke\|TestResolveClientAddress"` prints 10
|
||||
- `grep -c "func TestFrontendTokenScopes(" modules/pact/frontend_token_scopes_test.go`, `grep -c "func TestClientIP(" modules/towel/client_ip_test.go` and `grep -c "func TestTokenScopeNameParity(" modules/cabana/token_scope_name_test.go` each print 1
|
||||
- `grep -c "TestAccountContributionURL\|TestAccountContributionsIsolation\|TestRecordActionPromptLocalized\|TestAccountTokenRevokeEvent\|TestResolveBackendTokenRemote" scripts/check-phase19.7.sh` prints at least 5
|
||||
- `grep -c "hotkeysEditing.test.ts" scripts/check-phase19.7.sh` prints at least 1
|
||||
- `bash scripts/check-phase19.7.sh --go` and `--admin` exit 0
|
||||
</acceptance_criteria>
|
||||
<done>Every framework and SPA fix of plans 06 and 07 is pinned by a required test; the go, admin and docs stages are green.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 3: Plugin, site and demo tests for plan 08 (E-2's acceptance verbatim, E-1, the link proof, revocation), required by the gate; the whole gate green</name>
|
||||
<files>../sm-demo-app/plugins/golem15/user/backend_link_auth_time_test.go, ../sm-demo-app/plugins/golem15/user/backend_link_show_test.go, ../sm-demo-app/plugins/golem15/user/access_revocation_test.go, ../sm-demo-app/plugins/golem15/user/member_token_scopes_test.go, ../sm-demo-app/plugins/golem15/user/organisation_logo_test.go, ../sm-demo-app/plugins/golem15/user/mcp_guard_client_ip_test.go, ../sm-demo-app/plugins/golem15/user/api_token_events_test.go, ../sm-oauthserver-plugin/mcp_revocation_test.go, ../sm-journal-plugin/mcp_prompt_writes_test.go, ../sm-demo-app/mcp_revocation_e2e_test.go, ../sm-demo-app/vue-demo-app/app/lib/linkCode.test.ts, ../sm-demo-app/vue-demo-app, scripts/check-phase19.7.sh</files>
|
||||
<behavior>
|
||||
- TestMemberTokenScopes (user plugin, Postgres). A test plugin declares `servers:read` and `servers:write`. Each E-2 acceptance item is a subtest whose name quotes it:
|
||||
- "A plugin that declares servers:write can mint a usr_ token with that scope through POST /tokens, and a route with middleware.Scope(\"servers:write\") accepts it": POST `/_user/api/v1/tokens` with `{"name":"agent","scopes":["servers:write"]}` → 201 and a `usr_` token; `PUT /_test/servers/1/components/report` behind `APITokenGuard` and `middleware.Scope("servers:write")` → 200.
|
||||
- "A token without the scope gets 403 \"Missing required scope: servers:write\"": a `["read"]` token on that route → 403 with exactly that message.
|
||||
- "An undeclared scope still answers 422 at mint time": `["servers:delete"]` → 422 with `scopes: ["The selected scope is invalid."]`.
|
||||
- "Listing returns them in scopes": `GET /tokens` shows `["servers:write"]` in the token's `scopes`.
|
||||
- "The discovery answer lists the built-ins and the declared scopes with labels": `GET /tokens/scopes` behind `jwt.auth` → `read`, `write`, `ai` (built_in true, labels translated in en and pl) then `servers:read`, `servers:write` with their labels; no session → 401.
|
||||
- "Scopes are scoped to the application": a second application (app B), booted in the same test binary without the declaring plugin, answers 422 `The selected scope is invalid.` to POST /tokens with `["servers:write"]`. `MintAPITokenFor(appB, ...)` refuses it, and `MintAPITokenFor(appA, ...)` accepts it.
|
||||
- The legacy `MintAPIToken` accepts `servers:write` through the process-wide fallback and refuses a never-declared name. The fallback is cleared with `ResetDeclaredAPITokenScopes()` in `t.Cleanup`, so test order cannot leak declarations.
|
||||
- TestMemberTokenScopesBoot (user plugin, -short): "Two plugins declaring the same scope name … fails boot" (the error names both plugin ids); "a plugin redeclaring read, write or ai fails boot" (one subtest each); a bad name and an empty label fail; a `::` label naming a missing phrase key fails at Boot.
|
||||
- TestBackendLinkAuthTime (user plugin, Postgres, C-CR-02):
|
||||
- login, then request, then refresh, then confirm → 409 stale_session;
|
||||
- request, then login, then confirm → 200;
|
||||
- login, then request, then login again → 200;
|
||||
- a hand-made token without auth_time → 409;
|
||||
- a refreshed token after a post-request login keeps working (auth_time carried);
|
||||
- another user's token → 401.
|
||||
- TestBackendLinkShowPost (user plugin, Postgres, C-WR-01, C-IN-06):
|
||||
- POST show answers like GET (200 data shape, 404 for an unknown, used or expired code), and both carry Cache-Control no-store;
|
||||
- the My account action URL is `<frontend_url>/link-admin#code=<64 hex>` with no `?`;
|
||||
- `golem15.user.backend_links.page_path: /pl/polacz` changes the path;
|
||||
- a value without a leading slash gets one.
|
||||
- TestAccessRevocation (user plugin, Postgres, C-CR-01):
|
||||
- the ban bulk action, ResetPassword, ChangePassword and the admin password set each fire one AccessRevokedEvent in the write's transaction, with the right ids, Before and Reason;
|
||||
- for ResetPassword and the admin password set, the stored `tokens_valid_after` is whole seconds while `Before` is the untruncated instant (`Before` is not before the stored value and has a sub-second part when the clock has one). ChangePassword passes the stored cutoff plus one second;
|
||||
- the ban wrapper fires only for newly banned ids: selecting an already banned user together with an unbanned one fires one event naming only the second;
|
||||
- each fires MCPSubjectRevoked per user only after commit;
|
||||
- a failing listener rolls the password reset back;
|
||||
- `LinkedBackendUser` answers false for a banned linked user and true again after unban.
|
||||
- TestUserMCPGuardClientIP (C-IN-04): `MCPResolvePersonalToken` with `towel.WithClientIP` stamps `last_used_ip`; without it, the column is left as it was.
|
||||
- TestAPITokenRevokeEvent (C-IN-02): `DELETE /tokens/{id}` fires one revoked event with that TokenID.
|
||||
- TestOrganisationLogo (user plugin, Postgres, E-1):
|
||||
- the Organisations form schema has a `logo` fileupload next to `avatar`;
|
||||
- uploading through the admin form API stores a `system_files` row with field `logo`;
|
||||
- `OrganisationLogoFields` answers `has_logo true`, the disk name and a URL;
|
||||
- without a logo it answers `has_logo false` and nils;
|
||||
- the avatar still works;
|
||||
- deleting the organisation removes the logo attachment as it does the avatar.
|
||||
- TestMCPVerifierRevocation (oauthserver, Postgres, C-CR-01):
|
||||
- a token created before `tokens_valid_after` → ErrMCPTokenInvalid;
|
||||
- the same-second case: with `tokens_valid_after` stored as 12:00:05 (whole seconds), a token created at 12:00:05.300 is refused by the verifier and by the refresh rotation even with no revoke listener registered, and a token created at 12:00:06.000 is accepted;
|
||||
- `OnAccessRevoked` with `Before` = 12:00:05.700 revokes the 12:00:05.300 token (untruncated revoke) and leaves one created at 12:00:05.900;
|
||||
- a banned user → ErrMCPTokenInvalid, and valid again after unban for a token created after the cut-off;
|
||||
- the MCP refresh rotation refuses a banned user, a pre-cutoff refresh token and a deleted user.
|
||||
- TestMCPGrantRevocation (oauthserver, Postgres, C-CR-01, C-WR-02, C-IN-02):
|
||||
- AccessRevokedEvent revokes MCP access and refresh tokens created before Before, leaves later ones and OIDC-client tokens;
|
||||
- RemoveLink and a new SetLink revoke only `backend`-scoped MCP tokens of that user and fire revoked events with their ClientIDs after commit;
|
||||
- the authorization revoke route fires with ClientID.
|
||||
- TestJournalPromptWrites (Journal, -short): `draft_post` declares `Writes`; through pier's catalog (the TestJournalMCP harness) a read-only identity's prompts/list omits `journal_draft_post` and a read+write one lists it.
|
||||
- TestMCPRevocationEndToEnd (demo, tagged, Postgres) runs through the SDK client:
|
||||
- (a) a linked user with a `read write backend` grant is banned through the admin's bulk ban route: the next call is 401, the session is gone, and the refresh grant is refused;
|
||||
- (b) unlinking revokes the backend grant: the next call is 401, and a fresh consent without a link has no `backend`;
|
||||
- (c) a read-only grant lists no `journal_draft_post`;
|
||||
- (d) with `mcp.allow_personal_tokens: true`, revoking one `usr_` token ends only its session while the same user's OAuth session keeps working.
|
||||
- linkCode.test.ts (vue-demo-app): `readLinkCode('#code=<64 hex>')` returns it; uppercase, short, extra parameters and an empty hash give ''; `isLinkCode` is exact.
|
||||
</behavior>
|
||||
<read_first>
|
||||
- The plan 08 SUMMARY (final identifiers, commit shas, the go.work target, the submodule HEADs)
|
||||
- The existing harnesses: user plugin backend_link_test.go and api_tokens_test.go (admin and session helpers), mcp_guard_test.go; oauthserver mcp_verifier_test.go and harness_test.go; Journal mcp_test.go (`TestJournalMCP` compiles the declaration through pier); demo mcp_e2e_test.go (SDK client, consent helper, `internal/demotest`), testmain_test.go
|
||||
- The plan 08 smokes (`TestBackendLinkAuthTimeSmoke`, `TestMemberTokenScopesSmoke`, `TestMCPRevocationSmoke`): lift their seeds into the shared helpers of each repository instead of duplicating them
|
||||
- scripts/check-phase19.7.sh (USER_TESTS, OAS_TESTS, JOURNAL_SHORT_TESTS, DEMO_TESTS, `detect_vitest`)
|
||||
- 19.7-GAP-INPUTS.md E-2 "Acceptance" (the subtest names quote it)
|
||||
</read_first>
|
||||
<action>
|
||||
Write the tests in the behavior block under exactly those names, one commit per repository. Fix any production defect first in its own `fix(` commit in the owning repository.
|
||||
|
||||
Gate:
|
||||
- Add a `USER_SHORT_TESTS=(.:TestMemberTokenScopesBoot)` array, run in `run_plugins` with `GOWORK=off go_pkg_named "$USER_PLUGIN" -short`.
|
||||
- Append to `USER_TESTS`: `TestBackendLinkAuthTime`, `TestBackendLinkShowPost`, `TestAccessRevocation`, `TestMemberTokenScopes`, `TestOrganisationLogo`, `TestUserMCPGuardClientIP`, `TestAPITokenRevokeEvent`, `TestBackendLinkAuthTimeSmoke`, `TestMemberTokenScopesSmoke`.
|
||||
- Append to `OAS_TESTS`: `TestMCPVerifierRevocation`, `TestMCPGrantRevocation`, `TestMCPRevocationSmoke`.
|
||||
- Append to `JOURNAL_SHORT_TESTS`: `.:TestJournalPromptWrites`.
|
||||
- Append to `DEMO_TESTS`: `TestMCPRevocationEndToEnd`.
|
||||
- Add a `VUE_FILES=(app/lib/linkCode.test.ts)` array. `run_consumers` runs `pnpm --dir "$DEMO_APP/vue-demo-app" run test` through `detect_vitest` with those files required.
|
||||
- Add the docs-stage cooler check: `go doc ./modules/cooler KeyPrefix` line 4 starts with `KeyPrefix returns`, and `go doc -u ./modules/cooler keyPrefix` shows its own comment.
|
||||
|
||||
In sm-demo-app:
|
||||
- Commit the e2e test only.
|
||||
- Commit `app/lib/linkCode.test.ts` in vue-demo-app.
|
||||
- Commit the vue-demo-app pointer bump separately, staging only `vue-demo-app`.
|
||||
|
||||
Then run `bash scripts/check-phase19.7.sh --all` on a Docker host.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>bash scripts/check-phase19.7.sh --all</automated>
|
||||
<fails_when>non-zero exit, any stage printing FAIL, "refuse:" or "missing-named", or "NOT RUN docker" while a Docker daemon is reachable (run on a Docker host, through sudo -u golem if the socket is denied)</fails_when>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `grep -c "Missing required scope: servers:write" ../sm-demo-app/plugins/golem15/user/member_token_scopes_test.go` prints at least 1, and `grep -c "The selected scope is invalid." ../sm-demo-app/plugins/golem15/user/member_token_scopes_test.go` prints at least 1
|
||||
- `grep -c "func TestMemberTokenScopes(\|func TestMemberTokenScopesBoot(" ../sm-demo-app/plugins/golem15/user/member_token_scopes_test.go` prints 2
|
||||
- `grep -c "func Test" ../sm-demo-app/plugins/golem15/user/backend_link_auth_time_test.go ../sm-demo-app/plugins/golem15/user/backend_link_show_test.go ../sm-demo-app/plugins/golem15/user/access_revocation_test.go ../sm-demo-app/plugins/golem15/user/organisation_logo_test.go ../sm-demo-app/plugins/golem15/user/mcp_guard_client_ip_test.go ../sm-demo-app/plugins/golem15/user/api_token_events_test.go` prints at least 1 per file
|
||||
- `grep -c "func TestMCPVerifierRevocation(\|func TestMCPGrantRevocation(" ../sm-oauthserver-plugin/mcp_revocation_test.go` prints 2; `grep -c "func TestJournalPromptWrites(" ../sm-journal-plugin/mcp_prompt_writes_test.go` prints 1; `grep -c "func TestMCPRevocationEndToEnd(" ../sm-demo-app/mcp_revocation_e2e_test.go` prints 1
|
||||
- `grep -c "USER_SHORT_TESTS\|VUE_FILES" scripts/check-phase19.7.sh` prints at least 4
|
||||
- `bash scripts/check-phase19.7.sh --all` exits 0 on a Docker host with PASS for self-test, go, plugins, consumers, admin, docs, marker and docker
|
||||
</acceptance_criteria>
|
||||
<done>Every plugin, site and demo fix of plan 08 and both escalations are pinned by required tests (E-2 item by item), and the whole phase gate is green.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 4: Validation rows, the corrected and extended security review and contract (revert-checks for the critical findings), deferred items</name>
|
||||
<files>.planning/phases/19.7-generic-mcp-module/19.7-VALIDATION.md, .planning/phases/19.7-generic-mcp-module/19.7-SECURITY.md, .planning/phases/19.7-generic-mcp-module/19.7-SECURITY-REVIEW.md, .planning/phases/19.7-generic-mcp-module/deferred-items.md</files>
|
||||
<read_first>
|
||||
- 19.7-SECURITY-REVIEW.md (Method, Gaps fixed, Threat register rows T-19.7-03, -08, -17, -18, -22, -23, -31, -35, Cross-cutting findings 1 and 6, Accepted risks, Residual items) and 19.7-SECURITY.md (frontmatter, register, Accepted Risks Log, Audit Trail)
|
||||
- The `<threat_model>` blocks of 19.7-06, 19.7-07, 19.7-08 and this plan (T-19.7-42..T-19.7-65)
|
||||
- 19.7-REVIEW.md (finding ids for the corrections) and the 06, 07 and 08 SUMMARYs (commit shas per finding)
|
||||
- 19.7-VALIDATION.md (row format) and deferred-items.md (entry format, open items to update)
|
||||
</read_first>
|
||||
<action>
|
||||
**Revert-checks (P-41).** For each pair below:
|
||||
1. Create a scratch worktree of the owning repository at HEAD under the session scratch directory: `git -C <repo> worktree add <scratch>/<name> HEAD`.
|
||||
2. Run `git revert --no-commit <fix sha>` there.
|
||||
3. Run the single test: framework with `go test`; user plugin with `GOWORK=off`. The user plugin's relative replace resolves only from its real location, so in its worktree use `go mod edit -replace` to the absolute framework path (never committed), or `go work` in the scratch directory.
|
||||
4. Record FAIL with the failing line.
|
||||
5. Remove the worktree (`git worktree remove --force`).
|
||||
|
||||
The pairs:
|
||||
- A-CR-01 → `TestSessionReaper`;
|
||||
- C-CR-01 → `TestMCPVerifierRevocation` (the oauthserver verifier fix);
|
||||
- C-CR-02 → `TestBackendLinkAuthTime`: revert the user plugin confirm change. The framework auth_time cannot be "reverted" from the plugin worktree, so revert the plugin commit only.
|
||||
|
||||
At HEAD all three pass (Task 3's gate run).
|
||||
|
||||
**19.7-SECURITY-REVIEW.md:** add a section "Gap closure (plans 06-09, 2026-10-23)" with:
|
||||
- **Corrections**, a table of each earlier verdict the code review disproved: threat id, earlier verdict, finding, what was wrong, fix commit(s), pinning test. The rows:
|
||||
- T-19.7-22: the link proof compared `iat`, which refresh renews (C-CR-02); it was wrongly marked verified.
|
||||
- Cross-cutting finding 1: the link proof.
|
||||
- Cross-cutting finding 6: the claim that the link code never reaches a log was false; it reached nginx and site request logs and browser history (C-WR-01). Describe the old claim in your own words; do not quote its sentence.
|
||||
- T-19.7-23: code leakage.
|
||||
- T-19.7-03 and T-19.7-17: the mux cleaning was not part of the in-process chain (A-WR-01, A-WR-02).
|
||||
- T-19.7-18: the session leak and the cap race (A-CR-01, A-WR-03).
|
||||
- T-19.7-08: the `javascript:` URL sink (B-WR-03).
|
||||
- T-19.7-31: grant re-targeting after re-linking (C-WR-02).
|
||||
- T-19.7-35: grants surviving ban and reset (C-CR-01).
|
||||
- Rewrite cross-cutting findings 1 and 6 in place with the corrected text and a "corrected in gap closure" marker.
|
||||
- **Revert-checks:** a table with repository, fix sha, test, FAIL line with the fix reverted, PASS at HEAD.
|
||||
- **New threats:** register rows for T-19.7-42..T-19.7-65 in the existing evidence-table format (Mitigation as built, Implementation file:line, Test, Status). Each has a status of verified or fixed in <sha>. The E-2 rows (T-19.7-52, T-19.7-60) and the auth_time rows (T-19.7-51, plus T-19.7-22's correction) cite the decision note and the E-2 acceptance tests.
|
||||
- **Method:** add one step: a high or critical control is "verified" only with a test that fails without it; for the three critical findings this is shown by revert-check.
|
||||
|
||||
**19.7-SECURITY.md:**
|
||||
- The register gains T-19.7-42..T-19.7-65.
|
||||
- The corrected rows get their new status ("fixed in <sha>").
|
||||
- The Audit Trail gets a gap-closure entry.
|
||||
- `threats_open` stays 0 only if every high threat is verified or fixed. Otherwise set the real count and stop with a blocking note in the SUMMARY.
|
||||
|
||||
**19.7-VALIDATION.md:** one row per new behaviour (every test name of Tasks 1-3) with its final command, File Exists ✅ and Status ✅ green after the Task 3 gate run. Keep `nyquist_compliant: true`.
|
||||
|
||||
**deferred-items.md:**
|
||||
- New entries with reasons:
|
||||
- B-IN-05: the prompt dialog closes on a 422. The contract text was corrected in plan 07; keeping the dialog open changes the shared confirm primitive used by every action.
|
||||
- B-IN-06: no fix required; the visible link is the popup-blocker fallback.
|
||||
- OIDC grants on ban or password reset: an open question to the user; plan 08 revokes MCP grants only. Record whether OIDC refresh already refuses banned users (evidence). Also record that the extension is one line: add the OIDC client kind to the `kinds` list `Cascade.OnAccessRevoked` passes to `revokeUserGrants`, with a pinning test to add in TestMCPGrantRevocation.
|
||||
- The local oauthserver go.work retarget.
|
||||
- The demo's oauthserver and journal submodules detached at unreleased commits, plus the user submodule pointer, all waiting for the release.
|
||||
- The release order:
|
||||
1. framework v0.5.0;
|
||||
2. sm-user-plugin, sm-oauthserver-plugin and sm-journal-plugin tags requiring it;
|
||||
3. the demo, fonoteka.go and sm-bm-app bumps, with vue-demo-app pushed before sm-demo-app;
|
||||
4. sm-servers-app picks up E-1 and E-2 after the user plugin tag;
|
||||
5. then `PHASE197_RELEASED=1 bash scripts/check-phase19.7.sh --consumers`.
|
||||
- Update the status of earlier entries the gap closure resolved or superseded, for example the plan 05 test commits that are now part of the gap-closure release.
|
||||
|
||||
Commit each document as a planning doc.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>grep -q "threats_open: 0" .planning/phases/19.7-generic-mcp-module/19.7-SECURITY.md && for n in $(seq 42 65); do grep -q "T-19.7-$n" .planning/phases/19.7-generic-mcp-module/19.7-SECURITY-REVIEW.md && grep -q "T-19.7-$n" .planning/phases/19.7-generic-mcp-module/19.7-SECURITY.md || { echo "missing T-19.7-$n"; exit 1; }; done && grep -q "Revert-check" .planning/phases/19.7-generic-mcp-module/19.7-SECURITY-REVIEW.md && grep -q "B-IN-05" .planning/phases/19.7-generic-mcp-module/deferred-items.md && grep -q "TestMemberTokenScopes" .planning/phases/19.7-generic-mcp-module/19.7-VALIDATION.md && bash scripts/check-phase19.7.sh --all</automated>
|
||||
<fails_when>non-zero exit, a "missing T-19.7-NN" line, a missing section or row, or any gate stage printing FAIL, "refuse:" or "missing-named"</fails_when>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- Every id T-19.7-42 through T-19.7-65 appears in both security documents, and each row in 19.7-SECURITY-REVIEW.md names a file:line and a test and has the status verified or fixed in <sha>
|
||||
- The T-19.7-22 row of 19.7-SECURITY-REVIEW.md no longer has the bare status `verified`: `grep "^| T-19.7-22 " .planning/phases/19.7-generic-mcp-module/19.7-SECURITY-REVIEW.md | grep -c "fixed in"` prints 1
|
||||
- `grep -c "is never logged" .planning/phases/19.7-generic-mcp-module/19.7-SECURITY-REVIEW.md` prints 0 (the false claim is rewritten, not quoted)
|
||||
- The Revert-checks table has three rows, each with a FAIL line and a commit sha that exists (`git -C <repo> cat-file -t <sha>` prints commit)
|
||||
- `grep -c "TestSessionReaper\|TestAuthTime\|TestMemberTokenScopes\|TestBackendLinkAuthTime\|TestMCPVerifierRevocation\|TestMCPRevocationEndToEnd" .planning/phases/19.7-generic-mcp-module/19.7-VALIDATION.md` prints at least 6
|
||||
- `grep -c "B-IN-05\|B-IN-06\|PHASE197_RELEASED" .planning/phases/19.7-generic-mcp-module/deferred-items.md` prints at least 3
|
||||
</acceptance_criteria>
|
||||
<done>The security documents state only what tests prove, correct the earlier false verdicts, cover every new threat, and show the critical fixes failing without their code; the validation map and deferred items match the gap closure; the gate is green after the documentation commits.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<artifacts_this_phase_produces>
|
||||
## Artifacts this plan produces
|
||||
|
||||
**Gate:** `scripts/check-phase19.7.sh` with:
|
||||
- `workspace_check`;
|
||||
- the unreleased-pin policy and `PHASE197_RELEASED` strict mode;
|
||||
- the `USER_SHORT_TESTS` and `VUE_FILES` arrays;
|
||||
- the cooler doc check;
|
||||
- every new required name.
|
||||
|
||||
**Framework tests:**
|
||||
- pier: `TestSessionReaper`, `TestSessionReserve`, `TestLockScope`, `TestScopeWidening`, `TestStatelessFanOut`, `TestPersonalTokenBinding`, `TestDispatchHardening`, `TestCatalogPathFields`, `TestPromptWriteCeiling`, `TestTokenScopedRevoke`, `TestResolveClientAddress`;
|
||||
- bouncer: `TestAuthTime`;
|
||||
- pact: `TestFrontendTokenScopes`;
|
||||
- towel: `TestClientIP`;
|
||||
- cabana: `TestTokenScopeNameParity`, `TestAccountContributionURL`, `TestAccountContributionsIsolation`, `TestRecordActionPromptLocalized`, `TestAccountTokenRevokeEvent`, `TestResolveBackendTokenRemote`;
|
||||
- vitest: `admin/tests/app/hotkeysEditing.test.ts`, plus the extended `AccountContributions.test.ts`.
|
||||
|
||||
**Plugin, site and demo tests:**
|
||||
- sm-user-plugin: `TestMemberTokenScopes`, `TestMemberTokenScopesBoot`, `TestBackendLinkAuthTime`, `TestBackendLinkShowPost`, `TestAccessRevocation`, `TestUserMCPGuardClientIP`, `TestAPITokenRevokeEvent`, `TestOrganisationLogo`;
|
||||
- sm-oauthserver-plugin: `TestMCPVerifierRevocation`, `TestMCPGrantRevocation`;
|
||||
- sm-journal-plugin: `TestJournalPromptWrites`;
|
||||
- sm-demo-app: `TestMCPRevocationEndToEnd`;
|
||||
- vue-demo-app: `app/lib/linkCode.test.ts`.
|
||||
|
||||
**Documents:**
|
||||
- `19.7-VALIDATION.md` rows;
|
||||
- `19.7-SECURITY.md` and `19.7-SECURITY-REVIEW.md`: the corrections, T-19.7-42..65, and the revert-checks;
|
||||
- `deferred-items.md`: deferrals, the release order and the local-only wiring.
|
||||
|
||||
**Decisions:** P-40 and P-41.
|
||||
</artifacts_this_phase_produces>
|
||||
|
||||
<threat_model>
|
||||
## Trust Boundaries
|
||||
|
||||
| Boundary | Description |
|
||||
|----------|-------------|
|
||||
| gate script to six repositories | the script decides the phase verdict from test output across repositories and local workspace files |
|
||||
| security documents to the release decision | v0.5.0 is tagged on the strength of these documents |
|
||||
|
||||
## STRIDE Threat Register
|
||||
|
||||
| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan |
|
||||
|-----------|----------|-----------|----------|-------------|-----------------|
|
||||
| T-19.7-64 | Repudiation | a green gate built on a stray local workspace or on unreleased plugin drift | medium | mitigate | `workspace_check` refuses a repository resolving the framework or the user plugin elsewhere; unreleased drift is accepted only when the canonical checkout holds the commits, and is reported; `PHASE197_RELEASED=1` refuses any drift after the release; self-test cases for each outcome |
|
||||
| T-19.7-65 | Repudiation | a security document marking a control verified without a test that fails without it (the T-19.7-22 failure) | high | mitigate | revert-checks for the three critical findings recorded with FAIL and PASS evidence; the Method step requiring a failing-without-fix test for high and critical controls; the Corrections table for every disproved verdict |
|
||||
| T-19.7-SC | Tampering | package installs | low | mitigate | no package added; vitest runs use the committed locks (`npm --prefix admin ci` when needed, vue-demo-app's existing node_modules or `pnpm install --frozen-lockfile`) |
|
||||
</threat_model>
|
||||
|
||||
<verification>
|
||||
- `bash scripts/check-phase19.7.sh --all` on a Docker host: PASS for self-test, go, plugins, consumers, admin, docs, marker and docker
|
||||
- `bash scripts/check-phase19.6.1.sh --all` still passes (the earlier gate is not broken by the framework changes)
|
||||
- The framework's `go vet ./... && go test -count=1 ./...` with Docker, and `cd examples/hello && go test ./...`
|
||||
- `19.7-SECURITY.md` has `threats_open: 0`; `19.7-SECURITY-REVIEW.md` covers T-19.7-01..65 and T-19.7-SC, with the Corrections and Revert-checks sections
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- Unit tests are the last plan, and every gap-closure fix and both escalations have required, named tests (E-2's acceptance verbatim)
|
||||
- The gate is fail-closed against workspace and pin accidents, and green
|
||||
- The security review is corrected and extended, with revert-check evidence for the critical findings; the validation map and deferred items are current
|
||||
- No tag, push or deploy
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create `.planning/phases/19.7-generic-mcp-module/19.7-09-SUMMARY.md` when done, with a commit table per repository, the gate output of the final `--all` run, and the revert-check results. Commit the SUMMARY separately as a planning doc. The SUMMARY also names the g15office follow-up for the orchestrator's phase-advance step (CLAUDE.md rule 4).
|
||||
|
||||
Release rule: this plan does not tag, push or deploy. Every repository works against the local framework (go.work, or `replace ../summercms.go`), and every plugin's go.mod `require` line stays at its current version. The plans may commit locally in every repository. After the re-review, phase verification, Nyquist validation and UAT pass (user decision 2026-10-23), the releases happen in this order:
|
||||
1. framework v0.5.0;
|
||||
2. the sm-user-plugin, sm-oauthserver-plugin and sm-journal-plugin tags that require it;
|
||||
3. the application bumps;
|
||||
4. `PHASE197_RELEASED=1 bash scripts/check-phase19.7.sh --consumers` to confirm the pins.
|
||||
</output>
|
||||
Reference in New Issue
Block a user