Repository navigation
fix(plugin-security): explain's update verdict on a controlled_by_parent record comes from the master-detail write check - #22529
Conversation
…ntrolled_by_parent record comes from the master-detail write check explain asks ISecurityService.checkControlledByParentWrite for every update of a record that exists, with the explained context; deny and unresolvable refuse and name their leg or reason on the sharing layer, allow and not_applicable leave the report unchanged, and a rejection is reported fail-closed. The update's Layer 1 carries step 2.7's master-gate coverage vouch, so the ownership floor is handed to that check as the write path hands it. Claude-Session: https://claude.ai/code/session_01WYYhVJ78u7PhwFViWo1EmQ Co-authored-by: Claude <noreply@anthropic.com>
…ate verdict beside the PATCH Engine cells for every outcome of the master-detail write check, the registered service's explain beside the PATCH for every leg (and for another user), and the REST explain beside the REST PATCH on the parent-gates fixture, with the ownership floor armed. Claude-Session: https://claude.ai/code/session_01WYYhVJ78u7PhwFViWo1EmQ Co-authored-by: Claude <noreply@anthropic.com>
…arent update verdict Claude-Session: https://claude.ai/code/session_01WYYhVJ78u7PhwFViWo1EmQ Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 1 package(s): 4 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
What this run could not see
Coarse fallback — 16 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin a53e14fab06d00d4f0f167be5329233f3ccb06fa && git checkout a53e14fab06d00d4f0f167be5329233f3ccb06fa
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin ee8751d41e61a18f7819e4d3ad2c340f51ab2418 16a937c47dc85d93c986a9479012e44bb27318b2 && git checkout -B drift-repro ee8751d41e61a18f7819e4d3ad2c340f51ab2418 && git merge --no-ff 16a937c47dc85d93c986a9479012e44bb27318b2
node scripts/docs-audit/affected-docs.mjs --json ee8751d41e61a18f7819e4d3ad2c340f51ab2418
|
Contract reviewServed-tier: Inputs read: card #22514 (body, comments ① Derived judgments
② Semver level
③ Boundary flags
Out-of-scope, judged not this diff's:
Check-runs on Implemented-by: VERDICT: PASS Read at 2026-10-09T20:06Z · isolated contract-review subagent of the seat named on |
…eighteen write affordances read one affordance-to-grant map (objectui#12082) (objectstack-ai#12084) Part of objectstack-ai#12082 (the seat's answer B, `6094765692`: five write affordances that read no grant at all are this card's follow-up) Part of objectstack-ai#12081 (items 1, 5 and 6) Clause-②: yes A create form gated its fields on the EDIT grant, so a role that may create but not edit could not fill the form it is allowed to submit. This branch puts ONE affordance-to-grant map in `@object-ui/core`, read by the eighteen console write affordances it names (the five that read no grant at all are the card's follow-up, see Acceptance notes), and makes a create form ask the CREATE question, which follows the server's insert rule. Implemented by the dispatched os-dev agent for the `domain:ui#3` seat, session `https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8`. The dev wrote this body when the draft opened. The seat brought it to round 2 from the dev's report `6094735932` and the claim amendment `6094765692` on objectui#12082: the reader list, the pins' readings and the acceptance notes below are round 2. ## Measured on `main` first - **The card (p1), measured red on `main` `1b2d0160f`.** `packages/plugin-form/src/createFormGrant-12082.test.tsx` mounts every `ObjectForm` layout under `MePermissionsProvider` with the card's grant (`allowCreate: true, allowEdit: false`, no field entries). On `main`: every field of the create form drawn locked on all nine layouts, and the create body on the three one-step containers was `{}` (24 failed, 9 passed). With this branch: 33 passed. The empty body is the card's `400 VALIDATION_FAILED`: the outbound filter asked the same `write` question as the render gate. - **The server's insert rule (objectstack `main` `76bc1e03`, read in source).** `plugin-security`'s middleware step "2.5. Field-Level Security write enforcement" runs on `insert` and `update` alike through `computeForbiddenFieldWrites` → `FieldMasker.detectForbiddenWrites`, which refuses only a field in `getNonEditableFields` — a field whose explicit entry is not `editable`. `PermissionEvaluator.getFieldPermissions` builds entries only for fields a permission set names, so a field with no entry passes the field step. Object admission maps `insert` → `allowCreate` (`OPERATION_TO_PERMISSION`). So a create-mode field reads: explicit entry → its `editable`; no entry → the object's create grant. That is exactly what `checkField(object, field, 'create')` now answers; no client rule the server lacks is added. - **Item 1 (no header Edit for a caller whose update is allowed): the cause is server data, landed upstream.** On objectui `main` the header Edit is the object's resolved `edit` affordance (managed-object policy ∧ effective API operations) ∧ the explain engine's record verdict ∧ the `userActions.edit` predicate. For hotclm's `clm_payment_plan` (`sharingModel: 'controlled_by_parent'`, no `managedBy`, no `userActions`, no `apiMethods`) the only principal-dependent input is the explain record verdict, which `useRecordEditable` reads as `record.visible`. objectstack-ai/objectstack#22529 (the pull request for objectstack-ai/objectstack#22514, merged 2026-10-09, after the 17.7.0 tag — not an ancestor of the tag commit `4e4e8814`, control leg: the tag's 30th ancestor answers exit 0) made explain's `update` verdict on a `controlled_by_parent` record come from the master-detail write check; its own reproduction table shows the hotclm shape (`allowed: true`, `record.visible: false`, `PATCH` 200). objectstack-ai/objectstack#19963 is the older card on the same shape. Nothing on objectui's side hid Edit from that caller; the map's `recordEdit` row now also reads the update grant, and the pin holds that a caller with `allowEdit` and `update` in the operation set gets Edit. - **Item 5 (New / Import with `allowCreate: false`): does not reproduce as stated.** Both list surfaces (`ObjectView`, `ObjectDataPage`) already ANDed `can(object, 'create')` on `main` and in 17.7.0. hotclm binds every position to `clm_requester` as well as to its own set (`src/security/bind-position-sets.ts`), and `clm_requester` grants `allowCreate: true` on `clm_contract`, so the records manager's EFFECTIVE grant allows create: the server would accept the create, and New / Import were right to show. The rows are pinned anyway, now read through the map. - **Item 6 (lookup "Create new" without create on the target): reproduces on `main` by source.** `LookupField`'s `canCreate` read no grant at all. It now reads the `lookupCreateNew` row, asked of the target object. ## The map `packages/core/src/utils/affordanceGrants.ts`: `AFFORDANCE_GRANTS` names each affordance's CRUD-affordance bit, the object grant it exercises and, for an affordance that offers fields, the field question (`create` = insert rule, `write` = update rule). `resolveAffordance` is the one verdict — managed-object policy ∧ the server's effective API operation set (`getObjectApiOperations`, kept inside the map) ∧ the caller's grant — with the row's `userActions` predicates surfaced only when all three allow it. `resolveFieldAffordance` asks a field the row's question. **Why `core`, not `permissions`.** Every reader already depends on both packages, and `resolveEffectiveCrudAffordances` lives in `core`. `permissions` depends on `@object-ui/types` alone; homing the map there would add a `permissions → core` edge. In `core` the principal is a structural interface (`can`, `getObjectApiOperations`, `checkField`, `isLoaded`), so `core` gains no edge either. **Fail-open stays.** With no permission provider mounted, `usePermissions()` answers `can` with `true` and `isLoaded` with `false`: every grant reads open and no field question is asked, as the `fieldWriteGate.ts` docblock states. ## Readers Rows and readers (the census of `resolveEffectiveCrudAffordances`, `isObjectInlineEditable`, CRUD-verb `can(` / `check(`, and `checkField(…, 'write')` readers) — each becomes a map reader or is named below as outside the family: - create-form fields / edit-form fields and the form-wide lock — every `ObjectForm` layout through `fieldWriteGate.ts` (render pass, outbound filter, `closedFormAffordance`); - record-header Edit / Delete and the record body's in-place editing — `RecordDetailView` `resolveRecordHeaderActionGates`; - list New / Import — `ObjectView`, `ObjectDataPage`; the import wizard's writable target fields ask the create question (`importTargetFields`); - lookup "Create new" — `LookupField`, on the target object; - create-mode `MasterDetailForm` line cells ask the create question of the child object, since every line there is a new record; - the other census readers, each now a map reader: `RelatedRecordActionsBridge` (related lists), `RecordAttachmentsPanel` (Upload / delete on `sys_attachment`), `plugin-list` `ListView` (bulk Delete, inline-edit toggle), `plugin-grid` `ObjectGrid` + `rowCrudAffordances` (row Edit / Delete, in-place editing, the add-record row; `rowCrudAffordances` now takes the map's verdicts) and `ImportWizard` (template download), `plugin-detail` `DetailView` (operation set + grant; the object's own policy stays the host's channel per objectui#4419) and `record:details` (in-place editing reads the update grant), and the console `ProfilePage` (the language field). **Out of the family, with the reason:** `managedByEmptyState` (empty-state copy from the managed-object bucket; it shows or hides nothing); `useFieldPermissions` `canWrite` / `writableFields` (the resolver's own API); the read gates (`checkField(…, 'read')`). **Behaviour moves beyond the card's rows, all toward the server's refusal:** the grid's add-record row now also honours the managed-object policy and the effective `create` operation; `DetailView`'s object gate adds the effective operation set; `record:details` in-place editing reads the update grant; the form-wide lock and its notice also engage on a denied grant for the form's mode. ## Pins - the card's repro and its two controls (edit form under the same grant stays disabled; an explicit `editable: false` stays disabled and out of the body) — `createFormGrant-12082.test.tsx`; - the enumeration pin: every row × four grant shapes (create-only, edit-only, read-only, full) through the real `MePermissionsProvider`, the row set held to the test's own expectation table, and a census that refuses a console source file reading a CRUD grant outside the map; - earlier family pins stay green: objectui#4296, objectstack-ai#10107, objectstack-ai#11000, objectstack-ai#12047. **Readings (dev report `6094735932`):** - measured red on `main` `1b2d0160f` first: the card's pin 24 failed / 9 passed (every field locked, create body `{}`); the item-6 pin against `main`'s `LookupField` 3 failed / 2 passed (controls green). - at the branch head: `createFormGrant-12082` 35/35, `affordanceGrantMap-12082` 27/27, `LookupField.createGrant-12082` 5/5; the family pins' final run 161/161. - ablations (`scripts/ablation-replace.mjs`, predictions first, every restore blob-equal): A1 the create form's field question back to `'write'` ⇒ 19 red; A3 a grant read planted outside the map in `LookupField` ⇒ the census names that file; A4 `recordEdit`'s grant swapped ⇒ 4 red; A5 `MasterDetailForm` forced to edit mode ⇒ 1 red. - the Console's eager closure: +655 B gzipped (3,244,032 → 3,244,687 B over 290 eager chunks); `check:eager-closure` passes at the head. ## Acceptance notes - **The family's remainder (the card's follow-up, seat answer B `6094765692`):** five write affordances read no grant at all, so the grep census over grant reads could not see them: plugin-view `ObjectView`'s create button, plugin-calendar's quick-create and drag-to-reschedule, plugin-kanban's card move, and plugin-form `LineItemsPanel`'s add / remove lines. The census pin refuses grant reads outside the map; it does not catch an affordance that reads no grant, and the follow-up says so or closes it. - **objectui#12081 items 1 and 5, carried to the hotclm seat (noted, not filed):** item 1's cause is upstream (objectstack-ai/objectstack#22529, after the 17.7.0 tag), so hotclm's finance header Edit is re-measured once its `@objectstack` pin includes that merge; item 5's New / Import are correct for that user's effective grant. - `MePermissionsProvider.check` keys `objects` by the name as given, while `checkField` and `getObjectApiOperations` lowercase it first. Every reader passes the object's own (lowercase) name, so nothing diverges today; noted, not changed. - `LookupField` reads `allow_create ?? allowCreate` (two spellings of one key) — outside this card, noted. --- _Generated by [Claude Code](https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
Fixes #22514
Clause-②: yes (widening)
POST /api/v1/security/explainnow answers anupdateof acontrolled_by_parentrecord the way that record's own by-idPATCHis answered. The record verdict comes from the master-detail write check (ADR-0055) that the write path runs at step 2.8. explain reads it throughISecurityService.checkControlledByParentWrite, served since PR #22513, and keeps no second copy of the check or of itscontrolled_by_parentpredicate.Reproduction (before)
Measured on
origin/mainee8751d41, with an untracked scratch dogfood probe on PR #22513's fixture (cbp-parent-gates-fixture.ts, org-bound boot, so the platform ownership floorowner_only_writesbindsorg_member). The probe was deleted after the reading.cpg_contractiscontrolled_by_parentundercpg_account(public_read, edited by its owner).allowedrecordtruevisible: false,decidedBy: 'rls'PATCH403PERMISSION_DENIED(master, row-level security)userId)truevisible: false,decidedBy: 'rls'PATCH: 403)truevisible: false,decidedBy: 'rls'PATCH200truevisible: false,decidedBy: 'rls'DELETE200cpg_board(public_read_write)truevisible: truePATCH200What the reading says about the card. The card's mechanism holds: explain never asks the master-detail write check, and its sharing gate (
canEdit) abstains oncontrolled_by_parentand reads as writable. Its symptom needs restating. The card quotesallowed: true, which is the object-level field (may this principal updatecpg_contractat all). By the published contract and the parity table inexplain-enforce-parity.test.ts, a record question is answered byrecord.visible. Onmainthat verdict was decided by the ownership floor, which the write path hands over to the master check through step 2.7'smasterGateCoversThisWritevouch. explain did not vouch, because it ran no master gate (the comment in the wiring said so). The result was a two-sided divergence:rls), with no word about the master;PATCHthat answers 200.What changed
explain-engine.ts: a new optional dependency,checkControlledByParentWrite. For everyupdateof a record that exists, the engine asks it with the context being EXPLAINED. Asking for every update keeps the predicate in the member:not_applicablechanges nothing.security-plugin.ts(theexplainwiring only):this.checkControlledByParentWrite. That is the write path's own composition: the context prologue, step 2.8 for the principal and then for the delegator.masterGateCoversThisWrite, on step 2.7's condition (not on behalf of anyone). The ownership floor is handed over to the master check here exactly as the write path hands it.deletekeeps its floor, because explain asks no master check for it..changeset/22514-explain-cbp-update-master-check.md:minorfor@objectstack/plugin-security, not thepatchthe dispatch suggested. See the acceptance notes.Outcome to layer mapping
This mirrors the gates PR #22513 built and the review
6085973866's truth table. The door proceeds onallowandnot_applicable, and refuses on every other outcome.recordverdictsharinglayerrecordallownot_applicabledeny+legvisible: false,decidedBy: 'sharing'excluded; the detail names the leg (object_permission/row_level_security/record_sharing/master_chain) and the 403unresolvable+reasonvisible: false,decidedBy: 'sharing'not_evaluated; the detail names the reason and the status the update answers (422 / 404 / 422)visible: false,decidedBy: 'sharing'not_evaluated, no predicate; fail-closed fault detailvisible: false,decidedBy: 'sharing'not_evaluatedsharinglayer, the record's write gate.describeOwd's baseline wording stays, per triage.rls) still decides first, because step 2.7 runs above step 2.8. The not-found answer for a record the principal cannot read (the read question) still overrides both.allow. The member answersallowfor a system context on ANY object, which is the write path's first exit. Soallowis not evidence that the record's access derives from a master.explainAccessForCalleralready hands the engine the explained user's context (PM assumption 2 holds, and it is pinned below).Pins
PATCH403 naming the master's row-level security; explainvisible: false,decidedBy: 'sharing', legrow_level_securitynamedpackages/qa/dogfood/test/cbp-explain-master-write.dogfood.test.ts(REST explain beside the RESTPATCH)userIdand gets the member's refusal; the admin's own explain andPATCHadmitvisible: truebesidePATCH200, for a child they did not create, with the floor armed (assertArmed,org_member)controlled_by_parent:cpg_boardexplainvisible: truebesidePATCH200record_sharing,row_level_security,object_permission,master_chain) refused beside a refusedPATCH; four admitted updates explained writable beside an admittedPATCH; a system caller explaining the viewer byuserIdgets the viewer'sobject_permissionrefusalcontrolled-by-parent-write-member.test.ts(the registered service, the member's own double)security.explainallowandnot_applicableare byte-identical (toEqual) to the report without the member, oncontrolled_by_parent,public_read_write, and private objects with the gate admitting and refusing. The record's own RLS decides first. The member is asked once, with the explained context object itself, and never for read, create, delete, transfer, export, an object-level request or a missing recordexplain-controlled-by-parent-write.test.ts(engine, deps bag, no engine double)Ablations
Each leg went through
scripts/ablation-replace.mjs(wrap mode, with an anchor hit, and a restore proven by the blob equal to HEAD and an emptygit diff HEAD). Thenpnpm --filter @objectstack/plugin-security build, thenscripts/ablation-dist-preflight.mjs(marker present indist/), then the run. The restore leg rebuilt and ran the preflight with--absent(marker absent from all 6 built files, tree clean against HEAD).canEdit-only verdict backcheckControlledByParentWritekey renamed, so the engine sees no membervisible: true,decidedBy: 'object_crud'beside the 403), 2 green. member suite: 2 red (the same shape forc_other), 33 greenmasterGateCoversThisWriteforced falsedecidedBy, the editor control red, the plugin suites green (no floor in their harness)decidedBy: 'rls'on both refusals; the editor controlvisible: falsebesidePATCH200), the board green. plugin suites: 35/35 greenAblation B's first attempt is void: its marker was a comment, which the build strips, so the dist preflight could not prove the mutation landed (exit 1). It was re-run with a string-literal marker; the table reports the second run.
Local verification (HEAD
16a937c47)pnpm --filter @objectstack/plugin-security typecheck: exit 0. The test-layer program (tsconfig.test.json) lists both edited test files, counted with--listFiles.pnpm --filter @objectstack/plugin-security exec vitest run: 192 files, 3994 passed, 45 skipped, exit 0.pnpm --filter @objectstack/dogfood typecheck: exit 0.cbp-explain-master-write,cbp-parent-attachment-comment-gates,controlled-by-parentandshowcase-invoice-cbp: 4 files, 22 tests passed. The full dogfood suite (three CI shards) is declared to CI.node scripts/pm/dispatch-gates.mjs --commands: 71 commands derived and 71 run, each exit code recorded. The--ranreconciliation reads "71 run, 0 NOT-MEASURED (a DERIVED zero)".pnpm check:dual-build-cjs-loadsfirst answeredPREREQUISITE NOT MET(exit 3: eight unrelated packages had nodist/). After building those eight it answered exit 0, which is the reading recorded.pnpm lintowns the full run.eslint.config.mjslints**/*.{ts,…}, so the five changed.tsfiles are the touched population.--format jsonread 5 files, 0 errors, 0 warnings.parserOptions.project, stated in the config itself) and loads no import-graph plugin, so this diff cannot move any untouched file's verdict.Acceptance notes
minor, not the dispatch'spatch.ExplainEngineDepsis exported from@objectstack/plugin-security's index, and it gains an optional key. Under the WHICH LEVEL rule inpr-automation.yml's Check Changeset step, an additive widening of a published package's public surface takes at leastminor. TheClause-②line isyes (widening)(the seat amended the claim and line 2 at review): the optional deps key is a widening of a published type.explain's response payload gains no key, and no runtime accept set widens. The same reading is the one the review on PR fix(service-storage,plugin-audit,plugin-security)!: the attachment and comment parent gates judge a controlled_by_parent parent through its master #22513 gave its additive types.allowedis unchanged. It answers the object question and staystruefor a member who holdsupdateon the object. The record's bottom line isrecord.visible, as for every other record-level refusal (a private record the caller does not own reads the same way).deletekeeps today's answer. The member is declared for an UPDATE, so explain asks no master check for a delete and keeps the floor there. The measured gap is in the out-of-lane findings below.not_evaluated, with the fault detail, on an update of an existing record. That is refuse-only and matches the door, which refuses these contexts first. explain's earlier layers (object_crud, the D10 delegator handling) already decide those records, sodecidedBydoes not move. The review on PR fix(service-storage,plugin-audit,plugin-security)!: the attachment and comment parent gates judge a controlled_by_parent parent through its master #22513 named the same shape for the gates.record.visiblefor an update now show it to a master editor who did not create the child, and hide it from a non-editor for the right reason.transferwas not measured. explain'stransferrecord verdict asks the sharing gate, and the door runs step 2.8 fortransfertoo. This is a read-only inference, noted here and not filed.Out-of-lane findings (for the seat to file)
explaindeleteon acontrolled_by_parentrecord asks no master-detail check and keeps theowner_only_deletesfloor, which the door hands over to that check.ee8751d41, unchanged at this head): a principal who edits the master and holds delete on the child, but did not create it, gets explaindeleterecord.visible: false,decidedBy: 'rls'besideDELETE /api/v1/data/cpg_contract/ID200. For the non-editor the refusal is reported onrlswith no word about the master, beside aDELETE403 that names the master's row-level security.spec:ISecurityService.checkControlledByParentWrite(declared for an update only) →runtime:explain-engine.ts applyRecordAttribution(the delete branch askscanDeleteRecordalone).delete, or a sibling would have to be declared (spec lane) before explain can ask it. The legs themselves are operation-independent inassertControlledByParentWrite.explain delete controlled_by_parent master check,explain canDeleteRecord owner_only_deletes floor,checkControlledByParentWrite delete operation.Generated by Claude Code