golem15/summercms
4
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
ea4680eebe4e828db2f258087c3c2eb709f85622
summercms/.planning/phases/19.7-generic-mcp-module/19.7-REREVIEW.md

55 KiB
Raw Blame History

phase, reviewed, depth, kind, lanes, files_reviewed, findings, status
phase reviewed depth kind lanes files_reviewed findings status
19.7-generic-mcp-module 2026-10-23 deep re-review of the gap closure (plans 06-09)
A framework MCP core
B cabana and admin SPA
C plugins and demo
129
critical warning info total
1 10 17 28
issues_found

Phase 19.7 re-review of the gap closure

This re-reviews the changes made by gap-closure plans 06-09, after the first deep review in 19.7-REVIEW.md. That file is left unchanged because the plans and security documents cite its finding ids. Three deep lanes ran in parallel over: the framework since fef1cbb, sm-user-plugin since 32f9259^, sm-oauthserver-plugin since 671513d^, sm-journal-plugin since b129902^, vue-demo-app since 0125e29^ and sm-demo-app since 00f57bf^. Finding ids carry the lane prefix (RA-, RB-, RC-). Each lane also rules on every earlier critical and warning finding of its lane ("Earlier findings").

Lane Critical Warning Info Earlier findings not fully closed
A framework MCP core 0 6 6 A-WR-07 partial; A-WR-04 and A-WR-06 fixed with regressions
B cabana and admin SPA 0 2 4 B-WR-02 partial
C plugins and demo 1 2 7 C-CR-01 partial, C-WR-02 partial

Lane A

Phase 19.7 (lane A re-review): Code Review Report

Reviewed: 2026-10-23 Depth: deep Files Reviewed: 45 Status: issues_found

Summary

I re-reviewed lane A's gap-closure changes (git diff fef1cbb..HEAD) and the code they touch: the session reaper, the atomic per-binding reservation (memory and Redis Lua), the lock-scope rework, fingerprint-based widening, the stateless listen re-check and eviction close, the Redis warm-up, the narrowed revocation events, the dispatch hardening, the catalog boot checks, bouncer's auth_time, towel's client address and the gate script. I traced the calls into go-sdk v1.8.0 (Server.Sessions, changeAndNotify/notifySessions), into cabana (fireTokenRevoked, ResolveBackendToken, BackendPrincipal) and into the publishers of pact.MCPSubjectChanged in the OAuth server and user plugins. go vet -tags summercms_mcp on pier, surf and bouncer is clean. go test -race -short ./modules/pier passes.

Every earlier lane-A critical and warning has a fix in the code, not only in the plans. The reaper closes expired local copies and logs expired. Reserve is atomic in both stores. Path values are cleaned and required. Raw dispatch errors are logged, not returned. Personal tokens without TokenID are refused. The Redis store is warmed at mount. Eviction closes the evicted server's sessions. I found no identity or guard bypass. Widening adds tools to a cached server, but the receiving middleware still filters every list and call for the requesting identity, so no request gains reach it does not have. Revocations narrowed by TokenID and ClientID match the events cabana and the plugins publish. The ClientID == "" condition stops an admin-token revoke from also ending an OAuth session that acts as the same admin.

The new defects are in the concurrency and caching that the fixes introduced:

  • A-WR-07 is only partly fixed. Event fan-out still takes each per-session lock while it holds the router-wide lock. Widening and recomputing hold that per-session lock across guard I/O, and widening runs with no deadline. One slow check plus any event still stalls the instance.
  • The fingerprint includes the OAuth access-token id. Every token rotation re-filters the session. A stateless server shared by two live tokens of one binding flips its tool set on every request, which races calls into "unknown tool" errors.
  • A transient guard error during widening removes tools from a live session and records the new fingerprint, so the tools stay gone until the next rotation or event.
  • Capacity eviction now closes live listen streams and in-flight calls. With more concurrent bindings than the LRU capacity, the stateless path keeps forcing clients to reconnect.
  • Prompt Render errors still reach the client raw (the same class as A-WR-08).
  • The gate passes with demo submodules ahead of their go.mod pins by non-test commits unless PHASE197_RELEASED=1. The final "gate passed" line gives no caveat.

Earlier findings

Id Verdict Evidence
A-CR-01 fixed modules/pier/pier.go:143-148 starts reap. modules/pier/session.go:483-531: reapOnce snapshots sr.local, calls store.Get for each entry, then closeLocal and logs expired. A store error keeps the copy. reapInterval (:535-537) is between 1 s and 1 min. The memory store's Get honours expiry (store_memory.go:44-53). The Redis path relies on PX TTLs. reaper_test.go covers both stores. The reaper ctx is never cancelled in production (no shutdown hook, documented at pier.go:63-67).
A-WR-01 fixed modules/pier/dispatch.go:119-128 refuses empty, . and .. segments in every path value, and / in a single-segment value. Wildcard values are escaped per segment (:376-383).
A-WR-02 fixed Boot: modules/pier/catalog.go:220-224 refuses a path field that is not in schema.Required. Runtime: dispatch.go:142-148 refuses a missing path value.
A-WR-03 fixed modules/pier/session.go:143-173 reserves before connecting and releases on a failed connect or initialize. The memory Reserve counts and writes under m.mu (store_memory.go:101-117). The Redis reserveScript prunes, counts, SETs and SADDs in one EVAL (store_redis.go:144-182). lazyStore.Reserve forwards to it (:326-339). The non-reserver fallback (session.go:156-166, :187-193) is still check-then-act, but only a test-wrapped store reaches it (RA-IN-04).
A-WR-04 fixed, with regressions modules/pier/events.go:131-141 (widen) is called from session.go:332-335 and stateless.go:185-190. The fingerprint is identity.go:85-116. The new defects are RA-WR-02 and RA-WR-03.
A-WR-05 fixed modules/pier/pier.go:146-148 runs ls.warm at mount with a backoff (store_redis.go:279-294). route kicks the open on every request (stateless.go:248-250). open subscribes with context.Background() (pier.go:178).
A-WR-06 fixed, with a regression The listen re-check is at modules/pier/stateless.go:98-123. Capacity eviction closes the evicted server's sessions (:151-167). The new defect is RA-WR-04.
A-WR-07 partially fixed connect now runs without sr.mu (session.go:174-186, :308-331), and the stateless build runs without sp.mu (stateless.go:137-150). But matchingFunc still locks each ls.mu/e.mu while holding sr.mu/sp.mu (events.go:190-203, stateless.go:205-218), and widen/recomputeSession hold ls.mu across guard I/O (session.go:332-335, events.go:98-109, :138). See RA-WR-01.
A-WR-08 fixed (dispatch) modules/pier/dispatch.go:179-189 logs the cause and returns a fixed text. Only ErrMCPTokenInvalid and errSubjectUnavailable (backend_guard.go:73-86) get specific texts. The prompt path still returns raw errors (RA-WR-05).
A-WR-09 fixed modules/pier/identity.go:267-272 refuses TokenID == 0. The contract is documented at pact/mcp.go:241-249.

