Repository navigation
test(plugin-security): pin explain controlled_by_parent update/delete verdict under a Modify All Data master beside the door - #22760
Merged
objectstack-fleet[bot] merged 2 commits intoOct 11, 2026
Conversation
…delete verdict under a Modify All Data master beside the door The console record header asks `security/explain` with a `recordId` for `update` and `delete` and shows Edit and Delete on `record.visible`. A caller who holds `viewAllRecords` + `modifyAllRecords` on the master only, and plain CRUD on the controlled_by_parent detail, reaches the detail's rows through the master check alone. No suite pinned explain's verdict for that route beside the door: the existing pairs cover an owned master, an edit-shared master, an RLS-admitted master and a two-hop chain. This pins it over the real SecurityPlugin, ObjectQL with better-sqlite3, the real SharingService and its middleware, and the platform's member_default ownership floor armed for both callers: the bypass holder is explained writable for update and delete and the door admits; the master's reader without the bypass is refused by both, on the master check's leg. Claude-Session: https://claude.ai/code/session_01CBAfsWMSfM3EToQGVStEcp Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CBAfsWMSfM3EToQGVStEcp Co-authored-by: Claude <noreply@anthropic.com>
Contributor
📓 Docs Drift CheckNothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs. What this run could not see
Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
This was referenced Oct 11, 2026
objectstack-fleet
Bot
deleted the
claude/issue-22721-cbp-header-verdict
branch
October 11, 2026 04:12
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #22721
Clause-②: no
What this is
Stage 1 of #22721: the measurement the triage direction (comment 6101562558) asks for, plus one test file that pins the one route the measurement found unpinned. No source file changes.
The card's defect does not reproduce on
main. On a booted stack atorigin/maind7b26df5, which contains #22529 (d303b3e7af) and #22549 (86bf9ed7d5),git merge-base --is-ancestorexit 0 for both, the record-grainedsecurity/explainanswersrecord.visible: truefor bothupdateanddeleteof acontrolled_by_parentchild whose master another user owns and the caller may edit. The data door answers 200 for the same caller and row, through each of the three routes measured. A caller who may not edit the master is refused by both. The console header (objectuiorigin/main5330afd,packages/plugin-detail/src/useRecordEditable.ts) posts exactly{object, operation, recordId}forupdateanddeleteand readsdecision.record.visible, so onmainit offers Edit and Delete to the HotCRM sales manager. The card was measured on 17.7.0, which predates both corrections; hosted tenants receive them with cloud's v18 pin.The measurement, through the real HTTP doors
Booted with
@objectstack/verifybootStack: better-auth sign-ups, the REST explain route, the generic/datadoor, plugin-sharing and plugin-security. The boot isorgContext: true, so the ownership floor (owner_only_writes/owner_only_deletes, positionsorg_member) binds every caller. This was a scratch probe, untracked and deleted after the reading.hmx_account:sharingModel: 'private', owned and created by another member (rep).hmx_contract,hmx_quote,hmx_note: allcontrolled_by_parentunder it. Every child was inserted as the system, so no caller created any of them.hmx_contracthas HotCRM'scrm_contractgrant shape: edit, no delete, no write bypass.hmx_quotehas HotCRM'scrm_quotegrant shape: CRUD, withmodifyAllRecordson the detail too.hmx_notehas plain CRUD and no bypass, so a delete reaches the master check alone.permissionSets(themainspelling sincebfc15d27). Each caller's resolved positions and sets were asserted before anything was measured.1d7148bf)mgrviewAllRecords+modifyAllRecordson the mastersales_managerprofile oncrm_account(src/sales/profiles/sales-manager.profile.ts)teamaccessLevel: 'edit', recipient a position the caller holdsaccount_team_sharing(src/sales/sharing/account.sharing.ts)unitmgrunit_and_subordinates, edit); the caller is in the parent unit of the owner's unithierarchy-security(enterprise), anddefineStackrefuses it herecontrolThe master's own
PATCHanswered 200 for A, B and C, and 404 for X.Each cell below shows explain's
record.visible(withdecidedBywhere it is a refusal), then the door's status. Explain wasPOST /api/v1/security/explain {object, operation, recordId}, 200 on every cell. The door wasPATCHorDELETE /api/v1/data/OBJECT/ID.Explain's verdict equals the door's on all 36 cells.
allowDelete: falseoncrm_contract).visible: falsewith no decider.What was already pinned, and why one new file
Already pinned, explain beside the door, on
controlled_by_parent:controlled-by-parent-write-member.test.tscovers update, delete and transfer for four masters: an owned master, a master shared ateditto the caller (c_shared), an RLS-admitted master and a two-hop chain.packages/qa/dogfood/test/cbp-explain-master-write.dogfood.test.ts, covers the owned master.c_sharedshape at evaluation. The probe read the rule's grants as threesys_record_sharerows withrecipient_type: 'user'andaccess_level: 'edit'.Not pinned anywhere for explain: route A,
modifyAllRecordson the master alone, which is HotCRM's actual route.controlled-by-parent-detail-write-authority.test.tspins the door with the bypass on both objects and never asks explain. The master check reaches the bypass through two seams of its own:allow;canEdit.New:
packages/plugins/plugin-security/src/explain-controlled-by-parent-master-modify-all.test.ts. The rig is the realSecurityPlugin,ObjectQLover better-sqlite3, the realSharingServiceand its middleware, and the platform'smember_defaultfloor armed (org_member) for both callers.rlsrecord on a row neither caller created names the floor's predicate,{ created_by: CALLER }.visible: true, and the door admits.viewAllRecordson the master, withoutmodifyAllRecords): the door answersPERMISSION_DENIED/ 403 with "requires edit access to its master record". Explain answers{ visible: false, decidedBy: 'sharing' }, and its sharing detail names the master check's leg.Ablation of the negative pin
The mutation went through
scripts/ablation-replace.mjsin WRAP mode, inpackages/plugins/plugin-security/src/explain-engine.ts. It changesconst masterOrFault = MASTER_CHECKED_BY_ID_WRITES.has(engineOp) && recordExists && deps.checkControlledByParentWritetoconst masterOrFault = false && MASTER_CHECKED_BY_ID_WRITES.has(engineOp) && ..., so explain stops asking the master check. That is the pre-#22529 shape.1d8f2ff1toee54fcff.(
deletereads the same.)1d8f2ff1, equal to HEAD;git diff HEADis empty andgit status --porcelainis clean. The mutation was never staged or committed.7dc3c671and again atda157165(after the merge below), with the same reading both times.explain-engine.tsand the test are byte-identical across the merge.Tests and gates, at HEAD
da157165The branch merges
origin/main149294c02. Two commits there touchplugin-securityandplugin-sharing(#22671, #22745); the delta againstmainis still exactly the one new file (+275). Afterpnpm install --frozen-lockfile, the@objectstack/plugin-security...closure was rebuilt (18 tasks).@objectstack/plugin-securitysuite (vitest run --maxWorkers=2): 200 files passed, 4145 tests passed and 45 skipped. The lock reported VERDICT command-exit 0. Before the merge, at7dc3c671, it read 199 files, 4132 passed and 45 skipped.pnpm --filter @objectstack/plugin-security typecheck: exit 0. The test-layer program (tsconfig.test.json,--listFilesOnly) lists the new file, with 0 errors and 0 debt.node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack): 57 derived, all exit 0. Each exit code was recorded before any pipe. The--ranreconciliation reads: 57 derived, 57 run, 0 NOT-MEASURED (derived from the recorded codes), 0 UNRUN.7dc3c671, three gates exited 3, PREREQUISITE NOT MET.check-plugin-teardown-shape.mjs --self-testlacked its pinned fixture commit on this shallow clone. It exits 0 aftergit fetch --depth=1of621a4876.check:dual-build-cjs-loadsandcheck:i18nlacked other packages'dist/, which that battery'scheck:type-check-debtthen built. Both were rerun atda157165on post-merge builds.check:dual-build-cjs-loadsreports 107 require entry points across 66 packages load.check:i18nreports 9 packages, all bundles in sync.eslint --no-inline-config --format jsonreads 1 file, 0 errors and 0 warnings. eslint's own resolved config for the file (--print-config) is a linted config, and it carries noparserOptions.projectand noprojectService. With no type-aware linting, a new test file cannot move any other file's verdict, and the merge did not changeeslint.config.mjs. The repo-widepnpm lintis CI's.skip-changesetapplies.files[]isdist,README.mdandCHANGELOG.md. After a rebuild ofplugin-security,dist/carries 0 hits for the test's symbols, against 4 files each for two positive controls.Acceptance notes
sharinglayer detail says "The update answers 403 PERMISSION_DENIED" (and likewise for delete), while the door answers 404RECORD_NOT_FOUND. The top-level record verdict is the correct missing-record shape; only the layer prose names a status the door does not give. The read-absent rewrite inexplainAccessreplacesrecordand leaves layer prose as computed. Nothing here touches it.permissionSets, which ismain's ADR-0131 spelling. HotCRM on 17.7.0 binds itssales_managerset by naming it after the position. The route under test, the master check's answer for a bypass holder on the master, does not depend on which spelling delivered the set.Generated by Claude Code