feat: multi-family isolation — scope all data by familyId - #7
Conversation
Adds familyId to all 9 financial models (Child, ParentProfile, Account, Transaction, SpendingCategory, SpendingEntry, Investment, TokenLedgerEntry, Goal). Each new signup gets their own familyId. All queries and mutations are scoped by familyId — families cannot see or modify each other's data. Changes: 1. Schema: add familyId column + index to 9 models. FamilySettings changes from singleton to per-family (keyed by familyId). 2. Migration: SQL to add columns, backfill existing data to founder's family, make NOT NULL, add indexes. 3. auth.ts: auto-generate familyId on signup + founder setup. getAuthUser() returns familyId. 4. db-queries.ts: getFullState(familyId) scopes all queries. All mutation functions accept familyId, include it in creates, and verify ownership on updates/deletes. 5. API routes: /api/state and /api/mutations pass familyId from getAuthUser() to all queries/mutations. BEFORE MERGING: run the migration SQL on Neon.
📝 WalkthroughWalkthroughThis PR introduces multi-family data isolation across the application: a new ChangesMulti-family isolation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant APIRoute
participant AuthLib
participant DbQueries
Client->>APIRoute: Request (state/mutation)
APIRoute->>AuthLib: getAuthUser()
AuthLib-->>APIRoute: user with familyId or null
alt missing user or familyId
APIRoute-->>Client: 401 Not authenticated
else authenticated
APIRoute->>DbQueries: query/mutation(familyId, ...)
DbQueries-->>APIRoute: family-scoped result
APIRoute-->>Client: response with data
end
Related Issues: None referenced. Related PRs: None referenced. Suggested labels: database, security, backend Suggested reviewers: isaJrKai 🐇 A rabbit hops from house to house, 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
prisma/schema.prisma (1)
161-184: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAdd the missing
Goal.familyIdindex to the Prisma schema.The migration creates
"Goal_familyId_idx"and reads usewhere: { familyId }, but the Prisma model only declares@@index([ownerId]). This leaves schema drift and can cause future migrations to drop the production index.Suggested fix
model Goal { id String `@id` `@default`(cuid()) familyId String @@ @@index([ownerId]) + @@index([familyId]) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prisma/schema.prisma` around lines 161 - 184, The Goal Prisma model is missing the familyId index that the migration already creates and that queries rely on via where familyId. Update the Goal model in the Prisma schema to declare an index for familyId alongside the existing ownerId index so the schema matches the migration and prevents future drift. Use the Goal model definition and its @@index block to locate the change.src/lib/db-queries.ts (4)
178-221: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick winScope child and account balance updates by
familyId.
addTransactionwritesfamilyIdon the transaction row, but the balance mutations still targetchildId/accountIdglobally. A crafted payload can mutate another family’s child/account while recording the transaction under the caller’s family.Proposed fix
+ const childUpdateWhere = { id: input.childId, familyId: input.familyId }; + if (input.type === "save" || input.type === "redeem") { - await tx.child.update({ - where: { id: input.childId }, + const updated = await tx.child.updateMany({ + where: childUpdateWhere, data: { currentAmount: { increment: input.amount } }, }); + if (updated.count === 0) throw new Error("Child not found or access denied"); } else if (input.type === "invest" || input.type === "withdraw") { // Conditional update — prevents going negative const updated = await tx.child.updateMany({ - where: { id: input.childId, currentAmount: { gte: input.amount } }, + where: { id: input.childId, familyId: input.familyId, currentAmount: { gte: input.amount } }, data: { currentAmount: { decrement: input.amount } }, }); if (updated.count === 0) { - throw new Error("Insufficient balance for " + input.type); + throw new Error("Child not found, access denied, or insufficient balance for " + input.type); } } // Update linked account balance for save/withdraw if (input.accountId) { if (input.type === "save") { - await tx.account.updateMany({ - where: { id: input.accountId, balance: { gte: input.amount } }, + const updated = await tx.account.updateMany({ + where: { id: input.accountId, familyId: input.familyId, balance: { gte: input.amount } }, data: { balance: { decrement: input.amount } }, }); + if (updated.count === 0) throw new Error("Account not found, access denied, or insufficient balance"); } else if (input.type === "withdraw") { - await tx.account.update({ - where: { id: input.accountId }, + const updated = await tx.account.updateMany({ + where: { id: input.accountId, familyId: input.familyId }, data: { balance: { increment: input.amount } }, }); + if (updated.count === 0) throw new Error("Account not found or access denied"); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/db-queries.ts` around lines 178 - 221, addTransaction currently updates child and account balances using only childId/accountId, so scope every balance mutation by familyId as well. Update the tx.child.update, tx.child.updateMany, tx.account.updateMany, and tx.account.update calls in db-queries.ts to include familyId in their where clauses, using the same input.familyId already written to the transaction row. Keep the existing insufficient-balance checks and transaction flow intact, but ensure only records belonging to the caller’s family can be mutated.
372-395: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick winUse
familyIdin goal mutations.
updateGoalandcontributeToGoalacceptfamilyIdbut update byidonly, so any authenticated family can modify another family’s goal if they know the ID.Proposed fix
export async function updateGoal(familyId: string, id: string, patch: Partial<{ @@ }>) { - return db.goal.update({ where: { id }, data: patch }); + const updated = await db.goal.updateMany({ where: { id, familyId }, data: patch }); + if (updated.count === 0) throw new Error("Goal not found or access denied"); + return db.goal.findFirstOrThrow({ where: { id, familyId } }); } @@ export async function contributeToGoal(familyId: string, id: string, amount: number) { // Atomic increment — no race condition - return db.goal.update({ - where: { id }, + const updated = await db.goal.updateMany({ + where: { id, familyId }, data: { currentAmount: { increment: amount } }, }); + if (updated.count === 0) throw new Error("Goal not found or access denied"); + return db.goal.findFirstOrThrow({ where: { id, familyId } }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/db-queries.ts` around lines 372 - 395, `updateGoal` and `contributeToGoal` are missing the `familyId` ownership check, so they can mutate a goal by `id` alone. Update the `db.goal.update` calls in these functions to first verify the goal belongs to the given `familyId` (same pattern as `deleteGoal` using `findFirst`), then perform the mutation using the matched record’s `id` or a composite condition that includes `familyId`. Keep `deleteGoal` as the reference for the access-control pattern.
255-321: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick winEnforce family ownership for token and investment child IDs.
These paths create family-scoped rows but still aggregate/update by global
childId. Passing another family’s child ID can affect token balances, savings, or investments outside the caller’s family.Proposed direction
- const given = await tx.tokenLedgerEntry.aggregate({ - where: { childId, type: "parent_give" }, + const child = await tx.child.findFirst({ where: { id: childId, familyId } }); + if (!child) throw new Error("Child not found or access denied"); + + const given = await tx.tokenLedgerEntry.aggregate({ + where: { childId, familyId, type: "parent_give" }, _sum: { tokens: true }, }); const redeemed = await tx.tokenLedgerEntry.aggregate({ - where: { childId, type: "redeem" }, + where: { childId, familyId, type: "redeem" }, _sum: { tokens: true }, });Apply the same
{ id: childId, familyId }ownership check beforegiveTokenscreates rows, and includefamilyIdininvestNow’stx.child.updateManypredicate.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/db-queries.ts` around lines 255 - 321, The token and investment flows in giveTokens, redeemTokens, and investNow currently trust a global childId when reading or updating child state. Add a family ownership guard so these operations only affect rows where the child belongs to the provided familyId, using the same { id: childId, familyId } predicate before creating ledger/transaction rows and in investNow’s tx.child.updateMany filter. Also make the balance aggregates in redeemTokens family-scoped so another family’s child records cannot be counted or modified.
239-251: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winVerify owner records before creating family-scoped rows.
addSpendingEntryandcreateGoalpersist the caller’sfamilyId, but still trustownerIdfrom the payload. Validate that the referenced child/parent belongs toinput.familyIdbefore creating rows with cross-table ownership references.Also applies to: 352-368
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/db-queries.ts` around lines 239 - 251, Validate the referenced owner before inserting family-scoped records in addSpendingEntry and createGoal. Before calling db.spendingEntry.create or db.goal.create, look up the child/parent by input.ownerId and confirm it belongs to input.familyId, then use the verified record to populate childId/parentId or reject the request if it does not match. Keep the ownership check close to the create logic so the familyId and ownerId cannot be combined across families.src/app/api/child/state/route.ts (1)
10-17: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick winApply
familyIdto the child-state reads too.The new guard verifies
user.familyId, but the route still reads the child, token ledger, and goals without that scope. Rows with the samechildId/ownerIdbut a differentfamilyIdcan leak into this response and affect token balance/progress.Proposed fix
- const child = await db.child.findUnique({ where: { id: childProfile.childId } }); + const child = await db.child.findFirst({ where: { id: childProfile.childId, familyId: user.familyId } }); if (!child) return NextResponse.json({ ok: false, error: "Child record not found" }, { status: 404 }); - const tokenLedger = await db.tokenLedgerEntry.findMany({ where: { childId: child.id }, orderBy: { timestamp: "desc" }, take: 20 }); - const goals = await db.goal.findMany({ where: { ownerId: child.id }, orderBy: { createdAt: "desc" } }); + const tokenLedger = await db.tokenLedgerEntry.findMany({ where: { childId: child.id, familyId: user.familyId }, orderBy: { timestamp: "desc" }, take: 20 }); + const goals = await db.goal.findMany({ where: { ownerId: child.id, familyId: user.familyId }, orderBy: { createdAt: "desc" } });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/child/state/route.ts` around lines 10 - 17, The child-state route validates user.familyId but the subsequent reads in the route handler still query childProfile, child, tokenLedgerEntry, and goal without scoping to that family. Update the child-state lookup flow in route.ts so the child record and all dependent reads are constrained by familyId through the existing user.familyId guard, ensuring the response only includes rows belonging to the current family. Use the route handler’s childProfile, child, tokenLedgerEntry, and goals queries to apply the same family scope consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@prisma/migrations/20260625010000_add_multi_family_isolation.sql`:
- Around line 63-77: The FamilySettings backfill step is inserting per-family
rows before the schema has the required familyId column and unique constraint.
Update this migration to add FamilySettings.familyId and create the unique
key/index before the INSERT/DELETE block, then proceed with the backfill so
Prisma upserts using familyId match the database shape expected by
FamilySettings and its related upsert paths.
---
Outside diff comments:
In `@prisma/schema.prisma`:
- Around line 161-184: The Goal Prisma model is missing the familyId index that
the migration already creates and that queries rely on via where familyId.
Update the Goal model in the Prisma schema to declare an index for familyId
alongside the existing ownerId index so the schema matches the migration and
prevents future drift. Use the Goal model definition and its @@index block to
locate the change.
In `@src/app/api/child/state/route.ts`:
- Around line 10-17: The child-state route validates user.familyId but the
subsequent reads in the route handler still query childProfile, child,
tokenLedgerEntry, and goal without scoping to that family. Update the
child-state lookup flow in route.ts so the child record and all dependent reads
are constrained by familyId through the existing user.familyId guard, ensuring
the response only includes rows belonging to the current family. Use the route
handler’s childProfile, child, tokenLedgerEntry, and goals queries to apply the
same family scope consistently.
In `@src/lib/db-queries.ts`:
- Around line 178-221: addTransaction currently updates child and account
balances using only childId/accountId, so scope every balance mutation by
familyId as well. Update the tx.child.update, tx.child.updateMany,
tx.account.updateMany, and tx.account.update calls in db-queries.ts to include
familyId in their where clauses, using the same input.familyId already written
to the transaction row. Keep the existing insufficient-balance checks and
transaction flow intact, but ensure only records belonging to the caller’s
family can be mutated.
- Around line 372-395: `updateGoal` and `contributeToGoal` are missing the
`familyId` ownership check, so they can mutate a goal by `id` alone. Update the
`db.goal.update` calls in these functions to first verify the goal belongs to
the given `familyId` (same pattern as `deleteGoal` using `findFirst`), then
perform the mutation using the matched record’s `id` or a composite condition
that includes `familyId`. Keep `deleteGoal` as the reference for the
access-control pattern.
- Around line 255-321: The token and investment flows in giveTokens,
redeemTokens, and investNow currently trust a global childId when reading or
updating child state. Add a family ownership guard so these operations only
affect rows where the child belongs to the provided familyId, using the same {
id: childId, familyId } predicate before creating ledger/transaction rows and in
investNow’s tx.child.updateMany filter. Also make the balance aggregates in
redeemTokens family-scoped so another family’s child records cannot be counted
or modified.
- Around line 239-251: Validate the referenced owner before inserting
family-scoped records in addSpendingEntry and createGoal. Before calling
db.spendingEntry.create or db.goal.create, look up the child/parent by
input.ownerId and confirm it belongs to input.familyId, then use the verified
record to populate childId/parentId or reject the request if it does not match.
Keep the ownership check close to the create logic so the familyId and ownerId
cannot be combined across families.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: bcf365c2-9d1e-4e91-851d-b008c75e0c9a
📒 Files selected for processing (7)
prisma/migrations/20260625010000_add_multi_family_isolation.sqlprisma/schema.prismasrc/app/api/child/state/route.tssrc/app/api/mutations/route.tssrc/app/api/state/route.tssrc/lib/auth.tssrc/lib/db-queries.ts
| -- Step 4: Drop old FamilySettings singleton, create per-family row | ||
| -- First, get the founder's familyId and create a FamilySettings row for it | ||
| INSERT INTO "FamilySettings" ("id", "familyId", "annualTheme", "monthlyQuote", "currency", "createdAt", "updatedAt") | ||
| SELECT | ||
| 'fam_settings_' || "familyId", | ||
| "familyId", | ||
| COALESCE((SELECT "annualTheme" FROM "FamilySettings" WHERE "id" = 'singleton'), ''), | ||
| COALESCE((SELECT "monthlyQuote" FROM "FamilySettings" WHERE "id" = 'singleton'), ''), | ||
| COALESCE((SELECT "currency" FROM "FamilySettings" WHERE "id" = 'singleton'), 'UGX'), | ||
| NOW(), | ||
| NOW() | ||
| FROM "User" WHERE "platformRole" = 'FOUNDER' AND "familyId" IS NOT NULL; | ||
|
|
||
| -- Delete the old singleton row | ||
| DELETE FROM "FamilySettings" WHERE "id" = 'singleton'; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
Migrate the FamilySettings.familyId column and unique key before inserting per-family rows.
Line 65 writes "familyId", and downstream code upserts by where: { familyId }, but this migration never adds the column or creates the unique key. On the old singleton table this will fail, or the new app will not have the DB constraint Prisma expects.
Suggested fix
-- Step 4: Drop old FamilySettings singleton, create per-family row
+ALTER TABLE "FamilySettings" ADD COLUMN "familyId" TEXT;
+
-- First, get the founder's familyId and create a FamilySettings row for it
INSERT INTO "FamilySettings" ("id", "familyId", "annualTheme", "monthlyQuote", "currency", "createdAt", "updatedAt")
@@
-- Delete the old singleton row
DELETE FROM "FamilySettings" WHERE "id" = 'singleton';
+
+ALTER TABLE "FamilySettings" ALTER COLUMN "familyId" SET NOT NULL;
+CREATE UNIQUE INDEX "FamilySettings_familyId_key" ON "FamilySettings"("familyId");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| -- Step 4: Drop old FamilySettings singleton, create per-family row | |
| -- First, get the founder's familyId and create a FamilySettings row for it | |
| INSERT INTO "FamilySettings" ("id", "familyId", "annualTheme", "monthlyQuote", "currency", "createdAt", "updatedAt") | |
| SELECT | |
| 'fam_settings_' || "familyId", | |
| "familyId", | |
| COALESCE((SELECT "annualTheme" FROM "FamilySettings" WHERE "id" = 'singleton'), ''), | |
| COALESCE((SELECT "monthlyQuote" FROM "FamilySettings" WHERE "id" = 'singleton'), ''), | |
| COALESCE((SELECT "currency" FROM "FamilySettings" WHERE "id" = 'singleton'), 'UGX'), | |
| NOW(), | |
| NOW() | |
| FROM "User" WHERE "platformRole" = 'FOUNDER' AND "familyId" IS NOT NULL; | |
| -- Delete the old singleton row | |
| DELETE FROM "FamilySettings" WHERE "id" = 'singleton'; | |
| -- Step 4: Drop old FamilySettings singleton, create per-family row | |
| ALTER TABLE "FamilySettings" ADD COLUMN "familyId" TEXT; | |
| -- First, get the founder's familyId and create a FamilySettings row for it | |
| INSERT INTO "FamilySettings" ("id", "familyId", "annualTheme", "monthlyQuote", "currency", "createdAt", "updatedAt") | |
| SELECT | |
| 'fam_settings_' || "familyId", | |
| "familyId", | |
| COALESCE((SELECT "annualTheme" FROM "FamilySettings" WHERE "id" = 'singleton'), ''), | |
| COALESCE((SELECT "monthlyQuote" FROM "FamilySettings" WHERE "id" = 'singleton'), ''), | |
| COALESCE((SELECT "currency" FROM "FamilySettings" WHERE "id" = 'singleton'), 'UGX'), | |
| NOW(), | |
| NOW() | |
| FROM "User" WHERE "platformRole" = 'FOUNDER' AND "familyId" IS NOT NULL; | |
| -- Delete the old singleton row | |
| DELETE FROM "FamilySettings" WHERE "id" = 'singleton'; | |
| ALTER TABLE "FamilySettings" ALTER COLUMN "familyId" SET NOT NULL; | |
| CREATE UNIQUE INDEX "FamilySettings_familyId_key" ON "FamilySettings"("familyId"); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@prisma/migrations/20260625010000_add_multi_family_isolation.sql` around lines
63 - 77, The FamilySettings backfill step is inserting per-family rows before
the schema has the required familyId column and unique constraint. Update this
migration to add FamilySettings.familyId and create the unique key/index before
the INSERT/DELETE block, then proceed with the backfill so Prisma upserts using
familyId match the database shape expected by FamilySettings and its related
upsert paths.
What this does
Adds
familyIdto all 9 financial models. Each new signup gets their ownfamilyId. All queries and mutations are scoped byfamilyId— families cannot see or modify each other's data.This is Phase 1 of the "let families test the app independently" feature.
Before merging, run the migration SQL against Neon:
prisma/migrations/20260625010000_add_multi_family_isolation.sqlThe migration:
familyIdcolumn (nullable) to 9 tablesfamilyIdNOT NULLFamilySettingssingleton, creates per-family rowsSafe to run while the app is live — adds nullable columns first, backfills, then makes NOT NULL.
Changes
1. Schema (
prisma/schema.prisma)Added
familyId String+@@index([familyId])to:Child,ParentProfile,Account,TransactionSpendingCategory,SpendingEntry,InvestmentTokenLedgerEntry,GoalFamilySettingschanged fromid: "singleton"tofamilyId @unique(per-family rows).2.
src/lib/auth.tsperformFounderSetup()— assignsfamilyId: "fam_" + crypto.randomUUID()registerUserAsUSER()— assignsfamilyId: "fam_" + crypto.randomUUID()getAuthUser()— returnsfamilyIdin the user objectAuthUserinterface — includesfamilyId: string | null3.
src/lib/db-queries.tsgetFullState(familyId)— all 10findManycalls scoped byfamilyIdfamilyId:familyIdin thedatablockfindFirst({ where: { id, familyId } })before operatingsetFamilySettings(familyId, patch)—upsertbyfamilyIdinstead of singleton4. API routes
GET /api/state— passesuser.familyIdtogetFullState()POST /api/mutations— passesuser.familyIdto all 16 mutation functionsGET /api/child/state— checksuser.familyIdexistsFiles changed (7 files)
prisma/schema.prisma— +9familyIdcolumns, +9 indexes, FamilySettings restructureprisma/migrations/20260625010000_add_multi_family_isolation.sql(new) — Neon migrationsrc/lib/auth.ts— familyId on signup/setup, getAuthUser returns familyIdsrc/lib/db-queries.ts— all queries/mutations scoped by familyIdsrc/app/api/state/route.ts— auth gate + familyIdsrc/app/api/mutations/route.ts— auth gate + familyId on all mutationssrc/app/api/child/state/route.ts— familyId checkVerification
npx tsc --noEmit— cleannpx eslint— cleannpx prisma generate— cleanVercel env vars
None needed.
After merging
Next phases
Summary by CodeRabbit
New Features
Bug Fixes
Chores