Warnings

RA-WR-01: Event fan-out still nests per-session locks under the router-wide lock, while those locks are held across guard I/O with no deadline (A-WR-07 not closed)

File: modules/pier/events.go:190-203, :98-109, :131-141; modules/pier/stateless.go:205-218, :185-190, :171-181; modules/pier/session.go:295-297, :332-335 Issue: The fix moved connect out of sr.mu, but it did not change the lock order. sessionRouter.matchingFunc takes sr.mu and then each ls.mu in turn. statelessPath.matchingFunc does the same with sp.mu and e.mu. Meanwhile:

  • lookup holds ls.mu across widen, which calls catalog.allowed (database-backed MCPAllows/MCPAllowsMethod).
  • recomputeSession and recomputeStateless hold ls.mu/e.mu across refreshBackend (the link resolver) and catalog.allowed.
  • widen uses context.WithoutCancel(ctx), which removes the request's deadline as well as its cancellation. recomputeSession uses context.Background().

Failure scenario:

  1. Session S's request widens after a token rotation (the fingerprint changed, see RA-WR-02), or an event is recomputing S, while the database is slow.
  2. A second event arrives. The memory store runs every event on its own goroutine (store_memory.go:127-135), so two events in flight at once is normal: one role change fires one event per user.
  3. That event's matchingFunc holds sr.mu and blocks on S.ls.mu.
  4. Every stateful request on the instance now blocks: lookup at session.go:295, serveNew at :184, closeLocal, the reaper and the connect goroutine. The stateless path behaves the same way through sp.mu and hit().
  5. The guard check has no deadline. If the database waits on an exhausted pool, the stall lasts until a connection frees. The endpoint is down instance-wide.

TestLockScope only blocks an initialize, which no longer holds a lock. It never combines a blocked widen or recompute with an event, so it does not cover this path. Fix: Never wait on a per-session lock while holding the router lock. Snapshot the entries under sr.mu, release it, then evaluate the match:

func (sr *sessionRouter) matchingFunc(match func(Identity) bool) []*localSession {
	sr.mu.Lock()
	all := make([]*localSession, 0, len(sr.local))
	for _, ls := range sr.local {
		all = append(all, ls)
	}
	sr.mu.Unlock()
	var out []*localSession
	for _, ls := range all {
		ls.mu.Lock()
		hit := match(ls.identity)
		ls.mu.Unlock()
		if hit {
			out = append(out, ls)
		}
	}
	return out
}

Do the same in statelessPath.matchingFunc. Alternatively, keep identity behind its own small mutex or an atomic.Pointer[Identity] that is never held across I/O. Also give widen and the recomputes a bounded context, for example context.WithTimeout(context.WithoutCancel(ctx), d.cfg.CallTimeout). Extend TestLockScope with a guard that blocks inside widen, plus a concurrent MCPSubjectChanged, and assert that another session's call still finishes.

RA-WR-02: The fingerprint includes the OAuth access-token id, so a shared stateless server flips its tool set between two live tokens of one binding

File: modules/pier/identity.go:85-116 (TokenID at :103), :216-233; modules/pier/stateless.go:128-136, :185-190; modules/pier/events.go:131-141 Issue: For an OAuth identity, Frontend.TokenID and Backend.TokenID hold grant.TokenID, the access-token row id (sm-oauthserver-plugin/classes/mcp_verifier.go:110). It changes with every refresh. The stateless LRU keeps one server per binding (oauth:<client>:<user>), and every request runs refresh → widen with that request's identity. When two different tokens of the same binding are live at once, the fingerprint differs on every alternate request. Two cases are common:

  • The same static client signed in on two devices.
  • After a re-consent with wider scopes, an older narrower token is still valid on another device.

Each flip runs catalog.allowed (guard I/O) under e.mu (feeding RA-WR-01). When the scopes differ, applyDiff adds and then removes the write tools on every alternation. Failure scenario: Token A (read write) calls create_post. The receiving middleware checks A's own allowed set and passes the call. Meanwhile a request with token B (read) runs widen on the same server and calls RemoveTools("create_post"). The SDK's tools/call handler then answers "unknown tool" for A's legitimate call. Every flip also queues a list_changed notification for every listen stream of the binding. Before the gap closure the tool set was frozen. Now it flaps. Fix:

  • Leave token ids out of the fingerprint. The guard decision depends on scopes, subjects and the ceiling, not on which token row carried them.
  • Key the stateless LRU by binding + "|" + fingerprint, as the earlier review suggested, so identities with different reach never share or mutate one server.
  • At minimum, never remove tools in widen on the stateless path: the receiving middleware already filters down per request.

RA-WR-03: A transient guard error during widening permanently narrows a live session

File: modules/pier/events.go:131-141; modules/pier/filter.go:33-47, :49-64 Issue: catalog.allowed turns a provider error into "not allowed" (logged). widen then runs applyDiff, which removes every tool or prompt whose check errored, and stores the new fingerprint. Later requests with the same fingerprint skip widen. The receiving middleware can only filter down, so the removed tools cannot come back. They stay hidden until the fingerprint changes again or a subject event recomputes the session. Server construction already had a weaker form of this, but it ran once. Because of RA-WR-02, widening now runs on every OAuth token rotation. Failure scenario: A pool timeout during the widen at a token rotation hides every backend tool of a long-running agent session for a whole access-token lifetime. Calls to those tools fail as "not available to this caller" although nothing changed. Fix: Let widen tell an error apart from a refusal. For example, add an allowedStrict that returns an error, and on error leave the server untouched and leave *fp unchanged, so the next request retries:

tools, prompts, err := p.catalog.allowedStrict(bounded, id)
if err != nil {
	logger().Warn("pier: widening deferred", "error", err)
	return // keep *fp, retry on the next request
}
applyDiff(...)
*fp = next

