Skip to content

Commit 95e24b0

Browse files
fix(driver-sql,driver-turso)!: refuse an upsert whose conflict lands on another organization's row (#21185) (#21225)
Fixes #21185 Clause-②: no (narrowing) Executes ruling record 5934879010 on #21185 (letter A, refinements 1/2/3): a tenant-scoped `upsert` whose conflict lands on a row of another organization is refused with `UNIQUE_VIOLATION` and writes nothing, and the tenant column is insert-only (`insertOnlyUpsertColumns`), so an upsert never changes a row's organization. `update()` stays the deliberate path. Classes and positions only; the reproduction stays with the seats. ## The two `packages/spec` edits (patch round 1, on the seat's answer A) The ruled answer needed two `packages/spec` edits. The dispatch held them for the seat's word, and the seat answered A on #21185 (claim amendment 5937974892), under the spec lane's standing pre-approval. Neither edit moves the ruled `Clause-②: no (narrowing)`: no accept set widens. The precedent is the `driver-sql-calendar-day-methods-removed` changeset, which carries `registered` plus `Clause-②: no (narrowing)` with `@objectstack/spec` at `patch`. 1. **Error-code provenance** (`82f8120c`). `driver-sql` now stamps the registered `UNIQUE_VIOLATION` (`UPSERT_UNIQUE_VIOLATION_CODE` in `sql-driver.ts`), so one row, `'UNIQUE_VIOLATION'`, is added under the `@objectstack/driver-sql` owner key in `packages/spec/src/api/error-code-ledger.zod.ts`. It is provenance only: the `ErrorCode` union is byte-unchanged, and `api-surface` records the ledger by name only. `driver-turso` throws the same constructor through `SqlDriver`, so it needs no row. 2. **ADR-0087 disposition** (`82f8120c`, then `5682e8fe`). The ruled FROM → TO is a migration prescription, so the disposition is `registered driver-upsert-cross-organization-conflict-refused`. The semantic entry is `packages/spec/src/migrations/entries/semantic/18.driver-upsert-cross-organization-conflict-refused.ts`. Its prefix is the protocol-18 step that `registry.ts` records as accumulating and uncut, as for the precedent `18.driver-sql-calendar-day-methods-removed.ts`. `src/migrations/registry.ts` was regenerated with `gen:migration-registry`, the one artifact `check:generated --fix` proved stale; `spec-changes.json` and the upgrade guide do not move until the protocol major reaches 18. The changeset front matter carries `'@objectstack/spec': patch`. ## What changed, per face - **The fence** (`SqlDriver.upsertTenantGuard`): it applies when the call is tenant-scoped (non-empty `options.tenantId`) on an object with a tenant column. The value is the organization the row is written under, after `injectTenantOnInsert`, the reading #8807's identity probe already used. It holds for any conflict target, the primary key included. - **SQLite and PostgreSQL:** the predicate sits inside the merge statement: `ON CONFLICT … DO UPDATE SET … WHERE` the stored tenant column `IS` (SQLite) / `IS NOT DISTINCT FROM` (PostgreSQL) `excluded`'s. The read-back that already follows the statement now reads under the written organization exactly (the chokepoint scope plus equality on the tenant column), and a miss is the refusal. - **MySQL:** `ON DUPLICATE KEY UPDATE` takes no `WHERE`, so #8807's mechanism is reused. The statement and a read of the landed row under the written organization (`assertMergeLandedInWrittenOrganization`) run as one transaction whenever the call is fenced, not only when a rival UNIQUE key exists. Inside a caller's transaction they run in a nested transaction (a savepoint), so the refusal never depends on what the caller does next. When #8807's check is also owed, the new read carries its verdict. A miss is told apart by reading each rival key's sent values under the written organization: a hit there means the merge landed through a rival key in the caller's own organization, and #8807's refusal is kept; anything else answers `UNIQUE_VIOLATION`. - **The remote face (libSQL, `RemoteTransport.upsert`):** the same in-statement predicate. Of the ruling's two options, **the read-back is scoped**, not `rowsAffected`. When no column is left to merge the statement is `DO NOTHING`, which affects zero rows for a same-organization conflict too, so a `rowsAffected` verdict would refuse the control. The transport answers `null` on a fenced miss, and `TursoDriver` throws the local face's refusal. Measured as part of this: the remote face did not stamp the caller's organization on an upsert at all, so its insert leg wrote a row with no organization. It now calls `injectTenantOnInsert` on entry, as the local face does. - **Half 2:** the tenant column joined `insertOnlyUpsertColumns`, the one list both faces read. - **The refusal:** `code: 'UNIQUE_VIOLATION'`, `status: 409`, with the engine's duplicate-record sentence. It names no organization, no tenant column and no value of the landed row, and carries no `cause`. `isUniqueViolationError` recognises it. - **#8807 adjustment:** `assertMergeLandedOnSuppliedIdentity` no longer turns the payload's tenant into a scope on a call with no tenant context. With the tenant column insert-only, that scope missed a merge that really landed on the supplied id. This was measured on MariaDB 10.11 as a false #8807 refusal before the adjustment. `IDataDriver.upsert`'s docblock in `packages/spec/src/contracts/data-driver.ts` is one line and states no merge semantics, so `Clause-②` stays `no (narrowing)`. ## Tests The pins and the round-0 results below ran on `e746da26` (after merging `main`, which brought in #21178's remote-transport change). Patch round 1's head, `35d01c9d`, merges `main` again: #21163 (`c6b68891`, the `scanMaxNumericTail` region of `sql-driver.ts`, a clean merge) and #21067 (`682873d9`). On `35d01c9d` the driver suites were re-run (SQLite cell: `driver-sql` 211 files, 3545 passed; `driver-turso` 86 files, 2313 passed; `driver-sqlite-wasm` 675 passed), and so were the live PostgreSQL 16 and MariaDB 10.11 cells (92 of 92). Pins: - `packages/drivers/driver-sql/src/sql-driver-upsert-cross-organization-refusal.test.ts`. One dialect-cell matrix: SQLite, PostgreSQL, MySQL. - A cross-organization conflict is refused on the default `id` target, on an explicit `['id']` target and on a `unique: 'global'` column, with `code` and `status` asserted. The other organization's row reads back raw and byte-identical, and no row is added. - The refusal also holds inside a caller's transaction that commits afterwards. - The refusal names no organization, and it is recognised by the shared predicate. - A row with no organization is refused too. - Controls: a same-organization merge on both targets; an insert lands under the caller's organization; an explicitly named organization on an insert is not refused. - Half 2: no tenant context keeps the row's organization on both targets, and `update()` still moves it. - `packages/drivers/driver-turso/src/turso-local-remote-upsert-cross-organization-parity.test.ts`. The same pins on both `TursoDriver` faces, plus a parity pin that holds the two faces' answers equal. Local results: - SQLite plus a throwaway live PostgreSQL 16 plus a throwaway MariaDB 10.11 (standing in for MySQL): the new file plus `sql-driver-upsert-conflict-target-dialects.test.ts` pass 92 of 92. That includes #8807's tenanted pins. - Full suites (SQLite cell): - `@objectstack/driver-sql`: 209 files, 3491 passed. - `@objectstack/driver-turso`: 85 files, 2301 passed. - `@objectstack/driver-sqlite-wasm` (inherits the door): 675 passed. - Typecheck is green for both packages. MySQL 8.0 itself is **NOT MEASURED** locally (no server in the container). CI's `Temporal Conformance (live PG + MySQL)` job runs this package's whole suite with `OS_EXPECT_LIVE_DIALECT_MATRIX=1`. **Reverse verification.** The fix was committed first. Then `main`'s `sql-driver.ts` was restored under a trap and the new file was run on all three cells: 25 red and 14 green. The red ones are every refusal pin and half 2 on each cell. The fresh-id business-key pin is red on MariaDB only, because SQLite and PostgreSQL already answer it with their own unique violation. The file was then restored, with byte identity proved by blob hash and an empty `git diff HEAD`. The turso file ran red on `main` (10 of 15), including the remote insert leg writing no organization. **Gates.** On `35d01c9d`, `dispatch-gates --commands` derives 93 families (the spec paths added 30), and `--ran` reconciles 93 derived, 93 run, 0 NOT MEASURED, all exit 0. Among them: `check-adr-0087-registration --base origin/main`, `check:error-code-provenance` (336 stamp sites: 317 listed, 19 waived), `check:generated` (15 artifacts up to date), `check:migration-registry`, `check:spec-changes`, `check:upgrade-guide`, `check:api-surface` and `check:error-code-casing`. `check:driver-conformance` reads the same before and after (50 covered, 0 DEBT, 0 exempt), and `check:tenant-chokepoint` passes. ESLint, narrowed to the 8 changed `.ts` files, reports 0 errors and 0 warnings (`--format json`). The config enables no type-aware linting (`eslint.config.mjs` says so), so this diff cannot move the verdict on an untouched file. ## Acceptance notes - **The predicate is the written organization, as ruled.** A tenant-scoped call that names another organization in its payload uses `injectTenantOnInsert`'s documented admin authority, so it still merges into that organization's row. `update()` under the caller's scope would not reach that row. Under the `group` posture the fence is the written organization alone: an upsert onto a row of another member organization is refused, where `update()`'s union scope reaches it. - **Rows with no organization are fenced off too.** The written value is never NULL, so the predicate treats a stored NULL as distinct. - **No in-repo producer reaches a fenced cross-organization conflict.** The lifecycle archiver's `cold.upsert(object, row, ['id'])` carries no tenant context, so it gets half 2 only. The sandbox body runner's `ql.upsert` falls back to `insert`, and the knowledge adapters' `upsert` is a different interface. - **`driver-sqlite-wasm` inherits the fence.** Its knex dialect extends the SQLite3 compiler and its suite is green, but it has no tenant-scoped upsert pin of its own. - **`SqlDriver.create` on the MySQL family answers the insert id, not the record: filed as #21227.** Measured by this card's round-0 dev and then at the public doors: `builder.insert(...).returning('*')` returns `[insertId]` on MySQL, so `create()` returns `0` for a string-id table. Sign-up answers `400 FAILED_TO_CREATE_USER` and no user can sign in. `bulkCreate` has the same pattern. This PR's pins read the stored row back rather than `create`'s return value, and `upsert`'s own read-back is unaffected, so it does not bear on this change. --- _Generated by [Claude Code](https://claude.ai/code/session_017xfMoEjKUuSh2xYB8sCozp)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 3ddd3d0 commit 95e24b0

