Skip to content

fix(driver-memory): refuse a tenant-scoped call instead of returning cross-organization rows - #17005

Merged
yinlianghui merged 3 commits into
mainfrom
claude/issue-16589-memory-driver-scope-refusal
Sep 9, 2026
Merged

fix(driver-memory): refuse a tenant-scoped call instead of returning cross-organization rows#17005
yinlianghui merged 3 commits into
mainfrom
claude/issue-16589-memory-driver-scope-refusal

Conversation

@os-musk

@os-musk os-musk commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Fixes #16589

The memory driver silently ignored the engine's tenant scope for objects that omit a tenancy block. It now refuses the call instead of answering with every organization's rows.

Clause-②: yes

The ruling this implements, and the direction it closed

Maintainer ruling A, on this card (comment 5580215080, 2026-09-08), verbatim:

16589 内存驱动不需要支持多租户,业务上没有任何意义啊;其他同意

It confirms the standing 2026-08-12 ruling on #6915 (comment 5261729371): 「⛔ 不做处置 A(实现行级租户隔离)」.

Row-level tenant isolation on driver-memory is refused, read side included. This PR does not implement scoping and must not be read as a step toward it: the driver declines to answer. There is no predicate anywhere in this diff that filters rows by organization_id.

The defect

Two predicates decided "is this object tenant-scoped", and they disagreed on the default case:

  • the engine scopes an object unless it opts outEngine.buildDriverOptions: execCtx?.tenantId !== undefined && !isTenancyDisabled(objectSchema) && !isFederated;
  • the memory guard refused only an explicit opt-indeclaresTenantScope: tenancy.enabled === true.

An object that omits the tenancy block entirely fell between them: the engine scoped it, the guard never saw it, the posture check passed because the posture really was single, and the driver then discarded the scope and returned every organization's rows.

The four premises, re-taken on this branch

Re-measured on this PR's own tree, not quoted from the dispatch. Tree: b834b48e7a (the branch point; origin/main at cut time).

# premise reading on this tree control
the guard refuses only an explicit opt-IN memory-tenancy-guard.ts:113-115return (schema as ...)?.tenancy?.enabled === true; read directly, exact lines
the engine scopes unless the object opts OUT objectql/src/engine.ts:3925-3928 — the predicate above read directly, exact lines
memory-driver.ts never consumes the scope 0 hits for tenantId/tenantIds/organization_id in 2318 lines firing controls on the SAME file: assertObjectsNotTenantScoped = 2, assertSingleTenantPosture = 3; call site relocated by content at :1907 in syncSchema
the load-bearing docstring sentence present at :110-111, "and every object in a single-tenant deployment omits the block" — false read the docblock directly; the dispatch's warning that a multiline grep returned a zero whose own control also failed to fire is why this was not grepped

A fifth reading, taken while placing the refusal and worth recording: options is never dereferenced anywhere in memory-driver.ts — accepted in 14 signatures, read in none. That is premise ③ from the other side.

A refinement carried from the dispatch, stated the accurate way: the card's "zero hits across the whole driver-memory/src tree" is loose. memory-driver.ts itself is 0, which is the half that matters; package-wide there are hits in memory-tenancy-guard.ts and in memory-unique-constraint.ts — the latter a live organization_id key check about UNIQUE composites, a different concern, not a read-path scope. The read path knows nothing about tenants; the unique-constraint path does.

Where the refusal went, and why

It is a third seam beside the two boot seams, on the per-call path, built from the existing guard's shape. No new error family: it reuses MemoryMultiTenantUnsupportedError and the existing MEMORY_MULTI_TENANT_UNSUPPORTED code, with one optional seam parameter that varies only the wording.

The placement is justified by measurement, not preference:

  1. The boot seams are never handed the scope. Every engine call site spells syncSchema(tableName, obj) and dropTable(tableName) — two arguments and one argument, no DriverOptions at all. The scope is structurally invisible at syncSchema, which is exactly why the gap exists.
  2. The engine's decision is per-request. execCtx?.tenantId !== undefined is a property of the call, not of the schema or the deployment. At syncSchema time the fact does not exist yet.
  3. Widening the boot guard instead would over-refuse. Making declaresTenantScope match the engine (refuse anything not explicitly opted out) refuses at boot on objects that merely omit the block — including single-organization apps that never carry an organization context and are never scoped. That is wider than the ruled sentence, which refuses "when the engine hands the memory driver a tenant scope", not "when a schema could one day be scoped".

