feat(analytics,spec): executeAggregate 桥携带 ExecutionContext —— ADR-0021 D-C 第二层带子 (#3602)#3651
Merged
os-zhuang merged 4 commits intoJul 27, 2026
Merged
Conversation
…t — ADR-0021 D-C second belt (#3602) The analytics→engine bridge now forwards the request's ExecutionContext to `engine.aggregate`, so the engine's own middleware chain scopes analytics reads independently of the analytics layer's `getReadScope`. `BaseEngineOptions.context` has always been `.optional()`, so nothing forced the bridge to pass it — and it did not. An authenticated aggregate reached the engine with no principal, plugin-security's principal-less fall-open skipped its RLS injection, and the only thing left scoping the query was the strategy remembering to call `getReadScope`. #3597 was a strategy that did not, and both belts were off at once. `getReadScope` stays: the two resolve scope through different paths (engine middleware vs `security.getReadFilter`), and a deployment without plugin-security has only the analytics layer. Depth, not a replacement. - `StrategyContext` gains `context?: ExecutionContext`, bound per call by `AnalyticsService.callCtx` — unconditionally, including when no read-scope provider is configured, since that deployment needs the engine belt most. - `StrategyContext.executeAggregate` and the plugin/service `executeAggregate` config options gain `context?: ExecutionContext`. Additive: a custom bridge that ignores it behaves exactly as before. - `fetchRecordLabels` — the dimension display-label lookup — is row-granular (one row per record, real display names) and ran with neither read scope nor context. Its ids come from a scoped aggregate today, so it leaked nothing, but that was the caller's invariant, not the bridge's. Now ANDs the target object's read scope in (`$and`, never a key merge) and forwards the context. - `ObjectQLStrategy.generateSql` emitted no WHERE at all, so `/analytics/sql` read as an unscoped table scan while the real aggregate was scoped. Now renders the caller's filters and the read scope, and runs the same joined-scope guard `execute()` does. Never executed, so this was misleading output rather than a leak. - `BootOptions.analytics` lets a gate boot with the analytics belt off; the new dogfood case asserts the engine belt alone still scopes a member to their own rows. Verified to bite: reverting the context forwarding makes it count 5 instead of 2. Deliberately not fixed here: `ObjectQLStrategy.execute()` ignores `timeDimensions[].dateRange` entirely, so `generateSql` does not render a BETWEEN either. Filed as #3650. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013st2KArjmLQSuVzpDS91jj
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
documentation
Contributor
📓 Docs Drift CheckThis PR changes 4 package(s): 108 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
…ate-execution-context-cukwsp # Conflicts: # packages/services/service-analytics/src/analytics-service.ts # packages/services/service-analytics/src/dimension-labels.ts # packages/services/service-analytics/src/plugin.ts
os-zhuang
marked this pull request as ready for review
July 27, 2026 13:30
#3652 rewrote `ObjectQLStrategy.generateSql` (date_trunc for bucketed dimensions, COUNT DISTINCT, ORDER BY/LIMIT/OFFSET) and made `execute()` echo the rendered SQL on the result. Kept all of that. Resolved the WHERE clause in favour of this branch's version, which renders the same caller filters plus the two things #3652's inline loop does not: - the READ SCOPE (`compileScopedFilterToSql`), the point of #3602 residual 2 — without it the echoed string still reads as an unscoped table scan while the aggregate is scoped; - `assertJoinedScopesEnforceable`, so it never renders SQL for a query `execute()` would reject. Its filter rendering is also closer to what runs: `contains` binds `%v%` (a bare comparand under LIKE would only match exactly), and values go through `coerceFilterValueForObjectQL`, so the comparand shown is the one this path hands the engine. Scope VALUES stay in `params`, which the `execute()` echo discards — only the predicate shape reaches the browser, same as NativeSQL. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013st2KArjmLQSuVzpDS91jj
#3664 replaced `assertJoinedScopesEnforceable` with `assertNoCrossObjectReferences`, which rejects any cross-object reference regardless of read scope. That is strictly better than what this branch had: `engine.aggregate()` has no join and the SQL driver's aggregate emits none, so a dotted member was never merely "unscopeable" — it was silently wrong (one `(null)` bucket) or a hard error. Adopted as-is; this branch's narrower scope-conditional guard is gone. Made it derive its field set from `referencedFieldNames(cube, query)` instead of `execute()`'s built `groupBy`/`filter`, so `generateSql()` runs the IDENTICAL guard — the rendered SQL must not describe a query `execute()` would reject. Same field set either way, so `execute()`'s behaviour is unchanged. Also kept from this branch: the read scope rendered into `generateSql`'s WHERE (#3664 left rendering alone), and the `context` forwarded to the aggregate bridge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013st2KArjmLQSuVzpDS91jj
os-zhuang
added a commit
that referenced
this pull request
Jul 27, 2026
…#3654) (#3691) The ObjectQL fallback path (date-granularity bucketing, in-memory driver, federated) could not join, so cross-object grouping like `revenue by account.region` was rejected outright (#3664 stopgap). It now serves the common case by FK-expand: 1. group the base aggregate on the lookup FK column (`account`) — which the engine CAN do — scoped to the base object (and, per #3651's second belt, threading the ExecutionContext to the engine too); 2. resolve each FK id to the related attribute (`region`) with a read of the referenced object scoped to THAT object's own RLS; 3. re-bucket by the resolved attribute in memory, recombining measures (sum/count add, min/max take the extremum). A base row whose referenced record the caller cannot read buckets under an explicit `(restricted)` group: the measure still counts (grand totals preserved) but the hidden attribute never appears — no leak (ADR-0021 D-C / #3602). `/analytics/sql` renders the equivalent LEFT JOIN, so preview and execution accept/reject the same set. Bounded — still rejected LOUD (never silently wrong): cross-object in a MEASURE or FILTER, multi-hop dims, and non-recombinable measures (avg/count_distinct) with a cross-object dim. NativeSQLStrategy is unchanged. The pure re-bucketing step is isolated in cross-object-rebucket.ts and exhaustively unit-tested (recombination, restricted-total conservation, null-vs-restricted, multi-dim). Integration tests drive the real AnalyticsService with a two-call aggregate stub, asserting both the base and FK-resolution are scoped and the restricted bucket forms; reverting the strategy turns all three red. Re-applied cleanly on top of #3651/#3652 (executeAggregate carries context; generateSql renders scope) after those landed on main. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3602.
analytics→engine 的桥现在把请求的
ExecutionContext转发给engine.aggregate,于是 engine 自己的中间件链会独立于 analytics 层的getReadScope给聚合读加 scope。为什么
BaseEngineOptions.context一直是.optional(),所以类型系统从没强迫这座桥传它 —— 它也确实没传。一次已认证的聚合到达 engine 时不带任何 principal,plugin-security 的无 principal fall-open 跳过了自己的 RLS 注入,于是唯一还在起作用的就只剩"strategy 记得调getReadScope"这一条。#3597 就是某个 strategy 没调,两条带子同时失效。getReadScope保留:两者走的是不同的解析路径(engine 中间件 vssecurity.getReadFilter),而且没装 plugin-security 的部署只有 analytics 这一层。这是纵深,不是替代。改了什么
主项 —— context 贯通
StrategyContext新增context?: ExecutionContext,由AnalyticsService.callCtx逐请求绑定。无条件绑定,包括没有配置 read-scope provider 的情况 —— 恰恰是那种部署最需要 engine 帮它兜底,不能让第二条带子依赖第一条是否接上。StrategyContext.executeAggregate以及 plugin / service 两处executeAggregate配置项新增context?: ExecutionContext;auto-bridge 转发给engine.aggregate。纯增量 —— 自定义桥忽略它的话行为与之前完全一致。残留 1 ——
fetchRecordLabels的第二条带子#3639 已给它接上 analytics 层的带子(引用对象自己的 read scope)。本 PR 在
fetchRecordLabels/resolveDimensionLabels上再加一个尾部context参数并转发进executeAggregate,让 engine 也独立 scope 这次行级读。残留 2 ——
ObjectQLStrategy.generateSql之前完全不生成 WHERE,于是
/analytics/sql的预览读起来像一次无 scope 全表扫描,而实际执行(#3601 之后)是加了 scope 的 —— 排查"这行为什么不见了"的人拿到的是一段无法复现结果的 SQL。现在渲染调用方的 filter + read scope,并跑与execute()相同的联表 scope 守卫(不该渲染一条execute()会拒绝的查询)。这段 SQL 从不执行,所以此前是误导性输出,不是泄露。验证
execution-context-bridge.test.ts(14 例):context 转发、无 read-scope provider 时仍转发、同实例多租户各自拿到自己的 context、auto-bridge 落到engine.aggregate、label 查询上两条带子落在同一次 engine 调用(fix(analytics): scope the dimension-label lookup to the referenced object's RLS (#3602 residual 1) #3639 与本 PR 各自的单测都覆盖不到的交叉点)、generateSql的 WHERE / 参数 / 联表拒绝。BootOptions.analytics让 harness 可以把 analytics 那条带子显式关掉(getReadScope: () => undefined)启动,断言 member 仍然只数到自己的行 —— 此时唯一在起作用的就是桥递给 engine 的 context。确认它真的会咬:把
context: ctx.context临时改回undefined,该用例数到 5(全表)而非 2,正是 security(analytics): ObjectQLStrategy 不消费 getReadScope — NativeSQL 回落后聚合查询无 RLS/租户谓词(#2852 修复未覆盖的另一半) #3597 的泄露形状。service-analytics198 例全绿(含 fix(analytics): scope the dimension-label lookup to the referenced object's RLS (#3602 residual 1) #3639 新增用例)、两个 analytics dogfood 门禁 8 例全绿、pnpm build71/71、pnpm lint干净、check:api-surface通过。顺带发现,未在此修
ObjectQLStrategy.execute()完全忽略timeDimensions[].dateRange(只有where会到 engine),而 NativeSQL 和 preview-evaluator 都处理了。因为带dateGranularity的查询会强制 NativeSQL decline,所以任意驱动上的时间分桶趋势图都会画出全量历史。属于正确性 bug,与本 issue 正交 —— 已按 Prime Directive #10 单独开 #3650。generateSql里也因此故意不渲染 BETWEEN(渲染出来就是编造一条执行路径从不施加的谓词),代码注释里引了该 issue。🤖 Generated with Claude Code
https://claude.ai/code/session_013st2KArjmLQSuVzpDS91jj