RA-WR-04: LRU capacity eviction now ends live listen streams and in-flight calls, so the stateless path churns past capacity

File: modules/pier/stateless.go:151-167, :192-197, :55-62 Issue: A subscriptions/listen POST calls getServer once, when it starts. A quiet listener never touches its entry again, so the entry drifts to the back of the LRU (capacity max_per_binding * 16, 320 by default). The A-WR-06 fix makes eviction call closeSessions on the evicted server. That ends the listen stream, and it also ends any in-flight tools/call of that binding: the per-request ServerSession is in srv.Sessions(), and closing its connection cancels the dispatched route's context mid-handler. Failure scenario: More than 320 distinct bindings are connected with listen streams. Each new binding's first request evicts the oldest listener. That client reconnects and evicts the next one, and so on. Every stream keeps reconnecting, list changes sent during the gaps are lost, and a write tool call caught by an eviction is cancelled partway, so the agent gets an error for a request whose effect may or may not have committed. Fix:

  • Do not count entries with an open listen stream as evictable. For example, keep a per-entry count of active listens, raised in ServeHTTP around sp.handler.ServeHTTP for listen requests, and skip such entries when evicting.
  • Or move such an entry to the front on every bearer re-check tick.
  • Or leave the streams open on eviction (the 1-minute bearer re-check already bounds them). Then a revocation must also close the streams it can no longer find, for example through a binding-to-streams index that eviction does not clear.
  • Never close the sessions of a request that is still being served.

RA-WR-05: Prompt Render errors are returned to the MCP client verbatim

File: modules/pier/session.go:719-722 Issue: promptHandler returns pr.decl.Render's error as is. The SDK turns it into the JSON-RPC error message. A plugin's Render that reads the database (to list drafts, look up a record or build context) can then send pgx or gorm text, which may include the host, database and user names, to the outside client and its transcript. This is the leak A-WR-08 closed for dispatch, left open for prompts. Fix:

messages, err := pr.decl.Render(ctx, args)
if err != nil {
	logger().Warn("pier: prompt render failed", "prompt", pr.name, "error", err)
	return nil, &jsonrpc.Error{Code: jsonrpc.CodeInternalError, Message: "pier: the prompt could not be rendered"}
}

RA-WR-06: The gate passes while demo submodules carry non-test commits that the go.mod pins do not ship

File: scripts/check-phase19.7.sh:957-961, :977-982, :1158-1170 Issue: demo_pins now accepts a submodule that is ahead of its pin by non-test commits whenever the canonical checkout's HEAD contains them. It fails only with PHASE197_RELEASED=1, and the default is 0. The demo's own check-pins.sh exists to prevent exactly this: "they must be the same code, or the tests pass against something that is not shipped". With the default, the consumers stage runs the demo's builds and tests against the submodule code, while a release build (GOWORK=off) compiles the older pinned version. run_all still prints "Phase 19.7 gate passed". The acceptance is reported only as a mid-stage pins: line. The canonical check also accepts any commit on whatever branch the canonical checkout is on, pushed or not. Failure scenario: The phase is marked verified on "gate passed". Then the demo is tagged and deployed with the old pins, without the gap-closure plugin fixes the tests exercised. Fix:

  • Make the release check fail closed by default: require an explicit PHASE197_ALLOW_UNRELEASED=1 to accept unreleased drift.
  • When drift was accepted, end run_all with a non-pass verdict, for example Phase 19.7 gate passed WITH UNRELEASED PINS (not releasable), and a distinct exit code.
  • Optionally require that the commit exists on the canonical checkout's upstream (git branch -r --contains).

Info

RA-IN-01: lazyStore's doc comment says nothing dials at mount

File: modules/pier/store_redis.go:231-233 Issue: The comment still says "nothing dials at mount (route:list and tests never dial)". Mount now starts warm, which dials Redis in the background (pier.go:146-148). If a CLI command mounts pier while Redis is unreachable, it also logs "session store not open yet, retrying". Fix: Reword the comment to match Mount's and decide whether non-serving commands should skip warm.

RA-IN-02: Stream re-checks resolve the bearer without the client address

File: modules/pier/session.go:438; modules/pier/stateless.go:114 Issue: The bearer middleware puts towel.WithClientIP on the resolver context (pier.go:254), but the GET-stream and listen re-check goroutines call identities.resolve(ctx, raw) on a context without it. A re-check that fails, for example a revoked admin token, is logged by cabana.ResolveBackendToken with an empty remote. That is the case B-IN-04 set out to make traceable. Fix: Capture p.clientIP(r) when the stream starts and resolve with towel.WithClientIP(ctx, ip).

RA-IN-03: reserveScript touches keys it does not declare

File: modules/pier/store_redis.go:144-160 Issue: The script runs EXISTS on ARGV[1] .. m for every set member, so it touches keys missing from KEYS. A standalone redis.Client accepts this. Redis Cluster refuses it (cross-slot), and so do ACLs restricted by key pattern (undeclared keys). The binding set and the record key would not hash to one slot anyway. Fix: Document that the store needs a standalone Redis, or put a hash tag on the binding in both key names (mcp:{<binding>}:...) and keep the per-member keys in the same slot.

RA-IN-04: The non-reserver fallback in serveNew is still check-then-act and briefly exposes an unrecorded session

File: modules/pier/session.go:156-166, :184-193 Issue: Both shipped stores (and lazyStore) implement reserver, so only a test wrapper (WrapStore) reaches this branch. On it, the cap is TOCTOU, and the session sits in sr.local before its record is written, so a reaper pass in between closes it as expired. Fix: Make reserver part of Store, or delete the fallback and refuse a store without Reserve at mount.

RA-IN-05: A backend token whose scopes no longer resolve gets "account unknown or deactivated"

File: modules/pier/backend_guard.go:99-103, :79-86; modules/pier/dispatch.go:184-185 Issue: principal returns nil, nil when ScopeLimitFor reports known == 0, for example when a scope's plugin was removed. MCPDispatch then answers errSubjectUnavailable, and the agent reads that the administrator's account is unknown or deactivated, which is false. Fix: Return a distinct sentinel, for example errScopeUnavailable with "the token's scopes no longer grant this route", and map it in dispatch.go.

RA-IN-06: demo_pins passes when check-pins.sh exits non-zero with no output

