diff --git a/.changeset/sharing-rule-unlowerable-condition-gate.md b/.changeset/sharing-rule-unlowerable-condition-gate.md new file mode 100644 index 0000000000..7ddbee239a --- /dev/null +++ b/.changeset/sharing-rule-unlowerable-condition-gate.md @@ -0,0 +1,58 @@ +--- +"@objectstack/lint": minor +--- + +feat(lint): reject a sharing-rule condition the runtime can only skip (#4698) + +#4698 reported the same failure shape three times in one app in one day: a key +that is authored, is schema-valid, reads as meaningful — and is never consumed +by the runtime. Every check verifies that what is declared is *well-formed*, +never that it is *read*. The issue's third measured instance is a sharing rule +whose CEL `condition` uses `has(...)`: the seeder cannot lower it, skips the +rule, and the only signal is one WARN line at boot. The rule exists in +metadata, is absent from `sys_sharing_rule`, and grants nothing. + +**New rules, both `error`, on all three authoring commands:** + +- **`sharing-rule-unlowerable-condition`** — the condition is outside the + pushdown subset: a function call (`has(...)`, `size(...)`), arithmetic, a + ternary, or a cross-object path (`record.account.region`). +- **`sharing-rule-runtime-variable-condition`** — the condition reads + `current_user.*`. Criteria sharing rules are materialised (one static + `criteria_json` per rule, from which grants are written), so there is no + "current user" at compile time. The fix is a different mechanism, not a + different spelling, which is why it has its own id. + +Fix each by rewriting the predicate inside the lowerable subset — `==` `!=` +`>` `<` `>=` `<=`, `in`, `&&` `||` `!`, `== null` / `!= null`, and +`startsWith` / `endsWith` / `contains` over single-column `record.` +paths (ADR-0058 D2). Two specific migrations: `has(record.x)` → `record.x != +null` (`has()` is correct in an object *validation* rule, which is +interpreted, and wrong here, where the condition is compiled); and a related +record's field → denormalise it onto this object (formula/rollup) and test +that column, or share the related object instead. For per-user access, use an +RLS policy (`rowLevelSecurity[].using`), where `current_user.*` *is* resolved. + +**Why this one surface and not "unread keys" in general.** "Is this key read?" +is only a lint question when the answer is computable from the authored +metadata alone, and usually it is not — a repo-wide grep for a reader is not +evidence of absence, and a consumer may live in another package, another repo, +or an uninstalled plugin. A sharing rule's `condition` is the case where the +predicate is exact: its one runtime consumer +(`bootstrapDeclaredSharingRules`) does exactly one thing with the key — +`compileCelToFilter(condition, { variables: {} })` — and a condition that does +not lower means the rule is skipped outright. So the lint calls that same +compiler, from the same package, with the same options, instead of modelling +the consumer; the verdict is identical to the seeder's by construction and is +pinned in both directions by a test over a shared corpus. + +`error` rather than advisory, per the ADR-0078 claim `SharingRuleSchema`'s own +docblock makes ("the whole authorable surface is enforced — nothing here +validates and then silently does nothing"): there is no reading under which an +unlowerable condition does what it says. It fails closed, which is why it was +survivable, not why it was acceptable. Measured before shipping: every +sharing-rule condition declared anywhere in this repo lowers cleanly, so the +gate turns nothing red that works today. + +CEL *syntax* errors are deliberately left to `expression-invalid`, which +already gates this same field with a message written about syntax. diff --git a/packages/lint/src/authoring-rules.ts b/packages/lint/src/authoring-rules.ts index ac6b36d03c..dbf41dde34 100644 --- a/packages/lint/src/authoring-rules.ts +++ b/packages/lint/src/authoring-rules.ts @@ -120,6 +120,7 @@ import { validateSeedStateMachine } from './validate-seed-state-machine.js'; import { validateVisibilityPredicates } from './validate-visibility-predicates.js'; import { validateSecurityPosture } from './validate-security-posture.js'; import { validateOrgAxisRedLines } from './validate-org-axis-red-lines.js'; +import { validateSharingRuleEnforceability } from './validate-sharing-rule-enforceability.js'; import { validateActionLocations } from './validate-action-locations.js'; import { lintFlowPatterns } from './lint-flow-patterns.js'; import { lintLivenessProperties } from './lint-liveness-properties.js'; @@ -795,6 +796,31 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ surfaceReason: RUNTIME_NEEDS_FULL_SNAPSHOT, run: (stack) => validateOrgAxisRedLines(stack), }, + // #4698 — the "declared but never read" gate, for the one surface where the + // predicate is EXACT rather than inferred. A sharing rule's `condition` has a + // single runtime consumer (`bootstrapDeclaredSharingRules`) whose only use of + // the key is `compileCelToFilter(condition, { variables: {} })`; a condition + // that does not lower means the rule is SKIPPED at boot, so the grant is + // declared and does not exist. The lint calls that same compiler, from the + // same package, with the same options — the verdict cannot drift from the + // consumer's. Gating for the ADR-0078 reason `SharingRuleSchema`'s own + // docblock states: the whole authorable surface is enforced, and this was the + // one field where that sentence was not yet true. + { + name: 'validateSharingRuleEnforceability', + tier: 'gating', + input: 'parsed', + commands: ALL, + source: 'packages/lint/src/validate-sharing-rule-enforceability.ts', + surfaces: CLI_ONLY, + surfaceReason: + 'P2 (#4463): a sharing rule is not a `flow`, and P1 gates `flow` alone. The rule itself is ' + + 'snapshot-safe — it reads ONLY `stack.sharingRules[].condition` and needs no other collection — ' + + 'so widening it here is a `runtimeTypes: [\'sharing_rule\']` edit once the gate accepts that type, ' + + 'not new wiring. Recorded as pending rather than done, because a rule that has never run at a ' + + 'door should not claim it.', + run: (stack) => validateSharingRuleEnforceability(stack), + }, ]; // ─── Runner ───────────────────────────────────────────────────────── diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index 1ce022c37d..5743f01a25 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -195,6 +195,20 @@ export { } from './validate-org-axis-red-lines.js'; export type { OrgAxisFinding, OrgAxisSeverity } from './validate-org-axis-red-lines.js'; +// #4698 — "a key that nothing reads should not validate clean", for the one +// surface where "is it read?" is decidable: a sharing rule's `condition` is +// read ONLY through `compileCelToFilter`, so the lint calls that same compiler +// rather than modelling the consumer. +export { + validateSharingRuleEnforceability, + SHARING_RULE_UNLOWERABLE_CONDITION, + SHARING_RULE_RUNTIME_VARIABLE_CONDITION, +} from './validate-sharing-rule-enforceability.js'; +export type { + SharingRuleEnforceabilityFinding, + SharingRuleEnforceabilitySeverity, +} from './validate-sharing-rule-enforceability.js'; + export { validateDashboardActionRefs, DASHBOARD_ACTION_TARGET_UNDEFINED, diff --git a/packages/lint/src/validate-sharing-rule-enforceability.test.ts b/packages/lint/src/validate-sharing-rule-enforceability.test.ts new file mode 100644 index 0000000000..386a28acbc --- /dev/null +++ b/packages/lint/src/validate-sharing-rule-enforceability.test.ts @@ -0,0 +1,240 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import { compileCelToFilter } from '@objectstack/formula'; + +import { + validateSharingRuleEnforceability, + SHARING_RULE_UNLOWERABLE_CONDITION, + SHARING_RULE_RUNTIME_VARIABLE_CONDITION, +} from './validate-sharing-rule-enforceability.js'; +import { AUTHORING_RULES, runAuthoringRules } from './authoring-rules.js'; + +const ids = (stack: unknown) => validateSharingRuleEnforceability(stack).map((f) => f.rule); + +/** A complete, spec-shaped sharing rule with the condition swapped in. */ +const ruleWith = (condition: unknown) => ({ + sharingRules: [ + { + name: 'high_value_opps', + type: 'criteria', + object: 'opportunity', + accessLevel: 'read', + sharedWith: { type: 'team', value: 'deal_desk' }, + condition, + }, + ], +}); + +// ── Red: declared, schema-valid, and never read ────────────────────── +// +// Every source below parses as CEL and passes `SharingRuleSchema`. The seeder +// still drops the rule on the floor, which is the whole defect (#4698). + +describe('validateSharingRuleEnforceability — the declared-but-never-read cases go RED', () => { + it('flags `has(...)` — the measured instance from the issue (hotcrm#621/#633)', () => { + const findings = validateSharingRuleEnforceability( + ruleWith("has(record.owner_id) && record.stage == 'closed_won'"), + ); + expect(findings).toHaveLength(1); + expect(findings[0]).toMatchObject({ + severity: 'error', + rule: SHARING_RULE_UNLOWERABLE_CONDITION, + // The declaration site is named, not just the rule. + path: 'sharingRules[0].condition', + where: 'sharing rule "high_value_opps" on object "opportunity"', + }); + // It must say what actually happens at boot, not merely "unsupported". + expect(findings[0].message).toMatch(/SKIPS the rule at boot/); + expect(findings[0].message).toMatch(/never written to `sys_sharing_rule`/); + // …and prescribe the fix that works on THIS surface. + expect(findings[0].hint).toMatch(/record\.x != null/); + expect(findings[0].hint).toMatch(/INTERPRETED/); + }); + + it.each([ + ['a bare function call', 'size(record.tags) > 0'], + ['arithmetic', 'record.amount * 2 > 100'], + ['a cross-object path', "record.account.region == 'EU'"], + ['a ternary', "record.stage == 'won' ? true : false"], + ])('flags %s', (_label, source) => { + expect(ids(ruleWith(source))).toEqual([SHARING_RULE_UNLOWERABLE_CONDITION]); + }); + + it('gives `current_user.*` its own id — the fix is a different mechanism, not a respelling', () => { + const findings = validateSharingRuleEnforceability(ruleWith('record.owner_id == current_user.id')); + expect(findings).toHaveLength(1); + expect(findings[0]).toMatchObject({ + severity: 'error', + rule: SHARING_RULE_RUNTIME_VARIABLE_CONDITION, + path: 'sharingRules[0].condition', + }); + expect(findings[0].message).toMatch(/current_user\.id/); + // Points at the surface where `current_user.*` IS resolved. + expect(findings[0].hint).toMatch(/rowLevelSecurity\[\]\.using/); + }); + + it('reports the parsed tier identically — the envelope `ExpressionInputSchema` produces', () => { + expect(ids(ruleWith({ dialect: 'cel', source: 'size(record.tags) > 0' }))) + .toEqual([SHARING_RULE_UNLOWERABLE_CONDITION]); + }); + + it('names each offending rule separately, with its own index', () => { + const findings = validateSharingRuleEnforceability({ + sharingRules: [ + { name: 'ok', object: 'a', condition: "record.stage == 'won'" }, + { name: 'fn', object: 'b', condition: 'has(record.x)' }, + { name: 'var', object: 'c', condition: 'record.owner == current_user.id' }, + ], + }); + expect(findings.map((f) => [f.rule, f.path])).toEqual([ + [SHARING_RULE_UNLOWERABLE_CONDITION, 'sharingRules[1].condition'], + [SHARING_RULE_RUNTIME_VARIABLE_CONDITION, 'sharingRules[2].condition'], + ]); + }); +}); + +// ── Green: conditions that really are read ─────────────────────────── +// +// A false positive here is worse than the gap the rule closes: it rejects +// security metadata that enforces correctly today and hands the author a +// "correction" that would break it. + +describe('validateSharingRuleEnforceability — conditions the runtime DOES read stay green', () => { + it.each([ + ["record.health == 'red'"], + ["record.health == 'red' && record.budget > 100000"], + ['record.done == false'], + ["record.stage in ['closed_won', 'closed_lost']"], + ['record.closed_at == null'], + ["record.name.startsWith('ACME')"], + ["!(record.stage in ['draft']) || record.amount >= 1000"], + ])('accepts %s', (source) => { + expect(validateSharingRuleEnforceability(ruleWith(source))).toEqual([]); + }); + + it('accepts every sharing-rule condition the bundled examples declare', () => { + // Lifted verbatim from examples/app-showcase/src/security/sharing-rules.ts + // and examples/app-crm. The gate must not turn shipped apps red. + const shipped = [ + "record.health == 'red'", + "record.health == 'red' && record.budget > 100000", + "record.status == 'new'", + 'record.done == false', + ]; + for (const source of shipped) { + expect(validateSharingRuleEnforceability(ruleWith(source))).toEqual([]); + } + }); + + it('leaves CEL SYNTAX errors to validateStackExpressions — no double report', () => { + // Parses nowhere, but this rule stays silent: `expression-invalid` already + // gates the same field with a message written about syntax. + expect(compileCelToFilter('record.stage ==', { variables: {} })).toMatchObject({ reason: 'parse-error' }); + expect(validateSharingRuleEnforceability(ruleWith('record.stage =='))).toEqual([]); + }); + + it('ignores shapes Zod owns rather than inventing a second complaint', () => { + expect(ids(ruleWith(undefined))).toEqual([]); + expect(ids(ruleWith(''))).toEqual([]); + expect(ids(ruleWith(' '))).toEqual([]); + expect(ids(ruleWith(42))).toEqual([]); + expect(ids(ruleWith({ dialect: 'cel' }))).toEqual([]); + }); + + it('is a no-op on a stack that declares no sharing rules', () => { + expect(validateSharingRuleEnforceability({})).toEqual([]); + expect(validateSharingRuleEnforceability(undefined)).toEqual([]); + expect(validateSharingRuleEnforceability({ sharingRules: [] })).toEqual([]); + }); + + it('checks inactive rules too — the seeder compiles the condition regardless of `active`', () => { + // `bootstrapDeclaredSharingRules` carries `active` through to `defineRule`; + // it does not skip the compile. A rule that is off today and unlowerable is + // still a rule that will grant nothing the day someone switches it on. + expect(ids({ sharingRules: [{ name: 'r', object: 'o', active: false, condition: 'has(record.x)' }] })) + .toEqual([SHARING_RULE_UNLOWERABLE_CONDITION]); + }); +}); + +// ── The predicate is the consumer's, not a model of it ─────────────── + +describe('validateSharingRuleEnforceability — the verdict IS the seeder\'s verdict', () => { + const corpus = [ + "record.health == 'red'", + "record.health == 'red' && record.budget > 100000", + 'record.done == false', + "record.stage in ['a', 'b']", + 'record.closed_at != null', + 'has(record.owner_id)', + 'size(record.tags) > 0', + 'record.amount * 2 > 100', + "record.account.region == 'EU'", + 'record.owner_id == current_user.id', + ]; + + it('agrees with `compileCelToFilter({ variables: {} })` on every source, in both directions', () => { + for (const source of corpus) { + // This is exactly the call `bootstrap-declared-sharing-rules.ts` makes. + const seederWouldSeed = compileCelToFilter(source, { variables: {} }).ok; + const lintIsClean = validateSharingRuleEnforceability(ruleWith(source)).length === 0; + expect({ source, lintIsClean }).toEqual({ source, lintIsClean: seederWouldSeed }); + } + }); + + /** + * The "before" half of the proof, kept mechanical rather than asserted in + * prose. #4698's complaint is that the offending stack passes the WHOLE + * toolchain, so it is not enough to show the new rule goes red — it must also + * be shown that nothing else ever did. Running the full author-time registry + * over the fixture and demanding that every OTHER rule stays silent is that + * statement, and unlike a comment it keeps holding: if some future rule grows + * to cover this shape, this test fails and someone has to decide which of the + * two owns it, instead of the stack quietly acquiring a duplicate diagnostic. + */ + it('no OTHER author-time rule sees this — which is exactly why the gate was missing', () => { + const offending = { + objects: [ + { + name: 'opportunity', + label: 'Opportunity', + sharingModel: 'private', + fields: { + name: { type: 'text', label: 'Name' }, + owner_id: { type: 'text', label: 'Owner' }, + stage: { type: 'text', label: 'Stage' }, + }, + }, + ], + sharingRules: [ + { + name: 'closed_won_to_deal_desk', + type: 'criteria', + object: 'opportunity', + accessLevel: 'read', + sharedWith: { type: 'team', value: 'deal_desk' }, + condition: "has(record.owner_id) && record.stage == 'closed_won'", + }, + ], + }; + + const findings = runAuthoringRules('validate', { normalized: offending, parsed: offending }); + expect(findings.map((f) => f.rule)).toEqual([SHARING_RULE_UNLOWERABLE_CONDITION]); + + // And the fixture really did travel through every rule — a registry that + // silently stopped running would satisfy the assertion above vacuously. + expect(AUTHORING_RULES.filter((r) => r.commands.includes('validate')).length).toBeGreaterThan(20); + }); + + it('a match-all filter is unreachable from a lowering condition (so lint need not re-check it)', () => { + // The seeder's SECOND guard is `isMatchAllCriteria(f)`, which lives in + // plugin-sharing — a runtime `@objectstack/lint` must not import. This + // pins the claim in the module docblock that duplicating it would be dead + // code: every condition the compiler lowers yields a concrete predicate. + for (const source of corpus) { + const result = compileCelToFilter(source, { variables: {} }); + if (!result.ok) continue; + expect(Object.keys(result.filter as Record).length).toBeGreaterThan(0); + } + }); +}); diff --git a/packages/lint/src/validate-sharing-rule-enforceability.ts b/packages/lint/src/validate-sharing-rule-enforceability.ts new file mode 100644 index 0000000000..87249a2a07 --- /dev/null +++ b/packages/lint/src/validate-sharing-rule-enforceability.ts @@ -0,0 +1,248 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #4698 — "declared but never read", made decidable for ONE surface. + * + * The issue behind this file reported three shapes that pass `os validate`, + * `os lint`, `tsc` and a full test suite while the runtime never reads them. + * The useful invariant it names is the ledger discipline's: **a key that + * nothing reads should not validate clean.** The hard part is not the + * sentiment, it is the PREDICATE — "is this key read?" is only a lint question + * when the answer is computable from the authored metadata alone. Most of the + * time it is not: a repo-wide grep for a reader is famously not evidence of + * absence (#4604), and a consumer can live in another package, another repo, or + * a plugin nobody has installed yet (#4914). + * + * A sharing rule's `condition` is the case where the predicate is EXACT, and + * that is the whole reason this rule exists and its neighbours are deferred: + * + * The one runtime consumer of a stack-declared `sharingRules[].condition` is + * `bootstrapDeclaredSharingRules` (plugin-sharing), and the ONLY thing it + * does with the key is hand it to `compileCelToFilter(condition, { variables: + * {} })`. A condition that does not lower is not degraded, not partially + * applied and not deferred — the rule is SKIPPED (ADR-0049: never seeded as a + * permissive match-all), so it never reaches `sys_sharing_rule` and grants + * nothing at all. The only trace is one WARN line at boot. + * + * So this rule does not model the consumer, guess at it, or grep for it. It + * calls the consumer's own decision procedure, from the same package + * (`@objectstack/formula`), on the same input, with the same options. The + * verdict is bit-identical to the seeder's by construction — there is no + * heuristic to drift and no false positive that is not also a real skip. + * + * ## Why `error` + * + * `SharingRuleSchema`'s own docblock makes the claim this rule enforces: "The + * whole authorable surface is enforced — nothing here validates and then + * silently does nothing (ADR-0078)." Until now that sentence held for every + * part of the shape EXCEPT the one field carrying the author's intent. There is + * no reading under which an unlowerable condition does what it says: the grant + * does not exist. Per the severity bar `lint-flow-patterns.ts` states — gate + * when no reading of the metadata behaves as written — that is an `error`, not + * a warning. It fails closed (the recipient under-sees rather than over-sees), + * which is why it was survivable, not why it is acceptable. + * + * Measured before shipping: every sharing-rule condition and RLS predicate + * declared anywhere in this repo (examples, platform permission sets) lowers + * cleanly, so the gate turns nothing red that works today. + * + * ## The two ids, and why not one + * + * `compileCelToFilter` fails for three reasons; two of them are authoring + * mistakes with DIFFERENT fixes, so they get different ids rather than one id + * with a branchy message (allowlists and `--json` consumers key on the id): + * + * - `unsupported` → {@link SHARING_RULE_UNLOWERABLE_CONDITION}. The shape is + * outside the pushdown subset: a function call (`has(...)`, `size(...)`), + * arithmetic, a ternary, or a cross-object path (`record.account.region`). + * This is the issue's measured instance — an author following the guidance + * that is correct for object VALIDATIONS (where `has()` is interpreted, not + * lowered) writes silently inert security metadata. + * - `unresolved-variable` → {@link SHARING_RULE_RUNTIME_VARIABLE_CONDITION}. + * The condition reads `current_user.*`. Criteria sharing rules are + * MATERIALIZED — the seeder compiles one static `criteria_json` per rule and + * the evaluator writes `sys_record_share` grants from it — so there is no + * "current user" at compile time and the compiler correctly refuses. The fix + * is a different mechanism (RLS / an ownership-shaped grant), not a + * different spelling, which is exactly why it earns its own id. + * - `parse-error` → deliberately NOT reported here. CEL syntax is + * `validateStackExpressions`' surface and it already gates this same field + * (`stack.sharingRules[].condition`). Reporting it twice, in two + * vocabularies, would make the second report noise — and the first one is + * the better message, because it is written about syntax. + * + * ## What this rule deliberately does NOT do + * + * - **It does not re-implement `isMatchAllCriteria`.** The seeder's second + * guard (skip a condition that lowers to a filter constraining nothing) + * lives in `plugin-sharing`, and `@objectstack/lint` never depends on a + * runtime. Copying it here would fork the one definition of "this predicate + * constrains nothing" that `rule-criteria.ts` exists to be. It is also + * unreachable from this door: every AST the compiler lowers yields a + * concrete field predicate, so a lowering CEL condition cannot produce a + * match-all filter. `matchAllIsUnreachable` in the tests pins that claim + * against the compiler rather than asserting it in prose. + * - **It does not judge RLS `using` / `check`.** Same class, same compiler, + * and ADR-0056 D4 explicitly asks for this gate — but the RLS decision + * procedure is `isSupportedRlsExpression`, which first bridges the legacy + * SQL-ish subset through `sqlPredicateToCel`, and BOTH live in + * `plugin-security`. Judging RLS here means either importing a runtime + * (forbidden) or copying the bridge (forking the predicate — the thing this + * file's opening paragraph refuses to do). Hoisting `sqlPredicateToCel` into + * `@objectstack/formula` first would make it exact; that is a spec/runtime + * move, not a lint change, and is filed separately. + * - **It does not look at flow / hook `condition`s.** Those are INTERPRETED by + * the CEL engine, not lowered to a filter, so the whole language is in scope + * there and non-pushdownability means nothing. Reusing this predicate on + * them would reject working metadata — the false-positive direction that is + * worse than the gap. + */ + +import { compileCelToFilter } from '@objectstack/formula'; + +/** A `condition` outside the pushdown subset — the rule is never seeded. */ +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'; + +export type SharingRuleEnforceabilitySeverity = 'error' | 'warning'; + +export interface SharingRuleEnforceabilityFinding { + severity: SharingRuleEnforceabilitySeverity; + /** Diagnostic rule id (`sharing-rule-*`). */ + rule: string; + /** Human-readable location, e.g. `sharing rule "high_value_opps" on object "opportunity"`. */ + where: string; + /** Config path, e.g. `sharingRules[2].condition`. */ + path: string; + /** What is wrong. */ + message: string; + /** How to fix it. */ + hint: string; +} + +type AnyRec = Record; + +/** Coerce a collection (array or name-keyed map) to an array of records. */ +function asArray(v: unknown): AnyRec[] { + if (Array.isArray(v)) return v as AnyRec[]; + if (v && typeof v === 'object') { + return Object.entries(v as AnyRec).map(([name, def]) => ({ name, ...(def as AnyRec) })); + } + return []; +} + +function str(v: unknown): string { + return typeof v === 'string' ? v : ''; +} + +/** + * The `condition` value in the shape the compiler takes. + * + * Both authoring tiers reach this rule: `os lint` runs it on the NORMALIZED + * stack, where a bare string is still a bare string, while `os validate` / + * `os build` run it after `ExpressionInputSchema` has wrapped that string into + * `{ dialect: 'cel', source }`. `compileCelToFilter` accepts either, so the + * verdict does not depend on which tier asked — the property the wiring guard's + * `input: 'parsed'` entries all have to satisfy. + * + * Anything else (a number, an envelope with no `source`) is returned as `null`: + * the shape is Zod's to reject, and inventing a second complaint about it here + * would just be the double-report this file avoids for syntax. + */ +function toCompilerInput(condition: unknown): string | { source?: string } | null { + if (typeof condition === 'string') return condition.trim() ? condition : null; + if (condition && typeof condition === 'object') { + const source = (condition as AnyRec).source; + if (typeof source === 'string' && source.trim()) return { source }; + } + return null; +} + +/** What the author sees quoted back at them. */ +function sourceOf(condition: unknown): string { + const input = toCompilerInput(condition); + if (typeof input === 'string') return input; + return str(input?.source); +} + +const PUSHDOWN_SUBSET = + 'The lowerable subset is: `==` `!=` `>` `<` `>=` `<=`, `in`, `&&` `||` `!`, `== null` / `!= null`, ' + + 'and the string methods `startsWith` / `endsWith` / `contains` — over SINGLE-column `record.` ' + + 'paths (ADR-0058 D2).'; + +/** + * Gate stack-declared sharing rules on the ONE thing their runtime consumer + * does with `condition`: lower it to a `criteria_json` filter. + * + * Pure `(stack) => Finding[]`; tolerates the normalized and the parsed tier. + */ +export function validateSharingRuleEnforceability(stack: unknown): SharingRuleEnforceabilityFinding[] { + const findings: SharingRuleEnforceabilityFinding[] = []; + const cfg = (stack ?? {}) as AnyRec; + + asArray(cfg.sharingRules).forEach((rule, index) => { + const input = toCompilerInput(rule.condition); + if (input === null) return; + + // 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; + // 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 ' + + 'WARN line in the boot log. The rule is declared and grants nothing (ADR-0049: an unlowerable ' + + 'condition is never seeded as a permissive match-all).'; + + if (result.reason === 'unresolved-variable') { + findings.push({ + severity: 'error', + rule: SHARING_RULE_RUNTIME_VARIABLE_CONDITION, + where, + path, + message: + `Sharing-rule condition \`${source}\` reads a runtime variable (${result.detail}), ` + skipped, + hint: + 'A criteria sharing rule is MATERIALISED: the seeder compiles ONE static `criteria_json` per ' + + 'rule and the evaluator writes `sys_record_share` rows from it, so there is no "current user" ' + + 'for the condition to read. Express per-user access with the mechanism that runs per request ' + + 'instead — an RLS policy on a permission set (`rowLevelSecurity[].using`, where ' + + '`current_user.*` IS resolved), or the record-ownership path. Keep this rule for the part of ' + + 'the predicate that is a property of the RECORD (e.g. `record.stage == \'closed_won\'`) and ' + + 'name the audience through `sharedWith`.', + }); + return; + } + + findings.push({ + severity: 'error', + rule: SHARING_RULE_UNLOWERABLE_CONDITION, + where, + path, + message: + `Sharing-rule condition \`${source}\` is outside the pushdown subset the runtime can compile ` + + `(${result.detail}), ` + skipped, + hint: + 'Rewrite the predicate inside the lowerable subset. ' + PUSHDOWN_SUBSET + ' Two traps in ' + + 'particular: (1) `has(record.x)` is correct in an object VALIDATION rule, which is INTERPRETED, ' + + 'and wrong here, where the condition is COMPILED — write the null test as `record.x != null`; ' + + '(2) a related-record path (`record.account.region`) is a join, which the compiler refuses by ' + + 'design (ADR-0055) — denormalise the value onto this object (a formula/rollup field) and test ' + + 'that column, or share the related object instead.', + }); + }); + + return findings; +}