From 7364f0e903aa4b85cdfb6e2602c55875c6d5ec36 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 02:42:25 +0000 Subject: [PATCH 1/3] fix(lint)!: refuse a sharing-rule condition that compares a field with a json or multiple field Stage 2g of #19886 (the sharing-rule twin of stage 2f). The condition lowers, so the seeder seeds the rule, and driver-sql then refuses every criteria query it runs by declared type; SharingRuleService reads the refusal as matching no record, so the rule grants nothing. validateSharingRuleEnforceability now reports it, through the RLS rule's listHoldingComparisons (exported, not copied). Claude-Session: https://claude.ai/code/session_01Rjy9MeetSfq34PKn81CRiN Co-authored-by: Claude --- .../validate-rls-predicate-enforceability.ts | 7 +- ...-enforceability.list-holding-field.test.ts | 252 ++++++++++++++++++ .../validate-sharing-rule-enforceability.ts | 97 ++++++- 3 files changed, 346 insertions(+), 10 deletions(-) create mode 100644 packages/lint/src/validate-sharing-rule-enforceability.list-holding-field.test.ts diff --git a/packages/lint/src/validate-rls-predicate-enforceability.ts b/packages/lint/src/validate-rls-predicate-enforceability.ts index 2c4be42828c..6110324886a 100644 --- a/packages/lint/src/validate-rls-predicate-enforceability.ts +++ b/packages/lint/src/validate-rls-predicate-enforceability.ts @@ -1004,7 +1004,7 @@ function listHoldingDeclaration(meta: GraphField | undefined): string | null { } /** One lowered comparison between two columns, at least one of which holds a list or an object. */ -interface ListHoldingComparison { +export interface ListHoldingComparison { /** The comparison as the author wrote it, back in CEL. */ written: string; /** Each list-holding column, with what it is declared as. */ @@ -1017,8 +1017,11 @@ interface ListHoldingComparison { * the object graph. A column the graph cannot answer for (an object outside the * stack, no field map, a name it does not declare) is not judged here; the * reference pass above owns an unknown name. + * + * Exported for `validate-sharing-rule-enforceability.ts`, which judges the same + * class on a sharing rule's lowered `condition`: one classification, two rules. */ -function listHoldingComparisons( +export function listHoldingComparisons( graph: ObjectGraph, object: string, filter: Record, diff --git a/packages/lint/src/validate-sharing-rule-enforceability.list-holding-field.test.ts b/packages/lint/src/validate-sharing-rule-enforceability.list-holding-field.test.ts new file mode 100644 index 00000000000..05a76548e66 --- /dev/null +++ b/packages/lint/src/validate-sharing-rule-enforceability.list-holding-field.test.ts @@ -0,0 +1,252 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#19886] A sharing rule whose `condition` compares a field with a field that + * holds a LIST or an OBJECT is refused at authoring time, judged by the + * DECLARED type the stack's object graph carries — the sharing-rule twin of the + * RLS arm pinned in `validate-rls-predicate-enforceability.list-holding-field.test.ts`, + * through the same classification (`listHoldingComparisons`). + * + * `record.status != record.tags` (`tags` a `json` field or a `multiple` lookup) + * lowers to a legal `{ status: { $ne: { $field: 'tags' } } }`, so the seeder + * seeds the rule — and every criteria query it runs is then refused by + * driver-sql by declared type (`INVALID_FILTER` / 400), which + * `SharingRuleService` reads as "matches no record". Measured through the real + * plugin-sharing: the rule is seeded and grants nothing, at boot and on every + * later write. Before this arm every cell of the table below was clean at + * `os validate`. + * + * One table: operator × column class × operand order, then the scalar + * field-to-field controls, which stay clean. + */ + +import { describe, expect, it } from 'vitest'; + +import { + validateSharingRuleEnforceability, + SHARING_RULE_UNLOWERABLE_CONDITION, +} from './validate-sharing-rule-enforceability.js'; +import { validateRlsPredicateEnforceability } from './validate-rls-predicate-enforceability.js'; +import { runAuthoringRules } from './authoring-rules.js'; + +const deal = { + name: 'deal', + label: 'Deal', + sharingModel: 'private', + fields: { + status: { type: 'text', label: 'Status' }, + owner_name: { type: 'text', label: 'Owner name' }, + amount: { type: 'number', label: 'Amount' }, + budget: { type: 'number', label: 'Budget' }, + close_date: { type: 'date', label: 'Close date' }, + signed_date: { type: 'date', label: 'Signed date' }, + account: { type: 'lookup', label: 'Account', reference: 'account' }, + stage: { type: 'select', label: 'Stage', options: [{ label: 'Open', value: 'open' }] }, + tags: { type: 'json', label: 'Tags' }, + reviewers: { type: 'lookup', label: 'Reviewers', reference: 'account', multiple: true }, + }, +}; +const account = { name: 'account', label: 'Account', sharingModel: 'private', fields: { region: { type: 'text', label: 'Region' } } }; + +/** One criteria sharing rule on `deal`, the condition swapped in. */ +const stackWith = (condition: unknown, objects: unknown[] = [deal, account], extra: Record = {}) => ({ + objects, + sharingRules: [ + { + name: 'deal_desk_share', + type: 'criteria', + object: 'deal', + accessLevel: 'read', + sharedWith: { type: 'team', value: 'deal_desk' }, + condition, + ...extra, + }, + ], +}); + +/** Each operator an author can write between two fields, and what the finding quotes back. */ +const OPERATORS: ReadonlyArray<{ op: string; spell: (a: string, b: string) => string; quoted: string }> = [ + { op: '!=', spell: (a, b) => `record.${a} != record.${b}`, quoted: '!=' }, + { op: '!(==)', spell: (a, b) => `!(record.${a} == record.${b})`, quoted: '==' }, + { op: '==', spell: (a, b) => `record.${a} == record.${b}`, quoted: '==' }, + { op: '>', spell: (a, b) => `record.${a} > record.${b}`, quoted: '>' }, + { op: '>=', spell: (a, b) => `record.${a} >= record.${b}`, quoted: '>=' }, + { op: '<', spell: (a, b) => `record.${a} < record.${b}`, quoted: '<' }, + { op: '<=', spell: (a, b) => `record.${a} <= record.${b}`, quoted: '<=' }, +]; + +const COLUMNS = [ + { column: 'a json field', field: 'tags', declared: "`tags` is declared `type: 'json'`" }, + { column: 'a multiple lookup', field: 'reviewers', declared: "`reviewers` is declared `type: 'lookup'`, `multiple: true`" }, +] as const; + +/** The family's class sentence — the RLS arm's words, kept identical. */ +const 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, so the platform refuses the comparison instead of evaluating it'; + +/** The measured runtime consequence on the sharing path. */ +const CONSEQUENCE = + 'the rule is seeded into `sys_sharing_rule`, but every criteria query it runs 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 `SharingRuleService` reads a refused query as matching no record. No `sys_record_share` grant is ' + + 'ever materialised, at boot or on any later write, and the only signal is a WARN line in the server log. The ' + + 'rule is declared and grants nothing.'; + +describe('validateSharingRuleEnforceability — a condition comparing a field with a json / multiple field is REFUSED (#19886)', () => { + for (const { op, spell, quoted } of OPERATORS) { + for (const { column, field, declared } of COLUMNS) { + for (const order of ['scalar first', 'list first'] as const) { + const [left, right] = order === 'scalar first' ? ['status', field] : [field, 'status']; + const condition = spell(left, right); + it(`${op} · ${column} · ${order}: \`${condition}\``, () => { + const findings = validateSharingRuleEnforceability(stackWith(condition)); + expect(findings.map((f) => ({ severity: f.severity, rule: f.rule, path: f.path, where: f.where }))).toEqual([ + { + severity: 'error', + rule: SHARING_RULE_UNLOWERABLE_CONDITION, + path: 'sharingRules[0].condition', + where: 'sharing rule "deal_desk_share" on object "deal"', + }, + ]); + expect(findings[0].message).toBe( + `Sharing-rule condition \`${condition}\` lowers, but compares a field with a field that holds a list or ` + + `an object: \`record.${left} ${quoted} record.${right}\`, where ${declared}. ${CLASS_SENTENCE}: ${CONSEQUENCE}`, + ); + expect(findings[0].hint).toMatch(/^A field compared with a `json` or `multiple` field has no row-filter form/); + }); + } + } + } + + it('the table covers every field-to-field operator, both classes, both orders', () => { + expect(OPERATORS.length * COLUMNS.length * 2).toBe(28); + }); + + it('the parsed tier (`{ dialect, source }`) is refused identically', () => { + const findings = validateSharingRuleEnforceability(stackWith({ dialect: 'cel', source: 'record.status != record.tags' })); + expect(findings.map((f) => f.rule)).toEqual([SHARING_RULE_UNLOWERABLE_CONDITION]); + expect(findings[0].message).toContain('Sharing-rule condition `record.status != record.tags` lowers'); + }); + + it('an offending comparison inside a compound condition is refused, and a second one is named in the same finding', () => { + const one = validateSharingRuleEnforceability(stackWith("record.stage == 'open' && record.status != record.tags")); + expect(one.map((f) => f.rule)).toEqual([SHARING_RULE_UNLOWERABLE_CONDITION]); + + const two = validateSharingRuleEnforceability(stackWith('record.status != record.tags || record.reviewers == record.tags')); + expect(two).toHaveLength(1); + expect(two[0].message).toContain( + "object: `record.status != record.tags`, where `tags` is declared `type: 'json'`; " + + "`record.reviewers == record.tags`, where `reviewers` is declared `type: 'lookup'`, `multiple: true` and " + + "`tags` is declared `type: 'json'`. ", + ); + }); + + it('an inactive rule is judged too — the seeder compiles and seeds it regardless of `active`', () => { + const findings = validateSharingRuleEnforceability(stackWith('record.status != record.tags', [deal, account], { active: false })); + expect(findings.map((f) => f.rule)).toEqual([SHARING_RULE_UNLOWERABLE_CONDITION]); + }); + + it('speaks the RLS arm\'s class sentence, word for word — one class, one sentence, two surfaces', () => { + const rls = validateRlsPredicateEnforceability({ + objects: [deal, account], + permissions: [{ name: 'sales', label: 'Sales', rowLevelSecurity: [{ name: 'p', object: 'deal', operation: 'select', using: 'record.status != record.tags' }] }], + }); + const sharing = validateSharingRuleEnforceability(stackWith('record.status != record.tags')); + expect(rls[0].message).toContain(`${CLASS_SENTENCE}: `); + expect(sharing[0].message).toContain(`${CLASS_SENTENCE}: `); + }); +}); + +describe('validateSharingRuleEnforceability — every declared list-or-object class, read from the spec (#19886)', () => { + // The spec's two value-shape classes: `STRUCTURED_JSON_TYPES` and + // `isMultiValueField` (an inherently-multi option type, or a multi-capable + // type flagged `multiple: true`) — the same table the RLS arm pins. + const LIST_HOLDING: ReadonlyArray<[string, Record, string]> = [ + ['json', { type: 'json' }, "`type: 'json'`"], + ['composite', { type: 'composite' }, "`type: 'composite'`"], + ['repeater', { type: 'repeater' }, "`type: 'repeater'`"], + ['record', { type: 'record' }, "`type: 'record'`"], + ['location', { type: 'location' }, "`type: 'location'`"], + ['address', { type: 'address' }, "`type: 'address'`"], + ['vector', { type: 'vector' }, "`type: 'vector'`"], + ['multiselect', { type: 'multiselect' }, "`type: 'multiselect'`"], + ['checkboxes', { type: 'checkboxes' }, "`type: 'checkboxes'`"], + ['tags', { type: 'tags' }, "`type: 'tags'`"], + ['multiple select', { type: 'select', multiple: true }, "`type: 'select'`, `multiple: true`"], + ['multiple radio', { type: 'radio', multiple: true }, "`type: 'radio'`, `multiple: true`"], + ['multiple lookup', { type: 'lookup', reference: 'account', multiple: true }, "`type: 'lookup'`, `multiple: true`"], + ['multiple user', { type: 'user', multiple: true }, "`type: 'user'`, `multiple: true`"], + ['multiple file', { type: 'file', multiple: true }, "`type: 'file'`, `multiple: true`"], + ['multiple image', { type: 'image', multiple: true }, "`type: 'image'`, `multiple: true`"], + ]; + for (const [label, def, declared] of LIST_HOLDING) { + it(`${label}: refused, naming the declaration`, () => { + const objects = [{ ...deal, fields: { ...deal.fields, subject: { label: 'Subject', ...def } } }, account]; + const findings = validateSharingRuleEnforceability(stackWith('record.status != record.subject', objects)); + expect(findings.map((f) => f.rule)).toEqual([SHARING_RULE_UNLOWERABLE_CONDITION]); + expect(findings[0].message).toContain(`\`record.status != record.subject\`, where \`subject\` is declared ${declared}.`); + }); + } +}); + +describe('validateSharingRuleEnforceability — the one-value spellings stay CLEAN (#19886 controls)', () => { + const CONTROLS: ReadonlyArray<[string, string]> = [ + ['text != text', 'record.status != record.owner_name'], + ['text == text', 'record.status == record.owner_name'], + ['!(text == text)', '!(record.status == record.owner_name)'], + ['number > number', 'record.amount > record.budget'], + ['date <= date', 'record.close_date <= record.signed_date'], + ['a single lookup == text', 'record.account == record.owner_name'], + ['a single select != text', 'record.stage != record.status'], + ['a json field against a literal (not a field-to-field comparison)', "record.tags == 'a'"], + ['a json field null test', 'record.tags != null'], + ['a flat literal list', "record.status in ['open', 'pending']"], + ]; + for (const [label, condition] of CONTROLS) { + it(`${label}: \`${condition}\``, () => { + expect(validateSharingRuleEnforceability(stackWith(condition))).toEqual([]); + }); + } + + it('a single-valued field of a multi-capable type is one value: `select`, `lookup`, `user`, `file`', () => { + for (const def of [{ type: 'select' }, { type: 'lookup', reference: 'account' }, { type: 'user' }, { type: 'file' }]) { + const objects = [{ ...deal, fields: { ...deal.fields, subject: { label: 'Subject', ...def } } }, account]; + expect(validateSharingRuleEnforceability(stackWith('record.status != record.subject', objects)), def.type).toEqual([]); + } + }); +}); + +describe('validateSharingRuleEnforceability — the arm is the graph\'s, and reports once (#19886)', () => { + it('judges nothing the graph cannot answer: an anchor outside the stack, a field map it cannot read, an undeclared name', () => { + expect(validateSharingRuleEnforceability(stackWith('record.status != record.tags', [account]))).toEqual([]); + expect(validateSharingRuleEnforceability(stackWith('record.status != record.tags', [{ name: 'deal', sharingModel: 'private' }]))).toEqual([]); + expect(validateSharingRuleEnforceability(stackWith('record.status != record.nope'))).toEqual([]); + }); + + it('one defect, one finding: a condition the compiler already refuses keeps its own finding alone', () => { + const findings = validateSharingRuleEnforceability(stackWith('size(record.tags) > 0 && record.status != record.tags')); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(SHARING_RULE_UNLOWERABLE_CONDITION); + expect(findings[0].message).toMatch(/is outside the pushdown subset/); + expect(findings[0].message).not.toMatch(/lowers, but compares/); + }); + + it('reports beside the anchor arm, independently — two fields, two findings', () => { + const publicDeal = { ...deal, sharingModel: 'public_read_write' }; + const findings = validateSharingRuleEnforceability(stackWith('record.status != record.tags', [publicDeal, account])); + expect(findings.map((f) => [f.rule, f.path])).toEqual([ + ['sharing-rule-object-not-shareable', 'sharingRules[0].object'], + [SHARING_RULE_UNLOWERABLE_CONDITION, 'sharingRules[0].condition'], + ]); + }); + + it('reaches the author through `os validate`\'s rule table', () => { + const stack = stackWith('record.reviewers == record.status'); + const cli = runAuthoringRules('validate', { normalized: stack, parsed: stack }).filter((f) => f.rule.startsWith('sharing-rule-')); + expect(cli.map((f) => ({ rule: f.rule, path: f.path }))).toEqual([ + { rule: SHARING_RULE_UNLOWERABLE_CONDITION, path: 'sharingRules[0].condition' }, + ]); + expect(cli[0].message).toContain('`record.reviewers == record.status`, where `reviewers` is declared'); + }); +}); diff --git a/packages/lint/src/validate-sharing-rule-enforceability.ts b/packages/lint/src/validate-sharing-rule-enforceability.ts index 9c7fd987f44..f13d112c33e 100644 --- a/packages/lint/src/validate-sharing-rule-enforceability.ts +++ b/packages/lint/src/validate-sharing-rule-enforceability.ts @@ -108,6 +108,34 @@ * vocabularies, would make the second report noise — and the first one is * the better message, because it is written about syntax. * + * ## A field compared with a field that holds a list or an object (#19886) + * + * `record.status != record.tags`, with `tags` a `json` field or a `multiple` + * lookup, LOWERS — to `{ status: { $ne: { $field: 'tags' } } }` — because the + * compiler knows the predicate's text and not the object's field types. So the + * seeder's verdict is `ok` and the rule is seeded. The runtime refuses it one + * step later. Measured through the real `SharingServicePlugin` (seeding, hook + * binding, the boot backfill) on driver-sql and driver-sqlite-wasm: every + * criteria query such a rule runs answers `INVALID_FILTER` / 400, because + * driver-sql refuses a cross-field comparison against such a column by its + * DECLARED type; `SharingRuleService` reads the refused query as matching no + * record; and the rule grants nothing, at boot and on every later write, behind + * one WARN line per rule. The scalar field-to-field controls granted and + * enforced in the same run. (driver-memory reads a `{ $field }` comparand as a + * literal for EVERY field-to-field comparison, scalar ones included, so it is + * no evidence either way.) + * + * The compiler cannot see this, so this arm is the one place the rule judges + * more than the seeder's call: it reads the LOWERED filter against the declared + * field map, through the same classification the RLS rule uses + * (`listHoldingComparisons`, which reads the spec's `STRUCTURED_JSON_TYPES` and + * `isMultiValueField`, the two sets driver-sql refuses by). It keeps the + * unlowerable id: the fix is the same rewrite of the predicate, and the same + * class's literal spelling (`record.status == ['a', 'b']`) is already reported + * under that id, refused by the compiler rather than by the driver. A column + * the graph cannot answer for — an object this stack does not declare, a field + * map it cannot read, a name it does not declare — is not judged here. + * * ## What this rule deliberately does NOT do * * - **It does not re-implement `isMatchAllCriteria`.** The seeder's second @@ -138,9 +166,14 @@ import { compileCelToFilter } from '@objectstack/formula'; import { referenceCarrierOf } from '@objectstack/spec/data'; -import { recordsOf } from './object-graph.js'; +import { indexObjectGraph, recordsOf, type ObjectGraph } from './object-graph.js'; +import { listHoldingComparisons } from './validate-rls-predicate-enforceability.js'; -/** A `condition` outside the pushdown subset — the rule is never seeded. */ +/** + * A `condition` the runtime cannot evaluate as written: outside the pushdown + * subset (the rule is never seeded), or a lowered comparison with a field that + * holds a list or an object (seeded, and it grants nothing). + */ export const SHARING_RULE_UNLOWERABLE_CONDITION = 'sharing-rule-unlowerable-condition'; /** A `condition` reading `current_user.*` — unresolvable when grants are materialized. */ export const SHARING_RULE_RUNTIME_VARIABLE_CONDITION = 'sharing-rule-runtime-variable-condition'; @@ -206,6 +239,43 @@ const PUSHDOWN_SUBSET = 'and the string methods `startsWith` / `endsWith` / `contains` — over SINGLE-column `record.` ' + 'paths (ADR-0058 D2).'; +/** + * The finding for a lowered `condition` that compares a field with a field + * holding a list or an object, or `null` when it compares none (see this + * file's header). The consequence is the one measured through the real + * plugin-sharing on the SQL drivers. + */ +function listHoldingFinding( + graph: ObjectGraph, + object: string, + filter: Record, + at: { where: string; path: string; source: string }, +): SharingRuleEnforceabilityFinding | null { + const comparisons = object ? listHoldingComparisons(graph, object, filter) : []; + if (comparisons.length === 0) return null; + return { + severity: 'error', + rule: SHARING_RULE_UNLOWERABLE_CONDITION, + where: at.where, + path: at.path, + message: + `Sharing-rule condition \`${at.source}\` lowers, but compares a field with a field that holds a list or ` + + `an object: ${comparisons.map((c) => `\`${c.written}\`, where ${c.columns.join(' and ')}`).join('; ')}. ` + + '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: the rule ' + + 'is seeded into `sys_sharing_rule`, but every criteria query it runs 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 `SharingRuleService` reads a refused query as matching no record. No ' + + '`sys_record_share` grant is ever materialised, at boot or on any later write, and the only signal is ' + + 'a WARN line in the server log. The rule is declared and grants nothing.', + 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 — \"one of these values\" is `record.status in ['open', " + + "'pending']` — or keep the value the rule keys on in a single-valued field and compare with that.", + }; +} + /** * The object's effective sharing model, as `SharingService` computes it. * @@ -430,6 +500,8 @@ export function validateSharingRuleEnforceability(stack: unknown): SharingRuleEn const name = str(obj.name); if (name) objectsByName.set(name, obj); } + // [#19886] The declared field map, for the list-holding arm, built once. + const graph = indexObjectGraph(cfg); recordsOf(cfg.sharingRules).forEach((rule, index) => { anchorFindings(rule, index, objectsByName).forEach((f) => findings.push(f)); @@ -437,21 +509,30 @@ export function validateSharingRuleEnforceability(stack: unknown): SharingRuleEn const input = toCompilerInput(rule.condition); if (input === null) return; + const name = str(rule.name) || String(index); + const object = str(rule.object); + const where = `sharing rule "${name}"${object ? ` on object "${object}"` : ''}`; + const path = `sharingRules[${index}].condition`; + const source = sourceOf(rule.condition); + // The seeder's exact call: `compileCelToFilter(r.condition, { variables: {} })` // in `bootstrap-declared-sharing-rules.ts`. Same function, same options — // so `ok === false` here means "this rule will be skipped at boot", not // "this rule looks suspicious". const result = compileCelToFilter(input, { variables: {} }); - if (result.ok) return; + if (result.ok) { + // [#19886] Seeded — and refused by the driver on every criteria query + // when it compares with a list- or object-holding field (file header). + const listHolding = listHoldingFinding(graph, object, result.filter as Record, { + where, path, source, + }); + if (listHolding) findings.push(listHolding); + return; + } // Syntax belongs to `validateStackExpressions`, which already gates this // same field with a message written about syntax. if (result.reason === 'parse-error') return; - const name = str(rule.name) || String(index); - const object = str(rule.object); - const where = `sharing rule "${name}"${object ? ` on object "${object}"` : ''}`; - const path = `sharingRules[${index}].condition`; - const source = sourceOf(rule.condition); const skipped = 'so `bootstrapDeclaredSharingRules` SKIPS the rule at boot: it is never written to ' + '`sys_sharing_rule`, no `sys_record_share` grant is ever materialised, and the only signal is one ' + From 53275286b5f64c47986e26c03d463afaadbcc29e Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 02:43:39 +0000 Subject: [PATCH 2/3] chore(changeset): @objectstack/lint minor for the sharing-rule list-holding arm Clause-2: no (narrowing), BREAKING; ADR-0087 not-required (already-registered cel-predicate-one-value-comparand-refused). Claude-Session: https://claude.ai/code/session_01Rjy9MeetSfq34PKn81CRiN Co-authored-by: Claude --- ...list-holding-field-comparison-authoring.md | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 .changeset/19886-sharing-rule-list-holding-field-comparison-authoring.md diff --git a/.changeset/19886-sharing-rule-list-holding-field-comparison-authoring.md b/.changeset/19886-sharing-rule-list-holding-field-comparison-authoring.md new file mode 100644 index 00000000000..5ee212ef059 --- /dev/null +++ b/.changeset/19886-sharing-rule-list-holding-field-comparison-authoring.md @@ -0,0 +1,24 @@ +--- +"@objectstack/lint": minor +--- + +A sharing rule whose `condition` compares a field with a `json` or `multiple` field is refused when it is authored, at `os validate` / `os build` / `os lint`, instead of being seeded and then granting nothing (#19886). + +**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`, whose surface names `sharingRules[].condition` and this class. + +Clause-②: no (narrowing) + +`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 condition's text and not the object's field types, so the seeder seeds the rule. Measured before this change: 72 conditions (`==`, `!=`, `!(==)`, `>`, `>=`, `<`, `<=`; a `json`, `address`, `multiselect`, `multiple` lookup and `multiple` user field; both operand orders; plus a list-against-list and a compound spelling) were all accepted by the real `os validate`. The runtime refused every one of them, measured through the real plugin-sharing on driver-sql and driver-sqlite-wasm: the rule was seeded into `sys_sharing_rule`, every criteria query it ran answered `INVALID_FILTER` / 400, `SharingRuleService` read that as matching no record, and no `sys_record_share` grant was written, at boot or on a later insert or update. The recipient read nothing. The only signal was one WARN line per rule in the server log. + +What changes: + +- `@objectstack/lint`: `validateSharingRuleEnforceability` reports `sharing-rule-unlowerable-condition` for a condition that lowers but compares two fields (`==`, `!=`, `>`, `>=`, `<`, `<=`, on either side, under `!` too) where either column is DECLARED to hold a list or an object. It uses the same classification as the row-level-security rule's arm for this class (`listHoldingComparisons`, now exported from `validate-rls-predicate-enforceability.ts`), which reads 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`). The finding names each comparison and the declaration behind it, and states the run-time consequence. It keeps the unlowerable id because the fix is the same rewrite of the condition, and the literal spelling of the same class (`record.status == ['a', 'b']`) is already reported under that id. +- Inactive rules are judged too, as the rule already does for every condition: the seeder seeds them regardless of `active`. + +Not changed: a field compared with a single-valued field (`record.status != record.owner_name`, `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 anchor object from another package, an object with no field map, an undeclared name), which the rule does not judge. No row-level-security verdict changes. The rule still runs only at the CLI doors; the metadata save door for a `sharing_rule` does not run it, as before. + +No shipped condition moves: the 3 declared sharing-rule conditions in this repository's packages and examples compare a field with a literal, the cloud repository declares none, and the real `os validate` over `app-crm`, `app-multi-package`, `app-showcase` and `app-todo` reports no new `sharing-rule-*` finding. + +**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 ("one of these values" is `record.status in ['open', 'pending']`), or keep the value the rule keys on in a single-valued field and compare with that. + + From 79a2ad1e5146fb987c280fa585bd93b7b963580a Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 03:16:48 +0000 Subject: [PATCH 3/3] fix(lint): index only a lowering rule's anchor for the sharing list-holding arm Building the whole stack's object graph up front threw on an unreadable reference carrier before the anchor arm could refuse it with its own label, flipping the #18550 pin. The arm now indexes the rule's anchor lazily, once, and only for a condition that lowered: the only object a lowered sharing criterion can address. Claude-Session: https://claude.ai/code/session_01Rjy9MeetSfq34PKn81CRiN Co-authored-by: Claude --- .../validate-sharing-rule-enforceability.ts | 28 +++++++++++++++---- 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/packages/lint/src/validate-sharing-rule-enforceability.ts b/packages/lint/src/validate-sharing-rule-enforceability.ts index f13d112c33e..a95f00af123 100644 --- a/packages/lint/src/validate-sharing-rule-enforceability.ts +++ b/packages/lint/src/validate-sharing-rule-enforceability.ts @@ -251,7 +251,7 @@ function listHoldingFinding( filter: Record, at: { where: string; path: string; source: string }, ): SharingRuleEnforceabilityFinding | null { - const comparisons = object ? listHoldingComparisons(graph, object, filter) : []; + const comparisons = listHoldingComparisons(graph, object, filter); if (comparisons.length === 0) return null; return { severity: 'error', @@ -500,8 +500,23 @@ export function validateSharingRuleEnforceability(stack: unknown): SharingRuleEn const name = str(obj.name); if (name) objectsByName.set(name, obj); } - // [#19886] The declared field map, for the list-holding arm, built once. - const graph = indexObjectGraph(cfg); + // [#19886] The declared field map of a rule's ANCHOR, for the list-holding + // arm — the only object a lowered sharing criterion can address (a + // cross-object path does not lower). Built lazily, once per anchor, and only + // for a condition that lowered, so a stack this arm never judges is indexed + // exactly as before and the anchor arm's own refusal of an unreadable carrier + // keeps its precedence. + const graphs = new Map(); + const anchorGraph = (object: string): ObjectGraph | null => { + const target = objectsByName.get(object); + if (!target) return null; + let graph = graphs.get(object); + if (!graph) { + graph = indexObjectGraph({ objects: [target] }); + graphs.set(object, graph); + } + return graph; + }; recordsOf(cfg.sharingRules).forEach((rule, index) => { anchorFindings(rule, index, objectsByName).forEach((f) => findings.push(f)); @@ -523,9 +538,10 @@ export function validateSharingRuleEnforceability(stack: unknown): SharingRuleEn if (result.ok) { // [#19886] Seeded — and refused by the driver on every criteria query // when it compares with a list- or object-holding field (file header). - const listHolding = listHoldingFinding(graph, object, result.filter as Record, { - where, path, source, - }); + const graph = object ? anchorGraph(object) : null; + const listHolding = graph + ? listHoldingFinding(graph, object, result.filter as Record, { where, path, source }) + : null; if (listHolding) findings.push(listHolding); return; }