Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 55 additions & 0 deletions .changeset/tenant-chokepoint-read-doors.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
---
'@objectstack/driver-sql': patch
'@objectstack/driver-sqlite-wasm': patch
'@objectstack/driver-turso': patch
---

drivers: every SQL read door routes through the tenant chokepoint (#6792)

`SqlDriver.applyTenantScope()` owns read-side tenant isolation for the whole SQL family —
the `tenantId` early-out, the "object has no tenant field" early-out, the NULL-org
platform-row rule (#2734) and the ADR-0105 D2 union posture (#3623). Its own docstring
said "every CRUD method routes through it". Nothing ever checked that, and it was false
for as long as it had existed. **Three** read doors built their query through
`getBuilder()` and never arrived:

- **`findWithWindowFunctions()`** — the documented #4286 window door. It returns **rows**,
so on a deployment where the scope would have applied (`options.tenantId` set, object
has a tenant field) it returned rows belonging to **every** tenant. Measured with two
tenants seeded plus one NULL-org platform row: `tenantId: 'org_a'` returned
`[a1, a2, b1, b2, p1]` here against `find()`'s `[a1, a2, p1]` — another tenant's rows,
handed over at the driver layer.
- **`analyzeQuery()` / `explain()`** — returns a **plan**, not rows, so this is a smaller
fix and it is made on its own merits rather than folded into the one above. It is the
same defect #6577 fixed on these two methods one builder line lower: a plan is only
worth reading if it explains the statement `find()` would actually run, and a missing
tenant predicate changes selectivity and therefore which index the planner picks.
Compiled `select * from account` where `find()` sent the `organization_id` clause.
- **`distinct()`** — returns one column's **values** for every tenant. This one was in no
card. #6792 states the opposite, listing `distinct` among the scoped call sites; the
13th read site is `aggregate()`. It was found by measuring the invariant rather than
re-reading it.

All three now call `applyTenantScope()` beside their `getBuilder()` line, the position
`findRows()` uses. They route through the chokepoint rather than re-deriving a predicate:
a local equality would silently drop NULL-org platform rows (#2734) and collapse group
reads to active-org reach (#3623). Both of the chokepoint's early-outs are inherited
unchanged, so an unscoped admin/seed read (no `tenantId`) and any object without a tenant
field behave exactly as before.

**The durable half is a gate, not the three lines.** `pnpm check:tenant-chokepoint`
(`scripts/check-tenant-chokepoint.mjs`, wired into `.github/workflows/lint.yml`) re-derives
the invariant from the AST across the `SqlDriver` family on every run: a method that builds
through `getBuilder(object, options)` must call `applyTenantScope()` on that builder, or
carry a written exemption. Insert builders are exempt structurally — write-side tenancy is
`injectTenantOnInsert` — rather than by a name list. It is keyed on the **builder** and not
on the method signature, because the signature criterion the card sketches ("takes
`(object, …, options)` and returns rows") misses `distinct` (no `query` parameter) and
`analyzeQuery` (returns a plan). Verified red against the pre-fix tree, red against a
newly-added unscoped door, and silent once that door is scoped.

The chokepoint docstring no longer asserts the invariant; it names the gate that proves it.

If you call these doors directly on a multi-tenant deployment, pass `options.tenantId` as
you would to `find()` — that is what now takes effect. Callers that never passed it are
unaffected; that remains the documented unscoped/admin path.
38 changes: 38 additions & 0 deletions .github/workflows/lint.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -575,6 +575,44 @@ jobs:
- name: Spec type-alias convention gate (ADR-0122)
run: pnpm check:spec-parsed-alias

# Read-side tenant chokepoint gate (#6792, from #3724 / #6577).
# `SqlDriver.applyTenantScope()` owns read-side tenant isolation for the
# whole SQL family — the tenantId early-out, the no-tenant-field early-out,
# the NULL-org platform-row rule (#2734) and the ADR-0105 D2 union posture
# (#3623). Its own docstring claimed "every CRUD method routes through it".
# Nothing checked that, and it was FALSE for as long as it had existed:
# three doors built through `getBuilder()` and never arrived —
# `findWithWindowFunctions` (ROWS: a caller passing `tenantId` got every
# tenant's rows, measured `[a1,a2,b1,b2,p1]` against `find()`'s
# `[a1,a2,p1]`), `analyzeQuery`/`explain` (a PLAN for a statement `find()`
# would not run — the same defect #6577 fixed on these methods one builder
# line lower), and `distinct` (every tenant's values for one column).
#
# The third is the argument for gating rather than fixing. It was in NO
# card: #6792 asserts the opposite — that `distinct` is among the 13 scoped
# sites — and the triage comment and two rounds of measurement all
# inherited that sentence without re-deriving it. The 13th read site is
# `aggregate()`. Two of the three doors were found by a human reading the
# file for another reason; the third was found only by measuring, which is
# the thing a prose invariant can never do for itself.
#
# Keyed on the BUILDER, not the method signature. #6792 sketches "every
# method taking `(object, …, options)` and returning rows"; that criterion
# is measurably too narrow — `distinct(object, field, filters, options)`
# takes no query and `analyzeQuery` returns a plan, so it misses two of the
# three. `getBuilder()` is the single constructor of every statement this
# driver sends, so every builder is classified and one that cannot be
# classified is an error, never a default (#4690's family). Insert builders
# are exempt structurally, not by name: write-side tenancy is
# `injectTenantOnInsert`.
#
# Static AST over three files, no build needed, so it belongs in this job.
# Runs its own --self-test first, in both directions — the detector can be
# broken while every door is fine, and a scan that stops matching would
# report OK while reading nothing.
- name: Read-side tenant chokepoint gate
run: pnpm check:tenant-chokepoint

typecheck:
name: TypeScript Type Check
runs-on: ubuntu-latest
Expand Down
11 changes: 11 additions & 0 deletions content/docs/data-modeling/queries.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -590,6 +590,17 @@ const ranked = await sqlDriver.findWithWindowFunctions('employee', {
});
```

<Callout type="warn">
**Pass `options.tenantId` on a multi-tenant deployment.** Like `find()`, this door is
tenant-scoped only when the caller supplies it — the example above omits it, so it reads
across every tenant. That is the driver layer's documented contract (seed scripts and
cross-org tooling depend on the unscoped path), but it is a decision to make deliberately.

Until #6792 the door ignored `options.tenantId` even when you *did* pass it and returned
every tenant's rows regardless. It now routes through the driver's `applyTenantScope`
chokepoint like every other read.
</Callout>

For request-level analytics, use `aggregations` + `groupBy`, or model rankings in
report/dashboard metadata.

Expand Down
13 changes: 13 additions & 0 deletions content/docs/protocol/objectql/query-syntax.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -826,6 +826,12 @@ from presentation; Postgres and MySQL hand back their own native temporal value)
then re-deduplicates the presented values, because SQL `DISTINCT` compares the *stored*
form.

It is tenant-scoped on the same terms as `find()` — the `organization_id` predicate is
applied when the call carries `options.tenantId` (the fourth argument), and the example
above omits it, so it returns the column's values across every tenant. Until #6792 the
predicate was dropped even when `tenantId` *was* supplied, which made a scoped call
disclose every other tenant's values for that column.

### Full-Text Search

The `search` parameter does **not** reach a full-text index. The engine expands it into
Expand DownExpand Up@@ -959,6 +965,13 @@ projection, and niladic rendering means argument-taking functions (`LAG(field)`)
emit without their argument. For request-level analytics use `aggregations` +
`groupBy` (§5).

Tenancy works exactly as it does on `find()`: the driver applies its
`organization_id` predicate only when the call carries `options.tenantId`. The example
above omits it and therefore reads across every tenant — the intended unscoped/admin
path, but a deliberate choice rather than a default to inherit. (Until #6792 this door
dropped `options.tenantId` even when it was supplied, so a scoped call still returned
every tenant's rows.)

---

## 7. Pagination
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -80,6 +80,7 @@
"check:engine-double-contract": "node scripts/check-engine-double-contract.mjs --self-test && node scripts/check-engine-double-contract.mjs",
"check:resume-authority-declared": "node scripts/check-resume-authority-declared.mjs --self-test && node scripts/check-resume-authority-declared.mjs",
"check:spec-parsed-alias": "node scripts/check-spec-parsed-alias.mjs --self-test && node scripts/check-spec-parsed-alias.mjs",
"check:tenant-chokepoint": "node scripts/check-tenant-chokepoint.mjs --self-test && node scripts/check-tenant-chokepoint.mjs",
"check:stall-guard": "node scripts/run-with-stall-guard.mjs --self-test"
},
"keywords": [
Expand Down
Loading
Loading