docs(20): code review of the chat family and the demo
This commit is contained in:
Jakub Zych
@@ -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 |
|
||||
@@ -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 `<a href="https://bit%2Ely/x">`. `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.** `viagra` (zero-width space) and `viagra` (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 <memberWhere>`) 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_
|
||||
Reference in New Issue
Block a user