Repository navigation
Commit a2e7556
fix(service-analytics)!: a dataset dimension spelled through a relationship path the dataset does not declare is refused INVALID_FIELD on the native, ObjectQL and draft-preview faces (#28553)
domain:server#1 · os-project-manager · session_01R1q2xdtYgU6FKrZ7LgjPCC
<!-- claude-projects-attribution: {"github_login":"os-project-manager"}
-->
_Requested by **Leehom** · [project
thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1XSpYuanGsc8M8sJU9TUdUDH)_
Fixes #28258
Clause-②: no (narrowing) — a dimension named through a relationship path
the dataset's `include` does not declare is now refused `INVALID_FIELD`
/ 400: in a `queryDataset` selection on all three faces, and in an
`/analytics/query` or `/analytics/sql` request over a registered dataset
cube, where the ObjectQL strategy also served it before (the gate sits
in the call context those doors share). The draft preview
(`previewDrafts: true`) served it as one `null` group, and the ObjectQL
strategy served a real lookup (`account.name`) by joining it anyway;
both now refuse. The native strategy refused it `DATASET_INVALID` / 400
and now answers `INVALID_FIELD` / 400 for this case only. Nothing is
widened: no export, answer member, error code or status is added; both
codes were already on the wire.
Metadata-protocol: none
Skills-docs: none — `content/docs/data-modeling/analytics.mdx` already
says "Only declared paths are joinable", which this diff makes true on
every face; no file under `skills/**` or `content/docs` names the code a
selection-side refusal answers.
**BREAKING** accept-set narrowing on `queryDataset`
(`@objectstack/service-analytics`), and a code change on one native
refusal.
Card thread: `session_01FEkQ5pG4c8xoSCyPjcztJc`.
## 维护者速读
- **改了什么**:数据集查询里,维度若经由数据集 `include` 没声明的关系路径书写(如 `对象名.bogus`、或没 include
`account` 时写 `account.name`),三条执行面现在都在服务层同一道闸门按 400 `INVALID_FIELD`
拒,消息逐字相同,点名该维度、第一个没声明的路径、已声明路径和可用维度。原生 SQL 面这一种情形从 `DATASET_INVALID` 改为
`INVALID_FIELD`;草稿预览不再给一个 `null` 分组;ObjectQL 面不再绕过 include 去 join。
- **为什么改**:卡 #28258(分诊 6532297703,席位裁定
`INVALID_FIELD`):同一个写错的维度三面三种答复,草稿里还显示一个看似合理的总数。
- **风险与回滚**:只影响「选择里经未声明关系路径的维度」;已声明路径(`include: ['account']` 下的
`account.name`)、解析到已声明维度的点号写法(`对象名.region`)照旧服务;`runtimeFilter`
经未声明路径、手写 cube 的配置白名单仍走原生面的 `DATASET_INVALID`;读不了的关联对象仍先给
403。仓内唯一读到这个错误码的消费方(`@object-ui/data-objectstack` 适配器)对任何具名错误码一视同仁。回滚即
revert。
- **先看**:`packages/services/service-analytics/src/analytics-service.ts`
的新方法 `assertDimensionJoinsBacked` 及其两处调用(`callCtx`
里字段读闸门之后、`answerDataset` 预览的闸门列表末尾);`dataset-compiler.ts` 的
`unbackedRelationshipPath`。
- **两分钟验证**:`pnpm --filter @objectstack/service-analytics exec vitest
run src/__tests__/dotted-dimension-unbacked-join-parity.test.ts`,预期 9
passed。
- **需要你做的**:无,席位落地(收窄需先过合约评审)。
## What changes
**One gate, three faces** (`analytics-service.ts`, new private
`assertDimensionJoinsBacked`). For each `dimensions` entry that resolves
to no declared dimension (`declaredMemberEntry`, the strategies' own
`lookupMember` mirror) and is a dotted identifier path, it walks the
path hop by hop with `unbackedRelationshipPath` (new export of
`dataset-compiler.ts`, beside `joinAlias`; not re-exported from the
package entry) against the compiled dataset's `allowedRelationships`,
the set the native join allowlist reads. The first undeclared path is
refused through `invalidMemberError` (`dataset-refusal.ts`), the
member-level `INVALID_FIELD` / 400 constructor the other member refusals
use, carrying `member`, `param: 'dimensions'` and `cube`:
```
Dimension 'account.owner.name' on cube 'os28258_deals_by_account' traverses relationship path 'account.owner',
which the dataset does not declare in its `include`. Only fields along a declared relationship path are joinable
— declared paths: account. Valid dimensions: region.
```
It is asked in two places, one message, no copy:
1. `callCtx` (shared by `query()` and `generateSql()`, so both
strategies and the SQL echo), right after the object, stored-body,
internal-field and field-level verdicts and before the order-key door
and any strategy.
2. The draft preview's gate list in `answerDataset`, at the same point:
after `assertFieldsReadable`, before the evaluator.
**Why after the admission verdicts, not in `ensureCube`.** The first cut
asked it in `ensureCube` beside the dimension-field gate.
`packages/rest/src/analytics-relationship-path-admission.test.ts`
(#20933) then went red on both compositions: a dimension through an
undeclared path to an object the caller may not read must answer that
object's 403, as a declared one does. Asking the gate after `callCtx`'s
admission keeps that 403 (the same reason the order-key door sits
there).
**Scope kept to the ruling.**
- Only `dimensions`. A dotted `timeDimensions` entry already answers one
`INVALID_FIELD` on both live strategies (measured below), so it is left
alone.
- Only on a compiled dataset (`allowed` defined). A hand-authored cube's
configured `getAllowedRelationships` hook still reaches the native
strategy's `DATASET_INVALID`.
- The native strategy's allowlist (`native-sql-strategy.ts`) is
untouched; it still answers `DATASET_INVALID` for a `runtimeFilter`
member through an undeclared path, and for every other reason it is
raised today. `dataset-compiler.ts`'s own `assertDeclared` is untouched.
**One pin moved.**
`packages/rest/src/analytics-dataset-refusal-envelope.test.ts`'s row
"native-sql-strategy: a selection naming a join outside the allowlist"
used a dotted `dimensions` entry as its trigger. That is exactly this
card's case, now refused ahead of the strategy, so the row's trigger
becomes a `runtimeFilter` member (`{ 'account.region': 'emea' }`) that
still reaches the strategy allowlist: its audit trail (the producer's
`DATASET_INVALID` envelope, `listEntry: 'not backed by a declared
relationship'`) is kept, with the same message.
## Tests
New
`packages/services/service-analytics/src/__tests__/dotted-dimension-unbacked-join-parity.test.ts`:
real `ObjectQL` + `SqlDriver` (better-sqlite3, in memory) through
`AnalyticsServicePlugin.init`, the native face, the ObjectQL face and
the preview face (a pending seed draft), the #28201 harness.
- Refused three ways, `INVALID_FIELD` / 400, `param: 'dimensions'`,
`member`, `cube`, message first sentence, and the native, ObjectQL and
preview messages byte-equal: `os28258_deal.bogus` (the card's spelling),
`account.name` (a real lookup, no `include`), `account.owner.name` (only
`account` included, refused naming `account.owner`), and `account.name`
beside the declared `region`.
- Controls: backed `account.name` serves Acme 130 / Globex 50 on both
live strategies and is not refused on the preview; `os28258_deal.region`
(resolves to the declared `region`) serves amer 150 / emea 30 on both
live strategies and is not refused on the preview; undeclared plain
`bogus` keeps #28201's refusal ("groups by field 'bogus'") on every
face; `nope` beside `account.name` is answered about the measure on
every face; a `runtimeFilter` member `account.name` keeps the native
`DATASET_INVALID` / 400.
<details><summary>Red first on main eeec88d, and the per-face answers
measured there</summary>
The file run against the unchanged base: `Tests 4 failed | 5 passed
(9)`; the four refusal cases red ("expected a refusal, but the query
resolved": the preview served), the five controls green.
A probe on the same harness, same base, recorded each face:
| Selection (dataset) | native | ObjectQL | preview |
|:---|:---|:---|:---|
| `os28258_deal.bogus` (no include) | `DATASET_INVALID` / 400 "join
"os28258_deal" is not backed…" | `INVALID_FIELD` / 400 from the driver
("groupBy … names 'os28258_deal.bogus'") | served `[{
"os28258_deal.bogus": null, total: 180 }]` |
| `account.name` (no include) | `DATASET_INVALID` / 400 | **served**
Acme 130 / Globex 50 | served one `null` group, 180 |
| `account.name` (include account) | served | served | served one `null`
group, 180 |
| `os28258_deal.region` (no include) | served amer/emea | served
amer/emea | served one `null` group, 180 |
</details>
<details><summary>Green, ablation, suites</summary>
- Green: the new file 9/9; with the three preview-parity siblings
(`preview-undeclared-dimension-parity`,
`preview-filter-field-json-dimension-parity`,
`preview-undeclared-measure-parity`) 37/37.
- Ablation (`node scripts/ablation-replace.mjs --delete`, anchor the
preview's `this.assertDimensionJoinsBacked(q, cube,
compiled.allowedRelationships);`, i.e. the `null`-group behaviour put
back): anchor 1 → 0, blob `9b761f324397` → `5c8914397b7f`; `Tests 4
failed | 5 passed (9)`, the four refusal pins red (the preview
resolved), controls green; restore blob equals HEAD, `git diff HEAD`
empty. The suite imports the service from `src/`, so no build sits
between the mutation and the run.
- `pnpm --filter @objectstack/service-analytics exec vitest run`: Test
Files 201 passed (201), Tests 4823 passed | 282 skipped (5105).
- `pnpm --filter @objectstack/rest exec vitest run src/analytics` (after
building rest's dependency closure): Test Files 23 passed (23), Tests
298 passed | 22 skipped (320).
- `pnpm --filter @objectstack/service-analytics typecheck`: exit 0.
</details>
## Acceptance notes
- **Wire-change readers of `DATASET_INVALID` for this case** (measured
before changing it, per the order): `@object-ui/data-objectstack`'s
`queryDataset` adapter is the only runtime reader, and it sends every
named non-permission code to one generic "Dataset query failed: 400 —
detail" error (branch ⑤), so `INVALID_FIELD` lands identically; its two
tests use the code as fixture text only. No reader in `packages/client`,
the console, HotCRM (its one `DATASET_INVALID` mention is a currency
comment), `examples/`, `skills/**` or `content/docs` names this case.
In-repo test pins: the rest refusal-envelope row (moved, above);
service-analytics `dataset-refusal-envelope.test.ts` ⑤ and
`native-sql-rls.test.ts` call the strategy directly and are unchanged;
`dataset-rls-integration.test.ts` uses a declared dimension under a
configured hook and is unchanged. No real consumer, so both halves land.
- **ObjectQL face narrows too.** The card measured `OBJECT.bogus`; a
real lookup the dataset does not include (`account.name`) was served on
the ObjectQL strategy by joining outside the allowlist. The ruling's
contract covers it ("a dimension spelled through a join the dataset does
not back"), so it is refused there now and pinned; the changeset says
so. Because the gate sits in `callCtx`, the same narrowing reaches
`/analytics/query` and `/analytics/sql` over a registered dataset cube
(the ObjectQL strategy served there too); the Clause-② line and the
changeset name both doors (contract review, prose-only follow-up).
- **Preview vs a denied related object.** The preview reads no related
object (`cubeObjects` only), so it asks no admission for the path's
object; a path that is both undeclared and through an unreadable object
answers the field gate's verdict, then this gate's 400 on the preview,
where the live faces answer the object's 403. Not measured here; it
needs a security-wired preview harness.
- **Out of scope, measured on the same harness at `eeec88d14`, not fixed
here** (in the report for the seat):
- The preview joins nothing, so a BACKED dotted dimension
(`account.name` with `include: ['account']`) and a dotted spelling that
resolves to a declared dimension (`OBJECT.region`, `CUBE.region`) serve
one `null` group on the preview where both live faces serve real groups.
- A dotted `timeDimensions` entry (`account.created_at`, no include):
both live faces `INVALID_FIELD` / 400; the preview serves `[{ total: 180
}]`, dropping the time dimension.
- A `runtimeFilter` member through an undeclared path (`account.name`):
native `DATASET_INVALID`, ObjectQL `INVALID_FIELD` ("cannot evaluate a
cross-object filter"), preview `[]`.
- A declared path to a column the related object lacks (`account.bogus`,
include account): native `DATABASE_ERROR` / 500, ObjectQL
`INVALID_FIELD` / 400, preview one `null` group.
## Verification
Readings on `9b932230a` (code head: the union, the dogfood battery and
the full build ran on it). Head `6001927ad` adds one commit that only
respells two placeholders in the changeset prose (`OBJECT.bogus` for an
angle-bracket form); on it the changeset-reading families were re-run,
all exit 0: `check-adr-0087-registration --base origin/main`,
`check-changeset-no-major --base origin/main`, `check-empty-changeset
--base origin/main`, `check:changeset-gate-self-tests`,
`check:nul-bytes`, `check:issue-citations`,
`check-closing-keyword-parity`, `check-commit-card-trailers` (4
commits); the derived list on the new head is byte-identical to the one
run. Head `0fb1c3aa9` adds one more prose-only commit (the changeset's
Clause-② sentence and "Who sees it" name `/analytics/query` and
`/analytics/sql`, per the contract review); the same eight families
re-run there, all exit 0 (first reads exit 3 in a worktree without
`node_modules`, re-read after `pnpm install`), and the derived list is
again identical.
- **Derived gate families:** `node scripts/pm/dispatch-gates.mjs
--commands --repo objectstack-ai/objectstack` → 62 families, each run
with `< /dev/null` and its exit code captured before any pipe; `--ran`
reconciliation: `✓ dispatch-gates --ran: 62 derived famil(ies) accounted
for — 62 run, 0 NOT-MEASURED (a DERIVED zero — all 62 recorded an exit
code and none of them is 3)`. One first read was a container
prerequisite, re-read after meeting it: `check:dual-build-cjs-loads`
exit 3 ("Run `pnpm build` first") → full `pnpm build --concurrency=2`
(117 tasks, exit 0) → exit 0. `check:type-check-debt` exit 0 (`0 raw tsc
error(s) total, none above its recorded number`).
- **Hand gates:** `node scripts/check-platform-checklist.mjs
--changed-from origin/main` exit 0; `node
scripts/check-commit-card-trailers.mjs --range origin/main..HEAD` exit
0; `pnpm exec eslint --no-inline-config
packages/services/service-analytics/src
packages/rest/src/analytics-dataset-refusal-envelope.test.ts` exit 0, no
output (neither package has a `lint` script; the repo-wide `pnpm lint`
is CI's).
- **Open PRs touching these files:** none of the 14 open code PRs at
push time (#28512–#28531) changes `analytics-service.ts`,
`dataset-compiler.ts` or the rest refusal-envelope test.
- **Full dogfood** (owed by the `!` and the `(narrowing)` arm): after
building its closure, under the lock, `pnpm --filter
@objectstack/dogfood test` → Test Files 363 passed | 3 skipped (366),
Tests 2932 passed | 28 skipped (2960); no failure.
- **CI** families (Test Core, lint farm, type-check lanes) are read off
this PR's check-runs by the seat.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01FEkQ5pG4c8xoSCyPjcztJc
---
_Generated by [Claude
Code](https://claude.ai/code/session_01FEkQ5pG4c8xoSCyPjcztJc)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent bca27ad commit a2e7556
5 files changed
Lines changed: 356 additions & 4 deletions
File tree
- .changeset
- packages
- rest/src
- services/service-analytics/src
- __tests__
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
225 | 225 | | |
226 | 226 | | |
227 | 227 | | |
228 | | - | |
229 | | - | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
230 | 234 | | |
231 | 235 | | |
232 | 236 | | |
233 | | - | |
| 237 | + | |
234 | 238 | | |
235 | 239 | | |
236 | 240 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
0 commit comments