Skip to content

Commit 6f95c58

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-20295-rest-api-retire
2 parents f080e8e + de091b5 commit 6f95c58

32 files changed

Lines changed: 2854 additions & 395 deletions
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
---
2+
"@objectstack/formula": minor
3+
"@objectstack/plugin-security": minor
4+
"@objectstack/spec": patch
5+
---
6+
7+
A row-level write check that orders a field against a bound (`>`, `>=`, `<`, `<=`) is refused with `INVALID_FILTER` / 400 when that field holds a list or an object on the record being written, instead of comparing the list's string form and admitting the write (#19886).
8+
9+
**BREAKING** — an accept-set narrowing, shipped by `@objectstack/formula` and `@objectstack/plugin-security` as `minor` under the repo's launch-window convention for accept-set narrowings. The hand-migration prescription is registered under protocol major 18 as `rls-predicate-stored-list-ordering-refused`.
10+
11+
Clause-②: no (narrowing)
12+
13+
**Security fix for RLS write checks.** Measured through the real plugin-security on driver-sql and driver-memory: `record.tags > 'a'`, with `tags` a `json` field holding `['m']`, compared `'m' > 'a'` and admitted and stored the write. `record.meta < 'a'` with `meta` holding `{ a: 1 }` compared `'[object Object]' < 'a'` and did the same, and so did a `multiple` lookup. `using` stands in as the check for a policy that declares no `check`, so the same predicate in `using` was enforced the same way on writes. No shipped row-level or sharing-rule predicate orders a field at all.
14+
15+
What changes:
16+
17+
- `@objectstack/formula`: `matchesFilterCondition` refuses `$gt` / `$gte` / `$lt` / `$lte` and `$between` on a field whose value on the record is a list or a plain object, whatever the comparand, with the same `INVALID_FILTER` / 400 and the same message as the stage 2d refusals. The refusal is per record: a record whose `json` field holds one scalar is compared as before. `null` and `Date` values are unchanged, and so is every equality against a stored list (`$eq`, `$ne`, implicit equality, `$in`, `$nin`). `$between` is not produced by the CEL lowering, so it reaches this only through a filter passed to `matchesFilterCondition` directly.
18+
- `@objectstack/plugin-security`: a check insert or by-id update whose post-image holds a list or an object in an ordered field is refused 400 and stores nothing. That includes a by-id update that edits another field of a row whose stored `json` column holds a list, because the post-image merges the stored row.
19+
- `@objectstack/spec`: the migration registry carries the entry. The stage 2a entry `rls-predicate-array-comparand-refused` now ends "Scalar != and ==, null, Date comparands, and { $field } references between single-valued columns evaluate exactly as before", which is true since stage 2d.
20+
21+
Three moves, named:
22+
23+
1. **The write check now matches driver-sql's read.** driver-sql refuses every ordering comparison, and `$between`, on a column it stores as JSON text, by declared type (400, #7398). The in-process check now refuses the same predicate on the same row (400).
24+
2. **driver-memory's read parts from the write check.** driver-memory, a test driver, compares a stored list element by element on a read and keeps returning those rows (`record.tags > 'a'` reads a row holding `['m']`), while the check now refuses writing it. This is declared on #15104, as for stage 2d's `{ $field }` half.
25+
3. **A list written into a scalar field under an ordering check now answers 400.** `status: ['m']` into a `text` field under `record.status > 'a'`, or `amount: [500]` into a `number` field under `record.amount > 10`, was admitted, and driver-sql stored it as the text `'["m"]'` / `'[500]'`. It is now refused before anything is stored.
26+
27+
**The explain answer.** `security/explain` evaluates the business RLS predicate in-process on the fetched record, so it now answers `INVALID_FILTER` / 400 where the record holds a list or an object under an ordering predicate (this stage). It already answered 400 for a `{ $field }` comparison against a list-holding column (stage 2d). For both, per operation:
28+
29+
| explain operation | driver | the enforced operation answers | same as explain's 400? |
30+
|---|---|---|---|
31+
| `read` | driver-sql | 400 `INVALID_FILTER` (the driver's refusal) | yes |
32+
| `update` | driver-memory | 400 `INVALID_FILTER` (the post-image check) | yes |
33+
| `update` | driver-sql | 403 `PERMISSION_DENIED`: the pre-image gate fails closed on the driver's 400 | no — both deny |
34+
| `read` | driver-memory | the rows its element-wise read admits | no — the test driver's read |
35+
36+
Explain itself is unchanged.
37+
38+
**What to change.** Order a single-valued column (`record.priority > 2`), or test membership in the list with `in` (`record.status in ['open', 'pending']`). A `json` or `multiple` field has no ordering.
39+
40+
<!-- adr-0087: registered rls-predicate-stored-list-ordering-refused -->
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
---
2+
'@objectstack/spec': minor
3+
'@objectstack/lint': minor
4+
---
5+
6+
fix(spec)!: a `decision` branch with no `expression` — the key absent, or `null` — is refused at authoring (#19961)
7+
8+
Clause-②: no (narrowing)
9+
10+
<!-- adr-0087: registered flow-decision-branch-expression-absent-refused -->
11+
12+
**BREAKING** — an accept-set narrowing on one authored flow-node slot, shipped as
13+
`minor` under the launch-window convention (`check-changeset-no-major` refuses
14+
`major` until GA; breaking-ness is carried by this banner and the ADR-0087
15+
disposition above, not by the level).
16+
17+
**What changed.** `DecisionConditionSchema` declares a branch `{ label, expression }`
18+
with `expression` a required `z.string()`. Nothing enforced that: a decision node's
19+
`config` is an open record no schema is parsed against, and the expression ledger's
20+
resolver skipped an absent value as "not authored". So `conditions: [{ label: 'y' }]`
21+
passed `FlowSchema.parse`, `AutomationEngine.registerFlow` and `objectstack validate`,
22+
and the run then failed at that branch — the executor evaluates every branch it
23+
reaches, and a branch with no `expression` is a condition with no `source`, which
24+
`evaluateCondition` refuses. The ledger now marks the slot `required` (reconciled
25+
against the schema's own `required` list), and the branch is refused at all three
26+
doors through the walk and the function that already refuse a blank one — by
27+
`FlowSchema.parse` with a `custom` issue anchored at the slot (for example
28+
`nodes.1.config.conditions.0.expression`), by `registerFlow` and `objectstack validate`
29+
through that same parse, and by `validateStackExpressions` for a stack handed to it
30+
directly — with one message, led by the published `PREDICATE_SLOT_STRING_REFUSAL`
31+
sentence. `expression: null` is refused the same way, and so is a branch that wrote
32+
its predicate under `condition` (the edge's spelling), which has no `expression`
33+
either. The Studio flow designer writes the refused shape when a branch row's
34+
expression cell is left empty. Where such a branch already sits, the whole flow is
35+
refused: registered from the metadata
36+
registry or `sys_metadata` at boot, it is skipped with a `failed to register flow`
37+
warn naming it while the flows beside it register; a `defineStack({ flows })` source
38+
throws `StackSchemaInvalidError` for the whole stack; an artifact file is refused
39+
whole at load.
40+
41+
## FROM → TO
42+
43+
| you wrote | write instead |
44+
|:--|:--|
45+
| `conditions: [{ label: 'high' }]` on a `decision` node | the predicate you meant — `{ label: 'high', expression: 'record.amount > 10000' }` |
46+
| `conditions: [{ label: 'high', condition: 'record.amount > 10000' }]` | the same predicate under `expression` |
47+
| `conditions: [{ label: 'high', expression: null }]` | the predicate you meant, or `expression: 'false'` to keep the branch and never take it |
48+
49+
**One-line fix:** write the predicate under `expression`. `expression: 'false'` keeps
50+
the branch and its label and never takes it — a change of behaviour, not a preserved
51+
one: a run that reached the branch used to FAIL there, and now routes on to the next
52+
branch or the declared fallback. ⚠️ Do not drop a decision's only branch: the node
53+
then routes by its out-edges alone, and the out-edge that branch labelled is no
54+
longer held back.
55+
56+
**Unchanged.** A branch carrying a non-blank predicate parses, registers and
57+
validates as before; a blank one keeps its refusal and its own prescription
58+
(`flow-predicate-slot-blank-string-refused`); a `decision` with no `conditions`, or
59+
an empty list, still routes by its out-edges; an absent screen field `visibleWhen`
60+
is still legal (that slot is not required); and `PREDICATE_SLOT_STRING_REFUSAL`
61+
keeps its name and its text.
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
---
2+
'@objectstack/runtime': patch
3+
'@objectstack/rest': minor
4+
---
5+
6+
fix(runtime): the dispatcher's `/meta/:type` list prunes what `RestServer`'s list prunes, through one list gate that `@objectstack/rest` publishes (#20237)
7+
8+
Clause-②: yes
9+
10+
`GET /meta/:type` has two implementations: `RestServer`, and the runtime
11+
dispatcher's `/meta` domain. On a host that mounts only the `${prefix}/*`
12+
catch-all (`@objectstack/hono`'s `createHonoApp`, the documented embed shape,
13+
and any adapter built on the public `HttpDispatcher` API), the dispatcher is the
14+
only one that answers. Its list branch applied **no** per-caller filter. An
15+
authenticated member who does not hold `crm_admin` got these answers through
16+
`dispatch()`:
17+
18+
```
19+
request before after (= RestServer)
20+
GET /meta/doc?include=content the set-gated doc listed WITH its body the doc left out
21+
GET /meta/book the set-gated book listed the book left out
22+
GET /meta/app an app whose requiredPermissions the that app left out; the other app
23+
member lacks, and an ungated app with pruned of its gated entry
24+
its requiredPermissions-gated entry
25+
GET /meta/dashboard (anyone) every widget minus the widget whose service is off
26+
```
27+
28+
The plural spellings (`/meta/docs`, `/meta/books`, `/meta/apps`) answer the
29+
same. Holders are still listed everything in full. Object lists are masked as
30+
before. An anonymous caller still gets `401 UNAUTHENTICATED`.
31+
32+
**One gate, not two.** `RestServer`'s list filters moved unchanged into
33+
`createMetaListReadGate`, beside the item gate in
34+
`packages/rest/src/meta-item-read-gate.ts`. It covers the ADR-0046 §6.7 doc
35+
and book audience prunes, the app nav filter (`requiredPermissions`, the
36+
ADR-0045 §3 publish gate and the docs-audience entry arm) and the ADR-0057
37+
D10 dashboard widget gate. `RestServer`'s list route and the dispatcher's list
38+
branch both call it, over the same ports the item gate takes, and each rewraps
39+
the pruned items in its own list envelope. There is no second audience
40+
resolver in `packages/runtime`. `RestServer`'s list answers are unchanged.
41+
42+
Every exit of the dispatcher's list branch now runs the gate and the ADR-0106
43+
object mask: the protocol list and the two fallbacks, the runtime metadata
44+
service's list and the ObjectQL registry. The fallbacks used to serve
45+
unmasked object schemas as well as ungated docs, books and apps. A gate input
46+
that cannot be read (a doc list's books read throws) is answered as that fault,
47+
never as the unfiltered list.
48+
49+
**`@objectstack/rest`'s published export surface widens**, and that is why this
50+
changeset declares `Clause-②: yes`. Its only export subpath (`.`) gains one
51+
value:
52+
53+
- `createMetaListReadGate(sources, metaType)`: the list gate. It takes the
54+
same `MetaItemReadGateSources` the item gate takes, and it answers a judge
55+
from a list's items to the items this caller may be served.
56+
57+
It is public because the runtime dispatcher's `/meta` domain in
58+
`@objectstack/runtime` consumes this one gate. `@objectstack/rest` cannot import
59+
the runtime, so the shared decision has to live here and travel as an export,
60+
the way `createMetaItemReadGate` does. A caller outside the platform does not
61+
need it.
62+
63+
Nothing is removed or renamed from the package's exports, and no authorable key
64+
moves. The dispatcher's prunes are not the widening: they pull a second
65+
transport back to the rules the contract already declares (ADR-0046 §6.7,
66+
ADR-0045 §3, `apps.mdx`'s `requiredPermissions` row), which is why
67+
`@objectstack/runtime` stays a `patch`.

‎packages/formula/src/matches-filter-array-comparand.test.ts‎

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,15 @@
3434
* the rows above. The third is a `{ $field }` comparison whose column holds a
3535
* list: the shape is legal, so it is judged on the record — refused when either
3636
* compared column holds a list (or an object) on the record being judged.
37+
*
38+
* [#19886 stage 2e] The mirror of the first 2d row, with the list on the
39+
* RECORD's side and a literal bound — also judged on the record:
40+
*
41+
* | shape | before | after |
42+
* |---|---|---|
43+
* | `{ tags: { $gt: 'a' } }`, `tags` holding `['m']` | `'m' > 'a'` → `true`, the write **ALLOWED** | throws → 400 |
44+
* | `{ meta: { $lt: 'a' } }`, `meta` holding `{ a: 1 }` | `'[object Object]' < 'a'` → `true`, **ALLOWED** | throws → 400 |
45+
* | `{ tags: { $between: ['a', 'z'] } }`, `['m']` | `true` | throws → 400 |
3746
*/
3847

3948
import { describe, it, expect } from 'vitest';
@@ -235,3 +244,124 @@ describe('[#19886 stage 2d] a { $field } comparison whose column holds a list is
235244
}
236245
});
237246
});
247+
248+
describe('[#19886 stage 2e] an ordering comparison whose STORED operand holds a list or an object is refused on that record', () => {
249+
const m = matchesFilterCondition;
250+
251+
/** What a `json` column or a `multiple` lookup holds on a post-image. */
252+
const STORED: Array<[string, unknown]> = [
253+
['a one-element list', ['m']],
254+
['a multi-element list', ['a', 'z']],
255+
['an empty list', []],
256+
['a list of lists', [['m']]],
257+
['a plain object', { a: 1 }],
258+
['an empty object', {}],
259+
];
260+
261+
/** Every ordering operator, each with a bound the coerced string form used to satisfy or not. */
262+
const ORDERINGS: Array<[string, Record<string, unknown>]> = [
263+
['$gt', { $gt: 'a' }],
264+
['$gte', { $gte: 'a' }],
265+
['$lt', { $lt: 'z' }],
266+
['$lte', { $lte: 'z' }],
267+
['$between', { $between: ['a', 'z'] }],
268+
];
269+
270+
for (const [storedName, stored] of STORED) {
271+
for (const [opName, spec] of ORDERINGS) {
272+
it(`${opName} on a field holding ${storedName}: INVALID_FILTER / 400`, () => {
273+
const err = refusalOf({ tags: spec }, { tags: stored });
274+
expect(err.code).toBe('INVALID_FILTER');
275+
expect(err.status).toBe(400);
276+
});
277+
}
278+
}
279+
280+
it('the refusal reaches every depth the evaluation reaches, and a negation cannot turn it into an answer', () => {
281+
const record = { owner: 'u1', tags: ['m'] };
282+
const leaf = { tags: { $gt: 'a' } };
283+
for (const filter of [
284+
{ $and: [{ owner: 'u1' }, leaf] },
285+
{ $or: [{ owner: 'nobody' }, leaf] },
286+
{ $not: leaf },
287+
{ $and: [{ $or: [{ $not: leaf }] }] },
288+
]) {
289+
const err = refusalOf(filter, record);
290+
expect(err.code).toBe('INVALID_FILTER');
291+
expect(err.status).toBe(400);
292+
}
293+
});
294+
295+
it('a verdict already decided without the field does not reach it — judged per record, like the stage 2d refusal', () => {
296+
// Unlike the shape refusals, which judge the authored filter before any
297+
// record, this one judges a VALUE, so it fires where evaluation reads it.
298+
// Where it is not read, the answer does not depend on it either way.
299+
const record = { owner: 'u1', tags: ['m'] };
300+
expect(m(record, { $or: [{ owner: 'u1' }, { tags: { $gt: 'a' } }] })).toBe(true);
301+
expect(m(record, { owner: 'nobody', tags: { $gt: 'a' } })).toBe(false);
302+
});
303+
304+
it('whatever the comparand: a number, a Date, null or a { $field } reference', () => {
305+
const record = { tags: ['m'], status: 'a' };
306+
for (const spec of [
307+
{ $gt: 5 },
308+
{ $lt: new Date('2026-01-01T00:00:00.000Z') },
309+
{ $gte: null },
310+
{ $lte: { $field: 'status' } },
311+
]) {
312+
const err = refusalOf({ tags: spec }, record);
313+
expect(err.code).toBe('INVALID_FILTER');
314+
expect(err.status).toBe(400);
315+
}
316+
});
317+
318+
it('a list written into a scalar field is judged the same way (a text or number field under an ordering check)', () => {
319+
for (const [record, filter] of [
320+
[{ status: ['m'] }, { status: { $gt: 'a' } }],
321+
[{ amount: [500] }, { amount: { $gt: 10 } }],
322+
] as const) {
323+
const err = refusalOf(filter, record);
324+
expect(err.code).toBe('INVALID_FILTER');
325+
expect(err.status).toBe(400);
326+
}
327+
});
328+
329+
it('the SAME filter over a record holding one scalar is compared, not refused — the refusal is per record', () => {
330+
expect(m({ tags: 'm' }, { tags: { $gt: 'a' } })).toBe(true);
331+
expect(m({ tags: 'a' }, { tags: { $gt: 'a' } })).toBe(false);
332+
expect(m({ tags: 'm' }, { tags: { $between: ['a', 'z'] } })).toBe(true);
333+
expect(m({ tags: 5 }, { tags: { $lte: 10 } })).toBe(true);
334+
});
335+
336+
it('null, a missing field and a Date are untouched: no value is false, and a Date is a value', () => {
337+
for (const record of [{ tags: null }, {}]) {
338+
expect(m(record, { tags: { $gt: 'a' } })).toBe(false);
339+
expect(m(record, { tags: { $lt: 'z' } })).toBe(false);
340+
expect(m(record, { tags: { $between: ['a', 'z'] } })).toBe(false);
341+
}
342+
const at = new Date('2026-06-01T00:00:00.000Z');
343+
expect(m({ at }, { at: { $gt: '2026-01-01T00:00:00.000Z' } })).toBe(true);
344+
expect(m({ at }, { at: { $lt: '2026-01-01T00:00:00.000Z' } })).toBe(false);
345+
expect(m({ at }, { at: { $between: ['2026-01-01T00:00:00.000Z', '2026-12-31T00:00:00.000Z'] } })).toBe(true);
346+
});
347+
348+
it('equality against a stored list keeps the answer stage 2a pinned — only ordering moved', () => {
349+
const record = { tags: ['m'] };
350+
expect(m(record, { tags: 'm' })).toBe(false);
351+
expect(m(record, { tags: { $eq: 'm' } })).toBe(false);
352+
expect(m(record, { tags: { $ne: 'm' } })).toBe(true);
353+
expect(m(record, { tags: { $in: ['m'] } })).toBe(false);
354+
expect(m(record, { tags: { $nin: ['m'] } })).toBe(true);
355+
expect(m(record, { tags: { $exists: true } })).toBe(true);
356+
expect(m(record, { tags: { $null: false } })).toBe(true);
357+
});
358+
359+
it('the message withholds the field and the stored value', () => {
360+
const err = refusalOf({ secret_scope_column: { $gt: 'a' } }, {
361+
secret_scope_column: ['usr_member_one', 'usr_member_two'],
362+
});
363+
for (const secret of ['secret_scope_column', 'usr_member_one', 'usr_member_two']) {
364+
expect(err.message).not.toContain(secret);
365+
}
366+
});
367+
});

0 commit comments

Comments
 (0)