The seam judges the scope the engine actually handed over rather than re-deriving the engine's predicate from metadata. A driver that re-derived it would drift from the engine the first time that reasoning changed, and drift here is silent exposure.

It runs first in every door that accepts a DriverOptions, before any store access or delegation. That is what makes the closed round's measured F2 defect unreachable: upsert() delegates to update() or create(), and a refusal that fell through to the create arm would land a second row under one primary id.

declaresTenantScope itself is unchanged — its behaviour is still correct for what seam 2 uses it for.

The docstring correction

declaresTenantScope's closing sentence — "every object in a single-tenant deployment omits the block" — is false in both halves. single constrains the wall, not the number of organizations (the card measured 13 sys_organization rows in a single-posture run), and the omission it describes is not the absence of a tenant signal but its default presence.

The sentence is recorded as superseded rather than deleted, because it is what justified the predicate being an opt-in test and anyone re-reading that decision needs to see the reasoning that was withdrawn.

Acceptance — a control that can fail, in both directions

The card's own subject is a test that cannot fail, so the fixture is built against that. Three organizations are seeded with counts 2 / 3 / 7, chosen so every reading a driver could produce is a different number:

reading rows what it would mean
nothing 0 over-scoped
the correct subset for ORG_A 2 row-level isolation — the direction the ruling REFUSED
the union ORG_A + ORG_B 5 a widened tenantIds scope
everything 12 the #16589 defect

The closed round seeded only two organizations, so a [ORG_A, ORG_B] union covered the whole table and "the scope widened" and "no scope ran" were the same answer. With 2/3/7 they are 5 and 12.

The refusal is pinned as none of those readings — the driver produces no answer at all, asserted as a nullable count rather than with toHaveLength, which rejects a null target even under .not and would have passed the block for the wrong reason (this actually fired during development and was corrected).

That makes the control failable in both wrong directions: a revert to silent non-isolation answers 12, and an implementation of row-level isolation answers 2, and each one reds the file.

The three ways the same read is distinguishable:

  • before — an unscoped read returns all 12 rows across all three organizations. This is preserved deliberately: it is what a scoped read used to answer, because the scope was silently discarded.
  • after — the same read, scoped, refuses; the store is unchanged, and a scoped upsert by a foreign id creates nothing.
  • both — an unscoped read, an object declaring tenancy.enabled: false, an empty tenantIds, and options carrying only timezone / skipCache / bypassTenantAudit all keep working unchanged.

Ablation

Two legs, run on the committed implementation, under the shared verify lock — .turbo/cache is shared across worktrees, so an unlocked ablation can write a mutated artifact into a sibling agent's build cache.

The marker is a string literal in code, not a comment: comments do not survive the build, so a comment marker could never be proved to have reached dist.

step reading
pre-state marker in source 0, mutation anchor 1 — both directions readable before anything moved
mutate seam neutered with an early return; marker on disk 1; blob moved 1ad023cd to 42648ea3
reached dist ablation-dist-preflight --present exit 0
ablated run exit 1 — 5 failed / 6 passed (11)
restore git checkout HEAD -- (the guard path) — HEAD-pinned, never a bare git checkout --, which reads a polluted index
restore proved marker 0; blob back to 1ad023cd82d2f7dadbb94573112f7e9c80648b4d, byte-equal to git rev-parse HEAD:(path)
left no trace --absent preflight exit 0, and it independently reported the whole working tree clean against HEAD
restored run exit 0 — 11 passed (11)

Direction predicted before the run: red. Observed: red. The restore leg is proved by the blob hash and a whole-tree git status --porcelain, not by an exit code — a restore step that does nothing also exits 0.

Green accounting — every case that stayed green, and the layer that holds it

