Repository navigation
Commit 557ae7c
feat(spec): ISecurityService declares checkControlledByParentWrite, an optional member answering the master-detail write check (#22492)
Fixes #22464
Clause-②: yes (widening)
`ISecurityService` now declares one optional, feature-detected member
that answers the existing ADR-0055 master-detail write check:
`checkControlledByParentWrite(object, recordId, context)`. It resolves
with `ControlledByParentWriteOutcome`. This is the `packages/spec` half
of #22455, which triage split out. #22455 serves the member from
`plugin-security` and consumes it in the attachment and comment gates.
#22455 is not addressed here. The contract-tier review is the seat's.
No runtime source changes, and no behaviour changes. The member is
declared, not served.
## What is declared
- **`checkControlledByParentWrite?(object, recordId, context?)`**, which
resolves with a `ControlledByParentWriteOutcome`. The name and signature
follow `checkAuthoredRowWrite` in the same file.
- **`ControlledByParentWriteOutcome`**, a discriminated union on
`outcome`:
- `allow`: the check does not refuse. Every leg passes on every master
up the chain, for the principal and, on an on-behalf-of context, for the
delegator. A system context also answers `allow`, because the write path
never checks it.
- `deny`, with `leg: ControlledByParentWriteDenialLeg`:
`object_permission`, `row_level_security`, `record_sharing` or
`master_chain`.
- `not_applicable`: the object does not declare `controlled_by_parent`.
- `unresolvable`, with `reason:
ControlledByParentWriteUnresolvedReason`:
`master_detail_relation_missing`, `record_not_found` or
`master_reference_missing`.
- **A store fault is a rejection, not an arm.** The member rejects with
the engine's own error, unchanged, so the fault keeps its declared
`503`.
- **TSDoc** states the three things the card asks for:
- the answer equals what a by-id update of the same record gets from
this check, the ADR-0090 D10 delegator leg included;
- absence is the absence of a master check, not a policy that admits,
and the parent's own update gets the same answer there;
- the member is optional, so every consumer handles its absence, and the
unguarded call does not compile.
- The file header's list of failure stances gains one bullet for this
member.
## Premises, verified on `origin/main` `dee7692f0`
1. `git grep -n "assertControlledByParentWrite" origin/main --
packages/plugins/plugin-security/src/security-plugin.ts` finds the step
2.8 calls at `:3325` (principal) and `:3335` (the ADR-0090 D10 delegator
pass), and the private definition at `:9113`. Its legs are in the
private `assertMasterRowEditable` (`:9377`).
2. In `registered-security-service-members.pin.test.ts`,
`DeclaredOptionality` maps `keyof ISecurityService` with `-?` (`:52`),
`DECLARED_MEMBERS ... as const satisfies DeclaredOptionality` (`:78`),
and `SERVED_NOT_DECLARED` is empty (`:87`).
3. Read in full, `ISecurityService` declared 21 members, and none of
them runs this check. `checkAuthoredRowWrite` answers authored row-level
security only.
## What the check decides, and the arm each outcome maps to
The outcome type carries what `assertControlledByParentWrite` and its
legs decide for a by-id update, and nothing more.
| The check, for a by-id update | Arm |
|:---|:---|
| object not `controlled_by_parent` (returns early) | `not_applicable` |
| no `master_detail` relation, `MasterDetailRelationMissingError` (422
`INVALID_METADATA`) | `unresolvable` / `master_detail_relation_missing`
|
| target row absent, `DetailRecordNotFoundError` (404
`RECORD_NOT_FOUND`) | `unresolvable` / `record_not_found` |
| stored master FK empty, `MasterReferenceMissingError` (422
`MISSING_REQUIRED_FIELD`) | `unresolvable` / `master_reference_missing`
|
| leg 1, no object-level `update` on a master (403) | `deny` /
`object_permission` |
| leg 2, master row outside the write row-level security (403) | `deny`
/ `row_level_security` |
| leg 3, record sharing refuses and no authored update policy admits
(403) | `deny` / `record_sharing` |
| chain walk: a cycle, the depth bound, an ancestor with no relation, an
ancestor row absent, an ancestor FK empty (403) | `deny` /
`master_chain` |
| every hop passes, for the principal and then the delegator | `allow` |
| a read the check lets propagate faults (#7505) | rejection with the
engine's error |
The check tells its 403 legs apart only by the reason sentence it
throws. The leg vocabulary is closed, so a consumer never parses that
prose.
## Shape decisions, for the contract review
1. **The store fault is a rejection, not a fifth arm.** The check's own
docblock says that on a fault "the engine's own error propagates … and
this gate answers nothing". A rejection keeps the `503` without the
consumer doing anything. A value arm would keep it only if every
consumer remembered to rethrow, and a consumer that forgot would fail
open. This matches the dev's option A on #22455 ("a store fault thrown
so it keeps its declared 503"). The card's "the outcome type covers the
store fault" is met in the type's docblock, and in a compile pin that
`store_fault` is not a reason.
2. **`record_not_found` is the addressed record, not its master.** The
card and the ruling say "a missing master row". In the check, the 404
`DetailRecordNotFoundError` is the target row. A missing master row is
judged by the legs on the first hop, and is a `master_chain` refusal
above it. I followed the check, and the TSDoc says so. On the first hop
the legs can admit a dangling FK: `resolveSharingCanEdit` answers `true`
when sharing abstains on a public master with no write row-level
security. So "missing master row means refused" would have been a
verdict the check does not make.
3. **Contexts around the check.** The by-id update reaches step 2.8 only
after several earlier steps, so the TSDoc pins what each context
answers:
- a system context answers `allow`, because the middleware
short-circuits on it;
- a principal whose sets resolve to none answers `deny` /
`object_permission`, because `checkObjectPermission('update', master,
[])` is `false` (ADR-0056 D2);
- the three refusals of the context itself reject with the same `403
PERMISSION_DENIED`: no principal at all (ADR-0096 D5), a permission-set
resolution failure, and a dangling delegator (ADR-0090 D10).
The member answers the master check alone. The record's own gates are
not part of the answer.
4. **`master_chain` is a leg.** Above the first hop, the check answers
an unresolvable or dangling ancestor as an authorization refusal
(#7474's envelope, ADR-0055 amendment), so it is never `unresolvable`.
## The forced `plugin-security` row (claim amendment `6080901064`,
cross-lane declaration `6080912705`)
`DECLARED_MEMBERS` gains `checkControlledByParentWrite: 'optional'`, in
interface order. Nothing else in `plugin-security` changes. An optional
member that is declared and not served passes the pin's served-side
check, and `SERVED_NOT_DECLARED` stays empty. Serving the member is
#22455's.
## Proof each pin bites (predicted first, run from committed
`1a4557d0e`)
Each mutation went through `node scripts/ablation-replace.mjs` in wrap
mode. The anchor hit exactly once, the write was proven on disk, and the
restore was proven as blob == HEAD with an empty `git diff HEAD`. Each
leg ran the package's own `check-test-typecheck.mts` gate and then raw
`tsc -p tsconfig.test.json`. The spec test program imports
`./security-service` by relative path, so no `dist/` sits between the
spec mutations and the run.
| Leg | Mutation | Predicted | Observed |
|:---|:---|:---|:---|
| C-spec | none (control) | green, 0 diagnostics in the security-service
files | gate OK, 0 |
| A1 | `checkControlledByParentWrite?(` made required | red, 3: the
unguarded-call pin plus the two exhaustiveness gates | `(659,7) TS2578`
at the unguarded-call pin; `(70,3) TS2322` (the `makeService` literal);
`(122,11) TS2322` (`REQUIRED_MEMBERS`) |
| A2 | `deny`'s `leg` made optional | red, 1 | `(697,5) TS2578`, the
deny-without-leg pin |
| A6 | `unresolvable`'s `reason` made optional | red, 1 | `(699,5)
TS2578`, the unresolvable-without-reason pin |
| A3 | `'store_fault'` added to the reasons | red, 1 | `(725,5) TS2578`,
the fault-is-not-a-reason pin |
| A4 | `'abstain'` added to the `not_applicable` arm | red, 2 | `(705,5)
TS2578`, the abstain pin; `(740,17) TS2322` at the exhaustive switch's
`never` |
| C-psec | none (control) | green, 0 | gate OK, 0 |
| A5 | the `DECLARED_MEMBERS` row deleted | red, 1 |
`registered-security-service-members.pin.test.ts(79,12) TS1360`, the
`satisfies DeclaredOptionality` clause |
| R5 | A5 restored | green | gate OK, 0 |
A5 reads the spec through `dist/` (no `paths` rule for the spec in
`plugin-security`). The unmutated list compiling green at `1a4557d0e` is
what proves that program read the rebuilt `.d.ts`: against a stale one
without the member, the row itself would be the excess-property red. One
note: my first A4 attempt used an anchor that its replacement still
contained, so the tool refused it before running anything. That attempt
measured nothing, and A4 above is the re-run with a corrected anchor.
The three runtime rows show the consumer's shape: feature-detect,
proceed on `allow` and `not_applicable`, refuse otherwise, and propagate
a rejection. They run against stubs, so mutating the contract cannot
turn them red, and no ablation applies to them.
## Verification
All at `1a4557d0e`, under `scripts/pm/os-verify-lock.sh`.
- `pnpm --filter @objectstack/spec build`: exit 0. Then
`check:generated`: exit 1, with exactly `api-surface/` and
`export-origins/` stale. Then `check:generated --fix` regenerated those
two: three type-only exports each, and both checks now pass.
`api-surface-signatures.json` (it covers the `defineX` factories only)
and the reference pages (`check:docs`) are unchanged.
- `pnpm --filter @objectstack/spec typecheck`: exit 0.
`security-service.test.ts` has no `test-typecheck-debt.json` entry, so
its `@ts-expect-error` lines are live.
- `pnpm --filter @objectstack/spec exec vitest run --project local
--maxWorkers=2`: `Test Files 630 passed (630)`, `Tests 18796 passed | 1
todo`.
- `pnpm --filter @objectstack/spec exec vitest run --project repo
--maxWorkers=2`: `Test Files 54 passed (54)`, `Tests 915 passed (915)`.
- `pnpm exec turbo run build --filter='@objectstack/plugin-security^...'
--concurrency=2`: 17 of 17 successful.
- `pnpm --filter @objectstack/plugin-security typecheck`: exit 0, and
the test layer reports `0 file(s) / 0 error(s)`.
- `pnpm --filter @objectstack/plugin-security exec vitest run
--maxWorkers=2 src/registered-security-service-members.pin.test.ts`:
`Tests 3 passed (3)`. Narrowed on purpose: the package's only change is
this row. The full suite is CI's.
**Gates.** `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` at `1a4557d0e` derived 90 commands from the
merge-base change set (6 paths). I ran all 90 and recorded each exit
code before any pipe. 88 exited 0 on the first pass:
- `pnpm check:i18n` and `pnpm check:dual-build-cjs-loads` exited 3
(`PREREQUISITE NOT MET`): only the `plugin-security` closure had a
`dist/`.
- `pnpm check:dts-closure` and `pnpm check:sourcemap-no-sources-content`
exited 0, but they swept only the 17 packages built at that point.
After a full workspace build (`pnpm exec turbo run build
--filter='!@objectstack/docs' --concurrency=2`: 72 of 72 tasks, 71
replayed from the shared cache), I re-ran all four, and all four exited
0:
- `check-i18n-bundles: OK (9 package(s) — all bundles in sync …)`;
- `✓ check:dual-build-cjs-loads — 107 published require entry point(s)
across 66 package(s) load`;
- `check-dts-closure: 72 built package(s) swept`;
- `check-sourcemap-no-sources-content: 68 built package(s) swept - 544
map(s), none embed source text`.
`dispatch-gates --ran` over the recorded codes: `90 derived famil(ies)
accounted for — 90 run, 0 NOT-MEASURED`.
**Lint, narrowed (a measurement, not a skip).** `pnpm exec eslint
--no-inline-config --format json` over the three changed `.ts` files
reports `files 3 errors 0 warnings 0`. All three are in the configured
population: `eslint --print-config` resolves a rule set for each.
`eslint.config.mjs` enables no type-aware linting
(`parserOptions.project` and `projectService` are unset for all three),
so this diff cannot move the verdict on any untouched file. The full
`pnpm lint` is CI's.
**Consumers.** The public-surface change is one optional interface
member and three type-only exports. `git grep` over `packages/**` for
`Required` over `ISecurityService`, `keyof ISecurityService`,
`implements ISecurityService` and `satisfies ISecurityService` finds
only the `plugin-security` pin updated here. Every other reference holds
the service as a `Partial` or calls existing members, so no other
compiled verdict can move. The full workspace consumer sweep is CI's
`TypeScript Type Check`.
**Not merged with `main`.** `origin/main` is two commits ahead
(`8b713fad7`: CLI, core and dogfood tests, plus an unrelated changeset).
They share no file and no package with this diff. `dispatch-gates`
reports that none of them touched what its answer derives from. The
merge queue rebuilds on current `main`.
## Acceptance notes
Noted, not filed.
- **Step 2.8 is keyed on a user id.** Its guard requires a non-empty
resolved set list, a `userId` on the context, and the engine handle. So
a non-system principal that carries positions and no user id is not
master-checked on the write path. The guest envelope (`positions:
['guest']`, no user id) is the live shape, and it is dormant here: guest
anchor bindings refuse `allowEdit`
(`packages/spec/src/security/high-privilege.ts:218`), so the CRUD gate
refuses a guest update before step 2.8. The contract states the
composition and not that guard. Carrier: #22455, which serves the member
and decides whether the member mirrors the guard. This is a read-only
inference, not measured.
- **The registration log line** in `security-plugin.ts` (`:2391`) lists
members by hand. Carrier: #22455, which edits that file to serve the
member.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01DhTqaEHqPVSVnAkjG3jywn)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 6212cc6 commit 557ae7c
6 files changed
Lines changed: 320 additions & 0 deletions
File tree
- .changeset
- packages
- plugins/plugin-security/src
- spec
- api-surface
- export-origins
- src/contracts
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
68 | 68 | | |
69 | 69 | | |
70 | 70 | | |
| 71 | + | |
71 | 72 | | |
72 | 73 | | |
73 | 74 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
64 | 64 | | |
65 | 65 | | |
66 | 66 | | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
67 | 70 | | |
68 | 71 | | |
69 | 72 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
64 | 64 | | |
65 | 65 | | |
66 | 66 | | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
67 | 70 | | |
68 | 71 | | |
69 | 72 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
9 | 12 | | |
10 | 13 | | |
11 | 14 | | |
| |||
642 | 645 | | |
643 | 646 | | |
644 | 647 | | |
| 648 | + | |
| 649 | + | |
| 650 | + | |
| 651 | + | |
| 652 | + | |
| 653 | + | |
| 654 | + | |
| 655 | + | |
| 656 | + | |
| 657 | + | |
| 658 | + | |
| 659 | + | |
| 660 | + | |
| 661 | + | |
| 662 | + | |
| 663 | + | |
| 664 | + | |
| 665 | + | |
| 666 | + | |
| 667 | + | |
| 668 | + | |
| 669 | + | |
| 670 | + | |
| 671 | + | |
| 672 | + | |
| 673 | + | |
| 674 | + | |
| 675 | + | |
| 676 | + | |
| 677 | + | |
| 678 | + | |
| 679 | + | |
| 680 | + | |
| 681 | + | |
| 682 | + | |
| 683 | + | |
| 684 | + | |
| 685 | + | |
| 686 | + | |
| 687 | + | |
| 688 | + | |
| 689 | + | |
| 690 | + | |
| 691 | + | |
| 692 | + | |
| 693 | + | |
| 694 | + | |
| 695 | + | |
| 696 | + | |
| 697 | + | |
| 698 | + | |
| 699 | + | |
| 700 | + | |
| 701 | + | |
| 702 | + | |
| 703 | + | |
| 704 | + | |
| 705 | + | |
| 706 | + | |
| 707 | + | |
| 708 | + | |
| 709 | + | |
| 710 | + | |
| 711 | + | |
| 712 | + | |
| 713 | + | |
| 714 | + | |
| 715 | + | |
| 716 | + | |
| 717 | + | |
| 718 | + | |
| 719 | + | |
| 720 | + | |
| 721 | + | |
| 722 | + | |
| 723 | + | |
| 724 | + | |
| 725 | + | |
| 726 | + | |
| 727 | + | |
| 728 | + | |
| 729 | + | |
| 730 | + | |
| 731 | + | |
| 732 | + | |
| 733 | + | |
| 734 | + | |
| 735 | + | |
| 736 | + | |
| 737 | + | |
| 738 | + | |
| 739 | + | |
| 740 | + | |
| 741 | + | |
| 742 | + | |
| 743 | + | |
| 744 | + | |
| 745 | + | |
| 746 | + | |
| 747 | + | |
| 748 | + | |
| 749 | + | |
| 750 | + | |
| 751 | + | |
| 752 | + | |
| 753 | + | |
| 754 | + | |
| 755 | + | |
| 756 | + | |
| 757 | + | |
| 758 | + | |
| 759 | + | |
| 760 | + | |
| 761 | + | |
| 762 | + | |
| 763 | + | |
| 764 | + | |
| 765 | + | |
| 766 | + | |
| 767 | + | |
| 768 | + | |
| 769 | + | |
| 770 | + | |
| 771 | + | |
| 772 | + | |
| 773 | + | |
| 774 | + | |
| 775 | + | |
| 776 | + | |
| 777 | + | |
645 | 778 | | |
0 commit comments