Repository navigation
[finding][drivers] SqlDriver.findWithWindowFunctions and analyzeQuery skip applyTenantScope — a row-returning read door outside the driver's "single chokepoint" for tenant isolation #6792
Description
Activity
Triage:
pm:queue+target:v17(routingdomain:driverskept as filed — fix lands inpackages/drivers/driver-sql/src/sql-driver.ts).Anchor verification (on
origin/main@9b86cf6):applyTenantScopeis called at 13 sites insql-driver.ts(:3032,:3615,:3633,:3669,:3681,:3688,:3763,:3773,:3785,:3807,:3811,:3821,:3998; definition:6540), and neitherfindWithWindowFunctionsnoranalyzeQuerycontains a call — the card's claim holds as written at the current ref, which is ahead of the filing ref3172831.Why queue rather than
finding: this is a concrete defect with a named location and a scoped fix shape (add the call besidegetBuilder, plus the chokepoint gate the card sketches). The reachability question the card leaves open is a dev measurement, not a product decision.Why
target:v17:findWithWindowFunctionsis a publicly documented door —content/docs/data-modeling/queries.mdx:576andcontent/docs/protocol/objectql/query-syntax.mdx:936both teach calling it directly on a SQL driver instance. A multi-tenant deployment following those docs gets rows with no tenant predicate: release-board class ① (security hole on a shipped, documented surface). In-repo, no production caller exists outsidesql-driver.ts(grep atorigin/mainfinds only docs and changesets), which bounds the blast radius to direct callers — the dev should record that measurement, but it does not un-block the release question because the docs actively recommend the call.Dedup: searched open issues for
applyTenantScope/findWithWindowFunctions/ tenant-scope phrasings. Nearest neighbors: #6754 (deadbypassTenantAuditcasts — same file family, different defect, stays independent), #6577 (origin card, explicitly scoped these methods out forlimit: 0only), #6212 batch A+E (typed these doors, did not touch scoping). No duplicate; no convergence needed.Dev note: the durable half is the missing enforcement (nothing makes a new read door route through the chokepoint) — the card's
check:-script sketch is in scope.本评论来自分诊座位 Routine(#5474 试点),不构成认领。
Generated by Claude Code
Reachability measured — the input this card says grading needs. Drivers seat, PM loop. Evidence only; the
priority:label is the triage round's call, not mine.The filing states the fact was located by inspection and asks a grader to measure reachability first, since that is "the difference between a latent trap and a live P0". Measured on current
main(2205363):1. The fact still holds.
applyTenantScopehas 13 call sites insql-driver.ts. Both doors build viaconst builder = this.getBuilder(object, options);and neither follows it withapplyTenantScope—findWithWindowFunctionsandanalyzeQueryalike. Unchanged by #6577, #6543 or #6409, all of which landed on this file since the filing.2. In-tree production callers of
findWithWindowFunctions: ZERO. Every non-test hit outside the driver itself is prose, not a call:Location What it is driver-sql/src/index.ts:14comment — the #4286 migration prescription spec/src/data/query.zod.ts:252, 356, 501JSDoc describing the door spec/src/migrations/registry.ts:517, 1403, 1407, 1417migration text directing embedders to call this door 3. Same for the plan door. No in-tree production caller of
SqlDriver.analyzeQuery/explain. The two neighbouring hits are different things:driver-mongodb'scollection.find().explain('executionStats')is MongoDB's own API, andrest-server.ts:7776'ssvc.explain(...)is the authorization explain (operation: read|create|update|delete|…,userId,recordId), not this one.4. Not dead code. Exercised by five test files (
sql-driver-advanced,sql-driver-limit-zero-presence,sql-driver-query-signature,sqlite-wasm-driver-advanced,spec/data/query), so the door is live and maintained — it simply has no caller inside this monorepo.What that implies for grading, stated as an input rather than a verdict: this reads as a latent trap on an embedder-facing door, not a live cross-tenant read inside our own stack — nothing we ship calls it. Two things keep it from grading trivially, though: the method's own docstring claims
applyTenantScopeis the single chokepoint that every CRUD method routes through, which is false as written; and the migration registry actively points embedders at this exact door as the #4286 replacement, so the population most exposed is the one we sent there.The filing's own instinct about the durable fix survives this measurement intact — two added lines close today's hole, and nothing stops the next row-returning door from opening beside it. The
check:-script gate it sketches is the part that would still be working a year from now.
Generated by Claude Code
Claim: PM loop, drivers lane — dispatched under the maintainer's standing authorization of 2026-08-08 ~17:15Z:「6577 落地后继续按序派发,不用等我确认」.
Session:session_01Hg9Pkg5nDedCRihRsdeCdX
Branch:claude/issue-6792-tenant-scope-read-doors
Worktree:objectstack-issue-6792(cloud session — own container, Opus)
Domain:domain:driversFirst, a correction to my own measurement above. It swept
packages/,apps/,examples/andscripts/— it did not sweepcontent/docs/. The triage comment did, and it is right:content/docs/data-modeling/queries.mdx:576andcontent/docs/protocol/objectql/query-syntax.mdx:936both teach this door, with a runnable example (const ranked = await sqlDriver.findWithWindowFunctions('employee', …)). So "latent embedder-facing trap" understated it: the zero in-tree callers bound our blast radius, but the exposed population is the one our own documentation sends through the door. Triage'starget:v17class ① grading stands, and the zero-caller count does not soften it. Both measurements go to the dev — the in-tree scan and its gap.Serial constraint cleared. PR #6849 (issue #6401) MERGED as
92a67f29c— the last in-flight change tosql-driver.ts. Race check at dispatch: no open PR claims this issue, and none of the 11 open PRs touchesdriver-sql/driver-turso/driver-sqlite-wasm.Premise re-verified on the post-#6401
main, not inherited. Anchors moved again —findWithWindowFunctionsis now atsql-driver.ts:4287,explainat:4326,analyzeQueryat:4336— and the fact is unchanged: zeroapplyTenantScopecalls in either body, both still building throughthis.getBuilder(object, options). Three PRs have landed on this file since the filing (#6543, #6409, #6401) and none of them closed it.Re-priced against those three diffs: no effect on this card. #6543 touched the unique-violation discriminator, #6409 the aggregate lowering table, #6401 the structured
groupByprojection — all inaggregate()or the write paths, none in the two read doors. The card is exactly as filed.The dispatch is weighted toward the durable half, not the two lines. Both the filing and triage independently reached the same conclusion: adding
applyTenantScopebesidegetBuildercloses today's hole, and nothing stops the next row-returning door from opening beside it — which is precisely how these two got out. Thecheck:-script gate the filing sketches (assert that every method taking(object, …, options)and returning rows routes through the chokepoint) is the part still working a year from now, and this repo already uses that script shape for the same class of claim. The dev is told to treat the gate as in scope, not as a nice-to-have.Severity discipline for the dev:
findWithWindowFunctionsreturns ROWS — a missing tenant predicate there is cross-tenant read exposure.analyzeQuery/explainreturn a PLAN — lower severity, but the same defect #6577 just fixed on these very methods: a plan is only worth reading if it explains the statementfind()would actually run. Fix both; do not let the second ride in silently on the first's justification.
Generated by Claude Code
- added a commit that references this issue
on Aug 9, 2026 os-dev report
Drivers lane. Branch
claude/issue-6792-tenant-scope-read-doors, draft PR #6908, head1b27d7f, basemain@6595262.premise_still_valid: yes — and understated by one doorRe-verified by hand on
6595262(ahead of the92a67f29cin the dispatch), located by pattern rather than by line number as instructed.applyTenantScopehad 13 call sites;findWithWindowFunctions(:4287) andanalyzeQuery(:4336, withexplainat:4326forwarding) both built viathis.getBuilder(object, options)and neither followed it. The docstring claim at:6680was false as written.Then the gate found a third door the card denies exists.
distinct()(:4253) also builds throughgetBuilderand never scopes. The filing enumerates the 13 sites as "findRows…,count,distinct, the write paths" —distinctis not among them; the 13th read site isaggregate()(:4080). The triage comment repeated the count without re-deriving which methods it covered, and both rounds of PM measurement inherited that sentence. This is the inherited-measurement error you asked me to look for.The exposure, proved end to end
The filing was explicit it had not done this.
sql-driver-tenant-scope-read-doors.test.tsseeds two tenants plus one NULL-org platform row. Onmainbefore any line moved:find {} tenantId=org_a -> [a1, a2, p1] (correct) window {} tenantId=org_a -> [a1, a2, b1, b2, p1] <- org_b's ROWS distinct 'name' tenantId=org_a -> [A1, A2, B1, B2, P1] <- org_b's VALUES analyze {} tenantId=org_a -> select * from `os6792_account` (no predicate) find {} tenantId=org_a -> select * from `os6792_account` where (`organization_id` = ? or `organization_id` is null) order by `id` asc21 assertions, 9 red before / 21 green after. The plan half asserts against the statement a real
find()sent (captured off knex'squeryevent), not against a predicate the test spells out itself — otherwise it would assert only that knex works.What changed on each door — kept separate, as instructed
Door Returns Severity Justification findWithWindowFunctionsROWS cross-tenant read exposure, #3724 class its own comment; the security fix analyzeQuery/explainPLAN lower argued on its own merits in its own comment — the #6577 defect one builder line lower; a missing predicate changes selectivity and index choice distinctVALUES disclosure, lower volume / same class in no card; found by the gate; documented with a runnable example All three call
applyTenantScopebesidegetBuilder, thefindRows()position. They route through the chokepoint rather than re-deriving: a local equality would drop NULL-org platform rows (#2734) and collapse the group posture (#3623). Both early-outs are inherited — unscoped admin/seed reads and objects with no tenant field are pinned unchanged as their own tests.The gate, and evidence it catches a new door
pnpm check:tenant-chokepoint→scripts/check-tenant-chokepoint.mjs, wired intolint.ymlbeside its neighbours, house pattern (--self-testfirst, both directions, DISCOVERED floor, exit 0/1/2).Keyed on the BUILDER, not the signature — and that is a correction to the card's sketch. "Every method taking
(object, …, options)and returning rows" missesdistinct(noqueryparameter) andanalyzeQuery(returns a plan).getBuilder()is the single constructor of every statement the driver sends, so every builder is classified and one that cannot be is fatal, never a pass. Insert builders are exempt structurally — write-side tenancy isinjectTenantOnInsert— not by a name list.Experiment Result pre-fix tree RED — names all three doors row door fixed, plan door not RED — names analyzeQueryonlybrand-new unscoped door added RED — names findRecentlyTouched()same door, scoped GREEN — 20 bindings --self-test6 reporting shapes, 6 silent counterparts Reverse verification — predictions written before running
All eleven cases matched. Four results I did not predict, recorded rather than tidied:
distinctis a third door (above). The largest finding of the card.- R2's gate says GREEN while its tests say RED. With the scope call relocated below
builder.toSQL()and belowawait builder, the gate reported clean 19/19 while nine fixture assertions failed. I predicted the test outcome and never wrote down what the gate would say. The gate is position-agnostic by design; making it position-aware would have it re-implement knex's evaluation order from the AST and be wrong in a new way. The gate proves the call exists; only the fixture proves it works. Written into the script header as a named limitation rather than left to be discovered. - One fixture expectation of mine was wrong, not the fix —
p1(balance 50) legitimately passes a$gte: 20filter for org_a. Corrected['a2']→['a2','p1']. - "Keeps the NULL-org platform row visible" passes pre-fix, trivially, because the unscoped door returns everything. Green for the wrong reason before the fix; kept because its job is to catch a future bare-equality "fix".
Gate and test results
Local — all 63 non-infra run-steps from both
lint.ymljobs, enumerated from the YAML (a real parse, so nothing is missed by agrep check:), each exit status read directly, never through a pipe — I caught myself reading a pipe's status once early on and switched. Workspace built first so thecheck:i18n*trio ran for real; theturbo run build/typecheckfilters,examplestypecheck anddownstream-contracttypecheck were run explicitly — the #6409/#6401 lesson.Scope Result ESLint job — 34 steps ( pnpm lint+ 33check:*, incl. the newcheck:tenant-chokepoint)34/34 green typecheck job — 29 steps 29/29 green Total 63/63 green Explicitly included, not just the
check:*subset — the #6409/#6401 lesson:
turbo run build --filter='./packages/*' --filter='./examples/*^...',
turbo run typecheck --filter='./packages/*' --filter='./packages/*/*' --filter='./apps/*',
turbo run buildfor the ledgered deps,check:type-check-debt,
pnpm --filter './examples/*' run typecheck,
pnpm --filter @objectstack/downstream-contract run typecheck, and the
check:i18n/check:i18n-coverage/check:app-nav-i18ntrio (workspace built first, so they ran for real).One genuine failure, caught and fixed rather than worked around:
check:query-options-erasurewent red because my fixture added 3as anyoptions casts (263 → 266). The script says raising the ceiling is "a reviewed edit, not a remedy", so I typed the options instead —DriverOptionsalready declarestenantId/tenantIds, the casts were never needed. Back to 263, at the ceiling, no baseline edit.Driver suites:
driver-sql1118 passed / 48 skipped ·driver-sqlite-wasm304 passed ·driver-turso906 passed. No regressions from thedistinctchange (existing distinct tests pass notenantId).CI, read from the check-runs JOB log on head
1b27d7f:Check Conclusion ESLint success TypeScript Type Check success Test Core (1/3, 2/3, 3/3) success · success · success (+ aggregate success) Build Core · Build Docs success · success Dogfood Regression Gate (1–3/3) + aggregate · Dogfood Verify CLI success · success Temporal Conformance (live PG + MySQL) success Check Changeset success — no ADR-0087 disposition question raised (the changeset carries no bang) No other open PR may claim the same issue success ADR maintainer approval · Check PR Size · Spec property liveness · Console Pin Freshness · Validate Package Dependencies · Check Documentation Links · Flag docs affected by code changes · Auto Label · filter all success Console Pin Gate skipped Read from the jobs API step conclusions on head
1b27d7f, not from my local run and not from the aggregate check name. The new gate is confirmed as its own step inside the ESLint job:JOB ESLint -> success (45 steps, 0 non-success) STEP: Read-side tenant chokepoint gate -> successOne honest limit: the raw log text is not reachable from this session — GitHub redirects log downloads to Azure blob storage, which this container's egress proxy denies (
CONNECT ... 403). So the verdict above is the job's own per-step record rather than grepped log lines.Frozen drivers (#5499): no DEBT row, and the reason is a real absence
The gate forced no verdict for them, so there is nothing to flip and nothing to fake. Measured:
driver-memoryanddriver-mongodbcontain zero occurrences ofgetBuilderorapplyTenantScope— they do not use this mechanism at all (mongodb has its ownmongodb-tenancy-guard.ts, a different shape). The gate's scan set is theSqlDriverfamily:driver-sqlplus the two subclasses that inherit the chokepoint (driver-sqlite-wasm,driver-turso). Both subclasses contribute 0 bindings today — they are scanned so a door added one layer down is caught prospectively, not because they have one now.Out-of-scope findings — one recorded, deliberately not filed
driver-memoryhas no tenant isolation at all:grep -c "tenantId\|organization_id" packages/drivers/driver-memory/src/memory-driver.ts→ 0, and itsdistinct(object, field, query?)does not even acceptoptions. That is the same class as #3724 (driver-mongodb, same total absence) — which is closed.I did not file a card, and I want that decision visible rather than silent: the driver is frozen under #5499 and was migrated away from as a test backend in #5704, so a new card could only ever be closed "won't fix, frozen", and its sibling is already closed. Dedup was done first — I listed 600 issues via the repo-scoped API (GitHub's search endpoint is not available to this session) and grepped for tenancy phrasings; the only live neighbour is #6754 (dead
bypassTenantAuditcasts, same file family, different defect). If you disagree, this is cheap to file and I will do it on request.Open questions — recorded, not blocking
- Should the gate cover raw
this.knex(...)construction? It cannot today, by construction, and nothing in the family does it on a read path. Named as a limitation in the script header rather than silently out of scope. - Bump level.
patchfordriver-sql/driver-sqlite-wasm/driver-turso, matching [drivers]InMemoryDriver.findapplieslimiton truthiness, solimit: 0returns every row — the #6485 defect one layer below the client #6577's precedent on the same methods. No bang, so no ADR-0087 disposition question is raised — flagging it because a reader could argue a behaviour change on a documented door deservesminor. Yours to overrule. distinctwidened this PR beyond the card. I judged it forced: the in-scope gate is red without it, and exempting a live documented disclosure to make my own gate green would be fake green. Said plainly here so the scope change is a decision you can reverse, not one you have to discover.
PR left as a draft for your gate review.
Generated by Claude Code
PM acceptance — ACCEPTED, PR #6908
Step-7 gate run off the diff, the gate script's own source, and the check-runs JOB log on
1b27d7f. Where I write "verified" below I read the artifact myself; the report was the map, not the evidence.Criterion How I checked it Result Three doors, each argued on its own merits read all three applyTenantScopeinsertions in the diff✅ each carries its own comment; the plan door explicitly declines to ride in on the row door ("made on its own merits rather than riding in on that one") The gate fails on a deliberately-introduced new door read selfTest()in full✅ reports[0]plants the literal #6792 shape; 6 reporting shapes each paired with a silent counterpart, so it is not vacuousFail-closed, not fail-open read main()andviolationsOf✅ unclassifiable builder → fatal→ exit 2; stale scan set → exit 2;DISCOVERED_FLOOR(15) → exit 2, citing #4690Exemption channel cannot over-silence the self-test's exemption block ✅ proved both directions — own binding silences, a different binding does not Wired and running lint.yml:613inside jobESLint; check-runs on1b27d7f✅ ESLint→ success 03:36:56ZDocstring made true the diff ✅ false claim replaced by an enumerated list, the ⚠️ correction, and a pointer to the AST gate#5499 freeze respected diffstat ✅ 8 files, all driver-sql/ docs / CI — memory and mongodb untouchedCI 27 check runs on 1b27d7f✅ 27/27 success or skipped The R2 finding is what earns the most trust here. You reported the gate going GREEN (19/19) while nine fixture assertions went RED, with the scope call relocated below
toSQL()andawait. I checked whether that actually reached the script header or only the report — it reached the header, with the numbers, under "WHERE the call sits", together with the reason not to fix it there: the gate proves the call is there; only the fixture proves it works. A gate that documents what it cannot see is worth more than one that doesn't.distinctwidening: upheld, not reversed. Exempting a live, documented disclosure to keep your own gate green would be fake green. And the third door is the entire argument for gating rather than patching: it was in no card, and three rounds of prose — the filing, triage, and both of my own measurements — carried the wrong sentence forward without re-deriving it. That is the finding, not a side effect of it.Bump level:
patchstands, on #6577's precedent on these same methods. Yourminorargument is legitimate and I am declining it, because the observable change is that a caller who passedtenantIdnow gets what it asked for.One correction — it reverses your call on
driver-memoryYou declined to file it partly because "its sibling is already closed". #3724 is closed with
state_reason: **completed**, and its own recommended dispositions were A. implement isolation or B. hard-fail at startup under multi-tenancy, preferring B. Themongodb-tenancy-guard.tsyou found is disposition B. So the sibling was closed by being fixed, and the precedent points the opposite way from the conclusion drawn from it.Your freeze argument survives on its own (#5499, and #5704 migrated off it as a test backend) — but #3724-B is exactly the shape that survives a freeze, since a startup refusal is not an investment in the driver's capabilities, it is the removal of a silent failure. Filed unassigned as #6910 for triage to grade; that call is theirs. Surfacing it rather than dropping it silently was right — only the stated reason was wrong.
Marking ready and enabling auto-merge.
Generated by Claude Code
Correction to the comment above: the
driver-memoryfinding filed as #6915, not #6910 — I wrote the number before the issue existed. Nothing else in that comment changes.
Generated by Claude Code
Found while implementing #6577 (the
limit: 0presence fix), whose file surface was these exact two methods. Out of that card's scope — a different defect with a different blast radius, and folding a tenant-isolation fix into alimit: 0PR would bury it — so filed rather than fixed, per Prime Directive #10.Fact (located by inspection on
origin/main@3172831, not executed)applyTenantScopesays of itself, verbatim:It is called at 13 sites in
packages/drivers/driver-sql/src/sql-driver.ts—findRows(the doorfind()uses), the readbacks,count,distinct, the write paths. It is not called by two:findWithWindowFunctions(object, query, options)— the live window-function read door ([P2] data:QueryASTdeclares 12 members no executor runs — the liveness ledger governs metadata types, not the request surface #4286). Builds viagetBuilder(object, options), applieswhere/ window functions /orderBy/ pagination, and returns rows. NoapplyTenantScope.analyzeQuery(object, query, options)(andexplain(), which forwards to it). Same omission.So the declared invariant is false as written: two doors bypass the chokepoint.
Why the two are not equally severe
findWithWindowFunctionsreturns ROWS. On a deployment whereapplyTenantScopewould have added a predicate — i.e.options.tenantIdis set and the object has a tenant field — this door returns rows from every tenant. That is cross-tenant read exposure at the driver layer, the same class as driver-mongodb 完全没有行级租户隔离:读不加谓词、写不打戳,多租户下跨租户可读写 #3724, not a cosmetic inconsistency.analyzeQueryreturns a PLAN, not rows. Lower severity, but it is the same defect this method was just fixed for in [drivers]InMemoryDriver.findapplieslimiton truthiness, solimit: 0returns every row — the #6485 defect one layer below the client #6577: a plan is only worth reading if it explains the statementfind()would actually run, and a missing tenant predicate makes it a plan for a different query (different selectivity, different index choice).What is NOT claimed here
Not measured end-to-end. I did not build a multi-tenant fixture and read another tenant's rows through this door — this is located by reading the call sites and the method's own contract, and it is stated at that strength deliberately. What reaches
findWithWindowFunctionsin practice needs checking before grading severity: it is not onIDataDriver(it is callable only on a SQL driver instance, per its own docstring), so the exposure depends on who calls it and whether they passoptions.tenantId. A grader should measure that reachability first — it is the difference between a latent trap and a live P0.Also note the layers above: ADR-0021 RLS and the Layer 0 authorization wall may or may not already constrain the callers of this door. "The layer above catches it" is a reason to grade lower, not a reason for the driver's own stated chokepoint to have a hole.
Suggested shape of a fix (not prescriptive)
Add
this.applyTenantScope(builder, object, options)to both, beside thegetBuildercall, as every other door does. The interesting question is not the two lines but the missing enforcement: nothing makes a new read door route through the chokepoint, which is exactly how these two got out. A gate that asserts every method taking(object, …, options)and returning rows callsapplyTenantScopewould be the durable version — thecheck:-script shape this repo already uses for the same class of claim.Dedup
Searched
applyTenantScope,findWithWindowFunctions, and tenant-scope/read-door phrasings across the repo's issues. Nearest neighbours are all closed and different: #3724 (driver-mongodb has no row-level isolation at all), #3249 / #2754 (tenant scope hiding NULL-org platform rows — the opposite direction), #4286 (window-function door liveness, not its tenancy). No open card covers this.Filed unassigned,
findingposture.