File: scripts/check-phase19.7.sh:962-990 Issue: With rc != 0 and empty output, the here-string feeds one empty line, [[ -z "$line" ]] && continue skips it, and the function returns 0. The current check-pins.sh prints a line on every failure path, so this is unlikely, but the check fails open. Fix: After the loop, refuse unless at least one mismatch line was matched and accepted: track accepted=1 inside the branch, and [[ -n "$accepted" ]] || { echo "refuse: check-pins failed without a mismatch line" >&2; return 1; }.

Reviewed: 2026-10-23 Reviewer: Claude (msd-code-reviewer) Depth: deep

Lane B

Phase 19.7 (lane B re-review): Code Review Report

Reviewed: 2026-10-23T19:30:00Z Depth: deep Files Reviewed: 24 Status: issues_found

Summary

This re-review covers the lane B gap-closure changes between fef1cbb and HEAD (commits 9eedfbb, 24cf6cd, 44835e4, 88559d5 and the plan 09 tests). I re-read the code each fix touches and the code it calls:

  • the pier event path: listenEvents, handleEvent, revokedMatch, and how identities are resolved in identity.go;
  • lagoon.AfterCommit with no transaction;
  • towel.ClientIP with a nil ctx;
  • revokeAPIToken, normalizeTokenScopes (api_tokens.go:135-160);
  • FormView's <form> wrapper and save button disabled conditions;
  • PluginField's summer-change protocol;
  • every i18n key the SPA component uses, in en, pl and the fixture.

Verified correct:

  • Token revoke event. fireTokenRevoked is called with row.ID. The pier Event keeps TokenID through the store (token_id json tag). revokedMatch matches only subjects with TokenID == ev.TokenID && ClientID == "". An OAuth backend subject always has ClientID set (identity.go:227-233), so OAuth sessions survive. The revoke route has no transaction, so AfterCommit fires at once, which is correct.
  • URL rule (server). safeAccountActionURL refuses javascript:, data:, vbscript:, //host, /\host, https:host and http:/host, plus a leading space or tab/newline, because Go's url.Parse rejects control characters. I confirmed this by running the function.
  • Per-section isolation. A failing Status sets error (omitempty, so this only adds data). A failing Applies drops only that action. The SPA toasts on a failed GET and keeps the sections already shown.
  • OpenAPI. scripts/check-admin-openapi.sh --check exits 0, and scripts/check-admin-dist.sh reports that the dist matches.
  • Phrase keys. form.action_prompt_invalid, myaccount.action_confirm, myaccount.action_stale and myaccount.section_error sit under the right YAML sections in en and pl and are in the fixture.
  • Logging. ResolveBackendToken logs the remote address on all seven paths, and pier puts the address on the context (pier.go:254).
  • Frontend scope seam. cabana never reads HasFrontendTokenScopes (grep).
  • No raw HTML. AccountContributions.vue has no v-html and uses text interpolation only.
  • Checks run. go vet, the targeted cabana -short tests, pact and pier -short, TestDocsTree, the two vitest files and vue-tsc all pass.

No blockers. Two warnings:

  • The draft-commit fix for keyboard saves does not reach a focused input inside a plugin field type's shadow root.
  • The "one shared name rule" for token scopes is not actually one rule in cabana, because cabana trims the name before checking it and then registers the untrimmed name.

Earlier findings

ID Verdict Evidence
B-WR-01 fixed admin/src/app/hotkeys.ts:121-124 (originOf uses composedPath()[0]), :241 (used in onPlainKey), :113 (a custom element inside form/[role=form] counts as editable for closed roots; FormView wraps fields in <form> at views/FormView.vue:658). Tests: admin/tests/app/hotkeysEditing.test.ts:71-129.
B-WR-02 partially fixed admin/src/app/hotkeys.ts:227-233 blurs doc.activeElement before click(). This covers light-DOM fields (the taglist; tests at hotkeysEditing.test.ts:131-181). When focus is inside a shadow root, doc.activeElement is the shadow host, and host.blur() does not unfocus the inner input (see RB-WR-01).
B-WR-03 fixed Server: modules/cabana/account_contributions.go:217-221, :233-242. SPA: admin/src/components/account/AccountContributions.vue:31-39, :64-68, :128-135. The SPA check has a defense-in-depth gap the server covers (RB-IN-01).
B-WR-04 fixed modules/cabana/account_tokens.go:167 calls fireTokenRevoked(..., principal.ID, row.ID). modules/cabana/subject_events.go:51-65 puts TokenID on the event. modules/pier/events.go:21, :79-93 narrow the match to that token. Test: modules/cabana/account_token_events_test.go:20-61 (Postgres).
B-WR-05 fixed modules/cabana/admin_openapi.go:802, :812, :831-832. Regenerated into admin/src/api/schema.d.ts:3033, :5586. check-admin-openapi.sh --check exits 0.
B-WR-06 fixed (one residual gap, RB-IN-02) modules/cabana/account_contributions.go:113-138 (error flag on the section, failing action dropped). AccountContributions.vue:41-53 (toast on a failed GET), :125-127 (section error line). Tests: account_contributions_hardening_test.go:137-188, AccountContributions.test.ts "a 500 on the list reload".

Info items from the first review:

  • Fixed: B-IN-01 (AccountContributions.vue:74, :100), B-IN-02 (actions.go:249, :269), B-IN-03 (account_contributions.go:121), B-IN-04 (token_resolver.go:29).
  • Deferred: B-IN-05 (deferred-items.md:177).
  • Accepted: B-IN-06, with the visible link as the fallback.

Narrative Findings (AI reviewer)

Warnings

RB-WR-01: The draft-commit blur does not reach an input inside a plugin field type's shadow root (B-WR-02 residual)

File: admin/src/app/hotkeys.ts:227-233 Issue: The fix blurs doc.activeElement. When focus is on an <input> inside a shadow root (the plugin field types and widgets that modules/cabana/README.md:185 explicitly allows), document.activeElement is the shadow host, not the input. Calling blur() on a host that is not itself focused does nothing in two places:

  • The HTML spec. The unfocusing steps return early when the target is not in the current focus chain. The only exception is a host with delegatesFocus.
  • Firefox. ShouldBlur compares the element with the focused descendant, which is the inner input.

Chromium happens to blur here, because AdjustedFocusedElementInTreeScope() == this. The behaviour therefore differs by browser.

PluginField.vue:13-24 defines the value protocol as a summer-change event. A plugin that emits it from the inner input's native change event (fires on blur) still loses the typed value in Firefox:

  • Ctrl+S saves without it.
  • Ctrl+Enter saves without it and navigates away.

