Skip to content

Commit bef47d1

Browse files
committed
feat(spec,lint)!: refuse an RLS or sharing-rule comparison between two fields of different comparison classes when it is authored; regenerate api-surface and export-origins (#20347)
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QcAS3qiYYZNezaxZxaUdMV
1 parent 8c8cf98 commit bef47d1

6 files changed

Lines changed: 122 additions & 10 deletions

File tree

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/lint": minor
4+
---
5+
6+
A row-level-security predicate or a sharing-rule condition that compares two fields of different comparison classes — a text field with a number field, a field with a single image or file field, a field with a formula field — is refused when it is authored, at `os validate` / `os build` / `os lint` and, for a permission set, at the metadata save door (#20347). The classification it is judged by is exported once, from `@objectstack/spec/data`.
7+
8+
**BREAKING** — an accept-set narrowing in `@objectstack/lint`, shipped as `minor` under the repo's launch-window convention for accept-set narrowings. `@objectstack/spec` gains exports only.
9+
10+
Clause-②: yes (narrowing)
11+
12+
`record.status != record.amount` (text vs number) and `record.status != record.photo` (text vs a single image) lower to a legal `{ status: { $ne: { $field: … } } }` filter and hold no list, so no authoring rule refused them. Measured before this change, through the real `os validate` and the real plugin-security and ObjectQL on driver-sql: `os validate` reported both valid; the read a `using` scopes answered `INVALID_FILTER` / 400 and a by-id update or delete it scopes `PERMISSION_DENIED` / 403, because driver-sql compiles a column-to-column comparison only between two columns of one comparison class; and a single-record insert judged by the `check` — or by a `using` standing in as the check — was admitted and stored, because the in-process write check compares the two raw values. A formula field (`record.status != record.is_open`) answered the same three ways. The same-class control (`record.status != record.note`) read, updated, deleted and inserted normally. For a sharing rule, the condition lowers and is seeded, and every criteria query it runs meets the same driver-sql refusal.
13+
14+
What changes:
15+
16+
- `@objectstack/spec/data` (`filter-cross-field-comparison-class.ts`): the cross-field comparison classification. `CROSS_FIELD_COMPARISON_CLASSES` names the six classes (`numeric`, `text`, `boolean`, `date`, `datetime`, `time`); `CROSS_FIELD_NO_CLASS_REASONS` the three families with none (`list-or-object`, `file`, `formula`); `CROSS_FIELD_COMPARISON_TYPE_CLASSES` classifies every `FieldType` member exactly once, by reference to the existing value-class sets; `crossFieldColumnVerdict` answers one declared column (a multi-capable type flagged `multiple: true` holds a list); and `crossFieldComparisonVerdict` answers two (`comparable`, `cross-class`, `no-class`, or `unjudged` for a type outside `FieldType`). It is lifted case for case from driver-sql's cross-field boundary, and a pairwise parity test in driver-sql holds the two equal over every declared field type.
17+
- `@objectstack/lint`: `validateRlsPredicateEnforceability` reports `rls-predicate-unenforceable`, and `validateSharingRuleEnforceability` reports `sharing-rule-unlowerable-condition`, for every lowered field-to-field comparison (`==`, `!=`, `>`, `>=`, `<`, `<=`, either side, under `!` too) whose two declared columns are not `comparable`. It judges `using` and `check` on every operation, and sharing-rule conditions. The finding names each comparison, each column's declared type and class, and the clause's run-time consequence; the hint lists every class with the declared types it holds, read from the spec. A comparison either side of which holds a list or an object stays the existing list-holding finding, and a clause either arm refuses is not also handed to the engine's filter judge, so one defect earns one finding.
18+
19+
Not changed: driver-sql and the in-process write check keep their own behaviour here; moving both onto the exported classification is the engine-lane half. A comparison between two columns of one class (`record.amount > record.budget`, `record.stage == record.account`), a file or formula field compared with a literal or tested against `null`, and any column the stack does not declare or declares with a type outside `FieldType`, are not reported.
20+
21+
No shipped predicate moves: of the 163 `using` / `check` / `condition` string literals in this repository's packages and examples, the 105 that lower hold two field-to-field comparisons, both same-class (`spent > budget`, a hook condition; `a > b`, a gate fixture), and neither is an RLS predicate or a sharing-rule condition.
22+
23+
To keep such a rule, compare a field only with a field of the same class, or with a literal or a `current_user` value; test a file field with `!= null`; or store the value the rule keys on in a field of the right type. If the two columns really hold comparable values, one of them is declared with the wrong type, and the declaration is what to fix.
24+
25+
<!-- adr-0087: not-required (no-migration-prescription) nothing is renamed, retired or respelled: no metadata key, export or operator changes shape and no stored metadata is rewritten, so `objectstack migrate meta` has nothing to do; the author's remedy is to change which two columns a predicate compares, which is a change to the policy they meant, not to a spelling. -->

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

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ const deal = {
6363
account: { type: 'lookup', label: 'Account', reference: 'account' },
6464
tags: { type: 'json', label: 'Tags' },
6565
watchers: { type: 'lookup', label: 'Watchers', reference: 'account', multiple: true },
66+
photo: { type: 'image', label: 'Photo' },
6667
},
6768
};
6869
const account = { name: 'account', label: 'Account', fields: { region: { type: 'text', label: 'Region' } } };
@@ -293,3 +294,60 @@ describe('a field compared with a json / multiple field is refused at both doors
293294
});
294295
}
295296
});
297+
298+
/**
299+
* [#20347] A field compared with a field of ANOTHER comparison class — text vs
300+
* number, text vs a single image, text vs a formula field — is refused when it
301+
* is AUTHORED, at both doors, on every clause. None of these holds a list, so
302+
* the #19886 arm above lets them through; measured before this arm, the real
303+
* `os validate` reported `record.status != record.amount` and
304+
* `record.status != record.photo` valid, while through the real plugin-security
305+
* on driver-sql the read their `using` scopes answered `INVALID_FILTER` / 400
306+
* and the insert their `check` judges was admitted and stored. The rule judges
307+
* by the spec's classification (`crossFieldComparisonVerdict`); the full
308+
* operator × clause × class × order table is pinned beside the rule in
309+
* `@objectstack/lint`.
310+
*/
311+
describe('a field compared with a field of another comparison class is refused at both doors, on every clause (#20347)', () => {
312+
const ROWS: ReadonlyArray<{ label: string; clause: 'using' | 'check'; operation: string; predicate: string }> = [
313+
{ label: 'using on select, text != number', clause: 'using', operation: 'select', predicate: 'record.region != record.amount' },
314+
{ label: 'using on all, text != a single image', clause: 'using', operation: 'all', predicate: 'record.region != record.photo' },
315+
{ label: 'using on select, text != a formula field', clause: 'using', operation: 'select', predicate: 'record.region != record.is_open' },
316+
{ label: 'using on update, number > date', clause: 'using', operation: 'update', predicate: 'record.amount > record.close_date' },
317+
{ label: 'check on insert, text != number', clause: 'check', operation: 'insert', predicate: 'record.region != record.amount' },
318+
{ label: 'check on insert, the image first', clause: 'check', operation: 'insert', predicate: 'record.photo != record.region' },
319+
];
320+
const CONTROLS: ReadonlyArray<{ label: string; clause: 'using' | 'check'; operation: string; predicate: string }> = [
321+
{ label: 'using on select, text != text', clause: 'using', operation: 'select', predicate: 'record.region != record.owner' },
322+
{ label: 'check on insert, a single lookup == text (both text)', clause: 'check', operation: 'insert', predicate: 'record.account == record.owner' },
323+
{ label: 'using on all, an image null test', clause: 'using', operation: 'all', predicate: 'record.photo != null' },
324+
];
325+
const setFor = (row: { clause: string; operation: string; predicate: string }) =>
326+
permissionSet('', { operation: row.operation, [row.clause]: row.predicate });
327+
328+
for (const row of ROWS) {
329+
it(`REFUSED at both doors with one sentence — ${row.label}: \`${row.predicate}\``, async () => {
330+
const cli = cliDoor('', setFor(row));
331+
const saved = await runtimeDoor('', setFor(row));
332+
333+
expect(cli.map((f) => ({ severity: f.severity, rule: f.rule, path: f.path }))).toEqual([
334+
{ severity: 'error', rule: UNENFORCEABLE, path: `permissions[0].rowLevelSecurity[0].${row.clause}` },
335+
]);
336+
expect(cli[0].message).toContain('lowers, but compares two fields that share no comparison class');
337+
338+
expect(saved.accepted).toBe(false);
339+
expect({ code: saved.code, status: saved.status }).toEqual({ code: 'INVALID_METADATA', status: 422 });
340+
expect(saved.issues.map((i) => ({ rule: i.rule, path: i.path }))).toEqual([
341+
{ rule: UNENFORCEABLE, path: `permissions.sales.rowLevelSecurity[0].${row.clause}` },
342+
]);
343+
expect(saved.issues[0].message).toBe(cli[0].message);
344+
});
345+
}
346+
347+
for (const row of CONTROLS) {
348+
it(`ACCEPTED at both doors — ${row.label}: \`${row.predicate}\``, async () => {
349+
expect(cliDoor('', setFor(row))).toEqual([]);
350+
expect(await runtimeDoor('', setFor(row))).toEqual({ accepted: true, issues: [] });
351+
});
352+
}
353+
});