Four are held by the driver's ordinary read path, not by the seam, and stay green by design. Neutering a refusal cannot change what an unscoped read returns:

  • seeds three organizations whose every reading is a different number — the fixture's own precondition;
  • an UNSCOPED read returns every organization — this is the before leg. Its whole point is to be independent of the refusal: it is the answer a scoped read used to give;
  • an unscoped read on the same object is served exactly as before;
  • an object declaring tenancy.enabled: false is served — the engine sends no scope for an ADR-0066 opted-out object, so seam 3 is not on this path at all.

Two are the seam's negative half, and a removal ablation cannot red them by construction — they assert that something is not refused, so deleting the refusal can only make them greener:

  • options that carry no tenant scope pass straight through;
  • the DDL doors still sync and drop with no options.

Stated plainly rather than left as an unexplained green: this ablation does not demonstrate those two are discriminating. They discriminate in the opposite direction — against an over-eager seam — and the mutation that reds them is a predicate widened to treat a defined-but-empty tenantIds as a scope, which would refuse calls the engine never scoped. They are load-bearing precisely because the risk this card carries is a guard that refuses too much: driver-memory is the in-process store behind the dev stack and the example apps.

Changeset — decided on a measurement

minor with a **BREAKING** banner, plus an ADR-0087 disposition. Not assumed:

Disposition: not-required (no-migration-prescription) — nothing authorable is removed, renamed or re-shaped, so objectstack migrate meta has nothing to rewrite, and the operator is reached by the refusal message itself. check-adr-0087-registration accepts it (exit 0).

Verification

Gate reconciliation

Derived with node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack. The first derivation warned STALE TREE and named three files it derives from as changed across the gap — one of them a gate this PR runs — so origin/main was merged and it was re-derived clean. The change set stayed exactly these five paths.

57 derived · 57 run · 0 UNRUN · 0 NOT MEASURED. Every exit status captured by redirect-then-$?, never across a pipe.

Two were not green on the first pass, and neither was a finding:

gate first reading why that was not a verdict real verdict
check:dual-build-cjs-loads exit 3 its own words: PREREQUISITE NOT MET, "some package has no dist/ … this is NOT a pass: nothing was measured" exit 0 after a full pnpm build (73/73 tasks) — 104 require entry points across 67 packages load, 620 CJS files parse
check:type-check-debt exit 124 killed by my own 300s per-gate timeout, mid-report exit 0 — 5 ledger entries re-measured, 55 raw tsc errors, none above its recorded number

check:type-check-debt needed two further readings to become real, and the reason is worth recording: the resource-discipline cap NODE_OPTIONS=--max-old-space-size=4096 sits below the 6144 MB CI-shaped ceiling this gate pins for itself, and tsc OOMed — reported as exit 3, "refusing to record 0", not as a finding. Unsetting the variable fell through to this machine's 2096 MB default, lower still. Run at the pinned 6144 it is green. A tighter heap cap is not the conservative choice here; it is a different measurement.

Also run although the derivation scores it silent: check:tenant-chokepoint — exit 0. The tool flags its roster as living under packages/drivers, which these paths are in, and says in terms that such silence is not a clearance in either direction. It reports 20 getBuilder() bindings across 3 files (driver-sql, driver-sqlite-wasm, driver-turso), so seam 3 reading tenantId did not pull this driver into its scan — which is what the guard docblock now claims in prose.

Repo-wide pnpm lint — exit 0, run in full. No narrowing claimed.

Tests

suite reading
@objectstack/driver-memory test (post-merge) 47 files · 1159 passed · 15 todo — exit 0
@objectstack/driver-memory typecheck (both tsc legs) exit 0
@objectstack/runtime 248 files · 3509 passed — exit 0
@objectstack/service-datasource 32 files · 676 passed — exit 0
@objectstack/plugin-dev 7 files · 72 passed — exit 0
@objectstack/cli unit layer 189 files · 2624 passed — exit 0

Consumers were run because this changes on-the-wire behaviour, not merely an internal. The five dependents were enumerated from package.json, not guessed. On a first pass service-datasource reported 76 failures and the CLI 2 — every one a missing dist in a fresh worktree ("Failed to resolve entry for package @objectstack/driver-sql"; "packages/cli is not built … would fail for the wrong reason"). Those were NOT MEASURED, not reds; with the closures built, all six suites above are green.

