Uh oh!
There was an error while loading. Please reload this page.
fix(analytics): fail closed on cross-object aggregation the ObjectQL path cannot join (#3654) - #3664
Merged
Merged
Conversation
…path cannot join (#3654) engine.aggregate() has no join — it never expands a lookup and the SQL driver's aggregate emits no JOIN. A dotted dimension/measure like account.region reaching ObjectQLStrategy (the fallback NativeSQL declines on: date-granularity bucketing, in-memory driver, federated objects) failed SILENTLY: the in-memory path bucketed every row under one (null) group and summed the whole table into it — a plausible number that is actually a mislabelled full-table total; the native path errored on the unresolved column. ObjectQLStrategy now rejects any cross-object reference outright, with a clear message, before the query reaches the engine. This generalizes the #3597 guard (assertJoinedScopesEnforceable), which only rejected when the joined object carried a read scope AND early-returned when no read-scope provider was configured — so the silent (null) bucket still shipped on unsecured / in-memory setups. The new guard is unconditional and subsumes #3597: a rejected query never loads the joined object, so nothing is left unscoped. Cross-object datasets are unaffected on NativeSQLStrategy, which hand-compiles the LEFT JOINs (and scopes each). This only changes the fallback path — a silent wrong answer becomes a loud, actionable error. Full lookup-traversal in the aggregate path is left as follow-up (#3654). Tests: three cross-object cases (joined-object scope / base-only scope / NO provider — the #3654 silent-(null) case) now reject; reverting the guard turns all three red. The engine-level in-memory repro is in #3654. Co-Authored-By: Claude <noreply@anthropic.com>
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 6 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
Uh oh!
There was an error while loading. Please reload this page.
os-zhuang pushed a commit
that referenced
this pull request
Jul 27, 2026
#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
xuyushun441-sys pushed a commit
that referenced
this pull request
Jul 27, 2026
…#3654) 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 (#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; 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. 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 (the normal SQL path) 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 the FK-resolution are scoped and the restricted bucket forms; reverting the strategy turns all three red. Co-Authored-By: Claude <noreply@anthropic.com>
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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#3654(止血部分)。把静默出错的跨对象聚合从"塌成
(null)桶"变成响亮拒绝。问题(#3654)
engine.aggregate()没有 join —— 不展开 lookup,SQL driver 的 aggregate 也不发JOIN。于是点号维度/度量(如account.region)到了ObjectQLStrategy(NativeSQL 声明失败时的回退:日期分桶 / in-memory driver / federated)就静默出错:row['account.region']平铺取值恒 undefined → 所有行塌进一个(null)桶,度量被全表加总 —— 数字看着有值,其实是错标成(null)的全表汇总;已用
applyInMemoryAggregation纯函数在 #3654 里复现坐实。修法(fail-closed 止血)
ObjectQLStrategy在查询到达引擎之前,无条件拒绝任何跨对象引用,给出清晰错误。这泛化并涵盖了 #3601 的 guard(
assertJoinedScopesEnforceable):旧 guard 只在被联对象带 read scope 时拒绝,且无 read-scope provider 时直接早返回 —— 所以无安全 / in-memory 环境下静默(null)照旧发生。新 guard 无条件,且天然涵盖 #3597 的安全关切:被拒绝的查询从不加载联表 → 没有未加 scope 的联表数据可泄露。检测基于解析后的字段名(
resolveFieldName之后),所以被 cube 拍平成真实列的点号维度不误伤,只拦真正未解析的关系遍历。跨对象数据集在 NativeSQLStrategy 上不受影响(它手编 LEFT JOIN 且逐 join 加 scope)。本 PR 只改回退路径:静默错答 → 响亮报错。aggregate 路径的完整 lookup 遍历能力留作后续(#3654 方向 1)。
测试
三个跨对象用例现在都拒绝(经真实
AnalyticsService.query()→ 策略选择 → guard;executeAggregate stub 故意"不拦就返回垃圾(null)桶",所以通过=证明拒绝):(null)的正核心。回退 guard → 三个全红。service-analytics 185 passed;
tsc无新增错误(2 存量在未触文件)。路由(granularity 让 NativeSQL 退场 → ObjectQL)已被现有analytics-rls.dogfood.test.ts的 ObjectQL 用例覆盖。🤖 Generated with Claude Code