feat(lint): gate a sharing-rule condition the runtime can only skip (#4698) (#4985)

#4698 asks for a "declared but never read" check. The general form is not a
lint question: "is this key read?" is only decidable from authored metadata
when the consumer's decision procedure is in hand, and a repo-wide grep for a
reader is famously not evidence of absence (#4604, #4914).

A sharing rule's `condition` is the case where it IS decidable. Its one runtime
consumer, `bootstrapDeclaredSharingRules`, does exactly one thing with the key:
`compileCelToFilter(condition, { variables: {} })`. A condition that does not
lower is not degraded or partially applied — the rule is SKIPPED, never reaches
`sys_sharing_rule`, and grants nothing, with one boot WARN as the only trace.

So the new rule does not model the consumer. It calls the consumer's own
compiler, from the same package, on the same input, with the same options, and
is pinned in both directions against a shared corpus. Two ids, because the two
authoring mistakes have different fixes:

  - `sharing-rule-unlowerable-condition` — outside the pushdown subset
    (`has(...)`, `size(...)`, arithmetic, ternary, cross-object path). This is
    the issue's measured instance: `has()` is right in an object validation,
    which is interpreted, and wrong here, where the condition is compiled.
  - `sharing-rule-runtime-variable-condition` — reads `current_user.*`. Criteria
    rules are materialised, so there is no current user at compile time; the fix
    is RLS, a different mechanism, not a respelling.

Both `error`, on all three commands, 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 true. CEL syntax stays with
`expression-invalid` — one field, one complaint.

Measured before shipping: every sharing-rule condition and RLS predicate
declared anywhere in this repo lowers cleanly, so nothing that works today goes
red. The "before" half of the proof is in the test rather than in prose — the
full authoring registry runs over the offending stack and every OTHER rule must
stay silent, which keeps holding as rules are added.

Deliberately out of scope, filed instead of guessed:
  - RLS `using`/`check` (#4983) — same class, and ADR-0056 D4 explicitly asks
    for the gate, but its decision procedure needs `sqlPredicateToCel`, which
    lives in a runtime `@objectstack/lint` must not import. Hoisting the bridge
    into `@objectstack/formula` comes first; copying it would fork the predicate.
  - `validateOrgAxisRedLines` reads sharing-rule keys the spec rejects (#4984).
  - The tenant-scoped declared unique index (issue instance 2) is a driver-sql /
    objectql fix; tenancy is kernel-injected, not authored, so lint cannot judge
    it — the existing rule says so in its own comment.


Claude-Session: https://claude.ai/code/session_018iARDqtrhQgz6fVHDeDkbQ

Co-authored-by: Claude <noreply@anthropic.com>

Author: xuyushun441-sys

.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.<field>`
+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.

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 ─────────────────────────────────────────────────────────

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,

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<string, unknown>).length).toBeGreaterThan(0);
+    }
+  });
+});