Repository navigation
fix(engine): scope observer DM listing queries - #520
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to No confirmed issue prevents merging after normal checks; behavior for observer tokens without an explicit DM allowlist remains unverified. Pre-merge checks |
|
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary
dm_conversation_idsallowlist into the initial DM conversation queryWhy
A Relay Connect observer scoped to one DM still loaded and enriched every DM in the workspace before filtering. In the affected 5,268-conversation workspace that meant 203 sequential D1 statements for a capability allowed to see one conversation.
Validation
npm test --workspace @relaycast/engine(Node 22) — 117 files, 1,369 tests passednpm run typecheck --workspace @relaycast/engine— cleannpm run lint --workspace @relaycast/engine— cleangit diff --check— cleanCausal proof
The >100-conversation conformance fixture now checks both a 105-ID observer allowlist and a one-ID observer allowlist under D1's 100-bind ceiling. The one-ID observer returns only its authorized conversation and records exactly five DM-listing statements. Reverting only the engine/route pushdown makes the same test fail with 9 statements instead of 5; restoring the change passes.
Security
The scoped initial query retains the workspace predicate, deduplicates supplied conversation IDs, and keeps the existing
observerAllowsConversationpost-filter as defense in depth. No observer can load a conversation from another workspace.Remaining caveat
Unrestricted workspace-key and unscoped observer reads intentionally retain the workspace-wide enrichment path. This PR fixes the narrow observer-link hot path without changing response shape or dashboard polling behavior.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Note
Low Risk
Performance and query-scoping change on an existing read path; authorization still uses workspace bounds plus post-filter, with no new exposure surface.
Overview
GET /v1/dm/conversations/allnow applies an observer token’sdm_conversation_idsallowlist in the first conversation query instead of loading and enriching every workspace DM and filtering afterward.listAllDmConversationsaccepts optionalconversationIds, deduplicates them, returns early when empty, and uses D1-safe chunking when the allowlist is large. The route derives scoped IDs from normalized observer filters (include_dms/dm_conversation_ids) while workspace-key reads stay unscoped;observerAllowsConversationremains as defense in depth.Conformance coverage adds a single-conversation observer fixture that asserts one returned DM and five listing SQL statements with an
id IN (?)predicate on the initial select.Reviewed by Cursor Bugbot for commit b413333. Bugbot is set up for automated code reviews on this repo. Configure here.