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.
Replaced per-message user lookup with batch fetch using SQL IN clause.
Changes:
- Fetch all message user_ids in a single pass
- Use Users.get_users_by_user_ids() for batch lookup
- Build user mapping to avoid DB calls in loop
- Add early return for empty message lists
Performance: Reduces N+1 queries to 2 queries (messages + users)
Replaced per-message user lookup with batch fetch using SQL IN clause.
Changes:
- Fetch all message user_ids in a single pass
- Use Users.get_users_by_user_ids() for batch lookup
- Build user mapping to avoid DB calls in loop
- Add early return for empty message lists
Performance: Reduces N+1 queries to 2 queries (messages + users)
Timothy Jaeryang Baek