perf(db,webhook): backend follow-ups from the multi-user review

PR A of the post-review fix series. Addresses the two must-fix
backend findings (flow-media account scoping; webhook contact
lookup at scale) plus the two should-fix composite-index
additions, all in one migration + two TS files.

Migration 020 (`supabase/migrations/020_account_sharing_followups.sql`)
- Composite partial index `idx_automations_account_active_trigger`
  on `automations(account_id, trigger_type) WHERE is_active`.
  The 006-era partial index was account-blind so the planner had
  to post-filter by account_id on every webhook dispatch. With
  100+ admins in a shared account the post-filter step shows up
  in query plans; this index answers all three predicates from
  one lookup.
- Same shape for flows: `idx_flows_account_active` on
  `flows(account_id) WHERE status='active'` — paired to the
  `findEntryFlow` query in the runner.
- `flow-media` storage RLS rewritten to allow uploads / updates
  / deletes by any member of the account whose folder name
  matches `account-<account_id>/...`. Legacy `<auth.uid()>/...`
  paths remain writable by the original uploader so files
  uploaded pre-020 don't break. Reads stay public (the bucket
  was already public so Meta can fetch URLs without creds).

Webhook contact lookup (`src/app/api/whatsapp/webhook/route.ts`)
- The PR-8 conversion swapped `eq('user_id', userId)` for
  `eq('account_id', accountId)` but kept "pull every row, filter
  in JS" semantics. That was fine when account = one user but
  scales linearly with the team size × per-user contact count.