This is exactly the "plugin field type that commits on the native change event" case B-WR-02 named. The host's focus() afterwards is also a no-op for a non-focusable host, so focus is lost.

The new tests cover only the light-DOM taglist (hotkeysEditing.test.ts:131-181).

Fix: do what a pointer click does: focus the button before clicking it. Moving focus works in every browser and for closed roots too, because the inner input is unfocused and its blur/change fire.

// The deepest focused element reachable through open shadow roots.
let previous: Element | null = doc.activeElement
while (previous?.shadowRoot?.activeElement) {
  previous = previous.shadowRoot.activeElement
}
const restore = previous instanceof HTMLElement && previous !== target && previous !== doc.body ? previous : null
// Pointer-equivalent: mousedown focuses the button, which blurs whatever had
// focus, shadow-root inputs included (blur() on the host does not).
if (restore) {
  target.focus({ preventScroll: true })
}
target.click()
if (restore?.isConnected) {
  restore.focus()
}

At minimum, descend shadowRoot.activeElement for open roots before calling blur(). Add a test with an open-shadow-root input whose change handler emits summer-change.

RB-WR-02: A scope name is validated trimmed but registered untrimmed, so the shared name rule is not one rule

File: modules/cabana/token_scopes.go:106-118 (validation), :89-98 (registration); modules/cabana/api_tokens.go:146-152 Issue: validatePluginTokenScope validates strings.TrimSpace(scope.Name) with pact.ValidTokenScopeName. compileTokenScopes then stores scope.Name untrimmed, both in compiledTokenScope.Name and as the byName key. With a padded name such as " blog:read" or "blog:read\n", three things go wrong:

  • Boot accepts it. pact.ValidTokenScopeName(" blog:read") is false, so the parity this gap closure claims (P-30) does not hold. The docs say "Both catalogs check names with pact.ValidTokenScopeName" (docs/backend/users-and-permissions.md:119), and the HasFrontendTokenScopes doc promises that a bad name fails boot.
  • The scope is listed but can never be granted. It appears in GET /account/tokens/scopes under its padded name. Token creation trims every requested name (api_tokens.go:147), looks it up as byName["blog:read"], misses, and answers scope_unavailable.
  • The duplicate check depends on order. The check looks up the trimmed name. If " read" registers first and "read" second, the second is not detected as a duplicate, and both register.

TestTokenScopeNameParity (token_scope_name_test.go:12-44) has no padded names, so it does not catch any of this. The pre-existing trim mismatch matters now because this change is what turns ValidTokenScopeName into the advertised single rule.

Fix: validate exactly what is registered. Either refuse a name that is not already trimmed:

name := scope.Name
if name == "" || name != strings.TrimSpace(name) || !pact.ValidTokenScopeName(name) {
    return fmt.Errorf("cabana: plugin %s registered invalid token scope name %q", pluginID, scope.Name)
}

or register the trimmed name (Name: name, cat.byName[name]). Add " read" and "read\n" to the parity table, expecting refusal.

Info

RB-IN-01: The SPA URL check passes /<TAB>/host, which browsers resolve off-site

File: admin/src/components/account/AccountContributions.vue:31-39 Issue: safeUrl accepts any string that starts with one / and not // or /\. Browsers strip ASCII tab and newline from URLs, so "/\t/evil.example/x" and "/\n/evil.example" resolve to https://evil.example/... (confirmed with WHATWG URL). This is the same-origin bypass the check exists to stop.

The server check catches it, because Go's url.Parse rejects control characters, so this is a defense-in-depth gap only. The server and SPA rules are also documented as "the same rule" (:27-29), which they are not.

Fix: strip or refuse control characters first, e.g. if (/[\u0000-\u001f\u007f]/.test(u)) return ''. Or resolve with new URL(u, location.origin) and require protocol http(s) and, for relative input, origin === location.origin.

RB-IN-02: An action whose Applies fails disappears with no signal

File: modules/cabana/account_contributions.go:128-135; AccountContributions.vue:125-127 Issue: A Status failure sets error: true and shows the section error line. An Applies failure only drops the button. If the section's Status still succeeds, the admin sees a normal section (for example "Linked to …") with its Unlink button missing and no message. That is the "fails silently" half of B-WR-06 again, for a single action. Fix: set view.Error = true when any action's Applies fails as well (and document error as "status or an action could not be loaded"), so the SPA shows the section error line.

RB-IN-03: The CLI admin:token:revoke fires no revoke event

File: modules/cabana/token_commands.go:225-255 (interacts with account_tokens.go:167) Issue: The My account revoke now ends that token's MCP sessions. The operator revoke command fires nothing, so the token's open sessions (and any open GET listen stream) stay until the next request's bearer re-verification refuses them. The README entry for backend subject events says a revoked personal API token fires the event, without saying this applies to the HTTP route only. Fix: call fireTokenRevoked(ctx, app, row.BackendUserID, row.ID) after the update when the command runs with the application bus. Otherwise, document that only the My account revoke fires.

RB-IN-04: The custom-element-in-form rule also mutes Esc on focusable non-field widgets

File: admin/src/app/hotkeys.ts:113 Issue: Any focused element with a hyphenated tag inside a <form> counts as editable. That includes WidgetField hosts (components/form/fields/WidgetField.vue:61) that are focusable but not inputs, such as a chart or a button strip. While one of them has focus, Esc does not leave the form. This is an accepted trade-off for closed roots, but it is undocumented. Fix: mention it in the header comment at hotkeys.ts:15-25. Optionally narrow the rule to hosts rendered by PluginField (for example via a data-field-type attribute the SPA sets on plugin field elements).

Reviewed: 2026-10-23T19:30:00Z Reviewer: Claude (msd-code-reviewer) Depth: deep

Lane C

Phase 19.7: Code Review Report (lane C re-review: plugins and demo)

Reviewed: 2026-10-23T17:19:01Z Depth: deep Files Reviewed: 60 Status: issues_found

Abbreviations: U = sm-user-plugin (sm-demo-app/plugins/golem15/user, range 32f9259^..HEAD), O = sm-oauthserver-plugin (671513d^..HEAD), J = sm-journal-plugin (b129902^..HEAD), V = vue-demo-app (0125e29^..HEAD), D = sm-demo-app (00f57bf^..HEAD), F = summercms.go (framework, read for call chains only).

