Repository navigation
Commit c11b758
fix(plugin-security): explain's read verdict on a controlled_by_parent record takes the read door's answer on the master leg (#22813)
Part of #22792
This PR delivers item 1 of the card; item 2 (ruling A on #22795) stays
on the card, and #22792 remains open for item 2.
Clause-②: no
## What this changes
Class: explain-versus-door parity, the `read` verb. The family's triage
record on #22530 pinned every write verb; this widens that pin to
`read`. Position: `applyRecordAttribution`'s `read` branch in
`packages/plugins/plugin-security/src/explain-engine.ts`.
For a `controlled_by_parent` record, the read door scopes the find by
the record's master as well as by its own row-level security (ADR-0055).
explain's `read` branch modelled the record's own row-level security
only, so its record verdict could disagree with the read door. It now
asks the read door's own by-id read for that leg, and its verdict equals
the door's answer:
- **Asked when:** a record-grained `read` (or `export`, which streams
the same find) of a record that exists, on a `controlled_by_parent`
object, past the capability and object gates, where every leg the report
already models (the tenant wall, the record's business row-level
security, the sharing read filter) admits the record.
- **Asked how:** through the existing
`ExplainEngineDeps.recordAbsentToCaller`, the caller-context by-id read
the write doors already ask, with the EXPLAINED context. No second copy
of the master derivation.
- **The answer:** a record the door withholds is `visible: false`,
decided by the `sharing` layer (the layer the write verbs' master check
names), with that layer's record `excluded`. A record the door returns
leaves the report byte-identical. A rejection is reported fail-closed
(`not_evaluated`, no predicate), as every other dependency fault on the
record path is.
- **Unchanged:** every object that is not `controlled_by_parent`, every
write verb (the read-absent path of the by-id writes is not touched),
object-level reports, `allowed`, and the answer for an id no row
carries. The verdict keeps a decider; whether it should take the
nonexistent-id shape is item 2's round, not this PR.
### Why the existing dependency and not a new one
The PM route pointed at the read door's master-aware read (the security
plugin's master-derived read filter). Reaching it directly would add an
optional key to the exported `ExplainEngineDeps` type. On this family's
own precedent (the update half, which added
`checkControlledByParentWrite`) that is a widening of a published type,
`Clause-②: yes (widening)` with a `minor` changeset. The claim and this
dispatch declare `Clause-②: no` with a `patch`. `recordAbsentToCaller`
is the read door itself, so asking it is parity by construction, and no
public type moves. Only its docblock changes, to say it is now asked for
such a read too.
## Pins
| pin | where | what it holds |
|:--|:--|:--|
| **Family enumeration, REST** (the pin the #22530 triage record named,
widened in place) |
`packages/qa/dogfood/test/cbp-explain-master-write.dogfood.test.ts` |
The per-verb door table now gives `read` a by-id read door (`GET`) on
the fixture's private-master detail object. Three cells, each beside
that door: a record under a master the principal cannot read (door `404
RECORD_NOT_FOUND`, explain `visible: false`, decided by `sharing`);
CONTROL, a record under a master the principal reads (door `200`,
explain `visible: true`); CONTROL, an id no row carries (door `404
RECORD_NOT_FOUND`, explain unchanged, no decider). The table stays total
against `ExplainOperationSchema`. |
| **Engine, per verb** (the engine's verb classification, widened in
place) |
`packages/plugins/plugin-security/src/explain-controlled-by-parent-write.test.ts`
| `read` and `export` are classified `read_door_asked`. For each: a
withheld record is not visible on `sharing`; a returned one is
byte-identical to the report without the question; a rejection is
fail-closed with no predicate; asked once with the explained context,
and the write check is not asked; not asked where the record's own RLS
or the tenant wall already decides; not asked on a `private` or
`public_read_write` object, nor for an object-level request or a missing
record; `create` is not asked. |
No new test file. The registered-service enumeration
(`controlled-by-parent-write-member.test.ts`) is left as it was; see
Acceptance notes.
## Measurement and ablation
- **Before** (a temporary, uncommitted plugin-level harness over a real
`ObjectQL`, SQL driver and `SharingService`, on `179f7bf6c`, which
equals `680a86b4c` for `plugin-security`): the read door withheld the
record and explain's `read` verdict disagreed with it. PM assumption 1
holds. **After** (same harness, the changed source): the verdict equals
the door for the subject and both controls.
- **Ablation** (one leg, `scripts/ablation-replace.mjs` in wrap mode,
anchor 1 to 0, blob changed, under an outer `trap` restoring to `HEAD`
on EXIT, INT and TERM): the read master leg switched off behind a
marker. Then `pnpm --filter @objectstack/plugin-security build`, then
`scripts/ablation-dist-preflight.mjs` read the marker in 2 built files.
Predicted: the engine's withheld, rejection and asked-once cells red for
`read` and `export`, and the REST subject cell red, with every control
green. Observed: engine **6 red**, 67 green; REST **1 red** (the subject
cell), 17 green. Restore leg: blob equals `HEAD`, `git diff HEAD` empty,
rebuilt, and the preflight with `--absent` read the marker absent from
all 6 built files with the tree clean.
## Local verification (head `eb4274251`, after merging `origin/main`
`098481744`)
- `pnpm --filter @objectstack/plugin-security exec vitest run`: 199
files, 4053 passed, 45 skipped (run on `5ad8ac3f5`, before the merge,
which touched no file of this package). On `eb4274251`: the three
explain and master-check files, 107 passed; the widened dogfood file, 18
passed.
- `pnpm --filter @objectstack/plugin-security typecheck`: exit 0. `pnpm
--filter @objectstack/dogfood typecheck`: exit 0, after a full workspace
build.
- `node scripts/pm/dispatch-gates.mjs --commands`: 70 derived commands,
all run on `eb4274251`, all exit 0. Two first answered `PREREQUISITE NOT
MET` (exit 3, not a measurement) and were re-run green after the full
build. `--ran`: 70 derived, 70 run, 0 NOT-MEASURED.
- Lint, narrowed to the three touched TypeScript files and proved. All
three are in eslint's population (`--print-config` resolves each), the
changeset is outside its `files` globs, and `--format json` counts 3
files with 0 errors and 0 warnings. `eslint.config.mjs` enables no
type-aware linting, so this diff cannot move the verdict on any file it
does not touch.
- Not run locally, declared to CI: the repository-wide `pnpm lint`, the
full dogfood suite (only the touched file ran), and the path-scheduled
CI jobs.
## Acceptance notes
- **Delegated reads.** On an on-behalf-of read the door also composes
the delegator's scope. The record path still does not model the
delegator's row-level security on its own, so when the door withholds
such a record the sharing layer names the master and says that the
delegator's scope is composed in too. The visible verdict is the door's
either way. Not filed: no measurement.
- **The registered-service enumeration**
(`controlled-by-parent-write-member.test.ts`, `VERB_ROWS`) still
classifies `read` as `no_by_id_write`, which stays literally true. Its
fixture has no private master, so a read-door row there needs a fixture
change. The family's REST and engine enumerations carry `read`. Carrier:
none.
- **The object-level `readFilter`** on a `read` explanation is the `rls`
layer's artifact (`computeRlsFilter`). The service's `getReadFilter`
also ANDs the master-derived scope and the sharing filter. This is an
observation from reading code, not measured at a door, and it is the
object-level question, not this card's record verdict. Not filed.
Carrier: none.
- **Wiring comment.** The comment beside the `recordAbsentToCaller`
wiring in `security-plugin.ts` still calls it the write path's read
question. It is accurate, but it no longer covers every caller. Left
alone to keep the diff inside the declared territory.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01CBAfsWMSfM3EToQGVStEcp)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent e078506 commit c11b758
4 files changed
Lines changed: 249 additions & 18 deletions
File tree
- .changeset
- packages
- plugins/plugin-security/src
- qa/dogfood/test
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
Lines changed: 110 additions & 7 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
23 | 23 | | |
24 | 24 | | |
25 | 25 | | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
26 | 35 | | |
27 | 36 | | |
28 | 37 | | |
| |||
39 | 48 | | |
40 | 49 | | |
41 | 50 | | |
42 | | - | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
43 | 54 | | |
44 | 55 | | |
45 | 56 | | |
46 | 57 | | |
47 | 58 | | |
48 | 59 | | |
49 | 60 | | |
| 61 | + | |
50 | 62 | | |
51 | 63 | | |
52 | 64 | | |
53 | 65 | | |
54 | 66 | | |
55 | 67 | | |
56 | 68 | | |
| 69 | + | |
| 70 | + | |
57 | 71 | | |
58 | 72 | | |
59 | 73 | | |
| |||
77 | 91 | | |
78 | 92 | | |
79 | 93 | | |
| 94 | + | |
80 | 95 | | |
81 | 96 | | |
82 | 97 | | |
| |||
100 | 115 | | |
101 | 116 | | |
102 | 117 | | |
103 | | - | |
104 | | - | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
105 | 123 | | |
106 | | - | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
107 | 128 | | |
108 | 129 | | |
109 | 130 | | |
110 | 131 | | |
111 | 132 | | |
112 | | - | |
113 | | - | |
| 133 | + | |
| 134 | + | |
114 | 135 | | |
115 | 136 | | |
116 | 137 | | |
| |||
243 | 264 | | |
244 | 265 | | |
245 | 266 | | |
246 | | - | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
247 | 270 | | |
248 | 271 | | |
249 | 272 | | |
| |||
262 | 285 | | |
263 | 286 | | |
264 | 287 | | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
| 323 | + | |
| 324 | + | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
| 347 | + | |
| 348 | + | |
| 349 | + | |
| 350 | + | |
| 351 | + | |
| 352 | + | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
419 | 419 | | |
420 | 420 | | |
421 | 421 | | |
| 422 | + | |
| 423 | + | |
| 424 | + | |
| 425 | + | |
| 426 | + | |
| 427 | + | |
422 | 428 | | |
423 | 429 | | |
424 | 430 | | |
| |||
942 | 948 | | |
943 | 949 | | |
944 | 950 | | |
| 951 | + | |
945 | 952 | | |
946 | 953 | | |
947 | 954 | | |
| |||
1516 | 1523 | | |
1517 | 1524 | | |
1518 | 1525 | | |
| 1526 | + | |
| 1527 | + | |
| 1528 | + | |
| 1529 | + | |
| 1530 | + | |
| 1531 | + | |
| 1532 | + | |
| 1533 | + | |
| 1534 | + | |
| 1535 | + | |
| 1536 | + | |
| 1537 | + | |
| 1538 | + | |
| 1539 | + | |
| 1540 | + | |
| 1541 | + | |
| 1542 | + | |
| 1543 | + | |
| 1544 | + | |
| 1545 | + | |
| 1546 | + | |
| 1547 | + | |
| 1548 | + | |
| 1549 | + | |
| 1550 | + | |
| 1551 | + | |
| 1552 | + | |
| 1553 | + | |
| 1554 | + | |
| 1555 | + | |
| 1556 | + | |
| 1557 | + | |
| 1558 | + | |
| 1559 | + | |
| 1560 | + | |
1519 | 1561 | | |
1520 | 1562 | | |
1521 | 1563 | | |
| |||
1525 | 1567 | | |
1526 | 1568 | | |
1527 | 1569 | | |
1528 | | - | |
| 1570 | + | |
| 1571 | + | |
| 1572 | + | |
| 1573 | + | |
1529 | 1574 | | |
1530 | 1575 | | |
1531 | 1576 | | |
| |||
1560 | 1605 | | |
1561 | 1606 | | |
1562 | 1607 | | |
1563 | | - | |
1564 | | - | |
| 1608 | + | |
| 1609 | + | |
1565 | 1610 | | |
1566 | 1611 | | |
1567 | 1612 | | |
| |||
1585 | 1630 | | |
1586 | 1631 | | |
1587 | 1632 | | |
1588 | | - | |
1589 | | - | |
| 1633 | + | |
| 1634 | + | |
1590 | 1635 | | |
1591 | 1636 | | |
1592 | 1637 | | |
| |||
1778 | 1823 | | |
1779 | 1824 | | |
1780 | 1825 | | |
1781 | | - | |
1782 | | - | |
| 1826 | + | |
| 1827 | + | |
| 1828 | + | |
| 1829 | + | |
1783 | 1830 | | |
1784 | 1831 | | |
1785 | 1832 | | |
| |||
0 commit comments