Repository navigation
fix(service-storage,plugin-audit,plugin-security)!: the attachment and comment parent gates judge a controlled_by_parent parent through its master - #22513
Conversation
… a controlled_by_parent parent Claude-Session: https://claude.ai/code/session_01WYYhVJ78u7PhwFViWo1EmQ Co-authored-by: Claude <noreply@anthropic.com>
…Write from the write path's own composition Claude-Session: https://claude.ai/code/session_01WYYhVJ78u7PhwFViWo1EmQ Co-authored-by: Claude <noreply@anthropic.com>
…y-id update, and pin that it stays served Claude-Session: https://claude.ai/code/session_01WYYhVJ78u7PhwFViWo1EmQ Co-authored-by: Claude <noreply@anthropic.com>
…by_parent parent through its master Claude-Session: https://claude.ai/code/session_01WYYhVJ78u7PhwFViWo1EmQ Co-authored-by: Claude <noreply@anthropic.com>
…_parent parent through its master Claude-Session: https://claude.ai/code/session_01WYYhVJ78u7PhwFViWo1EmQ Co-authored-by: Claude <noreply@anthropic.com>
…eg for a floor-bound member Claude-Session: https://claude.ai/code/session_01WYYhVJ78u7PhwFViWo1EmQ Co-authored-by: Claude <noreply@anthropic.com>
…d master-detail write check Claude-Session: https://claude.ai/code/session_01WYYhVJ78u7PhwFViWo1EmQ Co-authored-by: Claude <noreply@anthropic.com>
…tachment-gate-master-write
…suite's pinned findOne double Claude-Session: https://claude.ai/code/session_01WYYhVJ78u7PhwFViWo1EmQ Co-authored-by: Claude <noreply@anthropic.com>
…tachment-gate-master-write
📓 Docs Drift CheckThis PR changes 3 package(s): 14 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 2 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 26 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 8cbe6098208a13f86b7279fd888e229a86941b87 && git checkout 8cbe6098208a13f86b7279fd888e229a86941b87
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 446c8b2a6420a61a2862e6f5140dda71a53316d1 15d05d2b697153d2e126285e037a9d49885fcd44 && git checkout -B drift-repro 446c8b2a6420a61a2862e6f5140dda71a53316d1 && git merge --no-ff 15d05d2b697153d2e126285e037a9d49885fcd44
node scripts/docs-audit/affected-docs.mjs --json 446c8b2a6420a61a2862e6f5140dda71a53316d1
|
…r-detail write check's system exit Claude-Session: https://claude.ai/code/session_01WYYhVJ78u7PhwFViWo1EmQ Co-authored-by: Claude <noreply@anthropic.com>
Contract reviewServed-tier: Inputs read: card #22455 (body and all 10 comments, the amended claim Check-runs on the head: every latest run concludes ① Derived judgmentsTruth tables against ruling A (
All four attachment limbs (attach, update row rule, delete row rule, re-point) and both comment moderation limbs (delete, update) go through that one function; the comment insert limb still asks READ only. Right. One composition on the plugin-security side. Serving note 1 (step 2.8's
Serving note 2 (registration log line read off the object). Published-surface changes, by exports map (
Security reading.
File list against the amended claim's surface. Outside the named files, all declared by the dev: ② Semver level
③ Boundary flagsDev flags (report
Implemented-by: VERDICT: PASS |
system-context.mdx: both sides' prose kept (main's row 9b from #22513, this branch's row-51 anchor packages/runtime/src/domains/i18n.ts#handleI18nRequest); counts re-derived by pnpm gen:system-context-census. registry.ts: re-derived by pnpm --filter @objectstack/spec gen:migration-registry (byte-identical to the text merge). Claude-Session: https://claude.ai/code/session_01BmsuLyUeuG5CNpZFMH1jzS Co-authored-by: Claude <noreply@anthropic.com>
Fixes #22455
Clause-②: no (narrowing)
The
sys_attachmentparent gate (@objectstack/service-storage) and thesys_commentmoderation gate (@objectstack/plugin-audit) now judge acontrolled_by_parentparent through its master: the answer the parent's own by-id update gets.@objectstack/plugin-securityserves the master-detail write check that card #22464 declared (ISecurityService.checkControlledByParentWrite) from the write path's own composition, and both gates ask it when the sharing service abstains. This executes seat 1's ruling A (6079158667) as carried by triage (6079448762) and the claim6083853580.Reproduction (before)
Real stack:
bootStack, org-bound, withStorageServicePluginandAuditPlugin. The fixture ispackages/qa/dogfood/test/fixtures/cbp-parent-gates-fixture.ts. A member holdsorg_member, the fixture's read/create/edit baseline, and a set grantingsys_attachmentandsys_commentcreate and delete. The mastercpg_accountispublic_read, so the member reads it, and it is owned by the admin.cpg_contractis acontrolled_by_parentchild of it.cpg_vault_itemis acontrolled_by_parentchild of a private admin-owned master. Measured with the new dogfood file on the unmodified basee148ca984(test commitbca810323):PATCH /data/cpg_contract/ID(precondition)PERMISSION_DENIEDPOST /data/sys_attachmenton thatcpg_contractPOST /data/sys_attachmentoncpg_vault_item, which answers the member 404DELETE /data/sys_attachment/IDof the admin's file on thecpg_contractDELETE /data/sys_comment/IDof the admin's comment on thecpg_contractsecurity.checkControlledByParentWriteundefined)Controls, green before and after: the master owner attaches (201); the member attaches under a master they own (201); a
public_read_writeparent admits the member (201); an uploader deletes their own file (200); an author deletes their own comment, and the master owner moderates a member's comment (200).What changed
plugin-security: servescheckControlledByParentWrite(object, recordId, context)on the typed service literal, so the compiler holds it to the contract.assertControlledByParentWritefor the principal's sets, then for the delegator's on an on-behalf-of context (step 2.8's two calls, in order). The first refusal is the answer.resolveOperationPrincipals, which the middleware and the member both call. Its refusals, their order and their wording are unchanged.WeakMapbeside the error, never on it, so the write path's envelope is byte for byte the same. The three non-verdict errors map to theunresolvablereasons. Anything else (a store fault, a refusal of the context) rejects unchanged.service-storage: one composition,mayEditParent, answers parent EDIT for all four limbs: attach, the row rule on update and delete, and the re-point's attach rule. It reads the tri-statecheckEdit, notcanEdit. The installer takes an optional trailing security resolver, andStorageServicePluginwires it.plugin-audit:canEditParentgets the same composition. It is the one function behind the moderation limb, so it covers comment delete (which reproduced) and comment update. Insert and re-point still ask READ.AttachmentSharingLike/CommentSharingLikenowPickcheckEdit. New port typesAttachmentSecurityLikeandCommentSecurityLikearePicks ofISecurityService.content/docs/permissions/system-context.mdx: one census row (9b), anchored atsecurity-plugin.ts#checkControlledByParentWrite, plus the census gate's own--fixcounts (121 to 122) and nothing else on the page. The reason: the served member answersallowfor a system context (the contract's first bullet), which is a newExecutionContext.isSystemread site, andcheck:system-context-censusrequires every such site to carry a row. The seat authorized this one row by amending the claim (6083853580), declared todomain:devx.Gate truth table (both gates, every limb)
checkEditcheckControlledByParentWriteallowdenyabstainallowabstainnot_applicablepublic_read_writecontrol)abstaindeny(any leg)abstainunresolvable(broken declaration, missing record, null master reference)abstain503abstainEnvelopes: attach and re-point answer 403
ATTACHMENT_PARENT_ACCESS. A delete of another user's file answers 403ATTACHMENT_DELETE_DENIED, and an update of it answers 403RECORD_NOT_ACCESSIBLE. Comment delete or update answers 403RECORD_NOT_ACCESSIBLE. A caller who cannot read the parent still gets the not-visible 403PERMISSION_DENIED.On
not_applicable: the gates ask the member on every abstention. The member is the one place that decides whether a parent iscontrolled_by_parent, so the gates carry no second copy of that predicate.not_applicableis therefore exactly the "any other abstain admits" arm of the ruling, and it matches the contract's consumer rule (proceed onallowandnot_applicable). The claim's line first readnot_applicableas fail-closed. That reading only arises if the gate decides "this parent is controlled_by_parent" itself, which this design never does. The seat accepted this reading and corrected the claim line.Pins and ablations
Every ablation went through
scripts/ablation-replace.mjs: the anchor hit once, the blob changed, and on restore the blob equals HEAD withgit diff HEADempty.service-storageattachment-access-hooks.test.tsif (verdict === 'allow') return true;becomesif (verdict !== 'deny') return true;(thecanEditfold)c078a57acb10= HEADplugin-auditcomment-access-hooks.test.ts7718798ea3e2= HEADdogfoodcbp-parent-attachment-comment-gates.dogfood.test.tsablation-dist-preflightfound the marker in 2 built files of each packageexpected 201 to be 403twice,expected 200 to be 403twice). Restore leg: sources equal HEAD, rebuilt,--absentpasses with a clean tree, rerun 10 / 10plugin-securitycontrolled-by-parent-write-member.test.ts,registered-security-service-members.pin.test.tsuserIdguard. C: refusals carry no leg8b1ac155b333= HEADThree earlier dogfood ablation attempts were not measurements, and none of them reached a test. Two mutations failed the declaration build (a narrowing error, then an unused parameter), and one was refused by
ablation-replacebecause the replacement contained the anchor. Each attempt restored the sources to HEAD.PATCH parity
controlled-by-parent-write-member.test.ts): the real engine middleware's by-id update and the served member run on one store for one caller.allowfor exactly the records the update admits: an owned master, a master shared atedit, a master admitted by authored write RLS, and a two-hop chain.denyleg (object_permission,row_level_security,record_sharing) matches the reason step 2.8's refusal states. The delegator leg is covered, and so ismaster_chain(an empty master reference above the first hop).unresolvablereasons each meet a refused update. A system context answersallow, and a non-controlled_by_parentobject answersnot_applicable.PATCHof the child answers 403, and its message saysrequires edit access to its master record (master 'cpg_account' not editable by this user (row-level security)). The member answers{ outcome: 'deny', leg: 'row_level_security' }.created_byownership floor bindsorg_memberon the master, and record sharing gives the member no basis to lift it. My first expectation wasrecord_sharing, and the measurement corrected it.allowand a 200PATCH. The member under their own master getsallowand a 200PATCH. Thepublic_read_writeboard getsnot_applicable.The two serving notes (landing record
6083231669)context.userIdguard is NOT mirrored.allowfor a principal whose master nothing measured. The contract'sallowcovers only a system context or every leg passing, and this lane does not edit the spec TSDoc.allowoutcome refuses. The record's own update already refuses such a principal at the object-level gate, because guest bindings refuseallowEdit, so the overall parity holds.denyonobject_permission) and ablated (mirroring the guard turns that pin red). The spec'sallowTSDoc needs no second clause.Object.keys(registeredSecurityService), sorted), so it cannot fall behind a served member again. The hand list named 16 of the 21 members served before this PR. It omitteddescribeDelegableScope,describeDelegationNarrowing,getEffectiveObjectPermissions,hasWriteBypassandresolveWriteScope. It now prints all 22.Local verification (HEAD
15d05d2b6)node scripts/check-system-context-census.mjsat15d05d2b6readscheck-system-context-census: OK — 122 elevation read sites in 20 packages across 57 files, living in 104 symbol(s); the page cites 117 symbol(s) against 117 required. The gate's own--fixis a no-op on the committed page (blob unchanged before and after).dispatch-gates --commandsderived 105 families at15d05d2b6. That is the 79 from the previous head plus 26 that the docs path adds (doc frontmatter, route spelling, section names, landing index, doc anchors, docs redirects, single H1, speccheck:docs, and others). All 105 were run with recorded exit codes, and every one exited 0.--ranreads:105 derived famil(ies) accounted for — 105 run, 0 NOT-MEASURED.turbo run build, 73/73 tasks) before the gates ran.bdce0149d, so these readings carry over. Atbdce0149d, after rebuilding:plugin-security: 106 passed;faea6141f(main has moved since, through merges that touch none of these packages):plugin-security: 191 files, 3969 passed / 45 skipped;service-storage: 47 files, 800 passed;plugin-audit: 42 files, 672 passed;typecheckfor all three, test layers included: exit 0..tsfiles: 14 results, 0 errors, 0 warnings. No type-aware linting is enabled, so untouched files' verdicts cannot move.mainmoved by one commit,4e9fe9ff6(the protocol version bump). It touches neither the census page, these packages nor anyisSystemread in code, so it was not merged this round.Acceptance notes
POST /api/v1/security/explainfor the member,{ object: 'cpg_contract', operation: 'update', recordId }, answersallowed: truewith the OWD layer reading "controlled_by_parent: rows are org-shared at this baseline". The same member'sPATCHof that record answers 403. Measured atfaea6141fwith an untracked scratch probe on this PR's fixture, deleted after the reading. The explain record write gate askssharing.canEdit(canEditRecord), whose abstention reads as permission. The member this PR serves is the parity path. Not fixed here.ISharingService.canEditTSDoc still names thesys_attachmentparent gate (and, under the write-depth paragraph, both parent-record gates) ascanEditcallers. Both now readcheckEdit. No runtime consumer; carrier: the spec lane's TSDoc follow-up, card spec(contracts): theControlledByParentWriteDenialLegTSDoc saysmaster_chainrefusals name a master above the record's own master; the walk's first hop reads the record's own master #22497.plugin-sharing'sSharingService.canEditdocblock. Doc only; carrier: none.minorforplugin-security, and the BREAKING banner rides only on the two packages that narrow. The ADR-0087 disposition isnot-required (no-migration-prescription). The remedy ("grant edit on the master") is written as prose, because the gate refuses that category when a FROM-TO block is detected and no other category applies to a runtime accept-set narrowing.scripts/engine-double-contract.pinned.jsongains one row, written by--writefor the new suite's pinnedfindOnedouble.Generated by Claude Code