Checks run: go vet ./... and go test -short ./... in U (with GOWORK=off, the module's own replace to the framework), O and J are green. With GOWORK=off, O does not build (see RC-IN-07). Docker-backed tests were not run.

Summary

Most of the gap closure holds up when traced through the code:

  • Account link proof (C-CR-02). The confirm handler reads auth_time again from the same token jwt.auth accepted, checks that its subject matches the authenticated principal, and refuses a token that has no auth_time. bouncer.Refresh copies auth_time unchanged (F bouncer/refresh.go:141-142). The plugin's only mint sites (mintFor: login, registration, activate-by-code) are real sign-ins. No path in U mints a frontend token without one.
  • Link code transport (C-WR-01). The code lives only in the fragment. The admin SPA opens the link with window.open(url, '_blank', 'noopener'), so the Referer is the admin page and never carries the code. Browsers never put a fragment in a Referer. The page moves the code to sessionStorage, strips it with replaceState, and sends /login?next=/link-admin without it.
  • Ban, password reset and admin-set password (C-CR-01). Each one revokes OAuth grants in the same transaction as the user write. The MCP verifier and the MCP refresh grant also refuse banned users and tokens issued before the cutoff, so the event is not the only protection.
  • Revocation scoping (C-IN-02). A token-specific revoke and a client-specific revoke each end only their own sessions. Pier tells the two apart by ClientID == "" (F pier/events.go:85-98), so a usr_ token id that equals an OAuth token id does not collide.
  • Journal (C-IN-01). The draft_post ceiling is enforced by pier's allowsPrompt. TestJournalPromptWrites would catch a regression because it checks both the read-only and the read-write grant.
  • Member token scopes (E-2). Mint, check, list and discovery work per application. A plugin cannot gain backend reach by naming a member scope backend, because pier grants a backend subject only on the OAuth path.
  • Organisation logo (E-1). The admin delete path (single and bulk) goes through cabana's deleteRecord, which runs FormAfterDelete inside the delete transaction.

What remains:

  1. RC-CR-01 (critical). A user's own password change does not end an attacker's MCP or OIDC grant if that grant's tokens were created or rotated after the current session's iat. An attacker controls rotation timing, so the C-CR-01 scenario ("the owner changes the password after a compromise") still leaves the attacker's agent working, including any linked-admin backend reach.
  2. RC-WR-01. C-WR-02 is closed for issued tokens only. A consent decided after the link changed, or an MCP code issued before the change and exchanged after it, still produces a backend grant for an administrator the consent page never showed. The token revocation also races with a concurrent refresh rotation.
  3. RC-WR-02. The oauthserver test for the verifier's ban refusal cannot fail. BanUsers also moves tokens_valid_after, so the cutoff check alone makes the token invalid.

Narrative Findings (AI reviewer)

Critical Issues

RC-CR-01: Self-service password change keeps every OAuth/MCP grant whose tokens were created or rotated after the session's iat

File: U controllers/api_controller.go:722-732; O classes/cascade.go:111-113 (created_at < before); O classes/mcp_store.go:252-266 (a rotated refresh token is a new row with created_at = now) and :385-391 (the new access token, the same); O classes/mcp_verifier.go:133-144 (issuedBeforeCutoff); U README.md:57; U access_revocation_test.go:156-170

Issue: ChangePassword sets tokens_valid_after = iat - 1s and fires FireAccessRevoked(..., before = cutoff + 1s = iat, ...). Here iat belongs to the session token that made the call. A refresh renews iat, so it is the time of the last refresh, not of the sign-in. The README sentence "tokens issued since that sign-in survive" is wrong on this point too.

revokeUserGrants revokes only access and refresh rows with created_at < before. The MCP refresh grant creates a new refresh row and a new access row with created_at = now on every rotation. The verifier's issuedBeforeCutoff and refreshUserUsable compare that same created_at with the cutoff. So any grant that was refreshed after the caller's iat:

  • is not revoked by the event,
  • passes the verifier, and
  • passes the refresh check,

and lives on indefinitely. The client chooses when to refresh: an OAuth client may present its refresh token at any time. An attacker's agent that refreshes every minute is therefore never older than the victim's session unless that session was minted within the last minute.

The ban, the password reset and the admin-set password all pass before = time.Now() and are not affected. Only the self-service change uses the session-relative cutoff, although nothing in the current request needs an OAuth grant kept alive.

Failure scenario: An attacker learns E's password, signs in, connects an MCP agent with read write backend (E is linked to admin A) and lets it refresh every few minutes. E notices odd posts, opens the site (session last refreshed 20 minutes ago) and changes the password from Settings. AccessRevokedEvent fires with Before = iat (20 minutes ago). The agent's current refresh token, created 3 minutes ago, is not revoked, and the verifier accepts it because created_at is after the cutoff. The agent keeps calling journal_update_post as admin A. TestAccessRevocation "ChangePassword fires with the stored cutoff plus one second" pins this behaviour instead of catching it.

For context: the same session-relative cutoff lets an attacker's frontend session JWT survive if it was refreshed after the victim's iat. That part predates this phase, but auth_time now makes a better cutoff possible.

Fix: Revoke OAuth grants as of now on a self-service change. The current request needs no OAuth grant, so nothing has to be kept:

// U controllers/api_controller.go, ChangePassword
cutoff := iat.Add(-time.Second) // keeps the calling session JWT, as today
revokedAt := time.Now()
if err := lagoon.Transaction(r.Context(), gdb, func(ctx context.Context, tx *gorm.DB) error {
	// ... Updates as today ...
	return classes.FireAccessRevoked(ctx, app, tx, []uint{user.ID}, revokedAt, classes.AccessRevokedPasswordChanged)
}); err != nil { ... }

Change the README sentence on line 57 to match. Rewrite the test to create an MCP grant, rotate it after the session's iat, change the password, and assert that the rotated grant is revoked and refused. Optionally, base the session cutoff on the calling token's auth_time, and have /refresh and the jwt guard compare auth_time with tokens_valid_after, so an attacker's refreshed session JWT also stops surviving.

Warnings

RC-WR-01: C-WR-02 is closed for issued tokens only: pending consents and unexchanged codes still bind backend to whichever administrator is linked later, and the revocation races with refresh rotation

File: O classes/cascade.go:136-138 (with a non-empty scope, the function returns before touching MCPCode rows), :198-216 (OnBackendLinkChanged), :111-134 (plain SELECT, then UPDATE ... WHERE id IN); O controllers/mcp_consent_api.go:82-89 (show reads the admin), :133-144 (decide checks only linked, not which admin); F pier/identity.go:225-235 (backend subject = whoever is linked at request time)

Issue: The fix revokes unrevoked access and refresh tokens that carry backend when a link is created or removed. Three paths still let a grant act as an administrator the consent page did not show:

  1. Decide after a link change. MCPConsentShow shows admin X. MCPConsentDecide checks only that some link exists (linked). If the link moves to Y between show and decide (an administrator's SetLink after an unlink, within the pending request's lifetime), the user approves "act as X" and gets a backend grant that pier resolves to Y.
  2. Code issued before, exchanged after. revokeUserGrants with scope = "backend" returns at line 136-138 and never deletes the user's unused MCP codes. A code issued while linked to X and exchanged after a relink to Y mints a fresh token (created_at after the revocation) with backend, acting as Y.
  3. Rotation race. The revocation reads candidate rows with a plain SELECT under READ COMMITTED, then updates them by id. If a refresh rotation of the same grant is in flight, the UPDATE waits on the old row's lock. Once the rotation commits, the new refresh row (inserted after the SELECT's snapshot) is never revoked and keeps backend across the link change.