- Now pre-filters in SQL by the phone's last-8-digit suffix
  (mirroring `phonesMatch`'s trunk-prefix tolerance). The
  trailing-suffix `like` can't use a B-tree index, but the
  account_id filter on top of `idx_contacts_account` keeps
  the scan to a small account-scoped subset. Re-applies the
  strict `phonesMatch` check in JS on the candidate set for
  correctness — typically 0-2 rows, basically free.

Flow-media upload path (`src/components/flows/forms/node-config-form.tsx`)
- Resolves the caller's `profile.account_id` before upload and
  pathes the file under `account-<account_id>/...` to match the
  new 020 RLS policy. Legacy `<user_id>/...` paths keep working
  via the back-compat clause in the policy.

Verification
- `npm run lint`      → 0 errors (same 19 pre-existing warnings)
- `npm run typecheck` → clean
- `npm test`          → 366/366 pass
- `npm run build`     → succeeds

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Arnas Donauskas 2026-05-31 19:18:58 +03:00
parent bf2addc7c9
commit 55da996e52
No known key found for this signature in database
4 changed files with 187 additions and 8 deletions

View File

@ -40,6 +40,28 @@ as the sole owner of their own account and sees identical data.
### Changed ### Changed
- **Flow-media storage is now account-scoped.** Migration 016
pathed uploaded files under `auth.uid()/...`, which orphaned
flow media when a teammate left a shared account. New uploads
go under `account-<account_id>/...` and any account member
with the right role can edit them. Legacy paths remain
writable by the original uploader for backward compatibility.
- **Webhook contact lookup now pre-filters in SQL.** Previously
pulled every contact in an account just to JS-filter to one
row by phone — fine when account = one user, painful when
account = team. Pre-filter by phone suffix on the database
side; re-apply `phonesMatch` on the (typically 0-2 row)
candidate set.
### Migration required
- `supabase/migrations/020_account_sharing_followups.sql` —
composite partial indexes on `automations(account_id,
trigger_type) WHERE is_active` and `flows(account_id) WHERE
status='active'` for the engine dispatch hot path; updated
`flow-media` storage RLS to allow account-member writes under
the new path convention. Idempotent.
- **Role-aware UI gating across the app.** The inbox composer's - **Role-aware UI gating across the app.** The inbox composer's
send button + textarea, the "New broadcast / automation / flow" send button + textarea, the "New broadcast / automation / flow"
buttons, the "Add pipeline / deal" buttons, and the "Add / buttons, the "Add pipeline / deal" buttons, and the "Add /

View File

@ -875,21 +875,42 @@ async function findOrCreateContact(
phone: string, phone: string,
name: string name: string
): Promise<ContactOutcome | null> { ): Promise<ContactOutcome | null> {
// Look up existing contacts for this account. Switched from // Look up existing contacts for this account. We pre-filter in SQL
// user_id to account_id in PR 8 of the multi-user series so an // by the phone's last-8-digit suffix so we don't ship every contact
// inbound message lands on the team's existing contact row even // in the account over the wire just to JS-filter to one row. This
// if a different teammate created it. // matters at scale: a shared account with 5 teammates × 100 contacts
// each is 500 rows — the prior implementation pulled all of them on
// every inbound message.
//
// The `phonesMatch` helper considers two phones equal if they share
// the last 8 digits (trunk-prefix tolerance). We mirror that here as
// a `like` pattern, then re-run the strict comparison in JS on the
// narrowed candidate set. The candidate set is typically 0-2 rows,
// so the JS pass is effectively free.
//
// The trailing-suffix `like` cannot use a B-tree index, but the
// `account_id` filter on top of `idx_contacts_account` (017) means
// we sequential-scan a small, account-scoped subset.
const normalizedSender = phone.replace(/\D/g, '')
const phoneSuffix =
normalizedSender.length >= 8
? normalizedSender.slice(-8)
: normalizedSender
const { data: contacts, error: contactsError } = await supabaseAdmin() const { data: contacts, error: contactsError } = await supabaseAdmin()
.from('contacts') .from('contacts')
.select('*') .select('*')
.eq('account_id', accountId) .eq('account_id', accountId)
.like('phone', `%${phoneSuffix}`)
if (contactsError) { if (contactsError) {
console.error('Error fetching contacts:', contactsError) console.error('Error fetching contacts:', contactsError)
return null return null
} }
// Use phonesMatch for flexible matching // Re-apply phonesMatch on the candidate set for correctness — the
// SQL `like` is a coarse pre-filter; phonesMatch handles edge cases
// like leading-`+` and explicit trunk-zero handling.
const existingContact = contacts?.find((c: ContactRow) => phonesMatch(c.phone, phone)) const existingContact = contacts?.find((c: ContactRow) => phonesMatch(c.phone, phone))
if (existingContact) { if (existingContact) {

View File

@ -913,8 +913,22 @@ function SendMediaForm({
if (userErr || !user) { if (userErr || !user) {
throw new Error("Not signed in."); throw new Error("Not signed in.");
} }
// Path convention matches migration 016's RLS policy: first // Resolve the caller's account_id so the path is account-
// segment must equal auth.uid(). Timestamp + random suffix // scoped rather than user-scoped. The 020 migration's RLS
// policy allows any account member to write under
// `account-<account_id>/...`, so a flow's media survives a
// teammate leaving the account (which was a data-integrity
// hole in the original 016 storage policies).
const { data: profile, error: profileErr } = await supabase
.from("profiles")
.select("account_id")
.eq("user_id", user.id)
.maybeSingle();
if (profileErr || !profile?.account_id) {
throw new Error("Could not resolve your account.");
}
// Path convention matches migration 020's RLS policy: first
// segment is `account-<uuid>`. Timestamp + random suffix
// prevents collisions between two builders open at once. // prevents collisions between two builders open at once.
const ext = file.name.split(".").pop()?.toLowerCase() ?? "bin"; const ext = file.name.split(".").pop()?.toLowerCase() ?? "bin";
const safeBase = const safeBase =
@ -922,7 +936,7 @@ function SendMediaForm({
.replace(/\.[^.]+$/, "") .replace(/\.[^.]+$/, "")
.replace(/[^a-zA-Z0-9_-]+/g, "_") .replace(/[^a-zA-Z0-9_-]+/g, "_")
.slice(0, 40) || "file"; .slice(0, 40) || "file";
const path = `${user.id}/${Date.now()}-${safeBase}.${ext}`; const path = `account-${profile.account_id}/${Date.now()}-${safeBase}.${ext}`;
const { error: upErr } = await supabase.storage const { error: upErr } = await supabase.storage
.from(FLOW_MEDIA_BUCKET) .from(FLOW_MEDIA_BUCKET)
.upload(path, file, { .upload(path, file, {

View File

@ -0,0 +1,122 @@
-- ============================================================
-- 020_account_sharing_followups.sql — review-board fixes for
-- the multi-user accounts series (#167-#177).
--
-- Two concerns this migration addresses:
--
-- 1. Engine dispatch indexes — the per-inbound automations and
-- flows lookups now scope by `account_id + trigger_type/status
-- + is_active/status='active'`. The pre-017 partial indexes
-- (`idx_automations_active_trigger`, no flows equivalent) were
-- account-blind. For shared accounts with 100+ teammates each
-- authoring rules, the planner ends up post-filtering by
-- account_id. Composite partial indexes drop the post-filter
-- cost to zero on the hot path.
--
-- 2. Flow-media storage scoping — migration 016 created the
-- `flow-media` bucket with per-user RLS policies keyed on
-- `auth.uid() = path[0]`. After the multi-user move, flows
-- are account-scoped but the storage paths remained user-
-- scoped: an agent who left the account would orphan every
-- flow node referencing media they had uploaded. This
-- migration switches the write policies to account-scoped
-- paths (`account-<account_id>/...`) while leaving the
-- legacy `<auth.uid()>/...` paths writable by their original
-- uploader for backward compatibility. The bucket is public,
-- so reads are unchanged.
--
-- Idempotent — safe to run multiple times.
-- ============================================================
-- ============================================================
-- COMPOSITE INDEXES — engine dispatch hot path
-- ============================================================
-- `runAutomationsForTrigger` queries
-- automations WHERE account_id = X AND trigger_type = Y AND is_active = TRUE
-- Migration 006 added a partial index on (trigger_type) WHERE is_active.
-- Composite + partial index lets the planner answer all three predicates
-- from one index lookup. The existing partial index can stay as belt-and-
-- braces for any code path that filters only by trigger_type.
CREATE INDEX IF NOT EXISTS idx_automations_account_active_trigger
ON automations(account_id, trigger_type)
WHERE is_active = TRUE;
-- `findEntryFlow` queries
-- flows WHERE account_id = X AND status = 'active'
-- Migration 017 only added `idx_flows_account`; this partial composite
-- is tuned for the engine's lookup and skips archived/draft rows.
CREATE INDEX IF NOT EXISTS idx_flows_account_active
ON flows(account_id)
WHERE status = 'active';
-- ============================================================
-- FLOW-MEDIA STORAGE — account-scoped writes
--
-- New path convention: `account-<uuid>/<timestamp>-<base>.<ext>`
-- Legacy path convention: `<uuid>/<timestamp>-<base>.<ext>` (where
-- the uuid is auth.uid() — preserved for back-compat).
--
-- Reads stay public (the bucket is public so Meta can fetch media
-- URLs without credentials). Only the write policies change.
--
-- Drop existing per-user policies and replace with account-aware
-- ones that accept either path convention.
-- ============================================================
DROP POLICY IF EXISTS "Users can upload their own flow media" ON storage.objects;
DROP POLICY IF EXISTS "Users can update their own flow media" ON storage.objects;
DROP POLICY IF EXISTS "Users can delete their own flow media" ON storage.objects;
DROP POLICY IF EXISTS "Members can upload flow media" ON storage.objects;
CREATE POLICY "Members can upload flow media"
ON storage.objects FOR INSERT
WITH CHECK (
bucket_id = 'flow-media'
AND (
-- New: any account member uploading under their account's folder.
-- `'account-' || account_id` is how we namespace the folder, so
-- two accounts that happen to be in the same Supabase project
-- can never accidentally collide.
EXISTS (
SELECT 1 FROM public.profiles p
WHERE p.user_id = auth.uid()
AND ('account-' || p.account_id::text) = (storage.foldername(name))[1]
)
-- Legacy: the original uploader keeps write access to files they
-- already uploaded under the pre-020 path convention.
OR auth.uid()::text = (storage.foldername(name))[1]
)
);
DROP POLICY IF EXISTS "Members can update flow media" ON storage.objects;
CREATE POLICY "Members can update flow media"
ON storage.objects FOR UPDATE
USING (
bucket_id = 'flow-media'
AND (
EXISTS (
SELECT 1 FROM public.profiles p
WHERE p.user_id = auth.uid()
AND ('account-' || p.account_id::text) = (storage.foldername(name))[1]
)
OR auth.uid()::text = (storage.foldername(name))[1]
)
);
DROP POLICY IF EXISTS "Members can delete flow media" ON storage.objects;
CREATE POLICY "Members can delete flow media"
ON storage.objects FOR DELETE
USING (
bucket_id = 'flow-media'
AND (
EXISTS (
SELECT 1 FROM public.profiles p
WHERE p.user_id = auth.uid()
AND ('account-' || p.account_id::text) = (storage.foldername(name))[1]
)
OR auth.uid()::text = (storage.foldername(name))[1]
)
);
-- Public read policy from 016 stays as-is; reads cross both path
-- conventions without modification.