From f3032950beff9f51adeb87fda8d798568e36c348 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Mon, 2 Nov 2026 00:53:14 +0100 Subject: [PATCH] docs(20): code review of the chat family and the demo --- .../20-REVIEW-DISPOSITION.md | 30 ++ .../20-REVIEW.md | 387 ++++++++++++++++++ 2 files changed, 417 insertions(+) create mode 100644 .planning/phases/20-demo-instance-with-every-shared-plugin/20-REVIEW-DISPOSITION.md create mode 100644 .planning/phases/20-demo-instance-with-every-shared-plugin/20-REVIEW.md diff --git a/.planning/phases/20-demo-instance-with-every-shared-plugin/20-REVIEW-DISPOSITION.md b/.planning/phases/20-demo-instance-with-every-shared-plugin/20-REVIEW-DISPOSITION.md new file mode 100644 index 00000000..ca74cf3f --- /dev/null +++ b/.planning/phases/20-demo-instance-with-every-shared-plugin/20-REVIEW-DISPOSITION.md @@ -0,0 +1,30 @@ +--- +phase: 20-demo-instance-with-every-shared-plugin +review: 20-REVIEW.md +recorded: 2026-11-02 +--- + +# Phase 20 review disposition + +One row per finding in 20-REVIEW.md. `open` means not yet triaged. + +| Finding | Severity | Disposition | Note | +|---|---|---|---| +| CR-01 | critical | open | Leaving or unmounting during start/join still connects and publishes camera and microphone | +| CR-02 | critical | open | Banned-link filter bypass through URL forms the browser normalises, rendered as working links | +| CR-03 | critical | open | Banned-word filter bypass through invisible characters, Unicode spaces and forum Markdown | +| WR-01 | warning | open | Logout leaves the chat store and the realtime socket of the previous user running | +| WR-02 | warning | open | Any member can pin or unpin any message in every public channel; the message is not checked against the channel | +| WR-03 | warning | open | Member-visible text outside the chat and forum seams is never moderated | +| WR-04 | warning | open | A link opens a call and turns the camera and microphone on with no confirmation | +| WR-05 | warning | open | A failed LiveKit connect posts `leave`, not `end`, leaving an empty ringing session behind | +| WR-06 | warning | open | The forum and friends clients have no 401 refresh; an expired token shows "not found" | +| IN-01 | info | open | Group calls in public channels are blocked only by the seeded participant count | +| IN-02 | info | open | chatvideo checks participant rows, not live membership | +| IN-03 | info | open | check-local passes member JWTs to curl in argv | +| IN-04 | info | open | A chat message that starts with "/" can never be posted | +| IN-05 | info | open | `demo:seed` brings back the default moderation lists after an admin empties them | +| IN-06 | info | open | nginx `/livekit/` exposes LiveKit's whole HTTP surface | +| IN-07 | info | open | The incoming-call runtime can leak a session handle and survives logout | +| IN-08 | info | open | The VAPID runbook pipeline runs without `pipefail` | +| IN-09 | info | open | `GET /chat/members` writes, and builds an unbounded `IN` list | diff --git a/.planning/phases/20-demo-instance-with-every-shared-plugin/20-REVIEW.md b/.planning/phases/20-demo-instance-with-every-shared-plugin/20-REVIEW.md new file mode 100644 index 00000000..14a84a3d --- /dev/null +++ b/.planning/phases/20-demo-instance-with-every-shared-plugin/20-REVIEW.md @@ -0,0 +1,387 @@ +--- +phase: 20-demo-instance-with-every-shared-plugin +reviewed: 2026-11-01T23:50:42Z +depth: standard +files_reviewed: 105 +files_reviewed_list: + - ../sm-chat-plugin/api/errors.go + - ../sm-chat-plugin/classes/service.go + - ../sm-chat-plugin/classes/writes.go + - ../sm-chat-plugin/contracts/rejection.go + - ../sm-chat-plugin/events/events.go + - ../sm-chatvideo-plugin/go.mod + - ../sm-demo-app/config/env/production/golem15.chatvideo.yaml + - ../sm-demo-app/config/golem15.chatvideo.yaml + - ../sm-demo-app/config/golem15.chat.yaml + - ../sm-demo-app/deploy/nginx/demo.summercms.io.conf + - ../sm-demo-app/deploy/supervisor/summercms-demo-livekit.conf + - ../sm-demo-app/deploy/centrifugo.json.example + - ../sm-demo-app/deploy/livekit.yaml.example + - ../sm-demo-app/deploy/env.example + - ../sm-demo-app/DEPLOY.md + - ../sm-demo-app/.gitmodules + - ../sm-demo-app/go.mod + - ../sm-demo-app/go.work + - ../sm-demo-app/plugins/demo/chat_api.go + - ../sm-demo-app/plugins/demo/chatbots.go + - ../sm-demo-app/plugins/demo/chat.go + - ../sm-demo-app/plugins/demo/classes/chat_context.go + - ../sm-demo-app/plugins/demo/classes/forum_access.go + - ../sm-demo-app/plugins/demo/config/config.yaml + - ../sm-demo-app/plugins/demo/console/reset.go + - ../sm-demo-app/plugins/demo/console/seed_chat.go + - ../sm-demo-app/plugins/demo/console/seed_forum.go + - ../sm-demo-app/plugins/demo/console/seed_friends.go + - ../sm-demo-app/plugins/demo/console/seed.go + - ../sm-demo-app/plugins/demo/moderation.go + - ../sm-demo-app/plugins/demo/plugin.go + - ../sm-demo-app/plugins/demo/realtime_access.go + - ../sm-demo-app/plugins/demo/routes.go + - ../sm-demo-app/plugins.gen.go + - ../sm-demo-app/scripts/check-all.sh + - ../sm-demo-app/scripts/check-chat.cjs + - ../sm-demo-app/scripts/check-deploy.sh + - ../sm-demo-app/scripts/check-live.sh + - ../sm-demo-app/scripts/check-local.sh + - ../sm-demo-app/scripts/dev.sh + - ../sm-demo-app/scripts/smoke.sh + - ../sm-demo-app/summer.yaml + - ../sm-demo-app/vue-demo-app/app/assets/css/site.css + - ../sm-demo-app/vue-demo-app/app/components/SiteHeader.vue + - ../sm-demo-app/vue-demo-app/app/components/SiteNav.vue + - ../sm-demo-app/vue-demo-app/app/components/TourCard.vue + - ../sm-demo-app/vue-demo-app/app/layouts/default.vue + - ../sm-demo-app/vue-demo-app/app/lib/realtime.ts + - ../sm-demo-app/vue-demo-app/app/pages/account/security.vue + - ../sm-demo-app/vue-demo-app/app/pages/call/[conversation].vue + - ../sm-demo-app/vue-demo-app/app/pages/chat.vue + - ../sm-demo-app/vue-demo-app/app/pages/forum/index.vue + - ../sm-demo-app/vue-demo-app/app/pages/forum/t/[id].vue + - ../sm-demo-app/vue-demo-app/app/pages/friends.vue + - ../sm-demo-app/vue-demo-app/app/pages/index.vue + - ../sm-demo-app/vue-demo-app/app/plugins/auth.ts + - ../sm-demo-app/vue-demo-app/app/utils/communityAuth.ts + - ../sm-demo-app/vue-demo-app/nuxt.config.ts + - ../sm-demo-app/vue-demo-app/pnpm-lock.yaml + - ../sm-demo-app/vue-demo-app/vitest.config.ts + - ../sm-forum-plugin/api/errors.go + - ../sm-forum-plugin/classes/forum_service.go + - ../sm-forum-plugin/classes/lifecycle.go + - ../sm-forum-plugin/contracts/rejection.go + - ../sm-forum-plugin/events/events.go + - ../sm-herald-plugin/go.mod + - ../sm-moderation-plugin/admin.go + - ../sm-moderation-plugin/classes/filter.go + - ../sm-moderation-plugin/go.mod + - ../sm-moderation-plugin/models/settings.go + - ../sm-moderation-plugin/plugin.go + - ../sm-moderation-plugin/config/config.yaml + - ../sm-moderation-plugin/models/settings/fields.yaml + - ../sm-moderation-plugin/updates/202610280001_create_golem15_moderation_settings.go + - ../sm-userfriends-plugin/go.mod + - ../vue-chat-layer/app/assets/css/chat-tokens.css + - ../vue-chat-layer/app/components/Chat/CommandPicker.vue + - ../vue-chat-layer/app/components/Chat/FloatingPanel.vue + - ../vue-chat-layer/app/components/Chat/Header.vue + - ../vue-chat-layer/app/components/Chat/ThreadPane.vue + - ../vue-chat-layer/app/components/Chat/Message.vue + - ../vue-chat-layer/app/components/Chat/MessageList.vue + - ../vue-chat-layer/app/components/Chat/LinkEmbed.vue + - ../vue-chat-layer/app/composables/useChat.ts + - ../vue-chat-layer/app/lib/client.ts + - ../vue-chat-layer/app/lib/chatText.ts + - ../vue-chat-layer/app/lib/markdown.ts + - ../vue-chat-layer/app/lib/linkEmbeds.ts + - ../vue-chat-layer/app/lib/realtime.ts + - ../vue-chat-layer/app/lib/slots.ts + - ../vue-chat-layer/nuxt.config.ts + - ../vue-chat-layer/vitest.config.ts + - ../vue-chatvideo-layer/app/composables/useCall.ts + - ../vue-chatvideo-layer/app/composables/useIncomingCalls.ts + - ../vue-chatvideo-layer/app/lib/callMachine.ts + - ../vue-chatvideo-layer/app/lib/videoClient.ts + - ../vue-chatvideo-layer/app/components/Chatvideo/Call.vue + - ../vue-chatvideo-layer/app/components/Chatvideo/CallButton.vue + - ../vue-chatvideo-layer/app/components/Chatvideo/IncomingCall.vue + - ../vue-chatvideo-layer/app/components/Chatvideo/Tile.vue + - ../vue-chatvideo-layer/nuxt.config.ts + - ../vue-forum-layer/app/components/Forum/Thread.vue + - ../vue-forum-layer/app/utils/forumClient.ts + - ../vue-userfriends-layer/app/components/Friends/Groups.vue + - ../vue-userfriends-layer/app/utils/friendsClient.ts +findings: + critical: 3 + warning: 6 + info: 9 + total: 18 +status: issues_found +--- + +# Phase 20: Code Review Report + +**Reviewed:** 2026-11-01T23:50:42Z +**Depth:** standard +**Files Reviewed:** 105 (the 86 listed files, plus the layer components, libs and deploy files the SUMMARYs under-listed) +**Status:** issues_found + +## Summary + +The review covered the phase's code in its sibling repositories. That is the chat and forum pre-save seams (`git diff v0.1.4..main` and `v0.1.0..main`; both tags exist), the new sm-moderation-plugin, the sm-demo-app host plugin, deploy files and scripts (commits since 2026-10-28), vue-demo-app since 44a5229, and the four new layers. + +Areas that held up when traced: +- **Pre-save seams.** Both are additive and silent without a listener, and they map `contracts.Rejection` to 422 on every chat and forum write path (send, reply, ReplyTo, edit; thread, reply, edit). +- **Core plugins.** The phase changed no core plugin (user, journal/blog, pages, payment). sm-user-plugin is consumed by tag and unchanged. +- **XSS.** `renderChatText` escapes everything before it adds tags, placeholders cannot be forged (`\u0000` is stripped), and `safeUrl` refuses `javascript:`, `data:` and `//` / `/\` paths. Forum `content_html` comes from goldmark plus bluemonday. Attachments download as octet-stream with the `download` attribute. No XSS was found. +- **Realtime authorization.** Every Centrifugo namespace goes through the subscribe proxy. The `user`, `chat`, `video` and `presentations` authorizers each check their own rule, so giving every member a connection token does not widen channel access. LiveKit tokens are scoped to one room, with no `roomAdmin`. +- **Runbook secrets.** The runbooks keep secrets out of argv and logs. + +Three defects need fixing before more traffic reaches the demo: +1. **Call race (CR-01).** The call composable connects to LiveKit and publishes the camera and microphone after the user has already left or navigated away. +2. **Link filter bypass (CR-02).** The banned-link filter misses URL forms that browsers normalise to the banned host. The chat then renders them as working links and link cards. +3. **Word filter bypass (CR-03).** The banned-word filter misses invisible characters, non-breaking spaces and forum Markdown that renders to the banned word. + +Each bypass was confirmed by running `classes.Match` against the probes; all of them returned `flagged=false`. + +## Critical Issues + +### CR-01: Leaving or unmounting during start/join still connects and publishes camera and microphone + +**File:** `../vue-chatvideo-layer/app/composables/useCall.ts:177-231` (also `258-266`, `274-276`; Leave is enabled while `starting`/`connecting` in `../vue-chatvideo-layer/app/components/Chatvideo/Call.vue:72,91`) + +**Issue:** `begin()` never re-checks the state or `leaving` after its awaits (`client.start`/`client.join` at line 184, `import('livekit-client')` at line 193). It also assigns `room` only after the import. `leave()` sees `room === null`, so it cannot disconnect anything. + +- **Caller:** Leave is clicked while the status shows "Connecting…" (the state is `starting`). `leave()` dispatches `ended`, posts `end` (404, because the session does not exist yet) and navigates away. Then `start` resolves. `begin` subscribes `video:session:{id}`, loads livekit-client, creates a `Room`, connects, and runs `setCameraEnabled(true)` and `setMicrophoneEnabled(true)`. The server session was created and the callee is ringing. If they accept, they see and hear a caller who believes they hung up. Nothing on the page can stop it, and the channel handle leaks. +- **Callee:** this path is worse. `begin(false)` dispatches nothing before `await client.join()`, so the state stays `idle`. Navigating away during the join runs `onBeforeUnmount → leave()`, which returns early on `status === 'idle'` (line 259). Then the join resolves: `transition` lets `joined` move `idle → connecting`, and the callee is live in the room with camera and microphone, with no UI and no leave posted. +- **Second join:** the same missing guard lets a second `begin(false)` start while the first is pending. That creates two Rooms, and the first is never disconnected. + +**Fix:** Add a per-attempt token and bail out after every await. Mark the attempt live before the first await. Make `leave()` work from the pre-room states. + +```ts +let attempt = 0 +async function begin(start: boolean) { + if (state.value.status !== 'idle' && isLive(state.value)) return + const mine = ++attempt + leaving = false + dispatch({ type: 'start' }) // also for the callee: leaves 'idle' before any await + const stale = () => mine !== attempt || leaving || !isLive(state.value) + let join: VideoJoin + try { join = start ? (takeStart(conversationId) ?? (await client.start(conversationId))) : await client.join(conversationId) } + catch (e) { if (!stale()) dispatch({ type: 'error', code: e instanceof VideoApiError ? e.status : 0, message: e instanceof Error ? e.message : '' }); return } + if (stale()) { void (start ? client.end(conversationId) : client.leave(conversationId)).catch(() => {}); return } + // ...same checks after import('livekit-client'), after r.connect(), and before each setXEnabled: + // if (stale()) { await r.disconnect(true); return } +} +async function leave() { + if (leaving || !isLive(state.value)) return // no early return on 'idle' while an attempt is pending + leaving = true; attempt++ // invalidates the in-flight begin + // ... +} +``` + +Add `useCall.test.ts` cases: "leave while start is pending" and "unmount while join is pending". Both must assert that `Room.connect` and `setCameraEnabled` are never called. + +### CR-02: Banned-link filter bypass through URL forms the browser normalises, rendered as working links + +**File:** `../sm-moderation-plugin/classes/filter.go:50,122-135`; `../sm-moderation-plugin/models/settings.go:137-168`; rendering in `../vue-chat-layer/app/lib/chatText.ts:87-93`, `../vue-chat-layer/app/lib/linkEmbeds.ts:40-47,69-83` and `../vue-chat-layer/app/components/Chat/LinkEmbed.vue:13` + +**Issue:** `hostPattern` matches the literal text and compares hosts and paths byte for byte. The WHATWG URL parser (browsers, and `new URL` in `linkEmbeds.ts`) percent-decodes and IDNA-maps the host, so each form below reaches the banned host. Every probe returned `flagged=false` against `Match(["viagra",...], ["bit.ly","example.com/promo"], …)`. With `node`, `new URL()` resolves each one to `bit.ly`: + +| Text sent | What the browser opens | +|---|---| +| `https://bit%2Ely/x` | `https://bit.ly/x` | +| `https://bit.l%79/x` | `https://bit.ly/x` | +| `https://ｂｉｔ.ｌｙ/x` (fullwidth) | `https://bit.ly/x` | +| `https://bit。ly/x` (U+3002) | `https://bit.ly/x` | +| `https://example.com:443/promo` | path rule skipped: the port stops the path group | +| `https://example.com/%70romo`, `//promo`, `https://example.com./promo` | path-prefix rules skipped | +| `https://sub.example.com/promo` | path rules never cover subdomains | +| punycode `xn--…` vs. Unicode host | either spelling of an IDN bypasses the other | + +The chat then renders the bypass as a clickable ``. `ChatLinkEmbed` makes it worse: its card shows the normalised host `bit.ly` and links to `https://bit.ly/x`. The forum renders the same through goldmark autolinks. Spammers can post any banned link, which defeats D-17 for links. + +**Fix:** Normalise both sides the way the browser does before comparing. In the filter, find every candidate URL or host, then: + +```go +// for each candidate: s := found[0]; if no scheme, s = "http://" + s +u, err := url.Parse(s) // then: +host, _ := url.PathUnescape(u.Hostname()) // %2E -> . +host = strings.Map(func(r rune) rune { // ideographic/fullwidth dots + switch r { case '。', '．', '｡': return '.' }; return r }, host) +host, err = idna.Lookup.ToASCII(norm.NFKC.String(host)) // fullwidth -> ASCII, Unicode -> punycode (x/net/idna) +host = strings.TrimSuffix(strings.TrimPrefix(strings.ToLower(host), "www."), ".") +path := path.Clean("/" + mustUnescape(u.EscapedPath())) // //promo, /./promo, %70romo +``` + +Apply the same `idna.Lookup.ToASCII` and NFKC to entries in `LinkEntry`. Let path rules also match subdomains, as host rules do. Widen `hostPattern` so its host class takes `%`, `．` and `。`, or tokenise on whitespace and parse each token. Add these probes to `filter_test.go`. `golang.org/x/net/idna` and `x/text` are already transitive dependencies of the framework; note them per the dependency rule. + +### CR-03: Banned-word filter bypass through invisible characters, Unicode spaces and forum Markdown + +**File:** `../sm-moderation-plugin/classes/filter.go:45,91,94,115`; seams `../sm-forum-plugin/classes/forum_service.go:284,341` and `../sm-forum-plugin/classes/lifecycle.go:235`; listener `../sm-demo-app/plugins/demo/moderation.go:65,86` + +**Issue:** The word regexp uses `[^\p{L}\p{N}_]` as its boundary and RE2's ASCII-only `\s+` between the words of a phrase. Confirmed with `classes.Match`: + +- **Invisible characters.** `via​gra` (zero-width space) and `vi­agra` (soft hyphen) display as "viagra" and are not flagged. Format characters (`\p{Cf}`) count as boundaries. +- **Unicode spaces.** `buy followers` (NBSP; likewise U+2009, U+3000) is not flagged. +- **NFD text.** Polish diacritics typed in decomposed (NFD) form do not match an NFC entry, and the combining mark counts as a boundary. +- **Forum Markdown.** The forum seam passes the raw Markdown body and title, but members see `breakwater.Render` output. `via**gra**` renders "via**gra**" as one word, `viagra` renders "viagra", and `[x](https://bit.ly/a)` renders a link to `bit.ly` because CommonMark decodes entities in link destinations. None of them are flagged. + +Moderation is therefore cosmetic against anyone who tries once. + +**Fix:** Normalise the text before matching, in `Filter.Check` (and `Match`): + +```go +t := norm.NFKC.String(text) +t = strings.Map(func(r rune) rune { + if unicode.Is(unicode.Cf, r) { return -1 } // ZWSP, ZWJ, soft hyphen, BOM... + if unicode.IsSpace(r) { return ' ' } // NBSP, thin, ideographic spaces + return r +}, t) +``` + +Use `[\p{Z}\s]+` in place of `\s+` between phrase words. Normalise entries with the same function in `ParseList`. + +For the forum, check the rendered text too. In the demo's `checkPost`, run the filter on `html.UnescapeString` of `breakwater.Render(body)` with tags stripped, and on the hrefs it contains. Alternatively, have forum's `PostSaving` carry the rendered HTML (an additive field) so every host gets it. + +## Warnings + +### WR-01: Logout leaves the chat store and the realtime socket of the previous user running + +**File:** `../sm-demo-app/vue-demo-app/app/composables/useDemoAuth.ts:83-92`, `../sm-demo-app/vue-demo-app/app/components/SiteHeader.vue:12-15`, `../sm-demo-app/vue-demo-app/app/utils/communityAuth.ts:57`; `reset()` in `../vue-chat-layer/app/composables/useChat.ts:1514-1516` + +**Issue:** `logout()` blacklists the JWT and clears the cookie. Nothing calls `useChat().reset()` or the realtime `disconnect()`, and the floating panel just unmounts. `start()` is the only caller of `reset()`, and it runs only when a chat view mounts again. After logout the tab therefore keeps: +- the Centrifugo connection and the `user:{me}` subscription, which deliver the old user's DM and call events until the connection token expires; +- the old user's messages and DMs in `useState`; +- a pending incoming-call prompt in `chatvideo:incoming`. + +`$chatAuth.unauthorized()` has the same gap: it only clears the cookie. The layer's own comment says reset exists so that a second account never sees the first one's chat, but the host never wires it. + +**Fix:** In `logout()` (and in `unauthorized`), after clearing the token, call `useChat().reset()` (it disconnects the shared client) and clear `useState('chatvideo:incoming')`. Alternatively, `watch(loggedIn)` in `plugins/auth.ts` and reset when it turns false. Add a vitest case for the logout path. + +### WR-02: Any member can pin or unpin any message in every public channel; the message is not checked against the channel + +**File:** `../sm-demo-app/plugins/demo/classes/chat_context.go:259-269` (the policy) reaching `../sm-chatchannels-plugin/classes/channel_service.go:1332-1353` and `../sm-chatchannels-plugin/api/pins.go:31-52` + +**Issue:** chatchannels gates `PinMessage` and `UnpinMessage` only with `requireChannelAccess`. The demo's `ChatAccess.CanAccessContext` answers true for every member, so any registered visitor can unpin the seeded welcome pins in #general, #announcements and #ogólny, or flood the pins panel. `PinMessage` also never checks that `messageID` belongs to the channel's conversation, so pins can point at another conversation's (DM) message ids. Contents do not leak, because the client's `GET /messages/{id}` needs participation, but the rows are bogus. Plan 03 noted "the visitor was allowed to pin … the pin succeeded" without treating it as a gap. + +**Fix:** Make pinning a manage action, for example additively in chatchannels: `PinMessage`/`UnpinMessage` require `CanManageContext`, or an optional `contracts.PinPolicy` the host implements. The demo then answers it from the moderator group. Also verify `message.conversation_id == *ch.ConversationID` before creating the pin. Until then, the demo layer can hide pin actions for non-moderators (`pinAllowed`), but the server must enforce the rule. + +### WR-03: Member-visible text outside the chat and forum seams is never moderated + +**File:** `../sm-demo-app/plugins/demo/moderation.go:28-44` + +**Issue:** The filter runs only on `MessageSaving.Content` and the `PostSaving` title and body. Members write other text that other members see: +- **Profile name and surname.** They appear in `/chat/members` for every member, in mention chips, in DM headers, as forum `author_name`, and as `caller_name` in the incoming-call prompt. +- **userfriends group names and descriptions.** They are shown to invitees in the code preview. +- **Chat attachment filenames.** + +A member registering as "free crypto bit.ly/x" puts that text in front of everyone, including in a ringing prompt. D-17 states that banned words and links are refused on every write path. + +**Fix:** Run `Filter.Check` on the profile name and surname at registration and profile update. Use a user-plugin saving event if one exists, otherwise the demo's own registration route, without changing the core user plugin. Do the same for userfriends group create and update if a seam exists, or add an additive `GroupSaving` seam, and for attachment filenames (chat `MessageSaving` could carry them additively). Record any path left out as an explicit, documented exception. + +### WR-04: A link opens a call and turns the camera and microphone on with no confirmation + +**File:** `../vue-chatvideo-layer/app/components/Chatvideo/Call.vue:34-36`, `../sm-demo-app/vue-demo-app/app/pages/call/[conversation].vue:17,30`; same-tab site links in `../vue-chat-layer/app/lib/markdown.ts:22` + +**Issue:** `ChatvideoCall` calls `begin()` on mount. `/call/{id}?start=1` starts a call (and rings the other side), and `/call/{id}` joins a live one. The chat renders site paths such as `/call/45?start=1` as same-tab links. One member can DM another `[see this](/call/45?start=1)`, where 45 is their shared DM. A click puts the victim in a call that publishes camera and microphone at once; once the origin has the media permission there is no prompt. A forged `?start=1` also bypasses the call button's intent. + +**Fix:** Require an explicit action on the call page. Render a "Start call" or "Join" button and call `begin()` from its click. Keep auto-start only for a `takeStart()` handoff, which proves the click came from `ChatvideoCallButton`, and drop the `?start=1` trigger for direct loads. + +### WR-05: A failed LiveKit connect posts `leave`, not `end`, leaving an empty ringing session behind + +**File:** `../vue-chatvideo-layer/app/composables/useCall.ts:204-208` + +**Issue:** When `r.connect()` fails, the caller posts `client.leave()`. As the starter and only connected participant, the session stays `pending` or `active` with nobody in it: +- the callee keeps ringing for up to 45 s, and Accept joins an empty room; +- Retry (`begin(true)`) gets the existing session back (`Created: false`), so it never rings again until the reconcile job runs. + +This is the "left-open session never rings again" bug that `leave()` itself fixed with "last one out ends". + +**Fix:** Use the same rule as `leave()`: `void (start || r.remoteParticipants.size === 0 ? client.end(conversationId) : client.leave(conversationId)).catch(() => {})`. + +### WR-06: The forum and friends clients have no 401 refresh; an expired token shows "not found" + +**File:** `../vue-forum-layer/app/utils/forumClient.ts:139-159`, `../vue-forum-layer/app/components/Forum/Thread.vue:41`, `../vue-userfriends-layer/app/utils/friendsClient.ts:135` + +**Issue:** The chat and video clients refresh once on 401 through `$chatAuth.refresh()`. `$forumAuth` and `$userfriendsAuth` are bare token getters, and neither client retries. After the JWT expires, a member on `/forum/t/{id}` sees "thread not found", because `Thread.vue` maps 401 to `forum.error.notFound`. Posting fails the same way. `/chat` keeps working in the same tab. + +**Fix:** Give both hooks the `{token, refresh, unauthorized}` shape (the demo can pass `chatAuth`), and add the chat client's single-flight refresh-and-retry to `request()`. Map 401 to its own "signed out" message rather than not-found. + +## Info + +### IN-01: Group calls in public channels are blocked only by the seeded participant count + +**File:** `../sm-demo-app/config/golem15.chatvideo.yaml:13-18`, `../sm-chatvideo-plugin/classes/video_service.go:91,109-112` + +**Issue:** The server has no DM-only rule (D-23 is UI only, as documented). Channel calls are refused today only because the four personas plus any visitor exceed `max_participants: 4`. If the cap is raised, or a channel is created with fewer seeded participants, any member can start a call that rings everyone who opened that channel. + +**Fix:** Add a host hook in chatvideo (for example an optional `CanStartCall(conversationType)`), or check `conversation.type == dm` in the demo, so the rule does not depend on seed data. + +### IN-02: chatvideo checks participant rows, not live membership + +**File:** `../sm-chatvideo-plugin/classes/video_service.go:425-443`, `../sm-chatvideo-plugin/classes/authorizer.go:54-79` + +**Issue:** A ban revokes the user's JWTs, so new calls are blocked. A LiveKit join token already minted (900 s) and an open `video:session` subscription still survive the ban, because chatvideo never consults `contracts.LiveMembership` as chat does. + +**Fix:** Optional: have chatvideo honour `LiveMembership` (via chat's `Service.IsParticipant`) in `requireParticipant` and the authorizer, and remove the participant from the LiveKit room on a ban. + +### IN-03: check-local passes member JWTs to curl in argv + +**File:** `../sm-demo-app/scripts/check-local.sh:452,514,601` (and the friends checks) + +**Issue:** The LiveKit join token is passed carefully through a curl config on stdin, but the member JWTs go into `-H "Authorization: Bearer $jwt"` arguments, which other local users can read in `/proc/*/cmdline`. This is local and uses throwaway users, but it is inconsistent with the stated pattern. + +**Fix:** Reuse the `curl -K -` approach with `header = "Authorization: Bearer …"`. + +### IN-04: A chat message that starts with "/" can never be posted + +**File:** `../vue-chat-layer/app/composables/useChat.ts:825-829` + +**Issue:** Every text starting with `/` (for example "/r/golang is great", or a path) is dispatched as a slash command and refused as unknown. + +**Fix:** Dispatch only when the first token matches a known trigger from `commands`, or support a `//` escape. + +### IN-05: `demo:seed` brings back the default moderation lists after an admin empties them + +**File:** `../sm-demo-app/plugins/demo/console/seed_friends.go:117-122` + +**Issue:** `seedModerationLists` refills both lists whenever both are empty, and every release runs `demo:seed`. An admin who deliberately clears both lists gets the defaults back at the next deploy. + +**Fix:** Seed only on the first run, for example by inserting the lists in a row-creation step or recording a seeded flag. + +### IN-06: nginx `/livekit/` exposes LiveKit's whole HTTP surface + +**File:** `../sm-demo-app/deploy/nginx/demo.summercms.io.conf:363-375` + +**Issue:** Browsers need only `/rtc` (and `/rtc/validate`). The location also exposes LiveKit's Twirp `RoomService` on the public origin. That service is protected by admin JWTs signed with the API secret, so exposing it publicly is unnecessary. + +**Fix:** `location ^~ /livekit/rtc { proxy_pass http://127.0.0.1:7890/rtc; … }`, and answer 404 for the rest of `/livekit/`. Adjust check-deploy. + +### IN-07: The incoming-call runtime can leak a session handle and survives logout + +**File:** `../vue-chatvideo-layer/app/composables/useIncomingCalls.ts:41-56,58-70` + +**Issue:** Two `video:call:incoming` events for the same session, both pending in `subscribe`, both pass the `pending.value?.sessionId === call.sessionId` check. The second overwrites `rt.channel` and the first handle is never closed. `pending` is also never cleared on logout (see WR-01). + +**Fix:** Keep a sequence number per `watchSession` and close the handle unless it is the latest. Clear `pending` from the logout path. + +### IN-08: The VAPID runbook pipeline runs without `pipefail` + +**File:** `../sm-demo-app/DEPLOY.md:533-540` + +**Issue:** The remote script uses `set -e` without `pipefail`. If `herald:vapid-generate` fails, `jq` emits nothing and `tee -a` appends nothing, but the `SUBJECT` line is still appended. The re-run guard (`grep -c "^…VAPID__" = 0`) then refuses ("already in .env") although no key pair exists, and recovery needs a hand edit of `.env`. The keys were already applied on rome; this matters for a rebuild. + +**Fix:** Use `set -eo pipefail`, generate into a variable first, check both keys are non-empty, then append all three lines in one `tee`. + +### IN-09: `GET /chat/members` writes, and builds an unbounded `IN` list + +**File:** `../sm-demo-app/plugins/demo/chat_api.go:338-358` + +**Issue:** A GET assigns handles (`AssignHandle`) to members without one. Two concurrent first loads race on the same rows. It also expands every member id into `id IN ?`, which fails with 500 past Postgres' 65535 bind-parameter limit. The production reset schedule is off, so users accumulate. + +**Fix:** Select with the `memberWhere` subquery (`WHERE id <> ? AND `) instead of an id list. Assign missing handles at registration or in `demo:seed`, not in a GET. + +--- + +_Reviewed: 2026-11-01T23:50:42Z_ +_Reviewer: Claude (msd-code-reviewer)_ +_Depth: standard_