objectql is not a dependent of this package, and separately: zero of its test files combine a tenantId with the memory driver, so nothing there was riding the discarded scope.

The new test file is inside a tsc program, proved rather than assumed: tsc --noEmit --listFiles lists it (1), a nonsense filename reads 0 (the instrument can return zero), and the guard source and the pre-existing guard test read 1 each — over 726 files, exit 0. That is leg 1 of this package's two-leg typecheck; leg 2 is tsconfig.typecheck.json, whose include is the root objectstack.config.ts alone.

Deliberately not done

Review posture

Draft, and it stays draft: this carries Clause-②: yes and needs:contract-review on both carriers, so it cannot land before an in-seat contract-tier review returns. Not ready-for-review, no auto-merge, not queued.


Generated by Claude Code

…f answering cross-organization rows

The engine scopes an object unless it opts OUT (buildDriverOptions), while the
boot guard refuses only an explicit opt-IN (declaresTenantScope). An object that
OMITS the tenancy block fell between them: the engine scoped it, the guard never
saw it, and the driver discarded the scope and returned every organization's
rows.

Adds seam 3 to memory-tenancy-guard: assertCallNotTenantScoped judges the scope
the engine actually handed over (DriverOptions.tenantId / tenantIds) rather than
re-deriving the engine's predicate, and refuses. Called first in every driver
door that accepts a DriverOptions, so a refusal leaves no partial effect.

Row-level isolation is NOT implemented here and is not the direction: the driver
declines to answer. Also records declaresTenantScope's false closing sentence as
superseded -- a `single` posture constrains the wall, not the number of
organizations.

Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg
Co-authored-by: Claude <noreply@anthropic.com>
… tenant-scope refusal

Seeds three organizations with counts 2/3/7 so that "returns nothing" (0), "the
correct subset" (2), "a widened union" (5) and "everything" (12) are four
distinct numbers -- the closed round of this card seeded only two orgs, where a
widened union covered the whole table and "the scope widened" was indistinguish-
able from "no scope ran".

The refusal is pinned as none of those readings, which makes the control failable
in BOTH wrong directions: a revert to silent non-isolation answers 12, and an
implementation of row-level isolation answers 2, and each reds the file.

Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/driver-memory, touching 26 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/drivers/driver-memory/src/index.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

23 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 8d4690b8f55d21da6b8530081ab76aa10ac524a7.

4 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/drivers/driver-memory/src/index.ts) — pages documenting those are invisible to this run
  • 6 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 60 of 216 client-bound route-ledger rows — the other 156 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 156: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 8 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 8d4690b8f55d21da6b8530081ab76aa10ac524a7packageMentionDocs.

Which tree this was computed on

This run read content/docs from 2c92a6a665765281a5049526ac5c3a66c0dd2c53 — the merge of head c8e9c33257c7fdd9ec8ab037464aef31e566ebea into base 8d4690b8f55d21da6b8530081ab76aa10ac524a7, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 2c92a6a665765281a5049526ac5c3a66c0dd2c53 && git checkout 2c92a6a665765281a5049526ac5c3a66c0dd2c53
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 8d4690b8f55d21da6b8530081ab76aa10ac524a7 c8e9c33257c7fdd9ec8ab037464aef31e566ebea && git checkout -B drift-repro 8d4690b8f55d21da6b8530081ab76aa10ac524a7 && git merge --no-ff c8e9c33257c7fdd9ec8ab037464aef31e566ebea

node scripts/docs-audit/affected-docs.mjs --json 8d4690b8f55d21da6b8530081ab76aa10ac524a7

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 8d4690b8f55d21da6b8530081ab76aa10ac524a7 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Sep 9, 2026
@yinlianghui
yinlianghui marked this pull request as ready for review September 9, 2026 02:38
@yinlianghui
yinlianghui added this pull request to the merge queue Sep 9, 2026
@os-musk
os-musk marked this pull request as draft September 9, 2026 02:59
Merged via the queue into main with commit 555a89c Sep 9, 2026
42 checks passed
@yinlianghui
yinlianghui deleted the claude/issue-16589-memory-driver-scope-refusal branch September 9, 2026 03:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

3 participants