Repository navigation
Commit eee0974
fix(lint)!: refuse a sharing-rule condition that compares a field with a json or multiple field when it is authored (#20375)
Fixes #19886
Clause-②: no
Stage 2g of #19886: the sharing-rule twin of stage 2f, and the only
remainder item of release `5861981062`. Seat answer A in the 2f ACCEPT
(`5861343773`): "Stage 2g (the `validate-sharing-rule-enforceability.ts`
arm) measures the sharing runtime (`rule-criteria.ts` → `engine.find` →
`driver-sql`) FIRST. It reuses this PR's classification". Claim
`5862004488`. Base `67047171`, head `79a2ad1e`. The changeset declares
`Clause-②: no (narrowing)`, BREAKING, `@objectstack/lint` minor,
following the 2d / 2e / 2f precedent.
## What changes
`validateSharingRuleEnforceability` (`packages/lint`) gets one arm. When
a sharing rule's `condition` lowers, the arm reads the lowered filter
against the declared field map of the rule's anchor object. It reports
`sharing-rule-unlowerable-condition` for every field-to-field comparison
(`==`, `!=` and the four ordering operators, either side, under `!` too)
in which either column is DECLARED to hold a list or an object.
- **One classification, not a copy.** The arm calls
`listHoldingComparisons` from
`validate-rls-predicate-enforceability.ts`, the 2f helper. That helper
reads the spec's `STRUCTURED_JSON_TYPES` and `isMultiValueField`, the
two sets driver-sql refuses such a comparison by. The share is the
smallest one that works: `export` on `listHoldingComparisons` and on its
`ListHoldingComparison` result type, plus one comment sentence. No RLS
line changes behaviour, and the 2f pins run unchanged (see Tests).
- **The anchor only, lazily.** The graph is indexed per anchor, once,
and only for a condition that lowered. A lowered sharing criterion can
only address its anchor's own columns, because a cross-object path does
not lower. Indexing the whole stack up front changed which refusal an
unreadable reference carrier raises and turned the existing `#18550` pin
red in the first run. The lazy anchor-scoped graph keeps that pin
byte-identical.
- **The id.** It keeps `sharing-rule-unlowerable-condition`. The fix is
the same rewrite of the condition, 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. The id's one-line doc
now names both halves. A separate id was weighed and not taken (see
Decisions).
- **Doors.** `os validate` / `os build` / `os lint` run this rule. The
metadata save door for a `sharing_rule` does not run it, as before (see
Acceptance notes). No gate is added and no runtime seam changes.
Files:
- the rule (+105 / −8);
- the RLS rule (+5 / −2: two `export` keywords, one comment sentence);
- a new table-driven pin beside the existing sharing tests;
- the changeset.
The cli admission parity test (`rls-policy-authoring-admission.test.ts`)
has no sharing-rule family, because the sharing rule never crosses the
save door, so it is untouched.
## The refusal text (new)
Message, from the real `os validate` at head:
> Sharing-rule condition `record.status != record.tags` lowers, but
compares a field with a field that holds a list or an object:
`record.status != record.tags`, where `tags` is declared `type: 'json'`.
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 class sentence is the 2f RLS arm's sentence, word for word; a pin
holds the two equal. The hint drops the RLS hint's `current_user`
alternative, because a sharing condition that reads `current_user` is
refused (`sharing-rule-runtime-variable-condition`).
## Zone 1 P1, measured FIRST (before any edit, at `67047171`)
A probe drove the real `SharingServicePlugin` over a real `ObjectQL`:
- `init`, then `start`, then the `kernel:ready` handlers: seeding
through `bootstrapDeclaredSharingRules` from `registerApp`'s
`sharingRules` collection, and hook binding;
- then the `kernel:bootstrapped` backfill;
- then a post-boot insert and update, to exercise the per-record hook
path.
Setup:
- 10 leak rules and 4 controls, each sharing READ with its own user, on
a `private` object;
- 3 seeded rows, including one whose `status` is a member of `tags`, so
a rule that "should match" exists;
- each recipient's read went through the sharing middleware.
| driver | leak rules (10: `json` list and object, `multiple` lookup,
`multiselect`, `multiple` user; `==`, `!=`, `!(==)`, `>`, less-or-equal;
both orders) | controls (text `!=` / `==` text, literal, `json != null`)
|
|---|---|---|
| driver-sql (better-sqlite3) | all 10 **seeded** with `criteria_json`;
every criteria query `INVALID_FILTER` / 400; one WARN `[sharing-rule]
criteria query failed` per rule at the backfill; **0 grants; the
recipient reads 0 rows**; after the insert and update, still 0 grants,
with only engine-level `Find operation failed` warns that name no rule |
granted and enforced exactly the matching rows (for example text `!=`
text: r1, r2, then r4 after the insert) |
| driver-sqlite-wasm | identical, cell for cell | identical |
| driver-memory (test/demo; #15104, frozen) | reads a `{ $field }`
comparand as a literal: `!=` grants every row and `==` / ordering grant
nothing | **equally wrong** (text `==` text grants nothing) — this is
#15104, not the list class |
- driver-mongodb: not live-measured. Its source (`mongodb-filter.ts`)
refuses every `{ $field }` comparand, scalar or list.
- driver-turso: local and replica mode extend `SqlDriver`. The remote
transport was not measured.
**Verdict: P1 holds.** On the SQL drivers the runtime refuses the
comparison, and the sharing path turns the refusal into a silent
zero-share: the rule is declared, seeded, and grants nothing. The
prohibition does not apply.
## Zone 2, measured
**1. The authoring doors.**
- The cells: 85 sharing rules in one stack.
- 72 leak cells: 7 operators × 5 list/object columns (`json`, `address`,
`multiple` lookup, `multiselect`, `multiple` user) × 2 orders, plus a
list-against-list cell and a compound one;
- 10 scalar controls;
- 3 firing controls.
- The doors: the real CLI (`node packages/cli/bin/run.js validate
--json`) and the save door (`saveMetaItem({ type: 'sharing_rule' })`
over a real `ObjectQL`, sqlite-wasm).
| tree | leak (72) | controls (10) | firing (3) | save door |
|---|---|---|---|---|
| `67047171` (base) | 72 clean | 10 clean | 3 reported (list literal and
function call → unlowerable; `current_user` → runtime-variable) | 85 /
85 accepted, firing controls included |
| `79a2ad1e` (head) | **72 refused, one finding each, one consequence
sentence and one hint across all 72** | 10 clean | 3 reported, rule and
message identical to base | 85 / 85 accepted (unchanged; this rule is
CLI-only) |
**2. The runtime.** P1 above. The door that answers is the driver,
through `engine.find`: driver-sql's `crossFieldComparisonClass`, by
declared type. `plugin-sharing` catches the refusal (`findMatchingRows`
/ `matchRecord`) and reads it as "no match".
**3. Where the arm lives.** `compileCelToFilter(condition, { variables:
{} })`, the seeder's exact call, returns the same lowered
`FilterCondition` shape the RLS rule walks. The probe's seeded
`criteria_json` shows it: `{"status":{"$ne":{"$field":"tags"}}}` and
`{"$not":{"status":{"$eq":{"$field":"tags"}}}}`. So the smallest arm is
one call to the 2f helper on `result.filter` when `result.ok`.
**4. Census (narrowing).**
- objectstack at `67047171`, `packages/**` + `examples/**`, non-test: 3
declared sharing-rule conditions (`app-showcase` ×2, a
`qa/downstream-contract` fixture), all field against literal. 0
field-to-field, 0 against a `json` / `multiple` column.
- cloud `main` `96eb092` (the `ls-remote` tip): 0 declared sharing-rule
conditions.
- The real `os validate` at head over `app-crm`, `app-multi-package`,
`app-showcase` and `app-todo`: 0 `sharing-rule-*` findings. Error and
warning counts are unchanged from base for the three apps that ran
there. `app-showcase` could not load at base before its dependency
closure was built, and at head it is valid with 0 errors.
## Tests
All at head `79a2ad1e`, in one run through `os-verify-lock.sh` (VERDICT
command-exit 0, held 603s on a shared box):
- `@objectstack/lint` build exit 0 (DTS emitted, 4/4 declaration files).
- `@objectstack/lint`, the targeted five files (the new pin, the
existing sharing tests, the 2f RLS list-holding pin, the rule-id barrel
and the wiring guard): 5 files / 480 tests pass.
- `@objectstack/lint`, full package: 113 files / 4704 tests pass. The
existing sharing-rule pins and the 2f RLS pins run unchanged, including
the `#18550` unreadable-carrier pin that the first draft of this arm
turned red.
- `@objectstack/lint` `typecheck`: exit 0 (source and test layer).
- `@objectstack/cli` `unit` project: 230 files / 3296 pass.
`integration` project, the admission parity pin: 30 / 30 (named by the
dispatch as a consumer suite; not touched).
- `@objectstack/plugin-sharing`, full package (read-only consumer; no
pin flips): 37 files / 913 pass.
The new pin has 64 tests:
- 28 cells: 7 operators × 2 classes × 2 orders, each asserting the
envelope (severity, rule id, path, where) and the whole message
verbatim, plus the hint's first clause;
- 16 declared classes read from the spec;
- 11 controls;
- the parsed tier, the compound and two-comparison cases, an inactive
rule, the RLS class-sentence parity, the not-judged cases,
one-defect-one-finding against an unlowerable condition, the anchor arm
reported beside it, and the `os validate` rule table.
The first run was at `53275286`, before the lazy-graph fix: `lint` full
1 failure out of 4704, the `#18550` pin; `cli unit` 230 / 3296;
`plugin-sharing` 37 / 913. That failure is why the graph is
anchor-scoped and lazy.
**Ablation**
From committed `79a2ad1e`, run by `ablation.sh` with an `EXIT INT TERM`
trap:
- **Mutation.** `scripts/ablation-replace.mjs` replaced `if
(listHolding) findings.push(listHolding);` with a guard that never
pushes, carrying the marker `ABLATION_19886G`. Anchor 1 to 0, marker 0
to 1, blob `a95f00af` to `f507e6a3`.
- **Build.** `pnpm --filter @objectstack/lint build` exit 0.
`ablation-dist-preflight.mjs @objectstack/lint` found the marker in all
4 built entries.
- **The new pin plus the existing sharing tests: 50 red / 64 green.**
Red: every refusal cell, class and property. Green: the 14 controls and
not-judged cases, and all 50 pre-existing sharing tests.
- **The real `os validate` over the 85-cell stack, with the ablated
dist:** the 72 leak cells are clean again, the 10 controls clean, and
the 3 firing controls unchanged. The arm alone carries the refusal,
through the built CLI.
- **Restore.** `git checkout HEAD -- ABS_PATH`: blob equal to HEAD
(`a95f00af`), `git diff HEAD` empty. Rebuilt with exit 0; the preflight
`--absent` reads the marker absent from all 14 built files and the tree
clean.
## Gates
Re-derived at head with `dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack`. The change set is these 4 paths against
merge base `67047171`, and it is the same 60 commands as at `53275286`.
Every one was run at `79a2ad1e`, with exit codes recorded to disk before
they were read. The `--ran` reconciliation reads **60 derived, 59 run, 1
NOT MEASURED, 0 UNRUN**:
- 54 light gates: exit 0.
- 5 build-reading gates exit 0: `check:dts-closure`,
`check:lean-entry-closure`, `check:published-files`,
`check:sourcemap-no-sources-content` and `check:type-check-debt`.
- NOT MEASURED: `check:dual-build-cjs-loads`, reason: exit 3,
PREREQUISITE NOT MET. Six packages outside the built closures have no
`dist/` (studio, client-react, embedder-openai, knowledge-ragflow,
organizations, service-cluster-redis), and the gate needs a whole-tree
`pnpm build`. As a proxy, lint's `dist/index.cjs` and `dist/runtime.cjs`
both `require()` cleanly.
- **ADR-0087:** `not-required (already-registered
cel-predicate-one-value-comparand-refused)`.
- The entry's `surface` names `sharingRules[].condition` and this exact
class: "a field compared with another field (==, !=, or an ordering
operator) where either column holds a list or an object on the record,
as a json column or a multiple lookup does".
- Its `replacement` carries the prescription.
- `check-adr-0087-registration` exits 0 and reads the marker as
`[BREAKING+clause-②-narrowing] not-required (already-registered)`.
## Decisions settled in this PR (open to the reviewer)
**The id: reuse `sharing-rule-unlowerable-condition` (chosen) or add
`sharing-rule-unenforceable-condition`.**
- Actual business need: 0 producers write this shape (census), so
neither choice moves an author.
- Long-term soundness: this file assigns ids by fix, and the fix here is
the unlowerable one, a rewrite of the condition. The class's literal
spelling already sits under that id. The cost is a constant whose name
says "unlowerable" for a condition that lowers, so its doc line now
names both halves. 2f made the same trade in RLS
(`rls-predicate-unenforceable`).
- Preventing AI authoring mistakes: the message states "lowers, but
compares…" and the real consequence, so a reader of `--json` is not sent
to "rewrite into the pushdown subset".
- Startup-stage scope: a new id is a new permanent public constant and
barrel line, for no author.
## Acceptance notes
- **The save door does not run this rule.**
`validateSharingRuleEnforceability` is `CLI_ONLY`. Its wiring entry in
`authoring-rules.ts` records the runtime crossing as pending: the
snapshot does not carry `sharingRules`. Measured at base and at head:
`saveMetaItem({ type: 'sharing_rule' })` accepts all 85 cells, including
the 3 firing controls this rule has always reported at the CLI. This is
pre-existing and not specific to this class, so it is not widened here.
- **driver-memory** mis-reads every `{ $field }` comparison as a
literal, scalar ones included. Measured on the sharing path: `!=`
over-shares every row. That is #15104 (frozen; test and demo only),
unchanged here.
- **The per-record path is quieter than the backfill.** After a write, a
refused criteria query is caught in `matchRecord` with no sharing-level
WARN for a non-`field` rule. Only the engine logs `Find operation
failed`, naming no rule. That is runtime diagnostics, read only here
(`plugin-sharing` is fenced). The new finding says "a WARN line in the
server log", which holds for the boot backfill.
- **The ADR-0087 entry's `reason`** describes the stage 2d outcome for
this class: the write-check evaluator refusing it. That fits RLS, not a
sharing condition, where the driver refuses the criteria query. The
`surface` and `replacement` fit, so no registration is owed. The reason
text is not edited here.
- The cross-class family (text vs number, text vs image, formula) is
#20347's and is not widened into this arm.
## What remains on #19886
The release's remainder list (`5861981062`) held stage 2g only, and this
PR closes it, hence `Fixes`. Carried elsewhere: #20347 (cross-class
comparisons, a different family) and #15104 (frozen, driver-memory).
---
_Generated by [Claude
Code](https://claude.ai/code/session_01Rjy9MeetSfq34PKn81CRiN)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 67c98f6 commit eee0974
4 files changed
Lines changed: 386 additions & 10 deletions
Lines changed: 24 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
Lines changed: 5 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1004 | 1004 | | |
1005 | 1005 | | |
1006 | 1006 | | |
1007 | | - | |
| 1007 | + | |
1008 | 1008 | | |
1009 | 1009 | | |
1010 | 1010 | | |
| |||
1017 | 1017 | | |
1018 | 1018 | | |
1019 | 1019 | | |
| 1020 | + | |
| 1021 | + | |
| 1022 | + | |
1020 | 1023 | | |
1021 | | - | |
| 1024 | + | |
1022 | 1025 | | |
1023 | 1026 | | |
1024 | 1027 | | |
| |||
Lines changed: 252 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
0 commit comments