docs(20): review disposition after the gap closure

- CR-01, CR-02, CR-03, WR-01, WR-04 and WR-05 fixed, with commits, tags
  and the live checks of release 195aa213c546
- WR-02, WR-03, WR-06 and IN-01 to IN-09 deferred, one todo each
This commit is contained in:
Jakub Zych
2026-11-03 23:07:32 +01:00
parent 2035563fc7
commit 399efed845
13 changed files with 255 additions and 19 deletions

View File

@@ -2,29 +2,30 @@
phase: 20-demo-instance-with-every-shared-plugin
review: 20-REVIEW.md
recorded: 2026-11-02
closed: 2026-11-03
---
# Phase 20 review disposition
One row per finding in 20-REVIEW.md. `open` means not yet triaged.
One row per finding in 20-REVIEW.md. `fixed` names the commits and the release tag and the check that confirmed it on the live demo (release 195aa213c546, deployed 2026-11-02 by plan 20-09); `deferred` points at the todo that tracks it. No row is open.
| 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 |
| CR-01 | critical | fixed | Leaving or unmounting during start/join still connects and publishes camera and microphone. vue-chatvideo-layer 69e8b83, 27ad81c (v0.1.1): a per-attempt token with stale checks after every await; leave is final at any moment. Live: `check-chat.cjs --call` ok against demo.summercms.io. |
| CR-02 | critical | fixed | Banned-link filter bypass through URL forms the browser normalises, rendered as working links. sm-moderation-plugin 266fc60, 9715ce7 (v0.1.1): link hosts and paths canonicalised like the URL parser; vue-chat-layer a798bb1 (v0.1.1): deceptive hosts render as text; sm-demo-app 262da37 (CheckHTML on the rendered forum post). Live: `check-moderation.sh` ok (fixed probe, sm-demo-app da481c9), every link form refused in chat, forum reply and thread title. |
| CR-03 | critical | fixed | Banned-word filter bypass through invisible characters, Unicode spaces and forum Markdown. sm-moderation-plugin 266fc60, 9715ce7, 334697b (v0.1.1): words matched after NFKC, format characters, Unicode spaces, diacritics, entities and inline markup; MatchHTML/CheckHTML; sm-forum-plugin ec1a82c (v0.1.2): PostSaving.HTML; sm-demo-app 262da37 (listener checks the rendered post). Live: `check-moderation.sh` ok, every word form refused. |
| WR-01 | warning | fixed | Logout leaves the chat store and the realtime socket of the previous user running. vue-demo-app 188ae25 (pinned in sm-demo-app 27c48cf with layers v0.1.1): logout and unauthorized reset the chat, close the realtime socket and dismiss the ring. Local: check-chat.cjs default mode, every /ws socket closed within 5 s of Log out (not run live: it posts in #general). |
| WR-02 | warning | deferred | Any member can pin or unpin any message in every public channel; the message is not checked against the channel. Todo: .planning/todos/pending/chatchannels-pin-policy.md |
| WR-03 | warning | deferred | Member-visible text outside the chat and forum seams is never moderated. Todo: .planning/todos/pending/moderation-profile-group-and-filenames.md |
| WR-04 | warning | fixed | A link opens a call and turns the camera and microphone on with no confirmation. vue-chatvideo-layer 5386903 (v0.1.1): the call page asks before media unless the call button or Accept handed over; vue-demo-app 9a12701 (pins). Live: `check-chat.cjs --call`, a direct /call load showed the confirmation with no LiveKit socket, tile or session before the click. |
| WR-05 | warning | fixed | A failed LiveKit connect posts `leave`, not `end`, leaving an empty ringing session behind. vue-chatvideo-layer 27ad81c (v0.1.1): a failed connect (or livekit-client import) ends a session nobody else joined, from GET status. Unit tests in useCall.test.ts; deployed with release 195aa213c546. |
| WR-06 | warning | deferred | The forum and friends clients have no 401 refresh; an expired token shows "not found". Todo: .planning/todos/pending/forum-friends-client-401-refresh.md |
| IN-01 | info | deferred | Group calls in public channels are blocked only by the seeded participant count. Todo: .planning/todos/pending/chatvideo-dm-only-host-hook.md |
| IN-02 | info | deferred | chatvideo checks participant rows, not live membership. Todo: .planning/todos/pending/chatvideo-live-membership.md |
| IN-03 | info | deferred | check-local passes member JWTs to curl in argv. Todo: .planning/todos/pending/check-local-jwt-out-of-argv.md |
| IN-04 | info | deferred | A chat message that starts with "/" can never be posted. Todo: .planning/todos/pending/chat-slash-text-escape.md |
| IN-05 | info | deferred | `demo:seed` brings back the default moderation lists after an admin empties them. Todo: .planning/todos/pending/demo-seed-moderation-lists-once.md |
| IN-06 | info | deferred | nginx `/livekit/` exposes LiveKit's whole HTTP surface. Todo: .planning/todos/pending/demo-nginx-livekit-rtc-only.md |
| IN-07 | info | deferred | The incoming-call runtime can leak a session handle and survives logout. The logout half was fixed by 20-08 (vue-demo-app 188ae25); the handle leak remains. Todo: .planning/todos/pending/chatvideo-incoming-handle-leak.md |
| IN-08 | info | deferred | The VAPID runbook pipeline runs without `pipefail`. Todo: .planning/todos/pending/demo-vapid-runbook-pipefail.md |
| IN-09 | info | deferred | `GET /chat/members` writes, and builds an unbounded `IN` list. Todo: .planning/todos/pending/demo-chat-members-read-only.md |

View File

@@ -0,0 +1,19 @@
---
title: Chat messages that start with "/" but name no command are posted as text
date: 2026-11-03
priority: low
area: vue-chat-layer app/composables/useChat.ts
files:
- ../vue-chat-layer/app/composables/useChat.ts
g15office:
---
## Problem
Every text starting with "/" (for example "/r/golang is great", or a path) is dispatched as a slash command and refused as unknown, so it can never be posted.
## Solution
Dispatch only when the first token matches a known trigger from the command catalog, or support a "//" escape.
Source: phase 20 code review (20-REVIEW.md IN-04).

View File

@@ -0,0 +1,21 @@
---
title: Make pinning in public channels a manage action, and check the pinned message belongs to the channel
date: 2026-11-03
priority: high
area: sm-chatchannels-plugin (classes/channel_service.go, api/pins.go), sm-demo-app plugins/demo/classes/chat_context.go
files:
- ../sm-chatchannels-plugin/classes/channel_service.go
- ../sm-chatchannels-plugin/api/pins.go
- ../sm-demo-app/plugins/demo/classes/chat_context.go
g15office:
---
## Problem
chatchannels gates PinMessage and UnpinMessage only with channel access, which the demo grants every member. So any visitor can unpin the seeded welcome pins or flood the pins panel. PinMessage also never checks that the message belongs to the channel's conversation.
## Solution
Additively in chatchannels: PinMessage and UnpinMessage require CanManageContext, or an optional contracts.PinPolicy the host implements; the demo answers it from its moderator group. Verify message.conversation_id equals the channel's conversation before creating the pin. Meanwhile the demo layer can hide pin actions for non-moderators, but the server must enforce the rule.
Source: phase 20 code review (20-REVIEW.md WR-02).

View File

@@ -0,0 +1,20 @@
---
title: A server-side DM-only rule for calls, not the seeded participant count
date: 2026-11-03
priority: medium
area: sm-chatvideo-plugin classes/video_service.go, sm-demo-app config/golem15.chatvideo.yaml
files:
- ../sm-chatvideo-plugin/classes/video_service.go
- ../sm-demo-app/config/golem15.chatvideo.yaml
g15office:
---
## Problem
The server has no DM-only rule; channel calls are refused today only because the seeded personas plus a visitor exceed max_participants: 4. Raising the cap, or a channel with fewer participants, would let any member ring everyone who opened that channel.
## Solution
Add an optional host hook in chatvideo (for example CanStartCall(conversationType)), or check conversation.type == dm in the demo, so the rule does not depend on seed data.
Source: phase 20 code review (20-REVIEW.md IN-01).

View File

@@ -0,0 +1,19 @@
---
title: The incoming-call runtime closes stale session handles
date: 2026-11-03
priority: medium
area: vue-chatvideo-layer app/composables/useIncomingCalls.ts
files:
- ../vue-chatvideo-layer/app/composables/useIncomingCalls.ts
g15office:
---
## Problem
Two video:call:incoming events for the same session, both pending in subscribe, both pass the pending-session check; the second overwrites rt.channel and the first handle is never closed. The logout half of this finding was fixed in phase 20 plan 08 (the demo's sign-out dismisses the ring); the handle leak remains.
## Solution
Keep a sequence number per watchSession and close the handle unless it is the latest.
Source: phase 20 code review (20-REVIEW.md IN-07).

View File

@@ -0,0 +1,20 @@
---
title: chatvideo honours live membership, so a ban ends call access at once
date: 2026-11-03
priority: medium
area: sm-chatvideo-plugin classes/video_service.go, classes/authorizer.go
files:
- ../sm-chatvideo-plugin/classes/video_service.go
- ../sm-chatvideo-plugin/classes/authorizer.go
g15office:
---
## Problem
chatvideo checks participant rows only. A ban revokes the user's JWTs, but an already minted LiveKit join token (900 s) and an open video:session subscription survive it, because chatvideo never consults contracts.LiveMembership as chat does.
## Solution
Have chatvideo honour LiveMembership (through chat's Service.IsParticipant) in requireParticipant and the authorizer, and remove the participant from the LiveKit room on a ban.
Source: phase 20 code review (20-REVIEW.md IN-02).

View File

@@ -0,0 +1,19 @@
---
title: check-local passes member JWTs to curl through a config on stdin, not argv
date: 2026-11-03
priority: low
area: sm-demo-app scripts/check-local.sh
files:
- ../sm-demo-app/scripts/check-local.sh
g15office:
---
## Problem
check-local still puts member JWTs into -H "Authorization: Bearer ..." arguments, readable by other local users in /proc/*/cmdline. The LiveKit join token and the new check-moderation.sh already use a curl config on stdin. Local and throwaway users only, but inconsistent with the stated pattern.
## Solution
Give request() an optional JWT that it writes as header = "Authorization: Bearer ..." into a curl config on stdin (curl -K -), as check-moderation.sh does, and drop every -H Authorization argument.
Source: phase 20 code review (20-REVIEW.md IN-03).

View File

@@ -0,0 +1,19 @@
---
title: GET /_demo/api/v1/chat/members is read-only and needs no id list
date: 2026-11-03
priority: medium
area: sm-demo-app plugins/demo/chat_api.go
files:
- ../sm-demo-app/plugins/demo/chat_api.go
g15office:
---
## Problem
The GET assigns handles to members that lack one (two concurrent first loads race on the same rows) and 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.
## Solution
Select with the memberWhere subquery (WHERE id <> ? AND <memberWhere>) instead of an id list, and assign missing handles at registration or in demo:seed, not in a GET.
Source: phase 20 code review (20-REVIEW.md IN-09).

View File

@@ -0,0 +1,20 @@
---
title: nginx proxies only LiveKit's /rtc paths, not its whole HTTP surface
date: 2026-11-03
priority: low
area: sm-demo-app deploy/nginx/demo.summercms.io.conf, scripts/check-deploy.sh
files:
- ../sm-demo-app/deploy/nginx/demo.summercms.io.conf
- ../sm-demo-app/scripts/check-deploy.sh
g15office:
---
## Problem
Browsers need only /rtc and /rtc/validate, but the /livekit/ location also exposes LiveKit's Twirp RoomService on the public origin. It is protected by admin JWTs signed with the API secret, so exposing it is unnecessary.
## Solution
Use location ^~ /livekit/rtc { proxy_pass http://127.0.0.1:7890/rtc; ... } and answer 404 for the rest of /livekit/; adjust check-deploy and check-live. A vhost change on rome needs its own go-ahead.
Source: phase 20 code review (20-REVIEW.md IN-06).

View File

@@ -0,0 +1,19 @@
---
title: demo:seed fills the default moderation lists only on the first run
date: 2026-11-03
priority: low
area: sm-demo-app plugins/demo/console/seed_friends.go
files:
- ../sm-demo-app/plugins/demo/console/seed_friends.go
g15office:
---
## Problem
seedModerationLists refills both lists whenever both are empty, and every release runs demo:seed. An admin who deliberately empties both lists gets the defaults back at the next deploy.
## Solution
Seed only once: insert the lists when the settings row is created, or record a seeded flag.
Source: phase 20 code review (20-REVIEW.md IN-05).

View File

@@ -0,0 +1,19 @@
---
title: The VAPID runbook step runs with pipefail and appends the keys only when both exist
date: 2026-11-03
priority: low
area: sm-demo-app DEPLOY.md (Chat family, VAPID keys)
files:
- ../sm-demo-app/DEPLOY.md
g15office:
---
## Problem
The remote script uses set -e without pipefail. If herald:vapid-generate fails, nothing is appended but the SUBJECT line, and the re-run guard then refuses ("already in .env") although no key pair exists; recovery needs a hand edit. The keys are already on rome; this matters for a rebuild.
## Solution
Use set -eo pipefail, generate into a variable first, check both keys are non-empty, then append all three lines in one tee.
Source: phase 20 code review (20-REVIEW.md IN-08).

View File

@@ -0,0 +1,21 @@
---
title: Single-flight 401 refresh in the forum and friends clients, and a signed-out message instead of not found
date: 2026-11-03
priority: medium
area: vue-forum-layer app/utils/forumClient.ts, vue-userfriends-layer app/utils/friendsClient.ts
files:
- ../vue-forum-layer/app/utils/forumClient.ts
- ../vue-forum-layer/app/components/Forum/Thread.vue
- ../vue-userfriends-layer/app/utils/friendsClient.ts
g15office:
---
## Problem
The chat and video clients refresh once on 401; the forum and friends hooks are bare token getters and never retry. After the JWT expires, /forum/t/{id} shows "thread not found" (Thread.vue maps 401 to notFound) and posting fails, while /chat keeps working in the same tab.
## Solution
Give both auth hooks the {token, refresh, unauthorized} shape (the demo can pass its chat auth), add the chat client's single-flight refresh-and-retry to request(), and map 401 to its own signed-out message.
Source: phase 20 code review (20-REVIEW.md WR-06).

View File

@@ -0,0 +1,19 @@
---
title: Moderate profile names, friend-group names and chat attachment filenames
date: 2026-11-03
priority: high
area: sm-demo-app plugins/demo/moderation.go (with seams in user, userfriends and chat)
files:
- ../sm-demo-app/plugins/demo/moderation.go
g15office:
---
## Problem
The filter runs only on chat message content and forum titles and bodies. Profile name and surname (shown in /chat/members, mentions, DM headers, forum author_name and the incoming-call prompt), userfriends group names and descriptions, and chat attachment filenames reach other members unmoderated.
## Solution
Run Filter.Check on name and surname at registration and profile update (a user-plugin saving event if one exists, otherwise the demo's own route, without breaking the core user plugin). Do the same for userfriends group create and update (an additive GroupSaving seam if needed) and for attachment filenames (MessageSaving could carry them additively). Document any path left out as an explicit exception.
Source: phase 20 code review (20-REVIEW.md WR-03).