security(engine): exempt the referential FK-clear write from the object-level CRUD check - #12941
Conversation
…lear The engine's `set_null` reference cleanup already tags its UPDATE with the server-derived `__referentialFieldClear` marker (#3023). The marker reached only the ownership-anchor guard, so the cleanup write still had to pass the object-level CRUD check on the referencing object: a role holding full delete rights on A and no grant at all on B could delete an A only while B was empty. plugin-security now exempts a marked UPDATE from the object-level CRUD grant check, and from that check alone. Field-level security on the FK column, the RLS `using` row scope and the RLS post-image `check` all stay enforced -- each pinned -- and the `cascade` arm keeps the operator's own delete authority, untouched. The write is not elevated: it stays the operator's identity, so audit attribution (`user_id`/`actor`, `updated_by`) is unchanged. Ruled by the maintainer on 2026-08-28 (#12597, second round, option B). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LZbWd2jNV1FErXTPSS4Dry
Committed BEFORE the mutation so the prediction cannot be written after the reading. Reverting the exemption (restoring the bare `if (permissionSets.length > 0)` at the object-level CRUD check) is predicted to turn SIX of the fourteen tests in `delete-reference-cleanup-system-identity.test.ts` RED: 1. `[#12597 - PIN INVERTED] a non-empty referencing table no longer needs ...` 2. `THE CONTRACT: full delete on A + NOTHING on B + a non-empty ...` 3. `GUARD 1 - field-level security on the FK column still refuses ...` 4. `GUARD 2 - the RLS \`using\` row scope ... still refuses ...` 5. `GUARD 2 CONTROL - the same policy admits the write when the row IS in scope` 6. `GUARD 3 - the RLS post-image \`check\` still refuses ...` Guards 1-3 are predicted red DESPITE still refusing, because each asserts its own gate's ADR-0112 envelope (`forbiddenFields`, `recordId` + "row-level security", "row-level CHECK") and the reverted CRUD check answers first with none of them. That is the whole reason those pins are written on envelopes rather than on the bare fact of a refusal. The other two new arms are predicted GREEN either way: the converse (no delete right on the target) and the ordinary unmarked update, neither of which the exemption is in a position to change. Note on the ablation's mechanics: the mutated file is `plugin-security/src/security-plugin.ts`, which this suite imports RELATIVELY (`./security-plugin.js`), so vitest reads the SOURCE in this checkout and no rebuild leg applies to it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LZbWd2jNV1FErXTPSS4Dry
MINOR by the maintainer's 2026-08-28 ruling: the change moves which deletes succeed, an observable behavioural contract change on the delete path rather than a patch-grade defect repair. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LZbWd2jNV1FErXTPSS4Dry
📓 Docs Drift CheckThis PR changes 1 package(s): 6 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 — 14 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 d77aef4034f1fdd34f451910460308990edd8dc2 && git checkout d77aef4034f1fdd34f451910460308990edd8dc2
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin e5f66214a3106edb7086f24d6acb0c12e7f865d2 0dd9a3496b89d561f55b494973873de10e1123c5 && git checkout -B drift-repro e5f66214a3106edb7086f24d6acb0c12e7f865d2 && git merge --no-ff 0dd9a3496b89d561f55b494973873de10e1123c5
node scripts/docs-audit/affected-docs.mjs --json e5f66214a3106edb7086f24d6acb0c12e7f865d2
|
The comment cited `delete-reference-cleanup-crud-exemption.test.ts`, a filename that resolves to nothing: the pins live in this package's `delete-reference-cleanup-system-identity.test.ts`, in its `#12597` describe. Caught in review. A citation that resolves to nothing is the defect class the ADR-anchor gate exists to name, and it is worse in a comment explaining a permission boundary, where the next reader is looking for the evidence. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LZbWd2jNV1FErXTPSS4Dry
Docs-drift bot report: all 8 rows are anchored on a token this diff introduces only in a COMMENTFiling this as a bot finding rather than acting on the rows. No code line in this diff contains …and a wrong row list is not a docs clearance, so the hand-read was done anywayThe bot names its own blind spot — a page that declares a rule by its inputs shares no token with the emitter — and it listed no No falsified sentence found. The two release-owned rows: audited, not edited
Both audited read-only: a code PR does not edit Review fix pushed
Generated by Claude Code |
Fixes #12597
Ruled twice by the maintainer. The authoritative record is the 2026-08-28 "Maintainer ruling applied (second round)" comment on the card (option B, verbatim 「6272 A1 其他同意」), plus its 2026-08-28 correction removing
needs:contract-review. Round 1's blanket-isSystemframing is superseded.What changed
packages/plugins/plugin-security/src/security-plugin.ts— the security middleware's object-level CRUD check (step 2) now skips a write that carries the existing server-derived__referentialFieldClearmarker, and skips nothing else:The marker has been stamped by
ObjectQL.cascadeDeleteRelations'sset_nullarm since #3023 (packages/objectql/src/engine.ts); until now it reached only the ownership-anchor guard, so the cleanup UPDATE still had to pass the CRUD grant check on the referencing object. That is the residue this card measured: a role with full delete on A and nothing at all on B could delete an A only while B was empty.Two narrowings inside the ruled scope, both stated in the code comment:
updateonly. A marker rides a context, and a context reaches whatever the write's own hooks do next. The engine stamps this one on theset_nullUPDATE alone, so any other operation carrying it is inheritance, never the ruled write.checkObjectPermissionondelegatorSets, same object, same operation). Leaving it standing would make the delete succeed directly and refuse on-behalf-of — the invisible coupling this card removes, reintroduced on the delegated path.Not
isSystem. That bypass at the top of the middleware is total — the repo's own page says so (content/docs/permissions/system-context.mdx: "Elevation is total, and it is not granular") — and the round-1 probe measured it switching off three guards that answer questions referential integrity does not ask.The
cascadearm is untouched.await this.delete(childName, { where: { id: depId }, context } as any)incascadeDeleteRelationsis byte-identical toorigin/main; the child-row DELETE carries no marker and keeps the operator's own delete authority. Option C stays excluded.The four pins, and what each measured
All in
packages/plugins/plugin-security/src/delete-reference-cleanup-system-identity.test.ts, driving the realSecurityPluginmiddleware over a realObjectQLengine.:1953(exempted):2293PERMISSION_DENIED/ 403,details.forbiddenFieldsnamesproduct, FK unchangedusingrow scope on update:2058PERMISSION_DENIED/ 403,details.recordId= the row, developer sentence says row-level security, FK unchangedcheck(product != null):2581PERMISSION_DENIED/ 403, developer sentence says row-level CHECK, FK unchangedEach refusal is asserted on its own gate's ADR-0112 envelope, never on the bare fact of a refusal — a refusal migrating between gates reddens instead of reading as "still refused". Three further arms: a control for the RLS
usingpin (same policy, in-scope row ⇒ the write is admitted, so the refusal above is the row scope and not the exemption failing to fire); the converse (no delete right on the target ⇒ still refused); and an ordinary, unmarked update on the referencing object by the same caller ⇒ stillPERMISSION_DENIEDop=update, which is what proves the exemption is scoped to the marker.One pin was inverted, deliberately and in place. The suite's constraint-1 assertion (
PERMISSION_DENIED,details.operation === 'update'on a non-empty referencing table) is what measured this card's residue; the ruling falsifies it. It now asserts the ruled behaviour and carries a comment naming this card and the round-2 ruling as the reason. It was not deleted and its expectations were not quietly edited to match new output.Audit attribution — the measured shape is preserved by construction
The write is not elevated at all: it stays the operator's own identity plus a marker, so both attribution channels the round-1 gate-(a) measurement found intact are untouched —
writeAudit'suser_id/actor(keyed onsession.userId) and theupdated_bystamp atplugin.ts:1028. The contract pin captures the context that actually reaches the engine for the cleanup write:userId: 'u_lead',isSystemnot true,__referentialFieldClear: true.Verification
Union re-run at
aae820b00, all throughscripts/pm/os-verify-lock.sh; verdicts read off the lock's own VERDICT line and vitest's own summary. After the review fix below, the target suite and the ratchet family were re-run at the new head0dd9a3496(working tree clean): Test Files 1 passed (1) · Tests 14 passed (14), withcheck:where-matcher,check:engine-double-contract,check:query-options-erasure,check:nul-bytesandcheck-comment-mask-adoptionall exit 0.pnpm --filter @objectstack/plugin-security test— Test Files 87 passed (87) · Tests 1585 passed (1585)pnpm --filter @objectstack/plugin-security typecheck— exit 0.tsconfig.jsonexcludes**/*.test.ts, verified withtsc --noEmit --listFiles(0 hits for the edited test file), so that green says nothing about the test file. The test file's correctness is measured by vitest executing it.pnpm --filter @objectstack/objectql exec vitest run src/engine-cascade-delete.test.ts src/engine-reference-check-system-identity.test.ts— 2 files, 21 tests passed (the marker's producer side, unchanged)pnpm lint(full repo,eslint . --no-inline-config) — exit 0, cleannode scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack), all exit 0:check:where-matcher,check:engine-double-contract,check:objectql-double-limit,check:cross-package-test-inputs,check:test-source-alias,check:type-source-resolution,check:type-check-coverage,check:query-options-erasure,check:published-files,check:page-declaration-shape,check:slot-lookup,check:nul-bytes,check:changeset-gate-self-tests,check:objectui-changeset,check:pm-half-states,check:i18n-stale-fill, pluscheck-adr-0087-registration,check-changeset-no-major,check-empty-changeset,check-ci-filter-parity,check-comment-mask-adoption,check-plugin-teardown-shape,release-rehearsal-clone --self-test,docs-audit/check-affected-docs.check:i18nrefused withPREREQUISITE NOT MET — the workspace CLI is not built. Narrowing by inspection: this package's extract config (scripts/i18n-extract.config.ts) runs--objects-onlyover a closed list of sixsrc/objects/*definitions plus the generated bundles; neither edited path is in that population, so the extract output cannot move. CI runs the gate.pnpm check:adr-anchorswas red onorigin/main(ADR-0006 D4 is cited by 1 file(s), but ADR-0006 declares no D4→packages/spec/src/data/object.zod.ts), tracked as [finding] The merge queue is blocked repo-wide:check-adr-anchorsis red onmain—object.zod.tscitesADR-0006 D4, which no ADR-0006 file declares #12913. It was not mine,packages/specis not this seat's surface, and ⛔check-adr-anchorswas not touched to make it go away. fix(spec): namespace tombstone stops citing ADR-0006 D4 — unblocks the merge queue's check-adr-anchors red #12917 has since landed (750fff559, inorigin/main's history) and the base is green on that gate: atorigin/maingrep -c 'ADR-0006 D4'on that file returns 0, with a positive control on the same ref and file (namespace) returning 8, so the zero is a real reading. Nothing here asserts a current base failure.Reverse verification (ablation)
Prediction committed before the mutation (
141ba0680, empty commit). Reverting the exemption to the bareif (permissionSets.length > 0):cfbb035f→9acdc533, anchoredgrep -cin both directions (guarded spelling 1 → 0, bare spelling 3 → 4);git hash-objectequal to the HEAD blob (cfbb035f),git diff HEADempty,git status --porcelainempty;trap … EXIT INT TERMwith absolute paths throughout../security-plugin.js), so vitest reads the source in this checkout, notdist/.Docs
Reviewer-of-record fix. The exemption comment cited
delete-reference-cleanup-crud-exemption.test.ts, a filename that resolves to nothing — the pins live in this package'sdelete-reference-cleanup-system-identity.test.ts. Corrected in0dd9a3496;git grepfor the dead name now returns 0 files, and the cited name resolves to a tracked file. A citation that resolves to nothing is exactly the defect class the ADR-anchor gate exists to name, and worse in a comment explaining a permission boundary.The docs-drift bot's 8 rows are anchored on a token this diff introduces only in PROSE. Measured independently:
No code line in this diff contains
set_null. So every listed row (data-api,field-types,fields,validation-rules,troubleshooting,protocol/objectql/types, and the two release pages) is anchoredvia set_null (literal)on explanatory comment text. Reported as a bot finding rather than acted on — anchoring on comment text over-lists, and this lane has now hit that mode more than once.Wrong rows are not a docs clearance, so the hand-read was done anyway. The bot's own blind spot is a page that declares a rule by its inputs, sharing no token with the emitter — and it listed no
content/docs/permissions/page at all, which is precisely where this diff could falsify something. One question was asked ofauthorization.mdx,access-matrix.mdxandsystem-context.mdx: does any sentence state or imply that the reference-cleanup write is subject to the caller's authority on the referencing object?authorization.mdx's layer table describes the ladder for caller-initiated operations (layer 3 = object CRUD, fail-closed 403); it does not assert that the ladder has no exemptions, and it already coexists with two documented-elsewhere layer-3 exemptions it does not caveat either — theisSystemshort-circuit and ADR-0056'spublicFormGrant.system-context.mdxis aboutisSystem, which this diff deliberately does not use; its row 18 (isSystemskips referential-integrity checks,engine.ts:3314) is untouched and still true, and its "Elevation is total, and it is not granular" is the sentence this design obeys rather than contradicts.access-matrix.mdxis the snapshot-gate page — the{read, create, edit, delete}map is per (permission set × object) and is not changed by this diff.content/docs/deployment/troubleshooting.mdx's "Cannot delete record: delete restricted" entry is aboutrestrictescalation andmultiple: truemember semantics, with zero permission/403 content.⇒ No falsified sentence found; nothing in
content/docs/is edited by this PR.The two release-owned rows were audited, never edited (
content/docs/releases/is release-owned):v15.mdx:847— "Engine-internal referential cascades (set_nullon user deletion) are exempted via a server-side__referentialFieldClearmarker that cannot be forged from a client (fix(security): 级联 set_null 豁免属主转移守卫 (#3023) #3048)". Still true. It describes the ownership-anchor guard's exemption, which is unchanged and still keyed on the same marker; this PR adds a second consumer of that marker, and the sentence never claims to be the only one. The un-forgeability half also still holds:assembleExecutionContextbuilds an inbound envelope from a closed field set that contains no__operation-private key.v17.mdx:3819—deleteBehavior: 'set_null'on amultiple: truereference removing the deleted member (finding(objectql): the delete-cascade path's two registry-read swallows are the #8895 shape one layer up —catch → returndisables every referential guard at once, silently #9002). Nothing about permissions; untouched by this diff and not falsified.Clause-② — self-assessed against the actual diff: no
The diff touches
packages/plugins/plugin-security/src/**and.changeset/only. No published contract's accept/reject face changes and no public surface widens: nopackages/specschema, no export added or removed, no authorable key, no REST parameter set. What changes is a runtime permission-gate verdict — the 安全/权限边界 manual floor, a different control, and one already exercised twice by the maintainer's own rulings on this card. This matches the maintainer's recorded correction removingneeds:contract-review; ⛔ that label is not re-attached.Changeset
.changeset/referential-field-clear-crud-exemption.md—@objectstack/plugin-security: minor. MINOR because this changes which deletes succeed: an observable behavioural contract change on the delete path, not a patch-grade defect repair.Generated by Claude Code
Generated by Claude Code