Any authenticated user could fetch a channel webhook's avatar, or be redirected to its external profile image URL, without belonging to the channel or holding any read access to it. This was the only webhook route with neither a channel check nor the channels feature gate.
The route now applies the same read gate every other route in this router uses: active membership for group and direct message channels, admin or a channel read grant otherwise, answering with 403 on denial and 404 when the webhook's channel row no longer exists. It also runs the channels feature and permission gate, so with channels disabled, or the permission withdrawn from regular users, the endpoint now refuses where it previously served the image.
Avatars keep rendering for channel members, and a denied request shows the default logo rather than a broken image, because the avatar component already falls back on an image error.
* perf: stop the Socket.IO session pool blocking the websocket event loop
With WEBSOCKET_MANAGER=redis the session pool is a synchronous Redis client, so every call into it blocks the whole worker's event loop, not just the caller. Two paths did it constantly: the orphan reaper walked the pool one round trip per session with no await anywhere, freezing the loop for the entire sweep every cycle, and nearly every socket event re-read the sender's session back out of Redis. Other users' events and every in-flight generation on that pod wait behind both.
The reaper now walks the pool in HSCAN batches and deletes in bulk, yielding between batches, and no longer sleeps past half the lock TTL, which previously guaranteed a failed renew every cycle. The per-event reads are gone: Socket.IO events only reach the worker holding the connection, and that worker already saved the same session dict locally when the user authenticated, so it was asking Redis for its own data. The writes stay, since those are what other pods read.
Measured at 4000 users / 16 containers, Redis 1.1 ms away:
| | before | after |
|---|---|---|
| reaper sweep, 5k sessions | 5.6 s, loop frozen throughout | 62 ms, 4.2 ms worst block |
| same, crash recovery with every session expired | 11.5 s | 96 ms |
| heartbeat / usage ping / disconnect | 2 / 3 / 2 round trips | 1 / 2 / 1 |
| 50-member channel post | 50 round trips, 57.2 ms block | 0 round trips, 0.02 ms |
| loop time per wall second at rest | 103 ms (10.3%) | 67 ms (6.7%) |
The alternative, converting RedisDict to the async client, fixes the same paths with a far larger blast radius (every call site gains await, and `in`/`[]`/`del` cannot be awaited so the dict interface goes) and still round-trips for data already in memory. Two deliberate behaviour changes: a heartbeat re-adds a session the reaper already removed, so a tab that survives a stall recovers instead of staying out of the pool until it reconnects; and disconnect no longer skips Yjs document cleanup when the pool entry is already gone, which previously leaked that document's update log forever.
Closes#28172
* perf: cut disconnect and user session lookup pool round trips, harden the session reaper
Follow-up on top of the session pool reaper branch. With WEBSOCKET_MANAGER=redis two paths still blocked the worker's event loop on synchronous Redis calls. Every disconnect listed all models in use cluster-wide and fetched each one individually, one blocking round trip per model. Disconnecting all sessions of a user (admin role change or deletion) pulled the entire session pool in one HGETALL and decoded every entry in a single uninterrupted block.
Disconnect now fetches the usage pool once with items(), going from 2+N+M round trips to 2+M (N models in use cluster-wide, M models the session used), and its delete of an emptied model entry is KeyError-guarded because another node can remove the same key between snapshot and delete; unguarded, that race aborted the handler and skipped its Yjs document cleanup. The user session lookup reuses the reaper's HSCAN batches and yields to the loop between pages. The reaper previously died permanently on the first Redis connection error, on every node at once during an outage; it now logs, releases the lock and returns to retrying acquisition.
* refac: keep the socket pool perf work to the round trips
A review pass on this branch turned up four changes riding along with the round-trip work without belonging to it, so they are backed out here. The `disconnect` handler keeps its `if sid in SESSION_POOL:` guard, so USAGE_POOL and ydoc cleanup stay off the path for sockets that never authenticated. `RedisDict.set()` keeps its own inline HDEL. `get_session_ids_by_user_id` stays synchronous over one HGETALL, since it runs on user delete and role change rather than per message. The crash-resilience wrapper around the reaper loop is dropped; if that guard is worth having, it belongs in its own change.
What stays is the perf part. The reaper now sweeps the pool in bounded HSCAN batches and deletes expired sids with one HDEL per batch, down from HKEYS plus an HGET and a per-sid HDEL across the whole pool. The `disconnect` handler reads USAGE_POOL with a single HGETALL, down from HKEYS plus one HGET per model in use. Session lookups in the socket handlers come from the local Socket.IO store, which removes one Redis GET from every heartbeat, usage, channel and ydoc event.
Naming and annotations follow the file: `get_session_pool_batches` for the module's `get_` prefix, `RedisDict.pop_many` so both reaper branches use one word for removing keys, a named `SCAN_BATCH_SIZE`, and types on the new helpers.
* fix: invalidate the RedisDict write signature on batch delete
RedisDict.set() skips the write when the payload fingerprint matches the last one this process wrote, so a mutation that goes around set() has to clear that fingerprint. The new batch delete did not, leaving a stale fingerprint behind: the next refresh with identical content is treated as already written and silently skipped, so the hash stays empty.
Renamed pop_many to delete_many. In a dict emulation pop removes and returns; this returns nothing and cannot without an extra HMGET, so the name promised something it does not do. delete_many matches __delitem__ and the HDEL underneath. Its only call site is the session pool reaper, whose behaviour is unchanged: same fields deleted, same batching, same return.
Fourteen modules import `json` without using it. Ruff flags every one with F401, and a word-boundary search for `json` in each file matches only the import line itself, including inside strings, comments and annotations.
Two exclusions, both deliberate. Migration files are left alone: the import is equally dead there, but those files are frozen history and not worth the churn. `models/chats.py` has the same dead import and is handled in its own change, so it is skipped here to avoid two changes touching the same line.
No behaviour change.
Create and publish Docker images with specific build args / build (map[arch:linux/arm64 runner:ubuntu-24.04-arm], map[build_args:USE_CUDA=true
USE_CUDA_VER=cu126
free_disk:true name:cuda126 suffix:-cuda126]) (push) Has been cancelled
Create and publish Docker images with specific build args / build (map[arch:linux/arm64 runner:ubuntu-24.04-arm], map[build_args:USE_CUDA=true free_disk:true name:cuda suffix:-cuda]) (push) Has been cancelled
Create and publish Docker images with specific build args / build (map[arch:linux/arm64 runner:ubuntu-24.04-arm], map[build_args:USE_OLLAMA=true free_disk:false name:ollama suffix:-ollama]) (push) Has been cancelled
Create and publish Docker images with specific build args / build (map[arch:linux/arm64 runner:ubuntu-24.04-arm], map[build_args:USE_SLIM=true free_disk:false name:slim suffix:-slim]) (push) Has been cancelled
Create and publish Docker images with specific build args / build (map[arch:linux/amd64 runner:ubuntu-latest], map[build_args:USE_OLLAMA=true free_disk:false name:ollama suffix:-ollama]) (push) Has been cancelled
Create and publish Docker images with specific build args / build (map[arch:linux/amd64 runner:ubuntu-latest], map[build_args:USE_SLIM=true free_disk:false name:slim suffix:-slim]) (push) Has been cancelled
Create and publish Docker images with specific build args / build (map[arch:linux/arm64 runner:ubuntu-24.04-arm], map[build_args: free_disk:false name:main suffix:]) (push) Has been cancelled
Create and publish Docker images with specific build args / build (map[arch:linux/amd64 runner:ubuntu-latest], map[build_args: free_disk:false name:main suffix:]) (push) Has been cancelled
Create and publish Docker images with specific build args / build (map[arch:linux/amd64 runner:ubuntu-latest], map[build_args:USE_CUDA=true
USE_CUDA_VER=cu126
free_disk:true name:cuda126 suffix:-cuda126]) (push) Has been cancelled
Create and publish Docker images with specific build args / build (map[arch:linux/amd64 runner:ubuntu-latest], map[build_args:USE_CUDA=true free_disk:true name:cuda suffix:-cuda]) (push) Has been cancelled
The channel message update and delete handlers enforced authorship only on group and dm channels. On standard channels the else branch accepted any caller holding write access on the channel, so a member who could post could also edit or delete messages authored by other members. Because the update form binds content, data and meta, and the model layer never touches message.user_id, an edited message kept the original author's attribution, so another member's message could be rewritten under their name.
Write access on a channel is the capability to post, not a moderation capability, and the frontend gates the edit and delete controls on authorship (message.user_id === user.id, or admin) for every channel type. The group and dm branch already encodes this with an explicit authorship check. Apply the same rule to the standard branch: the caller must hold write access on the channel and be the message author, unless they are an admin. Pinning is unchanged, since it is exposed to every member by design.
GET /api/v1/channels/{id}/messages/{message_id}/thread authorized only the URL channel, but get_messages_by_parent_id() appended the thread parent (loaded by id) without checking it belonged to that channel, so a caller could read a message from a channel they cannot access by passing its id as the thread root. Require the parent to be in the requested channel before returning it, and reject a posted parent_id/reply_to_id that does not belong to the channel.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace per-message database queries with batch IN-clause queries in
channel message handlers. This eliminates the N+1 query pattern that
caused ~102 queries per channel page load (50 messages × 2 queries each).
Changes:
- Add get_reactions_by_message_ids() to MessageTable: single query
fetches all reactions for multiple messages using IN clause with
User JOIN, returns dict[message_id, list[Reactions]]
- Add get_thread_reply_counts_by_message_ids() to MessageTable: single
GROUP BY aggregate query returns (count, max_created_at) per parent,
replacing full object loads just to call len()
- Refactor get_channel_messages(): 102 → 4 queries per page
- Refactor get_pinned_channel_messages(): 22 → 3 queries per page
- Refactor get_channel_thread_messages(): 53 → 4 queries per page
- Refactor send_notification(): N+1 membership check → batch set lookup
* fix: enforce message ownership in group/DM channel update + delete endpoints
`update_message_by_id` (channels.py:1348) and `delete_message_by_id`
(channels.py:1550) branch on `channel.type`. The `else` branch (standard
channels) correctly enforces `message.user_id != user.id` ownership before
mutating, but the `if channel.type in ['group', 'dm']` branch only checked
`is_user_channel_member` — channel membership alone, with no message
ownership verification.
Effect on group/DM channels: any verified member of the conversation could:
- overwrite another member's message content while the server preserved
`user_id=victim`, producing tampered content that renders to other
members as the original author's authentic post (integrity + authenticity);
- silently delete another member's messages, removing them from
conversation history without trace (integrity).
Reproduced end-to-end against v0.9.4 with three users (attacker, victim,
viewer) sharing a group channel: attacker overwrites victim's message and
deletes another, viewer reads the tampered content as victim-authored.
Two patches, identical shape, mirror the `else` branch's existing
ownership semantics:
- `update_message_by_id` group/DM branch: add
`if user.role != 'admin' and message.user_id != user.id: raise 403`
immediately after the `is_user_channel_member` check.
- `delete_message_by_id` group/DM branch: same.
The standard-channel branch is unchanged (it already enforced ownership).
Admins remain able to moderate any message, matching the existing semantic
in the standard-channel branch.
Reports consolidated under GHSA-wwhq-cx22-f7vv (earliest live filing of the
group/DM-specific variant). Same gap previously surfaced and partially
fixed under GHSA-jxwr-g6r6-j3fx (which addressed the standard-channel
branch only) — this completes the cohort.
* chore: trim comments
`pin_channel_message` (channels.py:1242) checked `permission='read'` on
the standard-channel branch before mutating `is_pinned` / `pinned_by` /
`pinned_at` via `Messages.update_is_pinned_by_id`. Pin/unpin is a write
operation; gating it on read access let any user with read-only channel
access pin or unpin any message in the channel, including admin posts.
One-character fix: change `permission='read'` to `permission='write'`.
Reported by kikayli in GHSA-5gc6-xhv4-2wg6.
Co-authored-by: kikayli <kikayli@users.noreply.github.com>
* perf(channels): batch user lookup in model_response_handler thread history
The thread-history builder in model_response_handler called
Users.get_user_by_id once per thread message (deduped via an intra-loop
dict), producing N individual SELECTs for a thread of N unique authors.
Replace with a single Users.get_users_by_user_ids call that returns all
authors in one WHERE id IN (...) query, matching the batch pattern
already used elsewhere in this file (lines 739, 804, 1320).
Behavior is preserved: deleted users still resolve to None and fall
through to the existing 'Unknown' fallback via .get().
* refac(channels): rename loop vars to full words per review
Address reviewer feedback to use descriptive names `message` and `user`
instead of single-letter `m` and `u` in the batch user-lookup
comprehensions.
---------
Co-authored-by: Claude <noreply@anthropic.com>
Unlike all other resource routers (knowledge, models, notes, prompts, tools, skills), the channel router did not call filter_allowed_access_grants. This allowed any user to set wildcard access grants on group channels, bypassing the admin's public sharing permission framework.
Adds filter_allowed_access_grants with the sharing.public_channels permission key to both create and update endpoints, matching the pattern used by all other resource routers.
The GET /channels/{id}/members endpoint checked membership for group/dm channels but had no access gate for standard channels, allowing any authenticated user with channels permission to enumerate members of private standard channels by UUID.
Fix `AttributeError` in `model_response_handler` when processing channel messages with `null` data field. The function iterates over thread messages to build conversation history, but some messages may have `data=None` causing a crash when accessing `thread_message.data.get()`. Added null check using `(thread_message.data or {}).get("files", [])` to safely handle messages without data.