9 files changed

Lines changed: 1269 additions & 16 deletions
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
---
2+
'@objectstack/driver-sql': minor
3+
'@objectstack/driver-turso': minor
4+
'@objectstack/spec': patch
5+
---
6+
7+
fix(driver-sql,driver-turso)!: an upsert whose conflict lands on another organization's row is refused with `UNIQUE_VIOLATION` and writes nothing, and an upsert never changes a row's organization (#21185)
8+
9+
Clause-②: no (narrowing)
10+
11+
<!-- adr-0087: registered driver-upsert-cross-organization-conflict-refused -->
12+
13+
**BREAKING for `upsert` callers on the SQL drivers and on `TursoDriver`.**
14+
15+
**What changed.** `upsert` resolves its conflict against the whole table, and the
16+
primary key and a `unique: 'global'` column are installation-wide, so the row a
17+
tenant-scoped call (`options.tenantId` on an object with a tenant column) collided
18+
with could belong to another organization. The merge wrote the payload onto that
19+
row, tenant column included. Now:
20+
21+
- **A tenant-scoped upsert merges only into a row of the organization the row is
22+
written under**, for any conflict target, the primary key included. A conflict
23+
that lands on a row of another organization, or on a row with no organization,
24+
is refused with `code: 'UNIQUE_VIOLATION'`, `status: 409`, and nothing is
25+
written. That is the answer `create()` gets for the same collision: from the
26+
caller's organization the call is an insert, and that insert collides. The
27+
refusal names no organization and no value of the row it collided with.
28+
- **The tenant column is insert-only** (`insertOnlyUpsertColumns`), like `id`,
29+
`created_at` and `auto_number` columns: an upsert with no tenant context merges
30+
into the row it lands on and keeps that row's organization.
31+
32+
Mechanism, per face: on SQLite, PostgreSQL and the remote (libSQL) face, the merge
33+
statement carries the organization predicate (`DO UPDATE … WHERE`), so another
34+
organization's row is never written. On MySQL, whose `ON DUPLICATE KEY UPDATE`
35+
takes no `WHERE`, the statement and a read of the landed row run in one
36+
transaction (a savepoint inside a caller's transaction), and the read's failure
37+
rolls the write back. The remote face now also stamps the caller's organization on
38+
the row it inserts, as the local faces do.
39+
40+
## FROM → TO
41+
42+
| you relied on | now |
43+
|:--|:--|
44+
| a tenant-scoped `upsert` merging into a row of another organization | refused with `UNIQUE_VIOLATION` / 409, nothing written |
45+
| an `upsert` payload's tenant value moving the row it merges into | the row keeps its organization; to move a row between organizations, use `update()` |
46+
47+
**What is not affected.** A tenant-scoped upsert whose conflict lands on a row of
48+
its own organization merges as before, on every target. An upsert that inserts
49+
lands under the caller's organization, or under the organization the payload
50+
names explicitly, as before.

0 commit comments

Comments
 (0)