A CRM contact's identity was decided by a column its subject can rewrite

[NewWorldVenture]

Three commits. A takeover of a stranger's crm_contacts row; a cleanup of the ILIKE class around it that also fixes a live regression on main; and the E2E fixture fix that unblocks main, which is red right now.

---

1 — The identity key was user-writable

upsertOnboardingContact runs as the service role against crm_contacts, whose RLS is admin-only, and the row its lookup matches is the row it then overwrites with the caller's name, email, phone and family_id.

Two independent faults, each sufficient alone:

1. The key. Both callers resolved email: profile?.email ?? auth.user.email. profiles.email is a plain text column, and profiles_update_self constrains which row you may update, not which columns — so its owner sets it to anything, including a stranger's exact address. No wildcard needed.
2. The pattern. That value went unescaped into .ilike('email', email), so % matched every contact and .limit(1) picked one.

Measured on a replayed database, as authenticated

Step	Result
attacker rewrites their own profiles.email to %	1 row
server reads profiles.email (preferred over the verified address)	%
.ilike matches a contact they never owned	yes
service-role update overwrites the victim's row	1 row — first_name=Attacker

Controls, same session, proving crm_contacts stayed shut to ordinary users throughout — so the only way in is the service-role path the app itself takes:

Control	Result
rows an ordinary user can read	0
rows changed by a direct write	0 (value verified untouched afterwards)

My first version of that second control tested for an insufficient_privilege exception and reported "blocked: f". That was wrong — an UPDATE matching no RLS-visible row changes 0 rows and raises nothing. Re-measured on ROW_COUNT.

Third consequence, same root cause: fireAutomationEvent took the same column, and runSteps sends to: recipient.email through Resend from the product's own FROM_EMAIL — so it also chose who receives branded mail on our behalf.

Fix: the verified auth.user.email now wins at all four call sites, and the three identity lookups escape. saveUserProfile's own email: profile.email is untouched — that's the user writing their own row. Escaping alone would not have been enough; the exact-address takeover needs no wildcard, so the preference change is the load-bearing half.

---

2 — Escaping exactly once, including what no guard could see

Rebased onto a50433ce, which consolidated escapeLike and added an enforcement test. I had written my own helper first; it's deleted and its call sites repointed rather than shipping a fifth copy.

That guard matches only template literals, so two shapes were invisible to it.

Bare-value sites — .ilike('email', email) has no backticks. Four were still raw: the three crm_contacts lookups, .ilike('category', b.category) in the digital twin, and raw search terms in meals and finances.

Values escaped twice — four sites already escaped by hand upstream, then had escapeLike() added at the call site:

const term = title.trim().replace(/[%_]/g, (m) => `\\${m}`);  // once
... .ilike('title', `%${escapeLike(term)}%`)                   // twice

50% becomes 50\\\%, which LIKE reads as a literal backslash then a literal percent. Measured in Postgres 16:

pattern	matches 50% off groceries
%50\% off groceries% (once)	t
%50\\\% off groceries% (twice)	f

So on main right now, findReminder, the task search, and the grocery and meal searches cannot find any row whose name contains % or _.

Fix: redundant upstream escapes removed (escaping stays at the call site, that commit's own convention), raw sites escaped, and the guard gains two rules — one for bare-value patterns, one forbidding a hand-rolled [%_] escape outside the helper. Both fail against main and pass here.

Two of those double-escapes were mine, introduced minutes earlier when a scripted inline→helper conversion overlapped a scripted raw-site fix and each added an escape. Typecheck caught one; a scan for const x = escapeLike(...) feeding escapeLike(x) caught the other. The new guard makes the class impossible to reintroduce quietly.

---

3 — The E2E fixture, which is why main is red

0299 refuses, at commit, an end state where a family has members but no active parent or adult. tests/e2e/authenticated.spec.ts set up its wallet probe by demoting the family's sole manager to child, so since aca5c7f2 that step fails 23514 and the probe never runs — on main as well as on every branch built from it.

The migration is right and the fixture was wrong: the probe is about the signed-in member being a child, not about the family losing its last manager. It now gives the household a second manager for the duration and restores the original shape afterwards. user_id: null is the same managed-profile row the 'Probe child' above already uses.

The order in finally is load-bearing and commented as such: restore the signed-in member first, then drop the stand-in.

Proved against a replayed database with the trigger present, driving each step as its own transaction — the constraint is DEFERRABLE INITIALLY DEFERRED and only fires at COMMIT, so a PL/pgSQL exception block around the UPDATE cannot see it (which is how my first probe managed to report that 0299 had refused nothing):

Sequence	Result
demote the sole manager	UPDATE 1, then COMMIT raises 23514 — reproduces CI exactly
insert stand-in, then demote	commits; role reads child
restore, then drop the stand-in	both commit; family back to 2 members
drop the stand-in before restoring	refused 23514

---

Verification

Condition	Result
New tests	9, of which 4 + 2 fail against the original code
Full suite	13,722 / 13,722
E2E in CI	✅ green — the two authenticated.spec.ts failures are gone
Escaping vs Postgres 16	checked directly, incl. a real underscore still matching

tsc, eslint and the Supabase query audit clean. No migration, so this reaches production with the deploy.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KRUgA6hD6QgzmtpSP6TUmP

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated
bubaly	Ignored		Sep 15, 2026 12:32am UTC

[NewWorldVenture]

E2E (public · a11y · authenticated · mobile device matrix) is failing here, and it is not this PR's.

Both failures are tests/e2e/authenticated.spec.ts:83:7 › … › wallet money writes require a manager even when legacy policies existed, on chromium and iphone, with:

code: '23514'
message: 'A family must keep at least one active parent or adult, otherwise nobody
          can manage it. Add or promote another member first.'

That is the constraint trigger added by 0299_family_keeps_a_manager.sql in aca5c7f2.

Red on the base branch too, with the identical two tests and the identical message:

run	head	E2E result
34902370855	aca5c7f2 (main)	2 failed · 429 passed
34903487803	344d49e6 (this PR)	2 failed · 430 passed

This PR touches no migration, no tests/e2e/, and nothing in family membership or wallet code — only ILIKE escaping and the CRM contact identity key.

Why it breaks. The step sets up its probe by demoting the family's sole member to child so it can assert a child cannot mint wallet credits:

// tests/e2e/authenticated.spec.ts:213
const { error: childRoleError } = await service.from('family_members')
  .update({ role: 'child' }).eq('id', membership.id);

0299 is right to refuse that committed end state — it is exactly the lockout the migration exists to prevent. The fixture, not the migration, is what is wrong: the probe is about the member client being a child, not about the family having no manager.

Proposed patch — give the family a second active manager before the demotion, so the invariant holds and the probe still tests what it means to test:

// Keep a manager, or 0299's deferred constraint refuses the demotion at commit.
const { error: coParentError } = await service.from('family_members').insert({
  family_id: familyId, user_id: null, role: 'parent',
  display_name: 'Probe co-parent', is_active: true,
});
if (coParentError) throw coParentError;

const { error: childRoleError } = await service.from('family_members')
  .update({ role: 'child' }).eq('id', membership.id);

I have not pushed this. It belongs with 0299, whose author has the context on whether a user-less member row is acceptable in that fixture or whether a second seeded account is wanted instead — and pushing it here would widen this PR into that change while it is being actively worked on.

No re-run spent: the failure is deterministic and already confirmed on the base branch, so a re-run would tell us nothing a second time.

---

Generated by Claude Code