Repository navigation
feat(spec): ISecurityService declares checkControlledByParentWrite, an optional member answering the master-detail write check - #22492
Conversation
…n optional member answering the master-detail write check The member exposes the existing ADR-0055 master-detail write check for an update of (object, recordId) under the caller's context, with the ADR-0090 D10 delegator leg. It resolves with ControlledByParentWriteOutcome, a discriminated union: allow, deny (with the refusing leg), not_applicable, and unresolvable (with the reason no verdict was reached). A store fault rejects with the engine's own error and keeps its declared status. The contract test gains three rows, including the type-level pin that an unguarded call does not compile. The plugin-security registered-member pin gains the one DECLARED_MEMBERS row its satisfies clause forces. Claude-Session: https://claude.ai/code/session_01DhTqaEHqPVSVnAkjG3jywn Co-authored-by: Claude <noreply@anthropic.com>
…lled-by-parent write outcome types Regenerated by `pnpm --filter @objectstack/spec check:generated --fix`, which proved exactly these two artifacts stale: three type-only exports each (ControlledByParentWriteOutcome, ControlledByParentWriteDenialLeg, ControlledByParentWriteUnresolvedReason). Claude-Session: https://claude.ai/code/session_01DhTqaEHqPVSVnAkjG3jywn Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 1 package(s): 5 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 4 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 139 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 5b88202554f10e54862a5b67da62e97491c33524 && git checkout 5b88202554f10e54862a5b67da62e97491c33524
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 35ef501e1301a2e992f4f785a9ef469ce79fc086 1a4557d0e213965045fe240833288f32bbc7d533 && git checkout -B drift-repro 35ef501e1301a2e992f4f785a9ef469ce79fc086 && git merge --no-ff 1a4557d0e213965045fe240833288f32bbc7d533
node scripts/docs-audit/affected-docs.mjs --json 35ef501e1301a2e992f4f785a9ef469ce79fc086
|
Contract reviewServed-tier: Inputs, and nothing else: card #22464 (body and every comment, the two claim records, the dev report ① Derived judgmentsAccept-set changes: none. The diff adds no authorable key, changes no schema, and touches no runtime source. Public-surface change A — one optional member, Public-surface change B — the outcome type, judged arm by arm against
Edge contexts the member TSDoc pins, each against the middleware:
The header bullet ("Gate outcomes refuse, and a fault rejects") is a fourth stance beside the three the file already lists, and it says what the type does. Right. Generated artifacts. The forced Contract tests. The three new rows pin: absence typed and reported as its own state; each arm's required field ( ② Semver level
③ Boundary flagsEvery deviation in the dev report
Out-of-scope findings, both carried to #22455 by the dev, each answered:
One finding of this review, not raised by the dev: in the Check-runs on the head, read at 2026-10-09T14:13Z, after waiting for every one to complete (35 check-runs, all Implemented-by: VERDICT: PASS |
Fixes #22464
Clause-②: yes (widening)
ISecurityServicenow declares one optional, feature-detected member that answers the existing ADR-0055 master-detail write check:checkControlledByParentWrite(object, recordId, context). It resolves withControlledByParentWriteOutcome. This is thepackages/spechalf of #22455, which triage split out. #22455 serves the member fromplugin-securityand 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 aControlledByParentWriteOutcome. The name and signature followcheckAuthoredRowWritein the same file.ControlledByParentWriteOutcome, a discriminated union onoutcome: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 answersallow, because the write path never checks it.deny, withleg: ControlledByParentWriteDenialLeg:object_permission,row_level_security,record_sharingormaster_chain.not_applicable: the object does not declarecontrolled_by_parent.unresolvable, withreason: ControlledByParentWriteUnresolvedReason:master_detail_relation_missing,record_not_foundormaster_reference_missing.503.Premises, verified on
origin/maindee7692f0git grep -n "assertControlledByParentWrite" origin/main -- packages/plugins/plugin-security/src/security-plugin.tsfinds 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 privateassertMasterRowEditable(:9377).registered-security-service-members.pin.test.ts,DeclaredOptionalitymapskeyof ISecurityServicewith-?(:52),DECLARED_MEMBERS ... as const satisfies DeclaredOptionality(:78), andSERVED_NOT_DECLAREDis empty (:87).ISecurityServicedeclared 21 members, and none of them runs this check.checkAuthoredRowWriteanswers authored row-level security only.What the check decides, and the arm each outcome maps to
The outcome type carries what
assertControlledByParentWriteand its legs decide for a by-id update, and nothing more.controlled_by_parent(returns early)not_applicablemaster_detailrelation,MasterDetailRelationMissingError(422INVALID_METADATA)unresolvable/master_detail_relation_missingDetailRecordNotFoundError(404RECORD_NOT_FOUND)unresolvable/record_not_foundMasterReferenceMissingError(422MISSING_REQUIRED_FIELD)unresolvable/master_reference_missingupdateon a master (403)deny/object_permissiondeny/row_level_securitydeny/record_sharingdeny/master_chainallowThe 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
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
503without 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 security(attachments): the attach / delete gate asks plugin-sharing's canEdit, which reads every controlled_by_parent object as public — a member with sys_attachment create/delete writes files on child records they cannot edit #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 thatstore_faultis not a reason.record_not_foundis the addressed record, not its master. The card and the ruling say "a missing master row". In the check, the 404DetailRecordNotFoundErroris the target row. A missing master row is judged by the legs on the first hop, and is amaster_chainrefusal above it. I followed the check, and the TSDoc says so. On the first hop the legs can admit a dangling FK:resolveSharingCanEditanswerstruewhen 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.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:
allow, because the middleware short-circuits on it;deny/object_permission, becausecheckObjectPermission('update', master, [])isfalse(ADR-0056 D2);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.
master_chainis a leg. Above the first hop, the check answers an unresolvable or dangling ancestor as an authorization refusal (assertControlledByParentWriteanswers a metadata defect and a missing row with the same403 PERMISSION_DENIED"requires edit access to its master record" #7474's envelope, ADR-0055 amendment), so it is neverunresolvable.The forced
plugin-securityrow (claim amendment6080901064, cross-lane declaration6080912705)DECLARED_MEMBERSgainscheckControlledByParentWrite: 'optional', in interface order. Nothing else inplugin-securitychanges. An optional member that is declared and not served passes the pin's served-side check, andSERVED_NOT_DECLAREDstays 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.mjsin wrap mode. The anchor hit exactly once, the write was proven on disk, and the restore was proven as blob == HEAD with an emptygit diff HEAD. Each leg ran the package's owncheck-test-typecheck.mtsgate and then rawtsc -p tsconfig.test.json. The spec test program imports./security-serviceby relative path, so nodist/sits between the spec mutations and the run.checkControlledByParentWrite?(made required(659,7) TS2578at the unguarded-call pin;(70,3) TS2322(themakeServiceliteral);(122,11) TS2322(REQUIRED_MEMBERS)deny'slegmade optional(697,5) TS2578, the deny-without-leg pinunresolvable'sreasonmade optional(699,5) TS2578, the unresolvable-without-reason pin'store_fault'added to the reasons(725,5) TS2578, the fault-is-not-a-reason pin'abstain'added to thenot_applicablearm(705,5) TS2578, the abstain pin;(740,17) TS2322at the exhaustive switch'sneverDECLARED_MEMBERSrow deletedregistered-security-service-members.pin.test.ts(79,12) TS1360, thesatisfies DeclaredOptionalityclauseA5 reads the spec through
dist/(nopathsrule for the spec inplugin-security). The unmutated list compiling green at1a4557d0eis 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
allowandnot_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, underscripts/pm/os-verify-lock.sh.pnpm --filter @objectstack/spec build: exit 0. Thencheck:generated: exit 1, with exactlyapi-surface/andexport-origins/stale. Thencheck:generated --fixregenerated those two: three type-only exports each, and both checks now pass.api-surface-signatures.json(it covers thedefineXfactories only) and the reference pages (check:docs) are unchanged.pnpm --filter @objectstack/spec typecheck: exit 0.security-service.test.tshas notest-typecheck-debt.jsonentry, so its@ts-expect-errorlines 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 reports0 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/objectstackat1a4557d0ederived 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:i18nandpnpm check:dual-build-cjs-loadsexited 3 (PREREQUISITE NOT MET): only theplugin-securityclosure had adist/.pnpm check:dts-closureandpnpm check:sourcemap-no-sources-contentexited 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 --ranover 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 jsonover the three changed.tsfiles reportsfiles 3 errors 0 warnings 0. All three are in the configured population:eslint --print-configresolves a rule set for each.eslint.config.mjsenables no type-aware linting (parserOptions.projectandprojectServiceare unset for all three), so this diff cannot move the verdict on any untouched file. The fullpnpm lintis CI's.Consumers. The public-surface change is one optional interface member and three type-only exports.
git grepoverpackages/**forRequiredoverISecurityService,keyof ISecurityService,implements ISecurityServiceandsatisfies ISecurityServicefinds only theplugin-securitypin updated here. Every other reference holds the service as aPartialor calls existing members, so no other compiled verdict can move. The full workspace consumer sweep is CI'sTypeScript Type Check.Not merged with
main.origin/mainis two commits ahead (8b713fad7: CLI, core and dogfood tests, plus an unrelated changeset). They share no file and no package with this diff.dispatch-gatesreports that none of them touched what its answer derives from. The merge queue rebuilds on currentmain.Acceptance notes
Noted, not filed.
userIdon 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 refuseallowEdit(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: security(attachments): the attach / delete gate asks plugin-sharing's canEdit, which reads every controlled_by_parent object as public — a member with sys_attachment create/delete writes files on child records they cannot edit #22455, which serves the member and decides whether the member mirrors the guard. This is a read-only inference, not measured.security-plugin.ts(:2391) lists members by hand. Carrier: security(attachments): the attach / delete gate asks plugin-sharing's canEdit, which reads every controlled_by_parent object as public — a member with sys_attachment create/delete writes files on child records they cannot edit #22455, which edits that file to serve the member.Generated by Claude Code