Skip to content

Messaging RLS gap: 13 policies and 3 membership predicates missing vs upstream #80

Description

@TortoiseWolfe

Split out of #77 (tier 3, item 19). Related to #75 (same class) and #69 (same subsystem).

Finding

Comparing RLS policy counts on the 16 tables both repos share, upstream is denser on 10 of them — 13 policies we don't have, concentrated exactly where message integrity lives:

Shared table Upstream Ours Δ
messages 13 10 −3
conversations 9 6 −3
payment_intents 5 4 −1
payment_results 5 4 −1
user_connections 5 4 −1
conversation_members 5 4 −1
subscriptions 4 3 −1
auth_audit_logs 3 2 −1
rate_limit_attempts 2 1 −1

(We are denser on user_profiles: 9 vs 6, from the worker-visibility work. Not all drift is a deficit.)

The gap tracks a set of helper functions upstream has and we don't:

  • is_conversation_member(), is_conversation_creator(), is_conversation_owner() — the predicates those extra messages/conversations policies are built on
  • enforce_message_update_columns() — a trigger that blocks column tampering on message UPDATE
  • reassign_group_owner_on_member_removal()

Why this is not a copy-paste job

Our messaging layer diverged deliberately and is ahead of upstream in places — isRLSError / isTransientFetchError discrimination, ensureKeys(), the 15s query timeout. Upstream went the other way (a provider seam for a .NET backend we don't have). #77 marks upstream's message-service.ts as do-not-import for exactly that reason.

So: port the policies and predicates, not the service file. And audit each of the 13 individually — some of the deltas are on payment tables we barely use, and a policy that makes sense upstream may be wrong here.

Specific item worth taking on its own

enforce_message_update_columns is self-contained, low-risk, and closes a real hole: without it, a recipient can UPDATE columns on a message they received. That is a small trigger and a good first commit.

Acceptance

  • Per-policy audit of all 13 deltas, with a written keep/skip decision for each
  • is_conversation_{member,creator,owner} added as SECURITY DEFINER with SET search_path, matching our existing 40/40 convention
  • Column-guard trigger on messages
  • tests/rls/ coverage that talks to PostgREST directly — per CI reports green on tests that cannot fail #76, a policy with no adversarial test is not evidence
  • All statements idempotent, inside the existing BEGIN;COMMIT; of the monolithic migration

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions