From a24982463bf671da3702bfdcde7d4cf416a98c12 Mon Sep 17 00:00:00 2001 From: Jakob Wennberg Date: Wed, 2 Sep 2026 17:57:34 +0200 Subject: [PATCH] fix(parties): skip nameless suppliers and customers in the party backfill (#2174) * fix(parties): skip nameless suppliers and customers in the party backfill The substrate migration failed on prod at the backfill: three rows (one supplier, two customers) have an empty name and ensure_party refuses a nameless party. The file never applied there, so it is corrected in place rather than chased with a migration that could not run before it. Rows without a name keep party_id NULL; the suggestion pipeline names them. Co-Authored-By: Claude Fable 5.1 * fix(parties): ensure_party writes only under the caller's own identity Authenticated callers must pass their own user id; the service role (auth.uid() NULL: migrations, MCP, cron) may act for another user. Same guard as apply_party_suggestions and decide_parties. Superagent P2 on #2172. Co-Authored-By: Claude Fable 5.1 --------- Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com> Co-authored-by: Claude Fable 5.1 --- DECISIONS.md | 1 + .../20260902160000_parties_substrate.sql | 11 ++++++++++- tests/pg/parties-substrate.pg.test.ts | 18 ++++++++++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/DECISIONS.md b/DECISIONS.md index c5262191..33c58eb7 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1500,3 +1500,4 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-09-02] Bank-sync cooldown is a durable lease column (bank_connections.sync_lease_until, migration 20260902150000) claimed with one conditional UPDATE, not a process-local attempt map: the security scan on PR #2165 showed the map is bypassed by a second serverless instance or a cold start, so two agent calls could each bill Enable Banking. A column add was chosen over reusing extension_data because PostgREST cannot express an atomic conditional upsert there; the nightly cron deliberately ignores the lease. [2026-09-02] Grok links carry auth=required like the claude.ai link (#2159), decided from a live test: on the lazy URL Grok's connector dialog listed all 150+ tools and never opened the sign-in, so it reads the 200 probe as an authless server exactly as claude.ai does. The flag lives in one helper (mcpServerUrl / sideDoorServerUrl in lib/onboarding/checklist.ts) so the settings row, the onboarding side door and the deep link cannot drift; ChatGPT stays lazy because its developer mode honours the 401 on the first protected call. [2026-09-02] parties children/roles reference parties(id, company_id) with composite FKs, not parties(id): a party UUID from another tenant is rejected by construction instead of relying on each writer to check; ON DELETE SET NULL (party_id) on customers/suppliers because a plain SET NULL would null company_id too (Superagent P2 on #2162) +[2026-09-02] Edited migration 20260902160000 after merge: its backfill failed on prod (ensure_party: name is required; 3 nameless rows) so it was never applied there, the Supabase main branch sat in MIGRATIONS_FAILED and every later migration was blocked behind it. An unapplied file is not a shipped schema; a follow-up migration could not run before it diff --git a/supabase/migrations/20260902160000_parties_substrate.sql b/supabase/migrations/20260902160000_parties_substrate.sql index 5d96b6e7..dd1640fd 100644 --- a/supabase/migrations/20260902160000_parties_substrate.sql +++ b/supabase/migrations/20260902160000_parties_substrate.sql @@ -269,6 +269,11 @@ DECLARE v_name text := btrim(coalesce(p_name, '')); v_id uuid; BEGIN + -- Authenticated callers write under their own identity; only the service + -- role (auth.uid() NULL: migrations, MCP, cron) may act for another user. + IF auth.uid() IS NOT NULL AND auth.uid() <> p_user_id THEN + RAISE EXCEPTION 'ensure_party: p_user_id must be the caller' USING ERRCODE = '42501'; + END IF; IF v_name = '' THEN RAISE EXCEPTION 'ensure_party: name is required'; END IF; @@ -298,7 +303,9 @@ GRANT EXECUTE ON FUNCTION public.ensure_party(uuid, uuid, text, text, text, text -- Suppliers first (they carry payment data), then customers, so a company -- that both buys from and sells to the same organisation ends up with one -- party carrying both roles. Rows without a valid org number get their own --- party; nothing is merged on name. +-- party; nothing is merged on name. Rows with an empty name (three exist on +-- prod: one supplier, two customers) keep party_id NULL: ensure_party +-- refuses a nameless party, and the suggestion pipeline names them later. DO $$ DECLARE r record; @@ -307,6 +314,7 @@ BEGIN SELECT id, company_id, user_id, name, org_number FROM public.suppliers WHERE party_id IS NULL AND company_id IS NOT NULL + AND nullif(btrim(name), '') IS NOT NULL ORDER BY created_at, id LOOP UPDATE public.suppliers @@ -318,6 +326,7 @@ BEGIN SELECT id, company_id, user_id, name, org_number FROM public.customers WHERE party_id IS NULL AND company_id IS NOT NULL + AND nullif(btrim(name), '') IS NOT NULL ORDER BY created_at, id LOOP UPDATE public.customers diff --git a/tests/pg/parties-substrate.pg.test.ts b/tests/pg/parties-substrate.pg.test.ts index bb422af9..1527b427 100644 --- a/tests/pg/parties-substrate.pg.test.ts +++ b/tests/pg/parties-substrate.pg.test.ts @@ -139,6 +139,24 @@ describe('parties substrate (pg)', () => { expect(visibleToStranger).toBe(0) }) + it('ensure_party refuses a p_user_id that is not the authenticated caller', async () => { + const mine = await seedCompany() + const other = await seedCompany() + await expect( + withUserContext(mine.userId, (client) => + client.query(`SELECT public.ensure_party($1, $2, 'Spoofed AB', NULL, 'company', 'manual')`, [mine.companyId, other.userId]), + ), + ).rejects.toMatchObject({ code: '42501' }) + const own = await withUserContext(mine.userId, async (client) => { + const { rows } = await client.query<{ id: string }>( + `SELECT public.ensure_party($1, $2, 'Own AB', NULL, 'company', 'manual') AS id`, + [mine.companyId, mine.userId], + ) + return rows[0]!.id + }) + expect(own).toMatch(/^[0-9a-f-]{36}$/) + }) + it('refuses to attach facts, identities, decisions, roles or merges to another company\'s party', async () => { const mine = await seedCompany() const theirs = await seedCompany()