Skip to content

Commit 79c3145

Browse files
os-helpclaude
andauthored
fix(analytics): refuse a { $field } comparand on both SQL-lowering doors instead of binding it (#7598) (#7694)
* fix(analytics): refuse a `{ $field }` comparand on both SQL-lowering doors instead of binding it (#7598) Measured on origin/main (5823d59), the premise of #7598 was inverted: neither door refused a field reference in a scalar comparand position — both BOUND the reference object as the comparison's value, producing a syntactically perfect predicate comparing a column against a value no row can hold. On the read-scope door that is an administrator's RLS predicate silently answering the wrong row set. Both doors now refuse, each in its existing envelope (INVALID_FILTER / 400 on the analytics `where` door, READ_SCOPE_COMPILE_FAILED / 500 on the read-scope lowering). Positions that already refused keep their exact wording, because each of those converges with driver-sql's own #5222 refusal arm. This does NOT port the #5222 capability: its four maintainer rulings turn on an object's declared field set and its tenant-isolation column, neither of which StrategyContext exposes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015fkdTyGmMD5s8ZtEifvuGy * chore(changeset): answer the ADR-0087 disposition for the #7598 refusal in writing The changeset declares a breaking change (`fix(analytics)!`), so check-adr-0087-registration requires the ledger question be answered. Disposition: not-required (no-migration-prescription) -- no authorable key is retired, packages/spec is untouched, and the FROM shape stays valid metadata that still executes on the ObjectQL engine path and both SQL drivers, so there is nothing for `objectstack migrate meta` to rewrite. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015fkdTyGmMD5s8ZtEifvuGy --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent e654bfd commit 79c3145

8 files changed

Lines changed: 798 additions & 3 deletions

File tree

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
---
2+
"@objectstack/service-analytics": minor
3+
---
4+
5+
fix(analytics)!: a `{ $field }` comparand is refused on both SQL-lowering doors instead of being BOUND as the comparison's value (#7598)
6+
7+
<!-- adr-0087: not-required (no-migration-prescription) This change retires NO key and adds none. `FieldReferenceSchema` stays declared in `packages/spec` exactly as it is, stays implemented by `@objectstack/formula`'s in-memory evaluator, and stays COMPILED by `driver-sql` / `driver-sqlite-wasm` under #5222 — `packages/spec` is untouched by this PR, no metadata schema gains or loses a key, and no authored or stored shape becomes unparseable. What moves is one COMPILER's posture at two doors of `@objectstack/service-analytics`: a shape that used to compile into a predicate binding the reference OBJECT as a value now refuses. There is therefore nothing for `objectstack migrate meta` to rewrite — the FROM shape is still valid metadata everywhere it was valid before, and rewriting it would be wrong, since the identical filter continues to execute on the ObjectQL engine path and on both SQL drivers. Nor is there a FROM/TO rule a ledger entry could state: the correct repair depends on which face the author's query routes to, which is a deployment fact rather than a metadata one. The channels that do reach an affected reader are this changeset's CHANGELOG text and the refusal message itself, which names the operator, the field, the referenced column, the faces that DO execute the shape, and why this compiler cannot — all shipped with this change. -->
8+
9+
**⚠️ Behaviour change.** A filter whose comparand is a field reference —
10+
`{ amount: { $gt: { $field: 'budget' } } }`, the shape
11+
`FieldReferenceSchema` declares and `compileCelToFilter` emits for a
12+
field-to-field comparison in a CEL permission / RLS rule — used to COMPILE on
13+
both of this package's doors. It now refuses: `INVALID_FILTER` / 400 on the
14+
analytics `where` door, `READ_SCOPE_COMPILE_FAILED` / 500 on the read-scope
15+
lowering (each door's existing envelope, unchanged).
16+
17+
#7598 was filed reading "these compilers still REFUSE `$field`". Measured on
18+
`origin/main` (`5823d593d`), nothing refused. For the six scalar comparison
19+
operators — exactly the ones #5222 taught `driver-sql` to compile into a
20+
same-table column-to-column comparison — the reference OBJECT went into the
21+
bind list:
22+
23+
| face | `{ amount: { $gt: { $field: 'budget' } } }` |
24+
|---|---|
25+
| `read-scope-sql` | `"person"."amount" > ?` · bound to `{"$field":"budget"}` |
26+
| `where``NativeSQLStrategy` | `WHERE amount > $1` · bound to the JSON TEXT `{"$field":"budget"}` |
27+
| `where``/analytics/sql` echo | `WHERE amount > $1` · bound to the reference OBJECT |
28+
| `where` → ObjectQL engine | reached `driver-sql`, which compiles it CORRECTLY since #5222 |
29+
30+
So the defect was a silent wrong answer, not a refusal: a syntactically perfect
31+
predicate comparing a column against a value no row can hold. Three of the four
32+
faces answered differently, and on the read-scope door the one answering wrongly
33+
is an administrator's RLS predicate. The gates assumed to be catching this
34+
(`isBindableComparand` / `isRenderableTextComparand`) had not drifted from
35+
`driver-sql` — they are simply never ASKED about that position, only about the
36+
LIKE family and `$in` / `$nin` / `$between` MEMBERS.
37+
38+
**What this does not do:** it does not bring the capability to these compilers.
39+
The four maintainer rulings that make a referenced column name safe in a SQL
40+
identifier position (same-table only, declared-only enumeration, tenant-isolation
41+
column forbidden on both sides, same comparison class) all turn on metadata
42+
`StrategyContext` does not expose — neither an object's declared field set nor its
43+
tenant-isolation column — so these compilers cannot enforce them, and shipping a
44+
port without them would open a comparison surface onto the tenant boundary.
45+
Implementing it here is a `packages/spec` contract question, left open on #7598.
46+
47+
Field-to-field RLS rules continue to work on the ObjectQL engine path, where the
48+
driver compiles them with the metadata it owns; they are now loudly refused,
49+
rather than silently mis-answered, on the raw-SQL analytics path.
50+
51+
Positions already refused before this change keep their exact wording — the LIKE
52+
family, `$in` / `$nin` members, and a bare `{ field: { $field: … } }` — because
53+
each of those refusals already CONVERGES with `driver-sql`'s own #5222 refusal
54+
arm. `minor` rather than `patch` follows #5234, the same class of change on the
55+
same two doors.

packages/services/service-analytics/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
"name": "@objectstack/service-analytics",
33
"version": "17.0.0-rc.6",
44
"license": "Apache-2.0",
5-
"description": "Analytics Service for ObjectStack implements IAnalyticsService with multi-driver strategy pattern (NativeSQL, ObjectQL, InMemory)",
5+
"description": "Analytics Service for ObjectStack \u2014 implements IAnalyticsService with multi-driver strategy pattern (NativeSQL, ObjectQL, InMemory)",
66
"type": "module",
77
"main": "dist/index.js",
88
"types": "dist/index.d.ts",
@@ -23,6 +23,7 @@
2323
"@objectstack/types": "workspace:*"
2424
},
2525
"devDependencies": {
26+
"@objectstack/driver-sql": "workspace:*",
2627
"@types/node": "^26.1.2",
2728
"@types/sql.js": "^1.4.11",
2829
"sql.js": "^1.14.1",

packages/services/service-analytics/src/__tests__/comparand-shape-refusal.test.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,9 +126,28 @@ describe('[#5234] the analytics `where` door refuses an uncompilable comparand',
126126
it('`{$field: …}` is refused here, converging with `driver-sql`', () => {
127127
// Not a special case: a field reference is an object, and this door had no
128128
// opinion about objects at all. `driver-sql` has refused it since #5041.
129+
//
130+
// ⚠️ [#7598] The convergence claim was re-measured after #5222 gave
131+
// `driver-sql` a real cross-field compiler, because that change was assumed
132+
// to have made this comment stale. It did NOT, for THIS case: #5222's
133+
// boundary admits the six scalar comparison operators and leaves the LIKE
134+
// family in its refusal arm (`$contains against a field reference is
135+
// refused` — a column-side LIKE pattern cannot be metacharacter-escaped
136+
// portably, and an unescaped one is the `%`-matches-every-row bypass). So
137+
// this pin still converges, verbatim, and is deliberately unchanged.
138+
//
139+
// What #5222 DID open is a position neither predicate in
140+
// `comparand-shape.ts` is ever asked about — the whole comparand of a
141+
// scalar comparison, which was BOUND rather than refused. That cell is
142+
// pinned in `cross-field-reference-refusal.test.ts` against the shared
143+
// corpus, not here, because it is a different question about a different
144+
// position.
129145
const err = refusalOf(() => tree({ name: { $contains: { $field: 'status' } } }));
130146
expect(err.code).toBe('INVALID_FILTER');
131147
expect(err.message).toContain('$field');
148+
// The wording stays the LIKE-family one — the #7598 gate deliberately does
149+
// not reach this operator, so a reader can tell the two refusals apart.
150+
expect(err.message).toContain('StringOperatorSchema');
132151
});
133152
});
134153

@@ -160,9 +179,22 @@ describe('[#5234] the analytics `where` door refuses an uncompilable comparand',
160179
it('`{$eq: {…}}` is deliberately UNTOUCHED — a separate account', () => {
161180
// #5526 pinned `toSqlBindValue({a:1})` → `'{"a":1}'`. Refusing it is the
162181
// analytics-side half of #5041, which this change does not open.
182+
//
183+
// ⚠️ [#7598] Still true, and now load-bearing in a second way: the
184+
// field-reference gate added there covers this very operator, so this case
185+
// is what proves the gate keys on the SHAPE `{$field: <string>}` and not on
186+
// "an object comparand". A gate that had widened to every object would turn
187+
// this row red — which is why the row is worth keeping rather than being
188+
// folded into the block above.
163189
expect(tree({ name: { $eq: { a: 1 } } })).toEqual({
164190
kind: 'leaf', member: 'name', operator: 'equals', values: [{ a: 1 }],
165191
});
192+
// The same distinction one step finer: `$field` present but NOT a string is
193+
// the ordinary object account too, exactly as on `driver-sql`, whose
194+
// `fieldReferenceOf` requires `typeof ref === 'string'`.
195+
expect(tree({ name: { $eq: { $field: 5 } } })).toEqual({
196+
kind: 'leaf', member: 'name', operator: 'equals', values: [{ $field: 5 }],
197+
});
166198
});
167199
});
168200
});

0 commit comments

Comments
 (0)