‎packages/lint/src/validate-rls-predicate-enforceability.cross-class-field.test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,8 @@ describe('validateRlsPredicateEnforceability — a field compared with a field o
154154
const using = validateRlsPredicateEnforceability(stackWith({ operation: 'select', using: 'record.status != record.amount' }))[0];
155155
expect(using.message).toContain(
156156
'every read this policy scopes is refused on the SQL drivers (`INVALID_FILTER` / 400: driver-sql refuses the ' +
157-
"comparison by the two columns' declared types).",
157+
"comparison by the two columns' declared types), and every by-id update or delete it scopes fails closed " +
158+
'(`PERMISSION_DENIED` / 403).',
158159
);
159160
expect(using.message).toContain('the same `using` is also the write check whenever no applicable policy');
160161

‎packages/lint/src/validate-rls-predicate-enforceability.ts‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -232,12 +232,17 @@
232232
* `record.status != record.photo` (text vs a single image) lower to legal
233233
* `{ status: { $ne: { $field: … } } }` shapes and hold no list, so the arm above
234234
* lets them through. Measured before this arm, through the real plugin-security
235-
* and ObjectQL on driver-sql: `os validate` reported them valid, the read their
236-
* `using` scopes answered `INVALID_FILTER` / 400 (driver-sql compiles a
237-
* column-to-column comparison only between two columns of ONE comparison
238-
* class, and refuses the file family and formula fields outright), and an
239-
* insert their `check` judges was ADMITTED and stored — the in-process write
240-
* check has no class rule, so the permissive answer sat on the write side of
235+
* and ObjectQL on driver-sql, for those two and for `record.status !=
236+
* record.is_open` (a formula field): `os validate` reported them valid; the
237+
* read their `using` scopes answered `INVALID_FILTER` / 400 (driver-sql
238+
* compiles a column-to-column comparison only between two columns of ONE
239+
* comparison class, and refuses the file family and formula fields outright)
240+
* and a by-id update or delete it scopes `PERMISSION_DENIED` / 403; and an
241+
* insert their `check` judges — or their `using`, standing in as the check —
242+
* was ADMITTED and stored. The in-process write check has no class rule and
243+
* compares the two raw values, so the write answer is whatever that comparison
244+
* happens to give (`record.amount > record.status` was refused 403, because
245+
* `5 > 'open'` is false in JS): the permissive answer sits on the write side of
241246
* an access policy. One policy, three answers.
242247
*
243248
* This arm refuses the comparison where it is written, by the same rule the
@@ -1231,9 +1236,10 @@ function crossClassConsequence(clause: 'using' | 'check'): string {
12311236
'comparison happens to hold — an answer the read path refuses to give';
12321237
return clause === 'using'
12331238
? 'every read this policy scopes is refused on the SQL drivers (`INVALID_FILTER` / 400: driver-sql refuses ' +
1234-
'the comparison by the two columns\' declared types). On an `insert`, `update` or `all` policy the same ' +
1235-
'`using` is also the write check whenever no applicable policy for that operation declares a `check` ' +
1236-
`(ADR-0058 D4), and there ${write}.`
1239+
'the comparison by the two columns\' declared types), and every by-id update or delete it scopes fails ' +
1240+
'closed (`PERMISSION_DENIED` / 403). On an `insert`, `update` or `all` policy the same `using` is also ' +
1241+
'the write check whenever no applicable policy for that operation declares a `check` (ADR-0058 D4), and ' +
1242+
`there ${write}.`
12371243
: `${write[0].toUpperCase()}${write.slice(1)}. The policy reads as a write rule and is enforced by an ` +
12381244
'accident of the two values.';
12391245
}

‎packages/spec/api-surface/data.json‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,9 @@
7474
"CREDENTIAL_KEY_SPELLINGS (const)",
7575
"CREDENTIAL_URL_QUERY_PARAMS (const)",
7676
"CREDENTIAL_URL_QUERY_PARAM_NAMES (const)",
77+
"CROSS_FIELD_COMPARISON_CLASSES (const)",
78+
"CROSS_FIELD_COMPARISON_TYPE_CLASSES (const)",
79+
"CROSS_FIELD_NO_CLASS_REASONS (const)",
7780
"CalendarDateValue (type)",
7881
"CalendarDateValueSchema (const)",
7982
"ClockTimeValue (type)",
@@ -95,6 +98,12 @@
9598
"ContextTokenPlaceholder (type)",
9699
"ContextTokenPlaceholderSchema (const)",
97100
"ContextTokenSchema (const)",
101+
"CrossFieldColumnVerdict (type)",
102+
"CrossFieldComparisonClass (type)",
103+
"CrossFieldComparisonFieldMeta (interface)",
104+
"CrossFieldComparisonTypeClass (interface)",
105+
"CrossFieldComparisonVerdict (type)",
106+
"CrossFieldNoClassReason (type)",
98107
"CrossFieldValidation (type)",
99108
"CrossFieldValidationParsed (type)",
100109
"CrossFieldValidationSchema (const)",
@@ -738,6 +747,8 @@
738747
"credentialFreeMongoOptions (function)",
739748
"credentialFreeUrl (function)",
740749
"credentialQueryParamOf (function)",
750+
"crossFieldColumnVerdict (function)",
751+
"crossFieldComparisonVerdict (function)",
741752
"defaultAggregateFor (function)",
742753
"defaultValueTokenIssue (function)",
743754
"defineCube (function)",

‎packages/spec/export-origins/data.json‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,9 @@
7171
"CREDENTIAL_KEY_SPELLINGS": "src/data/driver/common.zod.ts#CREDENTIAL_KEY_SPELLINGS (const)",
7272
"CREDENTIAL_URL_QUERY_PARAMS": "src/data/driver/common.zod.ts#CREDENTIAL_URL_QUERY_PARAMS (const)",
7373
"CREDENTIAL_URL_QUERY_PARAM_NAMES": "src/data/driver/common.zod.ts#CREDENTIAL_URL_QUERY_PARAM_NAMES (const)",
74+
"CROSS_FIELD_COMPARISON_CLASSES": "src/data/filter-cross-field-comparison-class.ts#CROSS_FIELD_COMPARISON_CLASSES (const)",
75+
"CROSS_FIELD_COMPARISON_TYPE_CLASSES": "src/data/filter-cross-field-comparison-class.ts#CROSS_FIELD_COMPARISON_TYPE_CLASSES (const)",
76+
"CROSS_FIELD_NO_CLASS_REASONS": "src/data/filter-cross-field-comparison-class.ts#CROSS_FIELD_NO_CLASS_REASONS (const)",
7477
"CalendarDateValue": "src/data/field-value.zod.ts#CalendarDateValue (type)",
7578
"CalendarDateValueSchema": "src/data/field-value.zod.ts#CalendarDateValueSchema (const)",
7679
"ClockTimeValue": "src/data/field-value.zod.ts#ClockTimeValue (type)",
@@ -92,6 +95,12 @@
9295
"ContextTokenPlaceholder": "src/data/context-tokens.zod.ts#ContextTokenPlaceholder (type)",
9396
"ContextTokenPlaceholderSchema": "src/data/context-tokens.zod.ts#ContextTokenPlaceholderSchema (const)",
9497
"ContextTokenSchema": "src/data/context-tokens.zod.ts#ContextTokenSchema (const)",
98+
"CrossFieldColumnVerdict": "src/data/filter-cross-field-comparison-class.ts#CrossFieldColumnVerdict (type)",
99+
"CrossFieldComparisonClass": "src/data/filter-cross-field-comparison-class.ts#CrossFieldComparisonClass (type)",
100+
"CrossFieldComparisonFieldMeta": "src/data/filter-cross-field-comparison-class.ts#CrossFieldComparisonFieldMeta (interface)",
101+
"CrossFieldComparisonTypeClass": "src/data/filter-cross-field-comparison-class.ts#CrossFieldComparisonTypeClass (interface)",
102+
"CrossFieldComparisonVerdict": "src/data/filter-cross-field-comparison-class.ts#CrossFieldComparisonVerdict (type)",
103+
"CrossFieldNoClassReason": "src/data/filter-cross-field-comparison-class.ts#CrossFieldNoClassReason (type)",
95104
"CrossFieldValidation": "src/data/validation.zod.ts#CrossFieldValidation (type)",
96105
"CrossFieldValidationParsed": "src/data/validation.zod.ts#CrossFieldValidationParsed (type)",
97106
"CrossFieldValidationSchema": "src/data/validation.zod.ts#CrossFieldValidationSchema (const)",
@@ -725,6 +734,8 @@
725734
"credentialFreeMongoOptions": "src/data/driver/common.zod.ts#credentialFreeMongoOptions (function)",
726735
"credentialFreeUrl": "src/data/driver/common.zod.ts#credentialFreeUrl (function)",
727736
"credentialQueryParamOf": "src/data/driver/common.zod.ts#credentialQueryParamOf (function)",
737+
"crossFieldColumnVerdict": "src/data/filter-cross-field-comparison-class.ts#crossFieldColumnVerdict (function)",
738+
"crossFieldComparisonVerdict": "src/data/filter-cross-field-comparison-class.ts#crossFieldComparisonVerdict (function)",
728739
"defaultAggregateFor": "src/data/aggregation-policy.ts#defaultAggregateFor (function)",
729740
"defaultValueTokenIssue": "src/data/default-value-shape.ts#defaultValueTokenIssue (function)",
730741
"defineCube": "src/data/analytics.zod.ts#defineCube (function)",

0 commit comments

Comments
 (0)