fix(service-analytics): guard fetchRecordLabels with assertReadScopeCannotVacate — the fourth read-scope door (#14329)

[claude]

Fixes #14329

AnalyticsServicePlugin's fetchRecordLabels hook was a fourth consumer of the same readScopeProvider output the three unified faces guard, and it met neither compileScopedFilterToSql nor assertReadScopeCannotVacate. 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 the ef8a4b9e named at dispatch):

• packages/services/service-analytics/src/plugin.ts:517-552 fetchRecordLabels — :533 builds idFilter, :534 is const filter = scope ? { $and: [idFilter, scope] } : idFilter;, and :538 hands that straight to await executeAggregate(targetObject, { groupBy: ['id', displayField], ..., filter, context }).
• grep -n 'assertReadScopeCannotVacate|read-scope-sql' plugin.ts exited 1 on the pre-fix tree: the file neither imported nor called the guard, and reaches the compiler on no path.
• The route is AnalyticsService.queryDataset → resolveScope (analytics-service.ts:1106-1108) → dimension-labels.ts:162 / :345 → this hook, which is why the StrategyContext.getReadScope inventory 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 touches StrategyContext.

The three sibling call sites — this is the fourth of the same shape

#	Site	Landed by
1	strategies/objectql-strategy.ts:628 — withReadScope, the ObjectQL engine merge	#13640
2	strategies/objectql-strategy.ts:550 — the /analytics/sql echo merge	#13926 / PR #14322
3	strategies/native-sql-strategy.ts:632 — applyReadScope	#13926 / PR #14322
4	plugin.ts:534 — fetchRecordLabels, this PR	#14329

A fifth in-package call site, objectql-strategy.ts:1111 in resolveFkAttr, is this hook's structural twin — same $and of an id filter with the referenced object's scope, same executeAggregate, 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:

+import { assertReadScopeCannotVacate } from './read-scope-sql.js';
+        if (scope) assertReadScopeCannotVacate(scope, targetObject);

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:

• sort-key pass (order on a lookup dimension, Dataset order sorts a select/lookup dimension by its stored value, not the label the user reads #3680) runs inside DatasetExecutor.execute, whose catch in queryDataset re-throws a declared ADR-0112 envelope untouched (hasDeclaredErrorEnvelope, analytics-service.ts:1158). The refusal reaches the caller as itself: READ_SCOPE_COMPILE_FAILED / 500.
• display pass (analytics: 让 executeAggregate 桥携带 ExecutionContext(#3597 的纵深防御第二层)+ 两处残留无 scope 调用 #3602) has its own catch (analytics-service.ts:1327) that degrades to a warn and 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.ts skips 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 toThrow would not distinguish that from a read that happened and then threw, so neither pin uses one — every refusal case asserts code and status per ADR-0112, plus the message, plus that executeAggregate was 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 one plugin.ts ships, not a stub standing in for it.

• an ordinary referenced-object scope still narrows the label lookup — asserted as the narrowed result set, not as "no throw": the engine receives { $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.
• the $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.
• the live fix(plugin-security): make the RLS emptied-membership deny guard polarity-aware #13570 RLS composite keeps own rows flowing — { $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 to 1 = 0 one arm from $not, and $nin: [] folds to 1 = 1 (constant TRUE) #13571's verdict rejected.
• no scope at all still reads the target, unchanged — the undefined arm. 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) is read-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 runs src/plugin.ts directly and no dist/ stands between the mutation and the measurement (the ablation script prints the suite's import specifiers to establish this rather than assuming it).

MARKER_COUNT before=1 after=0          # the anchor really matched
POST_MUTATION_HASH=36d2259c...  != HEAD_BLOB=60da818b...
git diff --stat -> 1 file changed, 1 deletion(-)

ABLATED_VITEST_EXIT=1
Tests  10 failed | 4 passed (14)       # predicted direction: refusals RED, controls GREEN

RESTORED_HASH=60da818b...  == HEAD_BLOB   RESTORE_MARKER_COUNT=1
git diff HEAD -> (empty)   git status --porcelain -> (empty)

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:

sort-key pass:  AssertionError: expected undefined to be 'READ_SCOPE_COMPILE_FAILED'
display pass:   AssertionError: expected [ { object: 'crm_account', ... } ] to deeply equal []

That second one is the leak itself: without the guard, the referenced object was read under a vacating scope.

Verification

Run	Result
pnpm --filter @objectstack/service-analytics test	Test Files 88 passed (88) · Tests 1885 passed (1885)
pnpm --filter @objectstack/service-analytics typecheck	clean — and tsc --noEmit --listFiles confirms the new test file is in the program, so this is not a green over source nothing read
pnpm lint (eslint . --no-inline-config, whole repo)	exit 0 — the full repo scan, not a narrowing
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands	36 derived + check:nul-bytes; 34 green, 3 NOT MEASURED

The 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 saved turbo run test log that CI tees and no local run produces), check:dual-build-cjs-loads and check:type-check-debt (both need a full workspace pnpm build closure 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 two export matches 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 new error-level site through a published sink shape either — the guard throws, and the one warn involved is pre-existing.

Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-analytics, touching 1 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

• content/docs/plugins/packages.mdx (via AnalyticsServicePlugin (symbol, a top-level class))

What this run could not see

• 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 5c9e40ad91028b57b0748e3ea0347189bac72ce9 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 40f01eadd61b56f7bf6fce75ab0c57ec36ac2252 — the merge of head aba91e6b02ed33da110bad7782eb5083e55c9412 into base 5c9e40ad91028b57b0748e3ea0347189bac72ce9, 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 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

⚠️ 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 5c9e40ad91028b57b0748e3ea0347189bac72ce9 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

[os-sales]

Landing provenance — ready + auto-merge at head aba91e6b

• Card: [finding] analytics fetchRecordLabels consumes the referenced object's read scope with neither compileScopedFilterToSql nor assertReadScopeCannotVacate in front — a fourth read-scope door outside the three faces #14322 unified #14329 (Fixes, closes on merge).
• ACCEPT: 14329#issuecomment-5504334261 (seat session_01AUF1NoViznQK32gqpK8wS8, os-sales), verified from the tree: 3 files, grep export on the diff empty, no packages/spec or content/docs/releases/ path, the guard hunk is one import plus if (scope) assertReadScopeCannotVacate(scope, targetObject); placed after the early returns and before the chunk loop.
• Clause-②: no, verified by the seat — an invariant restored on a consumer, no accept-set or public-surface change; no carrier was ever hung.
• CI on aba91e6b: every check green — Lint & Repo Gates success at 04:40:56Z, TypeScript Type Check success, Test Core (1/6) success at 04:35:10Z; mergeable_state: clean.
• Action: draft → ready and auto-merge enabled at ~09:24Z (delayed from 04:41Z by the account usage limit). On MERGED: strip pm:dispatched from the auto-closed [finding] analytics fetchRecordLabels consumes the referenced object's read scope with neither compileScopedFilterToSql nor assertReadScopeCannotVacate in front — a fourth read-scope door outside the three faces #14322 unified #14329.

---

Generated by Claude Code