The README now promises that "a grant never acts as an administrator the user did not consent to". These paths contradict it. Each window is short, so this is a warning.

Fix: Bind the grant to the consented administrator, so no revocation timing matters (this was option 2 of C-WR-02 and is additive):

  • At decide, take the backend_admin.login (or id) the page showed in the body and refuse with 409 when the current link differs.
  • Store backend_user_id on the MCP code, access and refresh rows. Copy it on code exchange and rotation, expose it in pact.MCPGrant, and have pier drop the backend subject when the current link's admin id differs.

As a cheaper stopgap: in OnBackendLinkChanged also delete the user's MCP codes with used_at IS NULL whose scopes contain backend, and lock the user's refresh rows (SELECT ... FOR UPDATE on user_id) before collecting ids, so a concurrent rotation is serialized behind the revocation.

RC-WR-02: The verifier's ban refusal is untested; the "banned user is refused" test passes even with the ban check removed

File: O mcp_revocation_test.go:138-160; code under test O classes/mcp_verifier.go:96-105, O classes/mcp_store.go:299-313 (refreshUserUsable ban branch)

Issue: The subtest bans with userclasses.BanUsers, which calls RevokeUserAccess and stamps tokens_valid_after = now. The grant was created before that, so the issuedBeforeCutoff check already returns ErrMCPTokenInvalid and the refresh is already refused. Deleting the ThrottleStates and banned[...] blocks from both the verifier and refreshUserUsable leaves the test green.

The ban check is the only protection when a ban is written without RevokeUserAccess, which the user README explicitly allows applications to do ("after writing a ban directly to user_throttle"). The refresh path's pre-cutoff branch has no isolated rotation test either. The suite covers rotation only for a deleted user, while the doc comment of TestMCPVerifierRevocation claims "the refresh rotation refuses a banned, a pre-cutoff and a deleted user". The demo's e2e step (a) cannot tell the three mechanisms apart either.

Fix: In the ban subtest, write the ban without moving the cutoff, then assert both refusals:

if err := env.gdb.Exec(`INSERT INTO user_throttle (user_id, is_banned) VALUES (?, TRUE)`, user.ID).Error; err != nil {
	t.Fatal(err)
}
invalid(t, "banned (no cutoff moved)", grant)
status, out := env.refreshStatus(t, grant)
refused(t, "banned (no cutoff moved)", status, out)

Add a rotation subtest for a pre-cutoff refresh token (setGrantCreated before setCutoff, then refreshStatus must be invalid_grant). The "same second" subtest only partly covers this.

Info

RC-IN-01: The server still accepts the link code in URLs

File: U routes.go:46-48; U controllers/backend_link_controller.go:36-54, 111-117; U controllers/api_controller.go:1166-1189 (readFields); D deploy/nginx/demo.summercms.io.conf:96-103 Issue: The shipped page never puts the code in a URL. The server, though, still serves the deprecated GET /backend-link/{code}, kept on purpose for compatibility. In addition, readFields falls back to r.ParseForm() for any non-JSON body, and r.Form includes the query string. So POST /backend-link/show?code=… and POST /backend-link/confirm?code=… both work, and nginx's default access log records those request lines. A third-party client of this core plugin can reintroduce the leak without noticing. Fix: In BackendLinkShowPost and BackendLinkConfirm, read the code only from a JSON body (ignore r.URL.Query()). Consider access_log off; (or a log_format without $request_uri) for location ~ ^/_user/api/v1/backend-link/ until the GET route is removed in a major release.

RC-IN-02: The auth_time proof has a one-second blind spot

File: U classes/backend_links.go:276 Issue: auth_time is truncated to whole seconds and compared with created_at truncated to the same second. A sign-in that happened earlier in the same second as the request, but before it, counts as fresh. The design accepts this, and it is not exploitable in practice. It is worth stating in the doc comment next to the D-05 claim. Fix: Document it, or compare auth_time >= created_at.Truncate(time.Second) + 1s and accept that a sign-in in the request's own second needs one more sign-in.

RC-IN-03: The logo URL fallback ignores the configured public path

File: U classes/organisation_logo.go:57-61 (copied from U classes/avatar.go:52-56) Issue: When Thumb fails, the fallback is "/" + attach.BlobKey(f.DiskName). That skips storage.uploads.public_path_prefix and public_url (F lagoon/attach/thumb.go:85-104). With the WinterCMS layout (/storage/app/uploads/public) the fallback URL is a 404. Fix: return f.URL(), true, nil (and the same in AvatarURL).

RC-IN-04: nginx's /mcp cap now equals pier's, so oversized bodies still get nginx's HTML 413

File: D deploy/nginx/demo.summercms.io.conf:217; F pier/pier.go:38 Issue: Both caps are 4 MiB. Bodies between 2 and 4 MiB now reach pier (C-IN-03 is fixed). A body over 4 MiB is still refused by nginx with an HTML 413, so the JSON-RPC error the new comment describes is never seen through nginx. Fix: Set client_max_body_size 5m; (and update the check-deploy.sh string), so pier's own refusal is the one clients get.

RC-IN-05: Test gaps and tests that pass without running

