Repository navigation
Commit cab6396
fix(plugin-security)!: a predicate-scoped update or delete matches only the rows the caller can read (#21900)
Fixes #21829
Clause-②: no (narrowing)
## What this changes
A predicate-scoped (`multi: true`) update or delete now matches only the
rows its caller can read. This is ruling B on #21829 (record
`5994120632`), which carries ruling A on #21771 (`5985885287`) from the
by-id write door to the predicate door: on the write doors, a row the
caller cannot read is a row that does not exist.
A row the read door would not return to the caller is not written, not
counted and not refused. So a predicate that matches only such rows
answers exactly what a predicate that matches nothing answers: success,
zero rows. A caller who can read a matched row but may not write it
keeps the answer it had.
**Where.** The `plugin-security` write middleware, at the seam PR #21812
used. In step 3, before `next()`, the middleware asks the read door its
question for the caller's own predicate (`options.where`, the predicate
step 2.9 already guards). The question is a caller-context read through
the engine (`readableRowsScopeForPredicateWrite`), so every data
middleware's and every kit's read visibility applies, the parent-derived
read gates included. The answer is composed onto the write's AST as an
id list, beside the write scopes the step already composes. Matched set
= readable ∩ writable. The engine reads its matched rows from that AST,
so the narrowing binds the matched-row read, the per-row hook dispatch
and the driver write alike. No engine source and no kit write hook is
edited.
**Binding constraints, as built:**
1. **Addressed versus nested (H2).** Only the predicate write the caller
addressed is narrowed (`addressedPredicateWrite`, the twin of
`addressedByIdWriteId`). A write that starts inside another engine
operation reads as nested through #21812's `engineOperationScope` and
keeps today's answer: a cascade, a hook's own predicate write. The
referential FK clear is excluded by its server-derived marker. No second
"is this addressed?" signal.
2. **By-id versus predicate** comes from the engine's own dispatch
predicates (`resolveEngineUpdateDispatch`,
`resolveEngineDeleteDispatch`): only their `multi` verdict is narrowed.
3. **One classifier (H3).** `absentUnderCallerRead` now delegates to
`readUnlessRefused`, which the predicate read question asks too. A
declared 4xx (the read grant withheld, a predicate the driver will not
compile) keeps the write's previous answer. A store fault propagates as
raised. No second visibility evaluator.
4. **The cap.** The id list is bounded by the platform's existing row
ceiling for one predicate write, `MAX_BULK_PER_ROW_HOOK_ROWS` (10 000).
Above it the write is refused with `400 INVALID_FILTER` before anything
runs. That is the rule the engine's nested-relation lowering follows for
the same shape. It is never a cut-off list, which would write fewer rows
than the caller can reach, and never "do not narrow", which would hand
the existence signal back to a padded predicate. The count is of rows
the caller can read, so the refusal discloses nothing the read door does
not. No error code is new.
5. **Fail closed** when the engine cannot be asked: a predicate write is
not run un-narrowed.
**H5, `security/explain`.** Measured: `ExplainInput` takes an object, an
operation, a context and an optional record id. It has no predicate, so
it models no predicate-scoped write. It is left alone.
## Client census (step 1, before any code)
The question: does any shipped client, or any test in this repo, read a
predicate-scoped update's or delete's `403` as "a hidden row exists", in
a way the move to success-with-zero-rows would break?
- `packages/client`: `data.update` and `data.delete` are by id.
`data.updateMany(records)`, `data.deleteMany(ids)`, `batch` and
`batchTransaction` reach REST routes that the protocol serves with
per-id by-id loops. (c), unrelated.
- `packages/client-react`: mutation wrappers over the same methods. (c).
- REST: no route admits a caller-supplied predicate with `multi` (the
delete-many ingress narrows its options to the batch-options bag). So no
client door issues a predicate write.
- `packages/cli`: the secret re-wrap calls the driver's `updateMany`
directly, below the middleware. The migration plugin lists method names.
(c).
- **objectui console, at the `.objectui-sha` pin `0abd4f9f`,
read-only:** the data adapter's bulk update and bulk delete send id
lists to the same per-id routes, with a per-id fallback. No console,
app-shell or adapter source passes `multi`. The console issues no
predicate write. (c).
- Server-side issuers that are not clients: the automation
`update_record` and `delete_record` nodes with `multi: true`. Any error
becomes a failed step with its message, and success records the written
count. Nothing reads a `403` as existence. (c).
- Tests: 43 files carry `multi: true`. None asserts a `403` for a
predicate that matches only rows hidden from the caller through the real
security middleware. The kit suites run without the security middleware.
The select-only and check-only write-scope suites already assert that
unreadable rows stay untouched. The bulk widener probe uses a
public-read object. The unscoped-delete gate reads the caller's raw
predicate.
**No class (a) dependency. No class (b) pin turned.** That was confirmed
empirically: every candidate suite below is green with the change.
## Premises
**Premise 2, the seam, holds.** The engine runs the middleware chain
around the predicate path's matched-row read, for update and delete
alike (`executeWithMiddleware` wraps `driver.find(object, ast, …)` on
both verbs). The AST is seeded from `options.where` before the chain
runs, and step 2.9 already reads `opCtx.options.where` (H1).
**Premise 1, the cost, holds.** Measured on the showcase app
(`bootStack`, the real `SecurityPlugin`, the storage and audit plugins),
with a caller holding `showcase_contributor`. Rows were seeded at the
driver, and one predicate update matching every row was timed. "Without"
is the same build with the narrowing ablated (marker proved in `dist/`,
restored and rebuilt after). All numbers are shared-box wall clock: the
lock excluded other locked runs only.
| object | rows | without | with | rows written (without / with) |
read-door id read |
|---|---|---|---|---|---|
| `showcase_task` (select-scoped RLS) | 1 000 | 67.7 s | 58.0 s | 1 000
/ 1 000 | 7 to 11 ms |
| `sys_attachment` (parent-derived read) | 1 000 | 2.51 s | 2.29 s | 1
000 / 1 000 | 12 to 13 ms |
| `showcase_task` | 10 000 | 378.8 s | 472.1 s | 10 000 / 10 000 | 27 to
45 ms |
| `sys_attachment` | 10 000 | 18.85 s | 19.87 s | 10 000 / 10 000 | 39
to 40 ms |
The end-to-end spread is box noise: the sign flips between 1 000 and 10
000. So the narrowing's own cost was measured directly at 10 000 rows,
as the median of 5 runs each:
- the read-door id read: 33 ms;
- the matched-row read: 125 ms plain, 144 ms with the id list;
- the driver's predicate update: 18 ms plain, 47 ms with the id list.
That is about 81 ms in all, against a write that pays 19 s (attachments)
to 379 s (tasks) for the same rows. The write's own budget is its
per-row hook dispatch.
**How the narrowing meets the kits' `MULTI_WRITE_AUTH_LIMIT` (1 000).**
It does not meet it. A 10 000-row predicate update on `sys_attachment`
lands on both builds. The engine's per-row dispatch binds `input.id`, so
the kits' row resolver takes its by-id branch. Their 1 000-row bound is
reached only on the whole-operation dispatch, which refuses the unscoped
shape before resolving anything. The ceiling that binds both legs is the
engine's per-row hook ceiling (10 000), which is also the narrowing's
cap.
## Pins
New `plugin-security/src/predicate-write-unreadable-not-matched.test.ts`
uses a real `ObjectQL`, a real SQL driver, the real middleware and a
kit-like per-row gate, on update and delete. It pins:
- a predicate matching only hidden rows equals a predicate matching
nothing;
- hidden and visible rows: only the visible rows change, and they alone
are counted;
- a reader who may not write keeps the gate's `403`;
- control: a visible, writable match is written;
- a read the read door refuses keeps the previous answer;
- a store fault on the read question propagates, with nothing written;
- a readable set over the cap is refused (`INVALID_FILTER`, 400), with
nothing written;
- a hook's own predicate write keeps the gate's `403`, while the same
caller addressing that predicate gets zero rows;
- the by-id doors keep #21812's answers.
New
`qa/dogfood/test/predicate-write-unreadable-not-matched.dogfood.test.ts`
runs on a real stack, at the engine's predicate door, under the context
the REST door resolves for the caller's own token. It covers both
principal classes (outside and inside the ownership floor's `org_member`
domain, each proven by arming probes), update and delete, and
`sys_attachment`, `sys_comment` and a plain row-level-security object
whose write-class policy reaches rows its read policy hides. 12 cells,
each asserting:
- hidden-only equals nothing (zero rows, nothing written);
- the reader keeps its answer;
- hidden and visible: count 1, only the visible row changed;
- the by-id door still answers `404 RECORD_NOT_FOUND` for the hidden
row.
The reader cells pin the answer each class had. The gate's named `403`
applies where the write scope reaches the row: outside the domain on
both verbs, and on delete inside it. Zero rows applies where the
ownership floor or the write-class policy already excludes the row.
**Doubles.** Two plugin-security harnesses (`security-plugin.test.ts`'s
middleware context and `tenant-layer0-verdict-on-operation.test.ts`'s
engine) gained a `find`, because a `ql` that cannot answer the read
question refuses the write. Their `findOne` now refuses what the real
engine refuses (`assertEngineFindOnePredicate`). Their assertions are
unchanged.
## Ablation
All ablations went through `scripts/ablation-replace.mjs`, with a trap
restore and the fix committed first.
- **A, the un-narrowed matched set (unit).** The narrowing call was
skipped. 8 of 17 unit pins went red: both verbs' hidden-only equality,
both verbs' hidden-and-visible count, both verbs' store-fault
propagation, the over-cap refusal, and the addressed control. Restore
was proven: blob == HEAD (`38bebf13`), `git diff HEAD` empty.
- **A, dist-mediated (dogfood).** A runtime-only guard was planted, and
the marker was proved present in 2 built `dist/` files. 10 of 12 door
cells went red, twice (once per measurement leg). The 2 cells that stay
green are inside-class updates on the two kit objects, where the
platform's ownership floor already kept the hidden row out of the write
scope before this change. Restore was proven both times: blob == HEAD,
rebuilt, `ablation-dist-preflight --absent` clean on the whole tree.
- **B, narrowing applied to nested writes too (unit).** The
hook's-own-write pin went red: zero rows instead of the gate's `403`.
Restore was proven: blob == HEAD.
## Verification
Head `23df2c8a` unless stated. The branch merges `origin/main` twice: at
`e864db56` (carrying #21873) and at `67c544cc` (carrying #21881, the
sibling `permission-set-projection` change).
- `plugin-security`: the full suite, 168 files, 3632 passed, 45 skipped,
0 failed. `typecheck` (`tsc`, scripts, `check:test-typecheck`) is green.
- `dogfood`: `typecheck` green. On this head, the new door file, the
#21812 door file, #21881's write-through binding file and the
owner-anchor bulk-write file: 4 files, 45 passed. On `44fa3fc1` (the
first merge), a batch of 10 files, with the #21812 door and
parent-derived files and every census candidate (owner-anchor bulk
writes, the bulk widener probe, the unscoped attachment gate, the engine
where-shape refusal, flow run-as, the attachment matrix, the
authored-row write scope), is 108 passed and 1 skipped. Also green: the
showcase declarative endpoints, 17 tests (its predicate delete flow runs
as system).
- `plugin-sharing`: 38 files, 954 passed. `service-automation`: the
write-node and bulk-intent suites, 27 passed. `runtime`: the
stored-metadata body boundary pin, 7 passed.
- `spec`: `check:migration-registry` reports the registry current (379
semantic entries). The migration and spec-changes surface suites: 176
passed. The ADR-0087 registration gate reads `registered
predicate-write-unreadable-row-not-matched` (new here).
- Derived gates: `dispatch-gates --commands` lists 124 commands on this
head. All 124 exit 0, each run with its exit code captured before any
pipe. The `--ran` reconciliation reads: 124 derived, 124 run, 0
NOT-MEASURED (a derived zero, from the recorded exit codes), 0 UNRUN.
One finding along the way was fixed: with `find` beside `findOne`, the
two harnesses became engine doubles to `check:engine-double-contract`.
Their `findOne` now opens with `assertEngineFindOnePredicate`, and the
pinned ledger (`scripts/engine-double-contract.pinned.json`) records the
grown coverage via the gate's own `--write`.
- Lint, narrowed: `eslint --no-inline-config` over the 7 changed
TypeScript files reports 0 errors and 0 warnings (counted from `--format
json`). The population is `eslint.config.mjs`'s `**/*.{ts,…}` glob minus
its never-linted list, and none of the 7 was ignored. The config enables
no type-aware linting (no `parserOptions.project`, no typed rules), so
this diff cannot move a verdict on an untouched file. The repo-wide run
is CI's.
## Docs
The sentences this made false are corrected:
- the multi-delete rule in the attachments access page, and its update
twin;
- the performance note in the permissions matrix;
- the write-widener floor paragraph in the RLS page. That paragraph also
still named a `403` for a hidden by-id target, which #21812 turned into
the not-found; it is corrected in the same sentence.
## ADR-0087
The changeset carries the FROM → TO a caller acts on, so `registered` is
the honest disposition. One D3 semantic entry,
`18.predicate-write-unreadable-row-not-matched`, sits beside
`18.by-id-write-unreadable-row-not-found`. It was generated with
`gen:migration-registry`, and `@objectstack/spec` is named in the
changeset (`patch`). No Zod schema, contract docblock or export moves.
## Acceptance notes
- **The read door's candidate window on the parent-derived objects.**
The attachment and comment read middleware pre-scans at most 2 000
candidate rows per read, and fails closed beyond that: rows past the
window are omitted, with a logged warning. The narrowing asks that read
door, so a predicate write that reaches more than 2 000 candidate rows
on those objects writes only the rows the read door returns. Measured: 3
000 attachments on 3 000 readable parents, one predicate update. Before,
3 000 were written. After, 2 000 were written, and the result says 2
000. This follows the ruling's text ("a row the read door would not
return is not matched"), and the count is honest. Whether that window
should be widened, or whether a narrowed write should refuse instead, is
a question for the seat, not decided here.
- **The cap is a narrowing too.** A predicate write whose predicate
matches more than 10 000 rows the caller can read is now refused (`400
INVALID_FILTER`), even where fewer of them are writable. On a composed
kernel every object measured carries per-row write hooks, so a write
matching more than 10 000 writable rows was already refused by the
engine's ceiling. The new refusal reaches only a predicate that matches
more readable than writable rows past that ceiling. The changeset
declares it.
- **Inside the `org_member` domain**, the ownership floor already kept
hidden rows out of predicate updates on the two kit objects. The change
there is on delete, and on the plain RLS object.
- **The kits' not-visible refusal** now reaches only writes the caller
did not address, and nested writes. The kits' docblocks are not edited
(no kit edit, per the ruling).
- **Not edited:** the sharing-rules page still names a `403` for a by-id
write to a row hidden on a `private` object. That has been stale since
#21812, and no sentence there is about predicate writes. Carrier: none.
- **Declarations:** the new dogfood file is on #6024 (`5994866670`). The
step-18 entry is the conditional spec declaration on #6017
(`5994857244`). The landing needs the at-tier contract review the ruling
requires.
---
_Generated by [Claude
Code](https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 866683f commit cab6396
12 files changed
Lines changed: 1118 additions & 33 deletions
File tree
- .changeset
- content/docs/permissions
- packages
- plugins/plugin-security/src
- qa/dogfood/test
- spec/src/migrations
- entries/semantic
- scripts
Lines changed: 30 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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
66 | 66 | | |
67 | 67 | | |
68 | 68 | | |
69 | | - | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
70 | 74 | | |
71 | 75 | | |
72 | 76 | | |
| |||
76 | 80 | | |
77 | 81 | | |
78 | 82 | | |
79 | | - | |
| 83 | + | |
| 84 | + | |
80 | 85 | | |
81 | 86 | | |
82 | 87 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
360 | 360 | | |
361 | 361 | | |
362 | 362 | | |
363 | | - | |
| 363 | + | |
364 | 364 | | |
365 | 365 | | |
366 | 366 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
88 | 88 | | |
89 | 89 | | |
90 | 90 | | |
91 | | - | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
92 | 96 | | |
93 | 97 | | |
94 | 98 | | |
| |||
0 commit comments