Uh oh!
There was an error while loading. Please reload this page.
fix(service-analytics): guard fetchRecordLabels with assertReadScopeCannotVacate — the fourth read-scope door (#14329) - #14400
Conversation
…annotVacate The record-label hook is a fourth consumer of the same readScopeProvider output the three unified faces guard, and it met neither compileScopedFilterToSql nor the vacancy guard: it $ands the referenced object's scope with `id $in [...]` and hands that straight to executeAggregate. A vacating spelling from an out-of-repo getReadScope producer therefore let a row-granular per-record read run effectively unscoped, surfacing exactly the display names the referenced object's RLS exists to hide. Call the already-exported assertReadScopeCannotVacate before the filter composition, in the same envelope as the siblings (READ_SCOPE_COMPILE_FAILED / 500). Placement mirrors ObjectQLStrategy.resolveFkAttr, this hook's structural twin: after the early returns (a call that reads nothing cannot widen anything) and before the chunk loop (one scope, one verdict). Zero compiler change. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
📓 Docs Drift CheckThis PR changes 1 package(s): 1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
What this run could not see
Coarse fallback — 8 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 40f01eadd61b56f7bf6fce75ab0c57ec36ac2252 && git checkout 40f01eadd61b56f7bf6fce75ab0c57ec36ac2252
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 5c9e40ad91028b57b0748e3ea0347189bac72ce9 aba91e6b02ed33da110bad7782eb5083e55c9412 && git checkout -B drift-repro 5c9e40ad91028b57b0748e3ea0347189bac72ce9 && git merge --no-ff aba91e6b02ed33da110bad7782eb5083e55c9412
node scripts/docs-audit/affected-docs.mjs --json 5c9e40ad91028b57b0748e3ea0347189bac72ce9
|
os-sales
commented
Sep 2, 2026
Landing provenance — ready + auto-merge at head |
Uh oh!
There was an error while loading. Please reload this page.
Fixes#14329
AnalyticsServicePlugin'sfetchRecordLabelshook was a fourth consumer of the samereadScopeProvideroutput the three unified faces guard, and it met neithercompileScopedFilterToSqlnorassertReadScopeCannotVacate. It now calls the already-exported guard on the referenced object's scope before composing the filter. Zero compiler change — the #13571 lowering residue is ruled and untouched.All numbers below were measured on head
aba91e6b0, the branch's final commit.Premise re-measured on the merged tree — it still holds
Triage's binding first step was to re-measure rather than trust the source reading in the card, since PR #14322 landed in between. Measured on
origin/main@1dcb995f2(which contains #14322 and #14354, i.e. newer than theef8a4b9enamed at dispatch):packages/services/service-analytics/src/plugin.ts:517-552fetchRecordLabels—:533buildsidFilter,:534isconst filter = scope ? { $and: [idFilter, scope] } : idFilter;, and:538hands that straight toawait executeAggregate(targetObject, { groupBy: ['id', displayField], ..., filter, context }).grep -n 'assertReadScopeCannotVacate|read-scope-sql' plugin.tsexited 1 on the pre-fix tree: the file neither imported nor called the guard, and reaches the compiler on no path.AnalyticsService.queryDataset→resolveScope(analytics-service.ts:1106-1108) →dimension-labels.ts:162/:345→ this hook, which is why theStrategyContext.getReadScopeinventory in fix(service-analytics): one verdict for a vacating read scope on all three analytics faces (echo + native SQL) #14322's body does not cover it: this path never touchesStrategyContext.The three sibling call sites — this is the fourth of the same shape
strategies/objectql-strategy.ts:628—withReadScope, the ObjectQL engine mergestrategies/objectql-strategy.ts:550— the/analytics/sqlecho mergestrategies/native-sql-strategy.ts:632—applyReadScopeplugin.ts:534—fetchRecordLabels, this PRA fifth in-package call site,
objectql-strategy.ts:1111inresolveFkAttr, is this hook's structural twin — same$andof an id filter with the referenced object's scope, sameexecuteAggregate, guarded since #13640 — and it is what fixes both the argument shape and the placement here: after the early returns (a call that reads nothing cannot widen anything, so refusing it would be pure over-denial) and before the chunk loop (one scope gets one verdict, not one per 500 ids). The guard's condition is spelled to match the composition on the next line exactly, so the set of scopes guarded and the set of scopes composed are provably the same set.The whole non-comment source diff is two lines:
Two label passes, two dispositions — both fail closed, both pinned
A refusal from this hook surfaces differently depending on which of
queryDataset's two label passes raised it. Both are pinned, because a reader who checks only one concludes the other is unguarded:orderon a lookup dimension, Datasetordersorts a select/lookup dimension by its stored value, not the label the user reads #3680) runs insideDatasetExecutor.execute, whose catch inqueryDatasetre-throws a declared ADR-0112 envelope untouched (hasDeclaredErrorEnvelope,analytics-service.ts:1158). The refusal reaches the caller as itself:READ_SCOPE_COMPILE_FAILED/ 500.analytics-service.ts:1327) that degrades to awarnand leaves raw ids rendering. That is not this PR weakening the envelope — it is the disposition analytics: 让 executeAggregate 桥携带 ExecutionContext(#3597 的纵深防御第二层)+ 两处残留无 scope 调用 #3602 already chose for this surface one frame up (dimension-labels.tsskips a dimension's labels rather than fetch unscoped when the scope cannot be resolved), and it is fail-closed: no name is fetched, so none can leak.The security property is therefore identical on both passes and is asserted as such: the referenced object is never read at all. A bare
toThrowwould not distinguish that from a read that happened and then threw, so neither pin uses one — every refusal case assertscodeandstatusper ADR-0112, plus the message, plus thatexecuteAggregatewas never called for the referenced object.Over-denial controls — the deliverable, not an optional extra
packages/services/service-analytics/src/__tests__/record-label-read-scope-vacancy.test.ts(new, 14 cases) drives the real plugin wiring —new AnalyticsServicePlugin(...).init(ctx)— so the closure under test is the oneplugin.tsships, not a stub standing in for it.{ $and: [{ id: { $in: ['acc1','acc2'] } }, { organization_id: 'org_A' }] }(the preservation half), and the rows come back['Acme Corp', 'acc2']— the in-tenant record renders its name, the out-of-tenant one keeps its raw id.$in: []zero-rows reduction still yields no labels and no refusal — the ruled 空组合子在同仓有两个对立答案:五个后端归约成布尔单位元,service-analytics 的两个编译器 fail-closed 抛错 —— #5239 的一致性表四条因此进不了表 #5322/fix(driver-sql): 空$and/$or/$not按布尔单位元编译,$or: []不再返回全表 (#5134) #5243 reduction at positive polarity passes the guard, reaches the engine, returns zero rows, and no label overwrites a raw id.{ $or: [{ owner: { $in: [] } }, { owner: 'u_me' }] }is not refused. Refusing it would 500 every analytics query for a user whose membership set resolves empty, the outcome read-scope-sql's emptied-membership folds are polarity-dependent at the lowering site itself:$in: []folds to1 = 0one arm from$not, and$nin: []folds to1 = 1(constant TRUE) #13571's verdict rejected.undefinedarm. Without this case a guard that refused everything would satisfy every refusal assertion above.What the suite deliberately does not re-derive is the engine's lowering of a vacating scope; that measured table (real
SqliteWasmDriver) isread-scope-vacancy-three-faces.test.ts's, and a second copy of one ruling is how two answers drift apart. Its fixture evaluator throws on any operator it was not written for, so an unjudgeable spelling fails loudly instead of manufacturing a comfortable answer.Reverse verification — run on the committed tree
Guard call deleted from
plugin.ts, mutation proven on disk before measuring, restored and the restore proven. The suite imports../plugin.js— a relative, same-package specifier — so vitest runssrc/plugin.tsdirectly and nodist/stands between the mutation and the measurement (the ablation script prints the suite's import specifiers to establish this rather than assuming it).The 10 reds are exactly the 10 refusal cases; the 4 greens are exactly the four over-denial controls, which is what makes them controls. The two failure texts name the defect directly:
That second one is the leak itself: without the guard, the referenced object was read under a vacating scope.
Verification
pnpm --filter @objectstack/service-analytics testTest Files 88 passed (88)·Tests 1885 passed (1885)pnpm --filter @objectstack/service-analytics typechecktsc --noEmit --listFilesconfirms the new test file is in the program, so this is not a green over source nothing readpnpm lint(eslint . --no-inline-config, whole repo)node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commandscheck:nul-bytes; 34 green, 3 NOT MEASUREDThe three NOT MEASURED are each the gate's own
exit 3"PREREQUISITE NOT MET — nothing was measured" branch, which every one of them states in its own text is neither a pass nor a finding:check-test-completeness(grades a savedturbo run testlog that CI tees and no local run produces),check:dual-build-cjs-loadsandcheck:type-check-debt(both need a full workspacepnpm buildclosure on disk). CI supplies all three. Every exit code above was captured before any pipe, and each verdict is quoted from the gate's own verdict line.Clause-② re-declared from the diff, not inherited:
git diff -U0 origin/main...HEAD | grep -E '^\+.*\bexport\b'returns nothing — the only twoexportmatches in the raw diff are the word "already-exported" in the changeset prose and a hunk header naming the enclosing class. No, as claimed and as triaged: this restores an invariant on a consumer and moves no published accept set. No newerror-level site through a published sink shape either — the guard throws, and the onewarninvolved is pre-existing.Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
Generated by Claude Code