File: U organisation_logo_test.go:138-142; V app/lib/linkCode.test.ts:13-21; the new U Postgres tests (backend_link_auth_time_test.go:25-27, backend_link_show_test.go:24-26, access_revocation_test.go:113-115, member_token_scopes_test.go:64-66, organisation_logo_test.go:58-60, api_token_events_test.go:17-19, mcp_guard_client_ip_test.go:17-19), O mcp_revocation_test.go:65-67, 184-186, D mcp_revocation_e2e_test.go:247-249 Issue:

  • The organisation delete test checks only that the system_files rows are gone. The after-commit attach.DeleteKeys part of FormAfterDelete (the blobs and thumbnails) is never asserted, so a broken bucket hand-off would pass.
  • In linkCode.test.ts, the case #next=/account&code=${code}x sits under "foreign", but it returns '' because of the trailing x. The behaviour the plan asked about (a valid code followed by other parameters) is not pinned at all.
  • The Postgres tests use if testing.Short() { return } instead of t.Skip, so -short runs report them as passed rather than skipped. Fix: After the delete, wait for the commit and assert that bucket.Exists(ctx, attach.BlobKey(logoDisk)) is false. Add expect(readLinkCode('#code=' + code + '&next=/x')).toBe(code). Use t.Skip("Postgres").

RC-IN-06: The legacy MintAPIToken fallback is process-global and is widened even when boot fails

File: U classes/api_token_scopes.go:125, 144-166; U plugin.go:127-146 Issue: CompileAPITokenScopes adds every declared name to the process-wide declaredScopes set before bootAPITokenScopes checks the phrase keys. A boot that then fails on a missing key has already widened what MintAPIToken accepts. In a process with several applications (tests, or a future multi-app host), MintAPIToken also accepts another application's scopes. Both are documented, and production runs one app per process. Fix: Add the names only after the phrase check passes (return the declared list from CompileAPITokenScopes and add it in bootAPITokenScopes just before app.Publish). Mark MintAPIToken as Deprecated: in favour of MintAPITokenFor.

RC-IN-07: sm-oauthserver-plugin builds only through its workspace file

File: O go.mod:6 (sm-user-plugin v0.1.10); O go.work (tracked, replace ... => ../sm-demo-app/plugins/golem15/user) Issue: O now uses userclasses.AccessRevokedEvent, FireSubjectEvent and FireSubjectEventAfterCommit, which v0.1.10 does not have. GOWORK=off go build ./... fails with undefined: userclasses.AccessRevokedEvent. The planned release order (tag U, then bump O) covers this, but nothing enforces it. Fix: Make the O release checklist require bumping sm-user-plugin to the new U tag and running a GOWORK=off go build ./... gate before tagging O.

Ruling: readLinkCode with other fragment parameters

readLinkCode('#code=<64 hex>&next=/x') returns the code. This is not a defect. URLSearchParams.get('code') returns the first code parameter, and the value still has to match ^[a-f0-9]{64}$ exactly, so nothing beyond a well-formed code is ever accepted. Other parameters are ignored, the fragment is stripped right away by replaceState (V link-admin.vue:55), and the code is only ever sent in a JSON body. Accepting extra parameters is the more robust behaviour: an analytics tool or a redirect that appends a fragment parameter does not break the link. The plan's expectation of an empty result was wrong. The one gap is that no test pins the behaviour (RC-IN-05).

Earlier findings

ID Verdict Evidence
C-CR-01 (ban / password reset keep MCP grants) partially fixed Fixed for ban (U controllers/users_admin_controller.go:651-683), reset (U controllers/api_controller.go:454-466) and admin-set password (U controllers/users_admin_controller.go:247-255). The verifier refuses banned users and pre-cutoff tokens (O classes/mcp_verifier.go:96-105), and so does the refresh grant (O classes/mcp_store.go:281-313). LinkedBackendUser excludes banned users (U classes/backend_links.go:437-443). Not fixed for the self-service password change: U controllers/api_controller.go:722-732 revokes only grants older than the session's iat (RC-CR-01).
C-CR-02 (fresh-sign-in proof defeated by refresh) fixed U controllers/backend_link_controller.go:102-110 reads auth_time again from the accepted token and checks the subject; U classes/backend_links.go:276 compares it with created_at; F bouncer/refresh.go:141-142 carries auth_time forward; the page shows the signed-in email with "Not you?" (V link-admin.vue:150-155). One-second blind spot only (RC-IN-02).
C-WR-01 (link code in logged URLs) fixed URL is …#code= (U account_contributions.go:120); the page reads the hash, stores the code in sessionStorage and strips it (V link-admin.vue:50-60); show is a POST with a JSON body (V link-admin.vue:89, U routes.go:48); the sign-in link carries no code (V link-admin.vue:24). The server still accepts the code in URLs (deprecated GET route, query fallback in readFields): RC-IN-01.
C-WR-02 (backend grant re-targeted by a later link) partially fixed Issued access and refresh tokens carrying backend are revoked on link and unlink (O classes/cascade.go:173-216, O plugin.go:103-105). Pending consents, unexchanged MCP codes and a refresh rotation racing the revocation still bind backend to a later admin (RC-WR-01).
C-IN-01 (draft_post for read-only grants) fixed J mcp.go:160-162 Writes: true; enforced by F pier/filter.go:52; J mcp_prompt_writes_test.go checks both grants.
C-IN-02 (one revoke ends every MCP session) fixed U controllers/api_tokens.go:142 (TokenID); O controllers/authorizations.go:122-126 (ClientID); F pier/events.go:85-98 tells the two apart.
C-IN-03 (nginx 2 MiB < pier 4 MiB) fixed D deploy/nginx/demo.summercms.io.conf:217, gated by D scripts/check-deploy.sh:254-256. Over-cap bodies still get the HTML 413 (RC-IN-04).
C-IN-04 (no last_used_ip on MCP personal tokens) fixed U classes/mcp_guard.go:172-176; pier sets the client IP (F pier/pier.go:254).
C-IN-05 (dead and duplicated code) fixed requestUsable calls IsUsableAt (U classes/backend_links.go:227-235); BackendLinksFor is used by both callers (U classes/backend_links.go:57-64, U account_contributions.go:57-59, U controllers/backend_link_controller.go:16-18); O's subject helpers delegate to U (O classes/subject_events.go:18-26).
C-IN-06 (hard-coded link page path) fixed golem15.user.backend_links.page_path (U classes/backend_links.go:85-104, U config/config.yaml:48-52), tested in U backend_link_show_test.go:94-109.

Reviewed: 2026-10-23T17:19:01Z Reviewer: Claude (msd-code-reviewer) Depth: deep

Reference in New Issue View Git Blame Copy Permalink