|
| 1 | +--- |
| 2 | +"@objectstack/lint": minor |
| 3 | +--- |
| 4 | + |
| 5 | +feat(lint): reject a sharing-rule condition the runtime can only skip (#4698) |
| 6 | + |
| 7 | +#4698 reported the same failure shape three times in one app in one day: a key |
| 8 | +that is authored, is schema-valid, reads as meaningful — and is never consumed |
| 9 | +by the runtime. Every check verifies that what is declared is *well-formed*, |
| 10 | +never that it is *read*. The issue's third measured instance is a sharing rule |
| 11 | +whose CEL `condition` uses `has(...)`: the seeder cannot lower it, skips the |
| 12 | +rule, and the only signal is one WARN line at boot. The rule exists in |
| 13 | +metadata, is absent from `sys_sharing_rule`, and grants nothing. |
| 14 | + |
| 15 | +**New rules, both `error`, on all three authoring commands:** |
| 16 | + |
| 17 | +- **`sharing-rule-unlowerable-condition`** — the condition is outside the |
| 18 | + pushdown subset: a function call (`has(...)`, `size(...)`), arithmetic, a |
| 19 | + ternary, or a cross-object path (`record.account.region`). |
| 20 | +- **`sharing-rule-runtime-variable-condition`** — the condition reads |
| 21 | + `current_user.*`. Criteria sharing rules are materialised (one static |
| 22 | + `criteria_json` per rule, from which grants are written), so there is no |
| 23 | + "current user" at compile time. The fix is a different mechanism, not a |
| 24 | + different spelling, which is why it has its own id. |
| 25 | + |
| 26 | +Fix each by rewriting the predicate inside the lowerable subset — `==` `!=` |
| 27 | +`>` `<` `>=` `<=`, `in`, `&&` `||` `!`, `== null` / `!= null`, and |
| 28 | +`startsWith` / `endsWith` / `contains` over single-column `record.<field>` |
| 29 | +paths (ADR-0058 D2). Two specific migrations: `has(record.x)` → `record.x != |
| 30 | +null` (`has()` is correct in an object *validation* rule, which is |
| 31 | +interpreted, and wrong here, where the condition is compiled); and a related |
| 32 | +record's field → denormalise it onto this object (formula/rollup) and test |
| 33 | +that column, or share the related object instead. For per-user access, use an |
| 34 | +RLS policy (`rowLevelSecurity[].using`), where `current_user.*` *is* resolved. |
| 35 | + |
| 36 | +**Why this one surface and not "unread keys" in general.** "Is this key read?" |
| 37 | +is only a lint question when the answer is computable from the authored |
| 38 | +metadata alone, and usually it is not — a repo-wide grep for a reader is not |
| 39 | +evidence of absence, and a consumer may live in another package, another repo, |
| 40 | +or an uninstalled plugin. A sharing rule's `condition` is the case where the |
| 41 | +predicate is exact: its one runtime consumer |
| 42 | +(`bootstrapDeclaredSharingRules`) does exactly one thing with the key — |
| 43 | +`compileCelToFilter(condition, { variables: {} })` — and a condition that does |
| 44 | +not lower means the rule is skipped outright. So the lint calls that same |
| 45 | +compiler, from the same package, with the same options, instead of modelling |
| 46 | +the consumer; the verdict is identical to the seeder's by construction and is |
| 47 | +pinned in both directions by a test over a shared corpus. |
| 48 | + |
| 49 | +`error` rather than advisory, per the ADR-0078 claim `SharingRuleSchema`'s own |
| 50 | +docblock makes ("the whole authorable surface is enforced — nothing here |
| 51 | +validates and then silently does nothing"): there is no reading under which an |
| 52 | +unlowerable condition does what it says. It fails closed, which is why it was |
| 53 | +survivable, not why it was acceptable. Measured before shipping: every |
| 54 | +sharing-rule condition declared anywhere in this repo lowers cleanly, so the |
| 55 | +gate turns nothing red that works today. |
| 56 | + |
| 57 | +CEL *syntax* errors are deliberately left to `expression-invalid`, which |
| 58 | +already gates this same field with a message written about syntax. |
0 commit comments