fix(cat): the two prune deletes named the row but not its owner

package.json

@@ -39,9 +39,10 @@
     "check:rpc-exists": "node scripts/check-rpc-exists.mjs",
     "check:one-current-user": "node scripts/check-one-current-user.mjs",
     "check:client-ip": "node scripts/check-client-ip.mjs",
+    "check:user-scoped-deletes": "node scripts/check-user-scoped-deletes.mjs",
     "check:ai-models": "node scripts/check-ai-models.mjs",
     "check:mdx": "node scripts/check-mdx.mjs",
-    "verify": "npm run ci:docs && npm run check:accent-ink && npm run type-check && npm run type-check:scripts && npm run check:sizes && npm run audit:routes && npm run lint && npm run check:duplication && npm run check:dead-fields && npm run check:migration-versions && npm run check:schema-columns && npm run check:currency-units && npm run check:rpc-exists && npm run check:one-current-user && npm run check:client-ip && npm run check:mdx && npm run test:unit -- --watchAll=false",
+    "verify": "npm run ci:docs && npm run check:accent-ink && npm run type-check && npm run type-check:scripts && npm run check:sizes && npm run audit:routes && npm run lint && npm run check:duplication && npm run check:dead-fields && npm run check:migration-versions && npm run check:schema-columns && npm run check:currency-units && npm run check:rpc-exists && npm run check:one-current-user && npm run check:client-ip && npm run check:user-scoped-deletes && npm run check:mdx && npm run test:unit -- --watchAll=false",
     "audit:schema": "node scripts/db/audit-schema-drift.mjs",
     "audit:routes": "node scripts/audit-routes.mjs",
     "gen:types": "bash scripts/db/gen-types.sh",

scripts/check-user-scoped-deletes.mjs

@@ -0,0 +1,79 @@
+#!/usr/bin/env node
+/* eslint-disable no-console */
+/**
+ * check-user-scoped-deletes.mjs — every DELETE on a per-user Cat table names
+ * the user.
+ *
+ * Cat's memory tables hold one row per user with no other tenancy boundary in
+ * the query itself. Six deletes in `src/services/cat/memory.ts` remove rows;
+ * four already filtered on `user_id`, and the two PRUNE paths did not — they
+ * deleted purely by `id`, on ids fetched moments earlier by a user-scoped
+ * query.
+ *
+ * That was safe, and only conditionally: the ids were right, and the client was
+ * RLS-scoped. Neither is guaranteed by the code that does the deleting. Three
+ * other paths in this service already use `getAdminClient()`, where RLS is not
+ * a backstop at all — hand one of those to a prune and it deletes across users,
+ * silently, from a function whose job is routine cleanup nobody watches.
+ *
+ * The second instance is the reason this gate exists rather than a third fix:
+ * `recordForgottenFacts` says "same pattern as memory pruning" in its own
+ * comment, and inherited the gap along with the pattern. bitbaum/orangecat#563
+ * finding 13.
+ *
+ * Deliberately narrow: it checks the Cat memory service, where the invariant is
+ * uniform and the tables are strictly per-user. Widening it to every table in
+ * the app would need a per-table notion of ownership that does not exist yet,
+ * and a gate that has to guess is a gate that gets muted.
+ */
+
+import { readFileSync } from 'node:fs';
+
+const FILES = ['src/services/cat/memory.ts', 'src/services/cat/economic-profile.ts'];
+
+/**
+ * A delete and the statement that follows it, up to the terminating `;`.
+ * Chains here are short and always end in one, so this needs no JS parser.
+ */
+function deleteStatements(source) {
+  const found = [];
+  const re = /\.delete\(\)/g;
+  let m;
+  while ((m = re.exec(source)) !== null) {
+    const end = source.indexOf(';', m.index);
+    const line = source.slice(0, m.index).split('\n').length;
+    found.push({ line, text: source.slice(m.index, end === -1 ? source.length : end) });
+  }
+  return found;
+}
+
+let offenders = 0;
+let checked = 0;
+
+for (const file of FILES) {
+  let source;
+  try {
+    source = readFileSync(file, 'utf8');
+  } catch {
+    continue;
+  }
+  for (const stmt of deleteStatements(source)) {
+    checked += 1;
+    if (!/\.eq\(\s*['"]user_id['"]/.test(stmt.text)) {
+      offenders += 1;
+      console.error(`✗ ${file}:${stmt.line} — DELETE does not filter on user_id`);
+      console.error(`    ${stmt.text.replace(/\s+/g, ' ').slice(0, 100)}`);
+    }
+  }
+}
+
+if (offenders > 0) {
+  console.error('');
+  console.error('  These tables are strictly per-user, and the delete is the last place');
+  console.error('  that can say so. RLS is a backstop, not the rule: three paths in this');
+  console.error('  service already run under getAdminClient(), where there is no backstop.');
+  console.error('  Add .eq(\'user_id\', userId) — the other deletes in these files all do.');
+  process.exit(1);
+}
+
+console.log(`✓ user-scoped deletes: ${checked} delete(s) checked, all filter on user_id`);

src/services/cat/memory.ts

@@ -402,7 +402,13 @@ async function recordForgottenFacts(
         .limit(count - MAX_FORGOTTEN_PER_USER);
       const ids = (oldest as Array<{ id: string }> | null)?.map(r => r.id) ?? [];
       if (ids.length > 0) {
-        await supabase.from(DATABASE_TABLES.CAT_FORGOTTEN_FACTS).delete().in('id', ids);
+        // Scoped by user_id as well as id, for the same reason pruneIfNeeded is
+        // — this inherited the pattern, and the gap with it.
+        await supabase
+          .from(DATABASE_TABLES.CAT_FORGOTTEN_FACTS)
+          .delete()
+          .eq('user_id', userId)
+          .in('id', ids);
       }
     }
   } catch (err) {
@@ -820,7 +826,20 @@ export async function extractAndStoreMemories(
   }
 }
 
-/** Keep the corpus bounded: delete the oldest memories beyond the per-user cap. */
+/**
+ * Keep the corpus bounded: delete the oldest memories beyond the per-user cap.
+ *
+ * The delete is scoped by user_id as well as by id. That is redundant today —
+ * the ids come from a query already filtered to this user, run through an
+ * RLS-scoped client — and it is exactly the redundancy worth having: this is an
+ * unconditional DELETE of rows the user never asked to remove, and the only
+ * thing standing between it and someone else's memories is that both of those
+ * conditions keep holding. Hand it a service-role client one day, as three
+ * other paths in this service already use, and RLS stops being the backstop.
+ *
+ * Every other delete in this file is written that way, including the
+ * suppression-lifting one twenty lines above. bitbaum/orangecat#563 finding 13.
+ */
 async function pruneIfNeeded(supabase: AnySupabaseClient, userId: string): Promise<void> {
   const { count } = await supabase
     .from(DATABASE_TABLES.CAT_MEMORIES)
@@ -837,7 +856,11 @@ async function pruneIfNeeded(supabase: AnySupabaseClient, userId: string): Promi
     .limit(count - MAX_MEMORIES_PER_USER);
   const ids = (oldest as Array<{ id: string }> | null)?.map(r => r.id) ?? [];
   if (ids.length > 0) {
-    await supabase.from(DATABASE_TABLES.CAT_MEMORIES).delete().in('id', ids);
+    await supabase
+      .from(DATABASE_TABLES.CAT_MEMORIES)
+      .delete()
+      .eq('user_id', userId)
+      .in('id', ids);
   }
 }