fix: guard upsert tenant scope and SQLite rebuild dependencies

.changeset/safe-conflict-tenant.md

@@ -0,0 +1,5 @@
+---
+"@maple-dev/effect-orm": patch
+---
+
+An upsert is single-tenant only when its conflict-update path also proves the existing row's tenant. A tenant-inclusive column target or an explicit tenant predicate can prove it; otherwise `requireTenantScope` refuses the statement before execution. This prevents an insert for one tenant from updating another tenant through a shared conflicting ID.

.changeset/safe-sqlite-rebuild.md

@@ -0,0 +1,5 @@
+---
+"@maple-dev/effect-orm": patch
+---
+
+Generated SQLite rebuilds check live inbound foreign keys before copying or dropping the old table. A referenced table is refused instead of letting a transaction-wrapped DROP fire CASCADE or SET NULL actions. The guard works in a D1 batch and rolls back with the migration; ordinary rebuilds still run. Older generated SQL must be reviewed or regenerated before applying it.

docs/inserts.md

@@ -219,7 +219,12 @@ Calling it again replaces them. Postgres has no insert settings, so compiling on
 An insert has a `tenantScope` like a query, worked out the same way. On a table with a
 `tenantColumn`, it is `"single-tenant"` when every row gives that column the same value or the
 same param, and `"cross-tenant"` when rows differ or a row uses another expression. An
-`onConflictDoUpdate` that sets the tenant column counts as one more row.
+`onConflictDoUpdate` that sets the tenant column counts as one more row. It must also prove
+that the existing conflict row has that tenant: include the tenant column in the conflict
+target, or pin it in the update's `where`, for example
+`where: (existing, excluded) => existing.orgId.eq(excluded.orgId)`. A constraint name alone
+does not prove its columns. Without this evidence the upsert is `"cross-tenant"`, even if
+the inserted values name one tenant. OR conditions do not supply tenant evidence.
 
 An `INSERT ... SELECT` into a tenant table is `"single-tenant"` when the SELECT is, and each row
 takes its tenant from a tenant column of the source or from the same param that pins the SELECT.

docs/migrations.md

@@ -450,7 +450,13 @@ What each change becomes:
 | New or changed index | `DROP INDEX`, `CREATE [UNIQUE] INDEX` |
 | Renamed table or column | `ALTER TABLE ... RENAME`, only with a `confirm_rename` hint |
 
