You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
fix(core,objectql,plugin-security): the RLS write check judges a lone scalar on a declared multi-valued field as the list it is stored as (#21253)
Fixes#21238
Clause-②: yes (widening)
The row-level write `check` now judges a lone scalar written to a
declared multi-valued field as the one-member list the write door stores
it as. The wrap rule moves into `@objectstack/core` as
`multiValueStorageForm`, beside `temporalStorageForm`. objectql's record
validator calls it, and `storedFormCheckJudge` (from #21235) folds it.
That is one rule for the write door and the check, with no second copy.
This is triage's route A (comment 5940327789), built on claim
5941247880.
### Cross-lane files (`domain:engine`), named before the change list
- `packages/core/src/utils/multi-value-storage-form.ts`, its test, and
the export line in `packages/core/src/index.ts`: the moved rule.
- `packages/objectql/src/validation/record-validator.ts`:
`normalizeMultiValueFields` calls the moved rule. What the write door
stores does not change (proof below).
### Premise, re-measured on `main` at `be5a83cf`
The harness is the one #21235's pins use: `ObjectQL.insert` +
`SecurityPlugin` + a SQL driver, a member resolving a permission set,
with `using` and `check` set to the same predicate. Each row was
measured on both driver families (`driver-sql` on better-sqlite3, and
`driver-sqlite-wasm`) with the same result. "Stored" and "read" come
from a system write of the same value.
| `check` | member writes | write on `main` | stored | read |
|---|---|---|---|---|
| `record.tags.contains('x')` | `'x'` | 403 `PERMISSION_DENIED` |
`["x"]` | shown |
| `record.tags.contains('x')` | `'xy'` | 403 | `["xy"]` | hidden |
| `record.tags.contains('x')` | `['x']` | admitted | `["x"]` | shown |
| `!record.tags.contains('x')` | `'x'` | **admitted** | `["x"]` |
**hidden** |
| `record.owners.contains('x')` (`select`, `multiple: true`) | `'x'` |
403 | `["x"]` | shown |
| `record.tags.contains('x')`, a by-id update | `'x'` | 403 | `["x"]` |
shown |
| `record.tags.contains('x')`, a predicate update | `'x'` | admitted |
`["x"]` | shown |
The premise holds. The fourth row goes further than the card's grading.
The card calls the split fail-closed, but the negated form fails OPEN: a
policy that forbids a member from tagging a row `x` is passed by sending
`'x'` instead of `['x']`. The insert seam runs before the write door
wraps, so the scalar is judged and the list is stored. The
predicate-update row was already right on `main`, because the engine
wraps before that seam.
### Changes
- `@objectstack/core`: new `multiValueStorageForm(value)`. It wraps a
string, a number or a boolean into `[value]`. It returns every other
value as the same value: a list, `null` / `undefined`, a blank string
(read as missing), an object, a `Date`. This is the validator's
per-value rule, moved as it was. The rule lives in core. The
declared-type predicate stays the spec's `isMultiValueField`.
- `@objectstack/objectql`: `normalizeMultiValueFields` keeps its column
selection (`SKIP_FIELDS`, `system`, `readonly`, `isMultiValueField`) and
calls the core rule for the value.
- `@objectstack/plugin-security` (`rls-check-stored-form.ts`):
`declaredMultiValueColumns(columns)` names the columns the declaration
(`declaredComparisonColumns`'s `type` + `multiple`) calls multi-valued.
Types come only from the declaration, never from values.
`storedFormImage` puts those columns' post-image values through the core
rule, copy-on-write. `storedFormCheckJudge` does this on every image it
judges: the insert seam, the by-id image, and both update seams.
- **Comparands are left as written**, on purpose. The read pairs none
with the wrap. `$contains` / `$notContains` take one member, and the
read refuses every scalar comparison on such a column
(`JSON_COLUMN_INCOMPATIBLE_OPERATORS`, `INVALID_FILTER` / 400). The
card's pins need only the image.
- Changeset: `@objectstack/core` minor (one new root export; this export
is the widening the `Clause-②: yes (widening)` line declares),
`@objectstack/plugin-security` minor (the accept set widens: rows 1, 5
and 6 above are now admitted; row 4 is now refused; a security-floor
behaviour change, not Clause-②), `@objectstack/objectql` patch (no
behaviour change).
### What the fold moves under a scalar-comparison policy
A policy that compares a multi-valued field with `==`, `!=`, `in` or an
ordering is refused by the read (`INVALID_FILTER` / 400) on both
families. On `main` the write check gave `'x'` the OPPOSITE of the
verdict `['x']` got. Now both get the verdict of the list the store
holds. Measured at base and at head on both families:
| `check` (read: 400) | `'x'` on `main` | `['x']` on `main` | `'x'` now
| `['x']` now |
|---|---|---|---|---|
| `record.tags == 'x'` | admitted | 403 | 403 | 403 |
| `record.tags != 'x'` | 403 | admitted | admitted | admitted |
| `record.tags in ['x']` | admitted | 403 | 403 | 403 |
| `!(record.tags in ['x'])` | 403 | admitted | admitted | admitted |
| `record.tags > 'a'` | admitted | 400 | 400 | 400 |
That the write check evaluates these at all, while the read refuses
them, is a separate read/write split. It is in the Acceptance notes and
in the dev report, and this PR does not fix it.
### The write door, byte for byte
- The validator's own tests, before and after: `pnpm --filter
@objectstack/objectql exec vitest run --maxWorkers=2 src/validation/`
gave 17 files / 917 passed with `record-validator.ts` restored from
`be5a83cf`, and 17 files / 917 passed at the change. The restore was
proven: blob == HEAD, `git diff HEAD` empty.
- A differential probe (scratch, not committed) ran the verbatim
`be5a83cf` body of `normalizeMultiValueFields` against the new one. It
covered 2750 cases: 5 schema shapes x 22 field names (every
multi-capable type with and without `multiple`, `system` / `readonly`,
`SKIP_FIELDS`, undeclared and prototype names) x 25 values (blank
strings, `NaN`, `Infinity`, `0`, booleans, lists, objects, operator
objects, `Date`, bigint, symbol, function). Result: **0 differences**,
with the identity of a list left in place compared too. Positive
control: one planted difference (a blank string wrapped) reads
`diffs=1`.
- The full objectql suite: 361 files / 7087 passed, in three chunks
under the verify lock.
### Pins (`rls-check-stored-form.test.ts`, 38 cells before, 61 now)
Both driver families. Each cell checks write == read on the same row,
and a refusal is asserted as `{ code: 'PERMISSION_DENIED', status: 403
}` with nothing stored.
- `tags: 'x'` under `contains('x')` is admitted, stored `["x"]` and
shown. `tags: 'xy'` is refused and hidden. `tags: ['x']` is admitted.
`!contains('x')` with `'x'` is refused and hidden. `owners` (`select`,
`multiple: true`) gives the same `'x'` / `'xy'` pair.
- Controls on a non-multi-valued `text` column: `title == 'x'` with
`'x'`, and `title.contains('x')` with `'xy'`, are both admitted
(substring, no wrap).
- A by-id update and a predicate update: `'x'` is admitted and stored
`["x"]`. `'xy'` is 403 and the row is unchanged.
- Unit cells: `declaredMultiValueColumns` names exactly the declared
multi-valued columns. The image wrap leaves lists (by reference),
blanks, single-value `select` / `lookup` and `text` alone. A lone scalar
gets its list's verdict under `contains`, `!contains`, `$notContains`
and an `$or`, and the comparands come back as the same object.
- #21235's 38 temporal cells are unchanged and green.
- `packages/core/src/utils/multi-value-storage-form.test.ts`: 16 cells
for the rule itself.
### Ablations
Each ran on committed head `a049a417` through
`scripts/ablation-replace.mjs` in WRAP mode. Every anchor hit 1 -> 0.
Every restore was proven: blob == HEAD and `git diff HEAD` empty.
- **A1, the fold removed** (the judge passes no multi-valued columns): 9
red. That is the scalar-admitted `tags` / `owners` cells x2, `!contains`
x2, the by-id update x2, and 1 unit cell. The predicate-update cell
stays green, because the engine already wraps before that seam.
- **A2, the wrap applied to a non-declared `text` column**: 8 red. That
is the three `text` controls x2, plus 2 unit cells.
- **A3, the judge admits every image**: 25 red. That is the 12 negative
pins x 2 families, plus 1 unit cell. Every negative pin can fail.
- **A4, the core rule stops wrapping, rebuilt into `dist/`**:
`ablation-dist-preflight.mjs` found the marker in
`packages/core/dist/index.js` and `index.cjs`. Core unit: 7 red.
objectql `record-validator.test.ts`: 1 red, reached through core's
`dist/`. plugin-security: 16 red. Both faces read the one rule. Restore
leg: core rebuilt, preflight `--absent` ok, the whole tree clean, and
the three suites green again (16 / 126 / 61).
### Tests and gates
- Core: 79 files / 2199 passed (both vitest projects). Typecheck for
core, objectql and plugin-security: all three exit 0, and each
`check:test-typecheck` is OK.
- plugin-security full suite: 157 files, 3406 passed / 23 skipped.
- These ran at `242331e8`. The final head `8f7c43bf` differs from it
only in the changeset's text.
- `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack
--commands` at `8f7c43bf` derives 70 families. All 70 ran and exited 0.
`--ran` reports 70 derived, 70 run, 0 NOT-MEASURED, 0 UNRUN.
- The first sweep (at `242331e8`) had three non-zero exits, all cleared.
`check:engine-split-ratio` refused on the shallow clone (exit 2) and
passed after the deepening its remedy names.
`check:dual-build-cjs-loads` and `check:i18n` exited 3 (PREREQUISITE NOT
MET) and passed after a full turbo build (72/72).
- Narrowed lint at `8f7c43bf`: `eslint --no-inline-config --format json`
over the 6 changed `.ts` files gives 6 files, 0 errors, 0 warnings. The
population is `eslint.config.mjs:971`
(`**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}`). `eslint.config.mjs:327-329`
enables no type-aware linting, so no untouched file's verdict can move.
The full `pnpm lint` is CI's.
- No control bytes in the 7 changed files. `AGENTS.md` stayed unmodified
throughout.
### Docs
I grepped `content/docs/**` (outside `releases/` and `references/`) and
`skills/**` for the scalar wrap (`single-element array`, `lone scalar`,
`wrap … scalar`, `normalizeMultiValueFields`), and for `contains` /
multi-valued under a `check`. No sentence is made false. Positive
controls: the grep reaches
`content/docs/data-modeling/validation-rules.mdx:473` ("A lone scalar
value is coerced into a single-element array" is still true) and
`content/docs/permissions/rls.mdx:64` (the `check` judges each row an
insert or update writes, which is still true).
## Acceptance notes
1. **Scalar comparisons on a multi-valued field: the write check
evaluates them, and the read refuses them (400).** The table above shows
it. Under `record.tags != 'x'`, a member's write is admitted and stored,
while every read under that policy answers `INVALID_FILTER`. The formula
matcher (the write check) does not consume
`JSON_COLUMN_INCOMPATIBLE_OPERATORS`, the set `driver-sql`,
`driver-memory` and objectql's per-aggregation filter refuse by. This is
reported to the seat as a finding and is not fixed here.
2. `security-plugin.ts:3294` (the comment above the
`storedFormCheckJudge` call) still names only `date` / `datetime` /
`time`. It points to `rls-check-stored-form.ts` for what the step
carries, which now says it. The file is outside this claim's surface
(carrier: none).
3. Boundary: the write door does not wrap a `system` / `readonly`
multi-valued column, and the check's declaration does not carry those
flags. A caller's value there is stripped by the engine before its
seams. Only a hook's own scalar write to such a column is judged wrapped
while it is stored as written. That is the boundary the insert seam
already states for platform-owned values.
4. The `Clause-②: yes (widening)` line declares the new
`@objectstack/core` root export `multiValueStorageForm`. The RLS
admission change is a security-floor behaviour change, not Clause-②. The
claim's original `no` was revised on #21238 (Clause-② claim revision),
and the changeset carries the same line from `e559c9ac`, where the
changeset and ADR-0087 families and `check:nul-bytes` were re-run green.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ)_
---------
Co-authored-by: Claude <noreply@anthropic.com>
fix(plugin-security): a row-level `check` judges a lone scalar written to a declared multi-valued field as the one-member list it is stored as, so the write and the read the same policy scopes give one answer for one row (#21238)
8
+
9
+
Clause-②: yes (widening)
10
+
11
+
The write door stores a lone scalar sent to a multi-valued field (`tags`, `multiselect`, `checkboxes`, or a `select` / `lookup` / `user` / `file` / `image` flagged `multiple: true`) as a one-member list: `tags: 'x'` is stored as `["x"]`. The row-level write `check` judged the value as sent on the insert and on a by-id update, because both images are formed before the write door runs. Measured through `ObjectQL.insert` with `SecurityPlugin` on two SQLite driver families, as a member resolving a permission set, with the same predicate as `using` and `check`:
12
+
13
+
|`check`| written | write, before | stored | read |
|`record.tags.contains('x')`, a by-id update |`'x'`| 403 |`["x"]`| shown |
18
+
19
+
Now the image's value on every field the object declares multi-valued goes through the same rule the write door stores it by, before the check is judged. The first and third rows are admitted. The second is refused: a policy that forbids a member from tagging a row `x` can no longer be passed by sending `'x'` instead of `['x']`. A lone scalar now gets exactly the verdict its stored list gets, on the insert, a by-id update and a predicate update. That includes a policy that compares such a field with a scalar comparison (`==`, `!=`, `in`, an ordering), which the read refuses with `INVALID_FILTER` / 400: there `'x'` used to get the opposite of the verdict `['x']` got, and now gets the same one.
20
+
21
+
Unchanged: a field the object does not declare multi-valued is judged as written; a list, `null`, a blank string and an object are judged as written, as the write door leaves them; the check's comparands are left as written, since `contains` takes one member; and refusals keep their code and status (`PERMISSION_DENIED` / 403).
22
+
23
+
**`@objectstack/core`** (one new root export, so `minor`; this export is the widening the `Clause-②: yes (widening)` line declares): `multiValueStorageForm(value)`, the rule itself. It wraps a string, a number or a boolean into a one-member list and returns every other value as the same value. `@objectstack/objectql`'s `normalizeMultiValueFields` now calls it, with no change in what the write door stores (`patch`). `@objectstack/plugin-security` is `minor` because the set of writes its check admits widens (the first and third rows above); that is a security-floor behaviour change, not the declared widening.
0 commit comments