Repository navigation
Commit 71a8b5e
fix(plugin-security): explain's record-grained write verdict composes the by-id write gate's authored-row-write deferral (#24713)
Fixes #24647
Clause-②: no
## What this changes
`security/explain`'s record-grained write verdict now composes the by-id
write gate's authored-row-write deferral, the composition the sharing
middleware already runs. When record sharing's per-record update or
delete gate refuses a row, the door asks whether an app-authored
row-level policy admits that row for the gate's verb
(`ISecurityService.checkAuthoredRowWrite`, reached through
`SharingService.probeAuthoredRowWrite`), and on `admit` hands the row to
the pre-image gate. Explain asked the gate alone, so it answered
`record.visible: false`, `decidedBy: 'sharing'` for a row whose `PATCH`
is admitted. The divergence was restrictive: explain refused what the
door admits, and nothing leaked.
- `packages/plugins/plugin-security/src/explain-engine.ts`: a new
optional dependency, `probeAuthoredRowWrite`, asked only when the write
gate answered `false`: for `update` and `delete` with the gate's verb,
and for `transfer` as an `update`. Only the literal `admit` lifts the
refusal. `abstain`, any other answer and a rejection keep it: the door's
probe catches its own failure and keeps its refusal, so a failing probe
is not reported as a failing request.
- `packages/plugins/plugin-security/src/security-plugin.ts`: the explain
binding wires it to the sharing service's own probe, when the service
has one, with the context the gate is asked with (`withWriteScope`). A
sharing service without the probe defers nothing, and explain reports
the gate's refusal as before.
- `.changeset/24647-explain-authored-row-write.md`:
`@objectstack/plugin-security` patch.
### What the record story reads for the new row
`record: { recordId, visible: true, decidedBy: 'rls' }`. `rls` is the
layer that last admitted the row, which is the published meaning of
`decidedBy`; no vocabulary value is added and `packages/spec` is
untouched. The `sharing` layer's record keeps `outcome: 'excluded'`
(record sharing itself still refuses), and its detail adds: "The by-id
update does not stop at that refusal: an app-authored row-level update
policy admits this record, so the write gate defers to row-level
security (see the rls layer)." The `rls` layer's record reads
`admitted`.
### Why the composition sits in the engine (a deviation from the
suggested route)
Composing at the binding alone (`canEditRecord` answering `canEdit ||
admit`) makes the row `visible: true` but leaves the story's `sharing`
layer at `admitted` with "The sharing service grants write on this
record (ownership or an edit/full share)" and `decidedBy: 'sharing'`.
That is a wrong "why" naming a share that does not exist, which is the
cost the card names. So the deferral is a dependency the engine composes
beside the gate, the way it already composes the master-detail check.
This touches `explain-engine.ts`, which the claim's file surface did not
list. No other open PR touches it.
## `delete` and `transfer`, included in place
- The door's delete path defers too: `buildSharingMiddleware` probes
with the refusing gate's verb for both `update` and `delete`. A
transfer's door is the by-id `PATCH` of `owner_id`, which the update
gate judges and the same deferral lifts, and explain's transfer verdict
rides the update gate. Measured before this change (the ablated run
below): all three answered `visible: false`, `decidedBy: 'sharing'`
beside an admitted door.
- The four in-place conditions hold: (1) the same defect class; (2) a
mechanical fix, one composition with the verb its gate asked; (3) no
other claim holds these files; (4) the same parity table, so no new
validation surface. Each verb has its own parity row.
- `restore` and `purge` are not deferred: they have no by-id door, and
the object gate refuses both to every principal.
## Security floor: no enforcement answer moves
- The door (`plugin-sharing`) and the `sys_attachment` / `sys_comment`
parent gates are not edited.
- Both parent gates ask record sharing's `checkEdit` first and refuse on
`deny` without asking explain. On the card's row the real
`SharingService.checkEdit` answers `deny` under the gates' envelope,
both with this change and with it ablated, so neither gate's answer can
change. Each gate's own parity table already pins "`sharing deny` ⇒
refused, explain never called", with an explain double that would admit
(`comment-parent-update-parity.test.ts`,
`attachment-access-hooks.test.ts`).
- No path was found where explain's verdict decides a write alone today.
The other `explain` callers ask object-level `create` or `read`, and
objectui reads the verdict for UI affordance only.
**Pins a sibling card flips:**
`packages/plugins/plugin-security/src/explain-enforce-parity.test.ts`,
test "an app-authored row-level write policy the by-id write gate defers
to (ADR-0105 D3) › the parent gates' first question, record sharing,
still refuses the row explain now admits". #24646 remains open. Its
ruling (option B, comment 6279880675) has both gates ask only explain's
record-grained update verdict, and that flips this pin: the gate then
admits exactly what the parent's by-id update admits, which for this
scenario, with this deferral, is admitted. The flip is pre-registered in
a comment beside the assertion.
## Tests
All on head `2fdf7b1987`, each through `scripts/pm/os-verify-lock.sh`,
verdict line `command-exit 0`:
- `pnpm --filter '@objectstack/plugin-security^...' build` (dependency
closure), then the `./packages/**` build after the `main` merge.
- `pnpm --filter @objectstack/plugin-security exec vitest run
--maxWorkers=2`: `Test Files 189 passed (189)`, `Tests 3794 passed | 45
skipped (3839)`.
- `pnpm --filter @objectstack/plugin-security typecheck`: exit 0, and
`check:test-typecheck: OK` (the test layer compiles under
`tsconfig.test.json`, so the new rows are type-checked).
- `vitest run --reporter=verbose src/explain-enforce-parity.test.ts`:
`Tests 106 passed (106)`. New cases: the table rows for update, delete
and transfer of a row an authored policy admits (door admitted, explain
`visible: true`), the two controls (a row no authored policy admits:
door 403 `PERMISSION_DENIED`, explain `visible: false`), and three
cases: the story (`decidedBy: 'rls'`, `sharing` layer `excluded`, `rls`
layer `admitted`), the parent gates' first question (`checkEdit` is
`deny` on the row explain now admits) and its positive control (the
writer's own row: `checkEdit` `allow`, explain and the door admit).
## Ablation (a one-time proof; no permanent test file)
`node scripts/ablation-replace.mjs` on `security-plugin.ts`, removing
the binding's wiring of the probe (anchor count 1 to 0), on head
`2fdf7b1987`:
- Mutated: `Tests 5 failed | 101 passed (106)`, failing in the expected
direction (red). The update, delete and transfer rows show explain at
`record: {"recordId":"l_theirs","visible":false,"decidedBy":"sharing"}`
beside `enforcement {"kind":"admitted"}`, which is the card's divergence
reproduced. The story case and the parent-gate case fail at the explain
assertion. The parent-gate case's `checkEdit` assertion (`deny`) passes
before that point, so the gates' first answer is the same with and
without the change. The positive control and both refusal controls stay
green.
- Restored: `git checkout HEAD -- ABS_PATH`, blob equal to HEAD
(`56d547d5ed1f`), `git diff HEAD` empty. The same ablation on the
pre-merge head `d2e245471` gave the same 5 failures.
## Gates
`node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack`, run on head `2fdf7b1987` (the merge head)
against the real diff: 69 commands, each run with its exit code captured
before any pipe. All 69 exit 0. `dispatch-gates --ran`: "69 derived
famil(ies) accounted for — 69 run, 0 NOT-MEASURED". The derived list is
identical to the one derived before the merge. Before the merge, three
families first answered `PREREQUISITE NOT MET` (exit 3):
`check-plugin-teardown-shape --self-test` (its pinned fixture commit was
not reachable yet), `check:i18n` (CLI not built) and
`check:dual-build-cjs-loads` (not every package built). All three were
re-run once their prerequisite was met, and all exit 0 in the final
union. CI's own check-runs on this PR are the authority for every family
CI runs.
## Acceptance notes
- `main` was merged once before this PR opened (merge commit
`2fdf7b1987`): the claim's serial constraint said whichever of this card
and #24623 lands second merges `main`, and #24623 landed (`d11fa8cc19`)
with a change to `security-plugin.ts` at a disjoint region. The merge
was clean. The build, the suite, the typecheck and the gate union were
re-run on the merge head.
- The deferral's code tags read `[#5493 / ADR-0105 D3]`, following this
file's existing tag for `checkAuthoredRowWrite`. ADR-0105 D3's own text
is about stripping only the platform's wildcard tenant policies; the
deferral's provenance is the #5493 ruling. A citation observation, not
filed.
- Explain's `purge` record verdict asks the update gate (`engineOp ===
'delete' ? canDeleteRecord : canEditRecord`), though a purge destroys
like a delete. The object gate refuses `purge` to every principal, so
the record verdict never reaches that gate. Dormant, not filed.
- `checkAuthoredRowWrite`'s docblock says a private-OWD cross-owner
by-id write is still refused end to end. That holds for a caller whose
read scope hides the row (the dogfood pin's principal). With `org` read
depth the write is admitted, which the new parity row measures. Not
filed.
---
_Generated by [Claude
Code](https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 0b4be0b commit 71a8b5e
4 files changed
Lines changed: 228 additions & 6 deletions
File tree
- .changeset
- packages/plugins/plugin-security/src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
559 | 559 | | |
560 | 560 | | |
561 | 561 | | |
| 562 | + | |
| 563 | + | |
| 564 | + | |
| 565 | + | |
| 566 | + | |
| 567 | + | |
| 568 | + | |
| 569 | + | |
562 | 570 | | |
563 | 571 | | |
564 | 572 | | |
565 | 573 | | |
566 | 574 | | |
567 | 575 | | |
568 | | - | |
| 576 | + | |
| 577 | + | |
| 578 | + | |
| 579 | + | |
569 | 580 | | |
570 | | - | |
| 581 | + | |
571 | 582 | | |
572 | 583 | | |
573 | 584 | | |
| |||
601 | 612 | | |
602 | 613 | | |
603 | 614 | | |
| 615 | + | |
| 616 | + | |
| 617 | + | |
| 618 | + | |
| 619 | + | |
| 620 | + | |
| 621 | + | |
604 | 622 | | |
605 | 623 | | |
606 | 624 | | |
| |||
613 | 631 | | |
614 | 632 | | |
615 | 633 | | |
| 634 | + | |
| 635 | + | |
| 636 | + | |
| 637 | + | |
| 638 | + | |
| 639 | + | |
| 640 | + | |
| 641 | + | |
| 642 | + | |
| 643 | + | |
| 644 | + | |
| 645 | + | |
616 | 646 | | |
617 | 647 | | |
618 | 648 | | |
| |||
645 | 675 | | |
646 | 676 | | |
647 | 677 | | |
648 | | - | |
| 678 | + | |
649 | 679 | | |
650 | 680 | | |
651 | 681 | | |
652 | 682 | | |
| 683 | + | |
| 684 | + | |
| 685 | + | |
| 686 | + | |
| 687 | + | |
| 688 | + | |
| 689 | + | |
| 690 | + | |
| 691 | + | |
| 692 | + | |
653 | 693 | | |
654 | 694 | | |
655 | 695 | | |
| |||
700 | 740 | | |
701 | 741 | | |
702 | 742 | | |
703 | | - | |
| 743 | + | |
704 | 744 | | |
705 | 745 | | |
706 | 746 | | |
| |||
935 | 975 | | |
936 | 976 | | |
937 | 977 | | |
| 978 | + | |
| 979 | + | |
| 980 | + | |
| 981 | + | |
| 982 | + | |
| 983 | + | |
| 984 | + | |
| 985 | + | |
| 986 | + | |
| 987 | + | |
| 988 | + | |
| 989 | + | |
| 990 | + | |
| 991 | + | |
| 992 | + | |
| 993 | + | |
| 994 | + | |
| 995 | + | |
| 996 | + | |
| 997 | + | |
| 998 | + | |
| 999 | + | |
| 1000 | + | |
| 1001 | + | |
| 1002 | + | |
| 1003 | + | |
| 1004 | + | |
| 1005 | + | |
| 1006 | + | |
| 1007 | + | |
| 1008 | + | |
| 1009 | + | |
| 1010 | + | |
| 1011 | + | |
| 1012 | + | |
| 1013 | + | |
| 1014 | + | |
| 1015 | + | |
| 1016 | + | |
| 1017 | + | |
| 1018 | + | |
| 1019 | + | |
| 1020 | + | |
938 | 1021 | | |
939 | 1022 | | |
940 | 1023 | | |
| |||
999 | 1082 | | |
1000 | 1083 | | |
1001 | 1084 | | |
| 1085 | + | |
| 1086 | + | |
| 1087 | + | |
| 1088 | + | |
| 1089 | + | |
| 1090 | + | |
| 1091 | + | |
| 1092 | + | |
| 1093 | + | |
| 1094 | + | |
| 1095 | + | |
| 1096 | + | |
| 1097 | + | |
| 1098 | + | |
| 1099 | + | |
| 1100 | + | |
| 1101 | + | |
| 1102 | + | |
| 1103 | + | |
| 1104 | + | |
| 1105 | + | |
| 1106 | + | |
| 1107 | + | |
| 1108 | + | |
| 1109 | + | |
| 1110 | + | |
| 1111 | + | |
| 1112 | + | |
| 1113 | + | |
| 1114 | + | |
| 1115 | + | |
| 1116 | + | |
| 1117 | + | |
| 1118 | + | |
| 1119 | + | |
| 1120 | + | |
| 1121 | + | |
| 1122 | + | |
| 1123 | + | |
| 1124 | + | |
| 1125 | + | |
| 1126 | + | |
| 1127 | + | |
| 1128 | + | |
| 1129 | + | |
| 1130 | + | |
| 1131 | + | |
| 1132 | + | |
| 1133 | + | |
| 1134 | + | |
| 1135 | + | |
| 1136 | + | |
| 1137 | + | |
| 1138 | + | |
| 1139 | + | |
| 1140 | + | |
| 1141 | + | |
| 1142 | + | |
| 1143 | + | |
| 1144 | + | |
| 1145 | + | |
| 1146 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
36 | 36 | | |
37 | 37 | | |
38 | 38 | | |
| 39 | + | |
| 40 | + | |
39 | 41 | | |
40 | 42 | | |
41 | 43 | | |
| |||
380 | 382 | | |
381 | 383 | | |
382 | 384 | | |
| 385 | + | |
| 386 | + | |
| 387 | + | |
| 388 | + | |
| 389 | + | |
| 390 | + | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
| 394 | + | |
| 395 | + | |
| 396 | + | |
| 397 | + | |
| 398 | + | |
| 399 | + | |
| 400 | + | |
| 401 | + | |
| 402 | + | |
| 403 | + | |
| 404 | + | |
| 405 | + | |
| 406 | + | |
| 407 | + | |
| 408 | + | |
| 409 | + | |
| 410 | + | |
| 411 | + | |
383 | 412 | | |
384 | 413 | | |
385 | 414 | | |
| |||
1558 | 1587 | | |
1559 | 1588 | | |
1560 | 1589 | | |
| 1590 | + | |
| 1591 | + | |
| 1592 | + | |
| 1593 | + | |
| 1594 | + | |
| 1595 | + | |
| 1596 | + | |
| 1597 | + | |
| 1598 | + | |
| 1599 | + | |
| 1600 | + | |
| 1601 | + | |
| 1602 | + | |
| 1603 | + | |
| 1604 | + | |
1561 | 1605 | | |
1562 | 1606 | | |
1563 | 1607 | | |
| |||
1697 | 1741 | | |
1698 | 1742 | | |
1699 | 1743 | | |
1700 | | - | |
| 1744 | + | |
| 1745 | + | |
| 1746 | + | |
| 1747 | + | |
| 1748 | + | |
1701 | 1749 | | |
1702 | 1750 | | |
1703 | 1751 | | |
| |||
1865 | 1913 | | |
1866 | 1914 | | |
1867 | 1915 | | |
1868 | | - | |
| 1916 | + | |
1869 | 1917 | | |
1870 | 1918 | | |
1871 | 1919 | | |
| |||
1893 | 1941 | | |
1894 | 1942 | | |
1895 | 1943 | | |
| 1944 | + | |
| 1945 | + | |
| 1946 | + | |
1896 | 1947 | | |
1897 | 1948 | | |
1898 | 1949 | | |
| |||
0 commit comments