-A rebuild follows [SQLite's procedure](https://www.sqlite.org/lang_altertable.html#otheralter)
+A rebuild first checks the live catalog for tables that reference it through a foreign key,
+including keys created by another tool and self-references. Any inbound key refuses the rebuild
+before rows are copied or the old table is dropped, even if its child table is empty. The error
+names `foreign_key_references_prevent_rebuild`. Use a custom migration with a tested plan that
+preserves those dependencies instead.
+
+With no inbound keys, the rebuild follows [SQLite's procedure](https://www.sqlite.org/lang_altertable.html#otheralter)
 the way drizzle-kit writes it: `PRAGMA foreign_keys=OFF`, `CREATE TABLE "__new_<table>"` with the
 new definition, `INSERT INTO "__new_<table>" (...) SELECT ...` for every column both tables
 have, `DROP TABLE`, `ALTER TABLE "__new_<table>" RENAME TO "<table>"`, `PRAGMA foreign_keys=ON`,
@@ -462,12 +468,17 @@ a new UNIQUE constraint over duplicate values. A column a rebuild leaves out nee
 
 > [!WARNING]
 > `PRAGMA foreign_keys=OFF` does nothing inside a transaction, and D1 and most migrators run each
-> migration in one. There, the rebuild's `DROP TABLE` deletes every row of the old table first, so
-> a table that references it with `ON DELETE CASCADE` loses its rows, and `ON DELETE SET NULL`
-> clears them. effect-orm declares no foreign keys, but a database may have them from another
-> tool. Before applying a rebuild of a referenced table, check for such keys
-> (`SELECT * FROM pragma_foreign_key_list('<child>')`) and, if there are any, run the rebuild
-> outside a transaction with foreign keys off.
+> migration in one. Without the guard, the rebuild's `DROP TABLE` would fire `ON DELETE CASCADE`
+> or `ON DELETE SET NULL` in referencing tables. Running outside a transaction is not an option
+> for a D1 batch, and `defer_foreign_keys` does not prevent those actions.
+
+Apply a migration's statements as one transaction (one `batch` on D1), and roll back on failure.
+The guard uses a short-lived `__effect_orm_rebuild_guard_<table>` table with a NOT NULL assertion,
+so it still works with `ignore_check_constraints` enabled. It drops the guard before a safe
+rebuild; a refused transaction rolls it back. A pre-existing table with that name causes an
+error without reusing or dropping it. This guard is in newly generated SQL: review or regenerate
+unapplied rebuilds generated by older versions. It checks inbound foreign keys; it does not
+preserve unmodelled triggers, views or other constraints.
 
 A rebuild renumbers rowids: the copy may read the old rows in any order. A table whose rows are
 referenced by rowid (rather than by a declared key) should declare an `integer` primary key, which

src/ch/compile.ts

@@ -1725,6 +1725,7 @@ const onConflictClause = (
 	insert: CHInsert<any, any, any, any>,
 	cell: (column: string, value: unknown, context: string) => string,
 	wrote: (column: string, value: unknown, sql: string) => void,
+	checked: (predicates: ReadonlyArray<TenantPredicate>) => void,
 ): string => {
 	const { table, conflict } = insert._state
 	if (conflict === undefined) return ""
@@ -1763,11 +1764,12 @@ const onConflictClause = (
 	}
 	if (conflict.action === "nothing") return `\nON CONFLICT${target} DO NOTHING`
 
-	const existing = createQualifiedColumnAccessor(table.name, undefined, table.columns)
-	const excluded = createQualifiedColumnAccessor("excluded", undefined, table.columns)
+	const existing = createQualifiedColumnAccessor(table.name, table.tenantColumn, table.columns)
+	const excluded = createQualifiedColumnAccessor("excluded", table.tenantColumn, table.columns)
 	const set = typeof conflict.set === "function" ? conflict.set(existing, excluded) : conflict.set
 	const assignments = setAssignments(table, set as Record<string, unknown>, cell, wrote, where, "onConflictDoUpdate")
 	const condition = conflict.where?.(existing, excluded)
+	checked(condition === undefined ? [] : tenantPredicatesOf(condition))
 	return `\nON CONFLICT${target} DO UPDATE SET ${assignments.join(", ")}${
 		condition === undefined ? "" : ` WHERE ${compileSqlFragment(condition.toFragment())}`
 	}`
@@ -1976,9 +1978,8 @@ function compileInsert(insert: CHInsert<any, any, any, any>, params: Record<stri
 	const dialect = currentDialect()
 	const { values, cell } = valueCells(table, params, where)
 
-	// A tenant table's insert is single-tenant when every row it writes (and an
-	// upsert's SET, if it writes the column) pins the tenant column to the same
-	// value or param. Any other expression, a NULL or a default could be anything.
+	// Every inserted row and any SET of its tenant must pin the same value.
+	// An upsert must also prove the tenant of the existing conflict row below.
 	const tenant = table.tenantColumn
 	const bounds = new Set<string>()
 	let pinned = tenant !== undefined
@@ -1990,6 +1991,7 @@ function compileInsert(insert: CHInsert<any, any, any, any>, params: Record<stri
 
 	const reads: Array<TenantSource> = []
 	let readBound: string | undefined
+	let conflictPredicates: ReadonlyArray<TenantPredicate> = []
 	const [columns, source, conflictSql, readScope] = withSubqueryCompiler(
 		recordingSubqueries(values, reads),
 		() => {
@@ -2034,9 +2036,10 @@ function compileInsert(insert: CHInsert<any, any, any, any>, params: Record<stri
 					else bounds.add(bound)
 				}
 			}
-			const conflictSql = onConflictClause(insert, cell, (column, value, sql) => {
-				if (column === tenant) pin(value, sql)
-			})
+			const conflictSql = onConflictClause(insert, cell,
+				(column, value, sql) => { if (column === tenant) pin(value, sql) },
+				(predicates) => { conflictPredicates = predicates },
+			)
 			return [columns, source, conflictSql, readScope] as const
 		},
 	)
@@ -2047,12 +2050,28 @@ function compileInsert(insert: CHInsert<any, any, any, any>, params: Record<stri
 		values,
 		dialect,
 	)
-	const tenantScope: TenantScope =
+	let tenantScope: TenantScope =
 		tenant === undefined
 			? (readScope ?? "untenanted")
 			: pinned && bounds.size === 1
 				? "single-tenant"
 				: "cross-tenant"
+	const conflict = insert._state.conflict
+	if (tenant !== undefined && tenantScope === "single-tenant" && conflict?.action === "update") {
+		const bound = [...bounds][0]!
+		const existingColumn = `${table.name}.${tenant}`
+		const excludedColumn = `excluded.${tenant}`
+		// A column target containing the tenant can only conflict on that same
+		// tenant. A constraint name alone says nothing about its columns.
+		const targetPins = conflict.target !== undefined && !("constraint" in conflict.target) && conflict.target.includes(tenant)
+		const updateScope = deriveTenantScope([
+			{ column: existingColumn, scope: "cross-tenant", bound: targetPins ? bound : undefined },
+			{ column: excludedColumn, scope: "single-tenant", bound },
+		], [{ predicates: conflictPredicates }], (value) => inlineParams(compileSqlFragment(value), values))
+		// SQLite resolves identifiers without case; a table named excluded can
+		// make the apparent existing/excluded equality a tautology.
+		if (table.name.split(".").at(-1)?.toLowerCase() === "excluded" || updateScope.scope !== "single-tenant" || updateScope.bound !== bound) tenantScope = "cross-tenant"
+	}
 	const own = withReads(
 		{
 			scope: tenantScope,

src/ch/insert-tenant.test.ts

@@ -0,0 +1,48 @@
+import { expect, it } from "vitest"
+import * as PG from "../postgres"
+
+const Rows = PG.table("rows", {
+	columns: { id: PG.text, org: PG.text, value: PG.text },
+	primaryKey: ["id"],
+	tenantColumn: "org",
+})
+const insert = PG.insertInto(Rows).values({ id: "same", org: PG.param.string("org"), value: "new" })
+const scope = (query: typeof insert) => PG.compileUnsafe(query, { org: "a" }).tenantScope
+
+it("does not infer an existing conflict row's tenant from the inserted values or SET", () => {
+	expect(scope(insert.onConflictDoUpdate({ target: ["id"], set: { value: "new" } }))).toBe("cross-tenant")
+	expect(scope(insert.onConflictDoUpdate({ target: ["id"], set: { org: "a" } }))).toBe("cross-tenant")
+	expect(scope(insert.onConflictDoUpdate({ target: { constraint: "rows_pkey" }, set: { value: "new" } }))).toBe("cross-tenant")
+})
+
+it("proves the conflict row's tenant through the target or an ANDed tenant predicate", () => {
+	expect(scope(insert.onConflictDoUpdate({ target: ["org", "id"], set: { value: "new" } }))).toBe("single-tenant")
+	expect(scope(insert.onConflictDoUpdate({
+		target: ["id"], set: { value: "new" },
+		where: (existing, excluded) => existing.org.eq(excluded.org).and(existing.value.neq("locked")),
+	}))).toBe("single-tenant")
+	expect(scope(insert.onConflictDoUpdate({
+		target: ["id"], set: { value: "new" }, where: (existing) => existing.org.eq(PG.param.string("org")),
+	}))).toBe("single-tenant")
+})
+
+it("does not accept OR, a different tenant, self-equality or a tenant-moving SET as proof", () => {
+	for (const query of [
+		insert.onConflictDoUpdate({ target: ["id"], set: { value: "new" }, where: (existing, excluded) => existing.org.eq(excluded.org).or(existing.value.eq("open")) }),
+		insert.onConflictDoUpdate({ target: ["id"], set: { value: "new" }, where: (existing) => existing.org.eq("b") }),
+		insert.onConflictDoUpdate({ target: ["id"], set: { value: "new" }, where: (existing) => existing.org.eq(existing.org) }),
+		insert.onConflictDoUpdate({ target: ["org", "id"], set: { org: "b" } }),
+	]) expect(scope(query)).toBe("cross-tenant")
+	expect(scope(insert.onConflictDoNothing())).toBe("single-tenant")
+	const declared = PG.compileUnsafe(insert.onConflictDoUpdate({ target: ["id"], set: { value: "new" } }).crossTenant(), { org: "a" })
+	expect(declared.tenantScope).toBe("cross-tenant")
+	expect(declared.crossTenantDeclared).toBe(true)
+})
+
+it.each(["excluded", "Excluded", "EXCLUDED", "app.excluded"])("does not confuse table %s with the excluded pseudo-table", (name) => {
+	const named = PG.table(name, { columns: { id: PG.text, org: PG.text, value: PG.text }, primaryKey: ["id"], tenantColumn: "org" })
+	const compiled = PG.compileUnsafe(PG.insertInto(named).values({ id: "same", org: "a", value: "new" }).onConflictDoUpdate({
+		target: ["id"], set: { value: "new" }, where: (existing, excluded) => existing.org.eq(excluded.org),
+	}))
+	expect(compiled.tenantScope).toBe("cross-tenant")
+})

src/ch/insert.test.ts

@@ -187,10 +187,10 @@ describe("insertInto", () => {
 			expect(compiled.parameters).toEqual(["o", "k", 1, false, true, false])
 		})
 
-		it("a SET that writes another tenant makes the insert cross-tenant", () => {
+		it("a SET that writes another tenant makes a tenant-keyed upsert cross-tenant", () => {
 			const insert = CH.insertInto(Counters).values(row)
 			const scope = (org: string) =>
-				PG.compileUnsafe(insert.onConflictDoUpdate({ target: ["key"], set: { org } })).tenantScope
+				PG.compileUnsafe(insert.onConflictDoUpdate({ target: ["org", "key"], set: { org } })).tenantScope
 			expect(scope("o")).toBe("single-tenant")
 			expect(scope("p")).toBe("cross-tenant")
 		})

src/kit/sqlite-generate.test.ts

@@ -124,6 +124,9 @@ it.each(rebuilds)("rebuilds the table for %s, keeps its rows and recreates its i
 	const second = await generateFrom(v2, { hints })
 	expect(second.plan.map((p) => p.label)).toEqual(["rebuild", "metadata"])
 	expect(second.plan[0]!.sql).toEqual([
+		expect.stringMatching(/^CREATE TABLE "__effect_orm_rebuild_guard_links"/),
+		expect.stringContaining("pragma_foreign_key_list"),
+		'DROP TABLE "__effect_orm_rebuild_guard_links"',
 		"PRAGMA foreign_keys=OFF",
 		expect.stringMatching(/^CREATE TABLE "__new_links" \(/),
 		// A NOT NULL column with a default copies through COALESCE, so old NULLs take the default.
@@ -186,7 +189,7 @@ it("asks to confirm a column a rebuild leaves behind", async () => {
 		],
 	})
 	expect(second.plan.map((p) => p.label)).toEqual(["rebuild"])
-	expect(second.plan[0]!.sql[2]).toBe('INSERT INTO "__new_links"("id", "url") SELECT "id", "url" FROM "links"')
+	expect(second.plan[0]!.sql).toContain('INSERT INTO "__new_links"("id", "url") SELECT "id", "url" FROM "links"')
 	await expectMigratedToEqualFresh(v2)
 })
 

src/schema/sqlite-ops.ts

@@ -9,6 +9,7 @@
 // indexes again (dropping the table dropped them).
 
 import { Schema } from "effect"
+import { sqliteDialect } from "../sqlite/dialect"
 import { SqliteColumnEntity, SqliteIndexEntity, SqliteTableEntity, SqliteUniqueEntity } from "./sqlite-entities"
 import { renderSqliteColumnDefinition, renderSqliteCreateIndex, renderSqliteCreateTable, sqliteIdent } from "./sqlite-render"
 
@@ -67,6 +68,24 @@ const copied = (column: SqliteColumnEntity | undefined, name: string): string =>
 		? `COALESCE(${sqliteIdent(name)}, ${column.default})`
 		: sqliteIdent(name)
 
+/**
+ * An offline schema cannot see foreign keys created by another tool. Check
+ * the live catalog before the rebuild can drop their parent. NOT NULL still
+ * applies with ignore_check_constraints enabled, unlike a CHECK assertion.
+ * No IF NOT EXISTS: a name collision must fail without touching that table.
+ */
+const rebuildGuard = (table: string): ReadonlyArray<string> => {
+	const guard = sqliteIdent(`__effect_orm_rebuild_guard_${table}`)
+	const column = sqliteIdent("foreign_key_references_prevent_rebuild")
+	return [
+		`CREATE TABLE ${guard} (${column} integer NOT NULL)`,
+		// Every FK declaration contains REFERENCES. Besides saving catalog work,
+		// this avoids inspecting D1's protected, FK-free _cf_METADATA table.
+		`INSERT INTO ${guard} (${column}) SELECT NULL FROM sqlite_schema AS s, pragma_foreign_key_list(s.name) AS fk WHERE s.type = 'table' AND instr(upper(s.sql), 'REFERENCES') > 0 AND fk."table" = ${sqliteDialect.quoteString(table)} COLLATE NOCASE LIMIT 1`,
+		`DROP TABLE ${guard}`,
+	]
+}
+
 /** The statements one op runs, in order. */
 export const renderSqliteOp = (op: SqliteMigrationOp): ReadonlyArray<string> => {
 	switch (op.op) {
@@ -89,6 +108,7 @@ export const renderSqliteOp = (op: SqliteMigrationOp): ReadonlyArray<string> =>
 			const temporary = rebuildName(op.table.name)
 			const created = renderSqliteCreateTable({ ...op.table, name: temporary }, op.columns.map((c) => ({ ...c, table: temporary })), op.uniques, { ifNotExists: false })
 			return [
+				...rebuildGuard(op.table.name),
 				"PRAGMA foreign_keys=OFF",
 				created,
 				// With no column in common the rows still carry over, every column taking its default: