Skip to content

Commit d498113

Browse files
fix(lint)!: refuse an RLS predicate that compares a field with a json or multiple field when it is authored (#20346)
Part of #19886 Clause-②: no Stage 2f of #19886: remainder item 1 of release `5860028604`, the compile-door arm ruled A in `5857706935` ("in the lint rule, serial after #20158"). Claim `5860056842`. Base `de091b50`, head `509728de`. The changeset declares `Clause-②: no (narrowing)`, BREAKING, following the 2d / 2e precedent. ## What changes `validateRlsPredicateEnforceability` (`packages/lint`) gets one arm. It reports `rls-predicate-unenforceable` for every lowered field-to-field comparison (`==`, `!=` and the four ordering operators, on either side, under `!` too) in which either column is DECLARED to hold a list or an object. - **Judged by declared type, from the rule's own `ObjectGraph`.** The class is read from the spec's value-shape classes, not from a list in lint: `STRUCTURED_JSON_TYPES` (`json`, `composite`, `repeater`, `record`, `location`, `address`, `vector`) and `isMultiValueField` (`multiselect`, `checkboxes`, `tags`, and `select` / `radio` / `lookup` / `user` / `file` / `image` with `multiple: true`). These are the same two that driver-sql builds its JSON-column set and its multi-valued test from, so the lint arm and the driver's cross-field refusal share one definition. - **Every clause.** It covers `using` on `select` / `all` / `update` / `delete` / `insert` and `check` on `insert` / `update` / `all` (Zone 2 item 3, decided from the runtime measurement below). - **One defect, one finding.** The arm runs before the #20158 engine-judge pass. A clause it refuses is never handed to `judgeFilter`, and the engine judge answers none of these cells anyway (Zone 2 item 1). - **Both doors.** `os validate` / `build` / `lint` and the metadata save door already run this rule, so both refuse with the same sentence. No gate is added, and no runtime seam. The 2d / 2e runtime refusals stay as the backstop. Files: the rule (+141), a new table-driven pin beside it, six refusal rows plus three controls added to the CLI/runtime parity pin, and the changeset. ## The refusal text (new) Message, for a `using` clause (a real `os validate` finding): > RLS using `record.status != record.tags` lowers, but compares a field with a field that holds a list or an object: `record.status != record.tags`, where `tags` is declared `type: 'json'`. A column that holds a list or an object is not one comparable value, on either side of a field-to-field comparison, so the platform refuses the comparison instead of evaluating it: every read this policy scopes is refused on the SQL drivers (`INVALID_FILTER` / 400: driver-sql refuses a cross-field comparison against such a column by its declared type), and every by-id update or delete it scopes fails closed (`PERMISSION_DENIED` / 403). On an `insert`, `update` or `all` policy the same `using` is also the write check whenever no applicable policy for that operation declares a `check` (ADR-0058 D4): then every single-record insert and by-id update whose record holds a list or an object in that column is refused (`INVALID_FILTER` / 400) and stores nothing, because the write check compares one value with one value and will not guess what a list means. For a `check` clause, the part after the colon is the write sentence alone, followed by: "The policy reads as a comparison and behaves as a refusal of every write that leaves a list or an object in that column." Hint: > A field compared with a `json` or `multiple` field has no row-filter form: a row filter compares one value with one value, and cannot test membership in a list another column holds. Compare with a single-valued column, or with a literal or a `current_user` value — "one of these values" is `record.status in ['open', 'pending']` or `record.owner in current_user.org_user_ids` — or move the condition into a validation rule or a hook. The class sentence ("A column that holds a list or an object is not one comparable value, on either side of a field-to-field comparison") follows the wording of 2d's `arrayComparandError`. The prescription follows the registered entry `cel-predicate-one-value-comparand-refused`. At authoring time the field names are the author's own text, so nothing is withheld. ## Zone 2, measured first **1. Is (a) still silent after #20265?** Yes, at both doors, and the engine judge refuses none of it. The cell table was written to disk before any edit (`cells-before.json`): - 400 cells: `==`, `!=`, `!(==)`, `>` and less-or-equal; five list-holding columns (`json`, `address`, `multiselect`, `multiple` lookup, `multiple` user); both operand orders; eight clause/operation pairs. - Doors: the real CLI (`node packages/cli/bin/run.js validate --json`) and the runtime save door (`saveMetaItem('permission')` over a real `ObjectQL` with the sqlite-wasm driver). | tree | leak cells (400) | scalar controls (48) | firing controls (16) | |---|---|---|---| | `de091b50` (base) | 400 clean at the CLI, 400 accepted at the save door | 48 clean / accepted | 10 refused at both: the list literal on all 8 clauses, the engine-judge text operator on select/all `using`; 6 clean, the engine judge on other clauses, as designed | | `509728de` (head) | 400 refused at both, one finding each, the same sentence at both doors (400 / 400) | 48 clean / accepted | unchanged | **2. Declared type.** The judgement uses the graph's declared `type` and `multiple`, never a value. An object outside the stack or with no field map is not judged; an undeclared name is the unknown-field finding alone (pinned). **3. Which clauses.** The runtime refuses every clause, so the arm covers every clause. Measured at `de091b50` through the real `SecurityPlugin` + `ObjectQL` + driver-sql (better-sqlite3), for `record.status != record.tags`, its mirror, `==` against `json`, and `!=` against a `multiple` lookup: | clause / operation | action | answer (36 of 36 leak cells) | |---|---|---| | `using` on select, all | `find` | `INVALID_FILTER` / 400 | | `using` on update, delete | by-id update / delete | `PERMISSION_DENIED` / 403, nothing changed | | `using` on insert, all (standing in as the check) | insert | `INVALID_FILTER` / 400, nothing stored | | `check` on insert, update, all | insert / by-id update | `INVALID_FILTER` / 400, nothing stored | The scalar control `record.status != record.reviewer` was admitted and enforced on all 9 pairs. **4. Producer census (narrowing).** - objectstack at `509728de`: 187 string literals under a `using` / `check` / `condition` key (111 distinct), in `packages/` and `examples/`, non-test. 2 lower to a `{ $field }` comparison. Both are scalar-to-scalar: the showcase hook condition `record.spent > record.budget` (currency) and a doc-formula script's `record.a > record.b`. Neither is an RLS or sharing predicate, and 0 compare with a list-holding field. - cloud `main` `96eb092`: 3 occurrences, 0 `{ $field }` comparisons. - The real `os validate` over `app-crm`, `app-multi-package`, `app-showcase` and `app-todo` at head: 0 `rls-predicate-*` findings. 0 over-refusals. ## Tests (head `509728de`, every run through `os-verify-lock.sh`) - `@objectstack/lint`, full package: 112 files / 4640 tests pass. The new pin has 336 tests: 224 cells (8 clause-operation pairs × 7 operators × 2 classes × 2 orders), 16 declared classes, 88 scalar controls on every clause, and the arm's own properties. `typecheck`: exit 0. - `@objectstack/cli` `unit` project: 229 files / 3251 pass. `integration` project, the parity pin (touched by this diff, so run locally): 30 / 30. `typecheck`: exit 0. - `@objectstack/metadata-protocol` (the save-door gate): 189 files pass, 3 skipped; 2728 tests pass, 19 skipped. - `@objectstack/plugin-security`, the unchanged 2d / 2e backstop pins (read only): 150 / 150. **Ablation** of the committed arm (`fdda49e5`; the rule's blob is the same at head). The mutation replaced the arm's call with an empty list through `scripts/ablation-replace.mjs` (anchor 1 to 0, marker 0 to 1, blob changed). `pnpm --filter @objectstack/lint build` then emitted JS; its DTS step exited 1 on the now-unused helper (TS6133). `ablation-dist-preflight.mjs @objectstack/lint` found the marker in all 4 built entries. - The new lint pin: **244 red**, 92 green. Red: every refusal cell and property. Green: the controls, the table-size check, and the not-judged and unknown-field cases. - The CLI parity pin: **6 red**, exactly the six new rows; 24 green. - Restored with `git checkout HEAD -- ABS_PATH`: blob equal to HEAD, `git diff HEAD` empty. Rebuilt with exit 0, and the preflight `--absent` was clean with a clean tree. ## Gates Re-derived on the actual paths with `dispatch-gates.mjs --repo objectstack-ai/objectstack`, all run at head, exit codes recorded before reading. `--ran` reconciliation: **61 derived, 60 run, 1 NOT MEASURED, 0 UNRUN**. The two families new over the dispatch lead are `check:cli-test-child-env` and bare `check-issue-citations.mjs`, both run, both exit 0. - NOT MEASURED: `check:dual-build-cjs-loads`, exit 3, PREREQUISITE NOT MET. Eight packages outside the built closures (studio, client-react, embedder-openai and five more) have no `dist/`, and the gate needs a whole-tree `pnpm build`. As a proxy, both of lint's `require` entries (`dist/index.cjs`, `dist/runtime.cjs`) load. - ADR-0087 disposition: `not-required (already-registered cel-predicate-one-value-comparand-refused)`. That entry already names this exact class in its `surface` and carries its prescription in its `replacement`. This PR moves where the registered class is refused (run time to authoring time) and adds no class, no prescription and no stored-metadata rewrite. `check-adr-0087-registration` passes. The #20158 precedent (`no-migration-prescription`) did not fit: this changeset carries a "What to change" line. ## Acceptance notes - **The stage 2d changeset's "Not changed" sentence.** It says a field compared with a `json` or `multiple` field "is not reported at authoring time". That no longer holds. This PR's changeset states the delta rather than editing an unreleased changeset, as 2d did for 2a. - **The sharing-rule twin (same class, another surface).** `sharingRules[].condition` `record.status != record.tags` and `record.reviewers == record.status` both pass the real `os validate` at head, while the firing control `record.status == ['a', 'b']` in the same run is reported. The arm belongs in `validate-sharing-rule-enforceability.ts`, which is outside this claim's surface (other `packages/lint` files are fenced). Its runtime answer on the sharing path was not measured. - **Other cross-field classes driver-sql refuses by declared type.** driver-sql's `crossFieldComparisonClass` refuses these too, and they are a different class from a list. A text column compared with a number column, a single-valued `image` column or a `formula` field all pass `os validate`. Measured through the real plugin-security on driver-sql for text against number and text against image: the `using` read answers `INVALID_FILTER` / 400, and the `check` insert is admitted and stored. The formula cell was not measured at run time. - **driver-memory** still reads a `{ $field }` comparand as a literal on a read (#15104, frozen; pointer `5857707647`). Unchanged here. ## What remains on #19886 The release's remainder list held item 1 only, and this PR closes it. It is `Part of` rather than `Fixes` because the sharing-rule twin above is the same authoring-door gap on a sibling surface. Whether it stays on this card or goes to its own card is the seat's call. If it moves off the card, the first line can take the closing keyword instead. --- _Generated by [Claude Code](https://claude.ai/code/session_01Rjy9MeetSfq34PKn81CRiN)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 26daf0b commit d498113

4 files changed

Lines changed: 500 additions & 7 deletions
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
---
2+
"@objectstack/lint": minor
3+
---
4+
5+
A row-level-security predicate that compares a field with a `json` or `multiple` field is refused when it is authored, at `os validate` / `os build` / `os lint` and at the metadata save door, instead of only when it runs (#19886).
6+
7+
**BREAKING** — an accept-set narrowing, shipped by `@objectstack/lint` as `minor` under the repo's launch-window convention for accept-set narrowings. The hand-migration prescription is already registered under protocol major 18 as `cel-predicate-one-value-comparand-refused`, which names this class.
8+
9+
Clause-②: no (narrowing)
10+
11+
`record.status != record.tags`, with `tags` a `json` field or a `multiple` lookup, lowers to a legal filter shape, because the CEL lowering sees the predicate's text and not the object's field types, and the engine's filter admission does not judge a `{ $field }` reference against the referenced column's type either. Measured before this change: 400 cells (`==`, `!=`, `!(==)`, `>`, `<=`; a `json`, `address`, `multiselect`, `multiple` lookup and `multiple` user field; both operand orders; `using` on `select` / `all` / `update` / `delete` / `insert` and `check` on `insert` / `update` / `all`) were all accepted by the real `os validate` and by the save door. The runtime refused every one of them, measured through the real plugin-security on driver-sql: a read the `using` scopes answered `INVALID_FILTER` / 400, a by-id update or delete it scopes `PERMISSION_DENIED` / 403, and every insert or by-id update judged by the `check` (or by a `using` standing in as the check) `INVALID_FILTER` / 400, with nothing stored.
12+
13+
What changes:
14+
15+
- `@objectstack/lint`: `validateRlsPredicateEnforceability` reports `rls-predicate-unenforceable` for every lowered field-to-field comparison (`==`, `!=`, `>`, `>=`, `<`, `<=`, on either side, under `!` too) in which either column is DECLARED to hold a list or an object. The declaration is read from the stack's own objects through the spec's value-shape classes, the same two driver-sql refuses such a comparison by: a structured JSON type (`json`, `composite`, `repeater`, `record`, `location`, `address`, `vector`), or a multi-valued field (`multiselect`, `checkboxes`, `tags`, or `select` / `radio` / `lookup` / `user` / `file` / `image` with `multiple: true`). It judges `using` and `check` on every operation. The finding names each comparison and the declaration behind it, and states the clause's run-time consequence. A clause it refuses is not also handed to the engine's filter judge, so one defect earns one finding.
16+
- Both doors run this rule already, so both refuse: `os validate` / `os build` / `os lint` fail, and a publish through the metadata save door answers `422 INVALID_METADATA` with the same sentence in `issues[]`. `OS_ALLOW_UNLINTED_METADATA_WRITES=1` still turns the save-door refusal into a logged warning.
17+
18+
Not changed: a field compared with a single-valued field (`record.status != record.owner`, `record.amount > record.budget`), a `json` or `multiple` field compared with a literal or tested against `null`, and any column the stack does not declare (an object from another package, an external object with no field map), which the rule does not judge. The runtime refusals of stages 2d and 2e stay as the backstop. The stage 2d changeset's sentence that a field compared with a `json` or `multiple` field "is not reported at authoring time" no longer holds: it is now reported at both doors.
19+
20+
No shipped predicate moves: 0 of the 187 `using` / `check` / `condition` strings in this repository's packages and examples, and 0 of the 3 in the cloud repository, compare a field with a `json` or `multiple` field, and the real `os validate` over `app-crm`, `app-multi-package`, `app-showcase` and `app-todo` reports no `rls-predicate-*` finding.
21+
22+
**What to change.** A field compared with a `json` or `multiple` field has no row-filter form: compare with a single-valued column, or with a literal or a `current_user` value ("one of these values" is `record.status in ['open', 'pending']`, or `record.owner in current_user.org_user_ids`), or move the condition into a validation rule or a hook.
23+
24+
<!-- adr-0087: not-required (already-registered cel-predicate-one-value-comparand-refused) The entry already names this exact class in its surface ("a field compared with another field (==, !=, or an ordering operator) where either column holds a list or an object on the record, as a json column or a multiple lookup does") and carries its prescription in its replacement ("A field compared with a json or multiple field has no pushdown form: compare with a single-valued column, or move the condition into a validation rule or hook"). This change moves where that registered class is refused, from run time to authoring time; it adds no class and no prescription the entry does not already hold, and it rewrites no stored metadata. -->

‎packages/cli/test/rls-policy-authoring-admission.test.ts‎

Lines changed: 64 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,25 +61,27 @@ const deal = {
6161
returnType: 'boolean',
6262
},
6363
account: { type: 'lookup', label: 'Account', reference: 'account' },
64+
tags: { type: 'json', label: 'Tags' },
65+
watchers: { type: 'lookup', label: 'Watchers', reference: 'account', multiple: true },
6466
},
6567
};
6668
const account = { name: 'account', label: 'Account', fields: { region: { type: 'text', label: 'Region' } } };
6769

68-
const permissionSet = (using: string) => ({
70+
const permissionSet = (using: string, policy: Record<string, unknown> = { operation: 'select', using }) => ({
6971
name: 'sales',
7072
label: 'Sales',
7173
objects: { deal: { allowRead: true } },
72-
rowLevelSecurity: [{ name: 'p', label: 'P', object: 'deal', operation: 'select' as const, using }],
74+
rowLevelSecurity: [{ name: 'p', label: 'P', object: 'deal', ...policy }],
7375
});
7476

7577
const RLS = (f: { rule: string }) => f.rule.startsWith('rls-predicate-');
7678

7779
/** `os validate` step 3, in process — see the file header. */
78-
function cliDoor(using: string): AuthoringFinding[] {
80+
function cliDoor(using: string, set = permissionSet(using)): AuthoringFinding[] {
7981
const config = {
8082
manifest: { id: 'com.example.rls', namespace: 'rls', version: '1.0.0', name: 'RLS', type: 'app' },
8183
objects: [deal, account],
82-
permissions: [permissionSet(using)],
84+
permissions: [set],
8385
};
8486
const normalized = normalizeStackInput(config as Record<string, unknown>);
8587
const lowering = lowerCallables(normalized as Record<string, unknown>);
@@ -126,10 +128,10 @@ interface SaveOutcome {
126128
issues: Array<{ rule: string; path: string; message: string }>;
127129
}
128130

129-
async function runtimeDoor(using: string): Promise<SaveOutcome> {
131+
async function runtimeDoor(using: string, set = permissionSet(using)): Promise<SaveOutcome> {
130132
const { protocol } = await runtimeHost();
131133
try {
132-
await protocol.saveMetaItem({ type: 'permission', name: 'sales', item: permissionSet(using) });
134+
await protocol.saveMetaItem({ type: 'permission', name: 'sales', item: set });
133135
return { accepted: true, issues: [] };
134136
} catch (err) {
135137
const e = err as { code?: string; status?: number; issues?: SaveOutcome['issues'] };
@@ -235,3 +237,59 @@ describe('the judge pass binds an app-staged membership key to [] (#20158)', ()
235237
expect(saved.issues.map((i) => i.message)).toEqual([cli[0].message]);
236238
});
237239
});
240+
241+
/**
242+
* [#19886] A field compared with a field that holds a list or an object — a
243+
* `json` field or a `multiple` lookup — is refused when it is AUTHORED, at both
244+
* doors, on every clause. The lowering sees the predicate's text and the
245+
* engine's admission does not judge a `{ $field }` reference against the
246+
* referenced column's type, so before this arm every row below was ACCEPTED at
247+
* both doors (measured) while the runtime refused it: the write check per
248+
* record (400), driver-sql on the read by declared type (400), and the by-id
249+
* update or delete a `using` scopes fails closed (403). The rule judges by the
250+
* DECLARED type its object graph carries; the full operator × clause × class ×
251+
* order table is pinned beside the rule in `@objectstack/lint`.
252+
*/
253+
describe('a field compared with a json / multiple field is refused at both doors, on every clause (#19886)', () => {
254+
const ROWS: ReadonlyArray<{ label: string; clause: 'using' | 'check'; operation: string; predicate: string }> = [
255+
{ label: 'using on select, != a json field', clause: 'using', operation: 'select', predicate: 'record.region != record.tags' },
256+
{ label: 'using on all, the json field first', clause: 'using', operation: 'all', predicate: 'record.tags != record.region' },
257+
{ label: 'using on update, a negated == a multiple lookup', clause: 'using', operation: 'update', predicate: '!(record.owner == record.watchers)' },
258+
{ label: 'using on delete, == a multiple lookup', clause: 'using', operation: 'delete', predicate: 'record.owner == record.watchers' },
259+
{ label: 'check on insert, != a json field', clause: 'check', operation: 'insert', predicate: 'record.region != record.tags' },
260+
{ label: 'check on update, > a multiple lookup', clause: 'check', operation: 'update', predicate: 'record.watchers > record.owner' },
261+
];
262+
const CONTROLS: ReadonlyArray<{ label: string; clause: 'using' | 'check'; operation: string; predicate: string }> = [
263+
{ label: 'using on select, text != text', clause: 'using', operation: 'select', predicate: 'record.region != record.owner' },
264+
{ label: 'check on insert, number > number', clause: 'check', operation: 'insert', predicate: 'record.amount > record.amount' },
265+
{ label: 'using on all, a json field null test', clause: 'using', operation: 'all', predicate: 'record.tags != null' },
266+
];
267+
const setFor = (row: { clause: string; operation: string; predicate: string }) =>
268+
permissionSet('', { operation: row.operation, [row.clause]: row.predicate });
269+
270+
for (const row of ROWS) {
271+
it(`REFUSED at both doors with one sentence — ${row.label}: \`${row.predicate}\``, async () => {
272+
const cli = cliDoor('', setFor(row));
273+
const saved = await runtimeDoor('', setFor(row));
274+
275+
expect(cli.map((f) => ({ severity: f.severity, rule: f.rule, path: f.path }))).toEqual([
276+
{ severity: 'error', rule: UNENFORCEABLE, path: `permissions[0].rowLevelSecurity[0].${row.clause}` },
277+
]);
278+
expect(cli[0].message).toContain('lowers, but compares a field with a field that holds a list or an object');
279+
280+
expect(saved.accepted).toBe(false);
281+
expect({ code: saved.code, status: saved.status }).toEqual({ code: 'INVALID_METADATA', status: 422 });
282+
expect(saved.issues.map((i) => ({ rule: i.rule, path: i.path }))).toEqual([
283+
{ rule: UNENFORCEABLE, path: `permissions.sales.rowLevelSecurity[0].${row.clause}` },
284+
]);
285+
expect(saved.issues[0].message).toBe(cli[0].message);
286+
});
287+
}
288+
289+
for (const row of CONTROLS) {
290+
it(`ACCEPTED at both doors — ${row.label}: \`${row.predicate}\``, async () => {
291+
expect(cliDoor('', setFor(row))).toEqual([]);
292+
expect(await runtimeDoor('', setFor(row))).toEqual({ accepted: true, issues: [] });
293+
});
294+
}
295+
});

0 commit comments

Comments
 (0)