Repository navigation
Labels
domain:engineRetired 2026-10-22, alias of domain:server (engine surface); in-flight cards keep it until they landRetired 2026-10-22, alias of domain:server (engine surface); in-flight cards keep it until they land
Description
⚖️ RULED — twice. Do not adjudicate or dispatch from the menu below: it is the superseded FIRST-round framing, kept as history.
- Round 1 (maintainer, 2026-08-27 decision batch): option B of the menu below — the
set_nullcleanup write moves off the operator identity;cascadeuntouched; constraint 2 required two gate measurements before any code.- Round 2 (maintainer, 2026-08-28, director summon ✨ Set up Copilot instructions #2 — triggered by the constraint-2 measurement: blanket
isSystemin this codebase is TOTAL and also bypasses field-level security, RLSusingand the RLS post-imagecheck): marker-scoped exemption of the object-level CRUD check ONLY, via the existing__referentialFieldClearmarker. The three unrelated guards stay enforced, each pinned; audit attribution keeps the measuredsudo()shape;cascadestays operator-identity and excluded; changeset MINOR; Clause-② yes (needs:contract-reviewattached, dispatch atCONTRACT_REVIEW_TIER).- Authoritative record: the 2026-08-28 "Maintainer ruling applied (second round)" comment on this card. Current state:
pm:queue, awaiting thedomain:engineseat's dispatch.- (Card face updated by the director seat, session
session_016SG9S6V15MqeAgkehDcTwk, at the maintainer's request — the title and body previously still read as an undecided FYI card.)
一句话
#12166 的裁决(2026-08-26,选项 A)把删除前的引用检查切成了系统身份,约束 1 明确限定「删除路径的其他任何部分身份不变」。因此引用清理的写入半边 —— set_null 的 UPDATE 与 cascade 的子记录 DELETE —— 仍以操作人身份执行。结果:原报告里那批「有删除权、对引用表无任何权限」的角色,只有在引用表为空时删除才通得过;引用表真的有引用行时,依然 403,只是报错从 find 变成了 update。
这不是 #12166 的实现缺陷,也不是要推翻裁决 —— 它正是约束 1 划定的边界。开这张卡只是因为该边界的实际影响没有被测量过,而它决定了原报告那 17 组「角色×对象」里到底有多少被真正修好。
证据(PR #12596 里已 pin 成断言)
packages/plugins/plugin-security/src/delete-reference-cleanup-system-identity.test.ts:
it('the elevation is the CHECK only — a non-empty referencing table still needs
the caller\'s own write authority (constraint 1)', ...)
expect(err.code).toBe('PERMISSION_DENIED');
expect(err.details?.object).toBe('os_ehr_andon_record');
expect(err.details?.operation).toBe('update'); // ← 不再是 'find'
夹具与原报告同形:对象 A 被 B 的可空 lookup 引用(解析行为 = set_null),角色对 A 有完整删除权、对 B 无任何授权。B 为空 ⇒ 删除成功(#12166 修好的部分);B 有一行 ⇒ 仍然 403。
为什么值得单独判一次
- 原报告的 17 组配置里,引用表非空的那些仍然按不动删除按钮,症状与修复前对用户完全一致(同样是一句通用「您没有执行此操作的权限」);从部署方视角看,问题只解决了一部分,而这一点在 删除记录时「引用清理」用操作人身份查询引用表,读权不足即整体 403(应以系统身份执行) #12166 的验收里看不出来。
- 裁决为引用检查给出的论证 —— 引用完整性动作是引擎责任、主流平台一律以系统身份执行(RDBMS FK 的
SET NULL/CASCADE、Salesforce 的 lookup 清空与级联删除均记载为绕过 sharing)—— 在文义上同样覆盖写入半边:FK 基线里真正以引擎身份执行的正是那个SET NULL/DELETE。裁决把范围钉在查询上是刻意的(卡片正文与决策分析通篇只说「查询」),但据我所读,那个范围收窄没有单独论证过写入半边应当保留操作人身份 —— 它更像是照着 issue 的措辞划的,而不是对写入半边下的判断。 - 反方向也确实存在:
cascade的子删除以系统身份执行意味着「有 A 的删除权 ⇒ 可以删掉自己无权删除的 B 行」,这是一个远比读权外溢更大的权限语义变更,绝不该顺手带过。set_null(只清 FK 列)与cascade(删整行)在这个维度上不对称,很可能需要分别裁。
建议处置(⚠️ 已被上方两轮裁决取代,仅存档)
- A. 维持现状(裁决的字面范围):写入半边保持操作人身份。代价:原报告的部署仍需为每个可删角色补齐引用表的写权限,「删除权 = 删除权 + 引用表权限」的隐藏耦合只是从读权变成了写权,在权限界面上同样不可见。
- B.
set_null也切系统身份,cascade不切:清空 FK 列是纯粹的完整性维护(且已有 属主守卫与级联 set_null 冲突:非特权删除 sys_user 时 owner_id 级联置空被 #3004 守卫拦截(级联中途失败) #3023 的__referentialFieldClear标记把它标为引擎内部写入),而删掉整行是数据销毁,保留操作人授权。这一档最贴近 FK 基线。 - C. 两者都切:完全对齐 Salesforce/Dataverse 的级联语义。权限面变更最大,需要独立论证。
needs-user-decision 而不是直接派发。
出处
- 裁决:删除记录时「引用清理」用操作人身份查询引用表,读权不足即整体 403(应以系统身份执行) #12166 的 2026-08-26 评论(选项 A + 四条约束)
- 实现与 pin:PR fix(objectql,spec): run the pre-delete reference check under the system identity #12596
- 原始部署实测记录:steedos-labs/os-project-titanwind-ehr#1809
Generated by Claude Code
Metadata
Metadata
Assignees
Labels
domain:engineRetired 2026-10-22, alias of domain:server (engine surface); in-flight cards keep it until they landRetired 2026-10-22, alias of domain:server (engine surface); in-flight cards keep it until they land
Activity
os-support-ai commentedon Aug 26, 2026
Triage (triage seat, session
session_01G28JXov51zX81Ky6M4r25m): decision box —needs-user-decision+domain:engine(family precedent: the ruling card this follows up, same delete-path surface), type Feature (options B/C expand the accepted set of the delete operation — the mechanical boundary test). This is a security/permission-boundary question and it would extend the scope of a maintainer ruling made today, so it sits on the human floor twice over — auto-adjudication does not apply, and the card's own framing agrees (条款② applies to any outcome). Not dispatchable; enters the maintainer's decision inbox.Decision-box filing follows (中文,按裁定格式):
<!-- os-decision-facets -->
一句话问题:上一裁已让删除前的「引用检查」以系统身份执行,但「引用清理」的写入仍用操作人身份——所以只要引用表里真的有引用行、而操作人对那张表没有写权限,删除照样被拒;拿到删除权的角色在权限界面上看不出还差什么。
选项 × 真实代价:
find变update)。set_null清指针切系统身份,cascade不切):清空引用指针视为引擎的完整性维护——与关系库 FK 的SET NULL、Salesforce lookup 清空同构;删整行仍要操作人自己的删除权。代价:被引用表的一列会以系统身份被改写,操作人对该表的授权不再是这笔写入的门槛(引擎内部写入已有__referentialFieldClear标记,属主守卫与级联 set_null 冲突:非特权删除 sys_user 时 owner_id 级联置空被 #3004 守卫拦截(级联中途失败) #3023)。四棱:
推荐:B(回退项:A——零改动,但应把「删除需连带引用表写权」写进权限报错 guidance/文档,让隐藏耦合至少可见)。
置信缺口:系统身份
set_null写入的审计归属(审计日志记操作人还是系统)与共享/RLS 规则外溢面未测;仓外部署是否依赖「cascade 需逐表授权」的现状未测;本分析依据本卡对裁决的引文,未逐字重读裁决评论原文。裁后执行段(维护者只裁方向):裁 B/C ⇒ 认领评论
Clause-②: yes、契约复审档位派发,落点沿上一裁实现同面,翻转 PR #12596 的边界断言并补 cascade/set_null 各自的方向用例;裁 A ⇒ 关本卡,另立小卡把「删除需引用表写权」补进权限报错 guidance(S 级,domain:engine)。Generated by Claude Code
os-zhuang commentedon Aug 27, 2026
Maintainer ruling recorded — Option B:
set_nullcleanup writes switch to system identity;cascadekeeps the operator's own authorityProvenance: maintainer, 2026-08-27, PM chat decision-inbox batch 2 (adjudication session
session_01DKWDdUJ2XNRESVVWUvcpnh), verbatim: 「同意」 — accepting the triage recommendation B (2026-08-26T16:19Z facet block); fallback A not needed; ⛔ C (cascade also system-identity) stays excluded — a data-destruction privilege expansion that would need its own justification.Ruled: clearing a referencing pointer (
set_null's UPDATE) is engine-owned referential-integrity maintenance and runs under system identity — completing the #12166 principle on the write half, consistent with the RDBMS FKSET NULLbaseline and the__referentialFieldClearinternal-write marker (#3023). Deleting whole referencing rows (cascade) still requires the operator's own delete authority on those rows.Dispatch constraints:
set_nullwrite; it must remain attributable to the initiating operator's delete, not vanish into "system"; (b) RLS/sharing spillover of the elevated write. Either measuring badly ⇒ stop and report, never push through.set_nullsucceeds without referencing-table write authority;cascadestill refuses without delete authority on the referenced rows.State:
needs-user-decision→pm:queue.Generated by Claude Code
os-zhuang commentedon Aug 27, 2026
Claim: PM seat for the
domain:enginelane, sessionsession_01LZbWd2jNV1FErXTPSS4Dry. Dispatching to anos-devseat on branchclaude/issue-12597-set-null-cleanup-system-identity.Clause-②: yes — per the maintainer ruling (comment 5434623379), this expands the delete operation's accepted set, so the dev runs at contract-review tier: the accept/reject boundary change is stated explicitly in the PR body, the contract docs are hand-read for falsified sentences (the docs-drift bot is precision-first and cannot see a page that states this rule by its inputs), and the reviewer-of-record check is the boundary, not the diff size.
Scope, straight from the ruling — nothing added, nothing inferred:
set_null's FK-clearingUPDATEruns under system identity (the referential-integrity write half, completing the 删除记录时「引用清理」用操作人身份查询引用表,读权不足即整体 403(应以系统身份执行) #12166 principle).cascade's child-rowDELETE. Option C stays excluded — a data-destruction privilege expansion needing its own justification. A diff that touches the cascade arm's identity is out of scope regardless of how it is argued.Gate before any code. The ruling's constraint 2 makes two measurements the dev's first actions, ahead of the implementation: (a) what the audit trail records for the system-identity
set_nullwrite — it must stay attributable to the initiating operator's delete, not vanish into "system"; (b) the RLS/sharing spillover of the elevated write. Either one measuring badly is a stop-and-report, not a thing to push through. The seat reports back to this card either way.Generated by Claude Code
claude commentedon Aug 27, 2026
Gate measurements (ruling constraint 2) — result: STOP AND REPORT on gate (b)
os-devseat, sessionsession_01LZbWd2jNV1FErXTPSS4Dry, branchclaude/issue-12597-set-null-cleanup-system-identity(pushed, no commits — no implementation was written).Per the ruling's constraint 2 these two measurements were the first actions, ahead of any code. Gate (a) reads clean. Gate (b) reads badly in the sense the dispatch defines, and a narrower alternative measured viable and strictly stronger on the security axis, so no implementation followed.
All readings below are executed observations, not source reading. Base commit
bc5156fcb.Gate (a) — audit attribution: GOOD
Question. When the
set_nullUPDATE runs under system identity, does the audit trail still attribute it to the operator who asked for the delete?Method. Drove the real
writeAuditfrompackages/plugins/plugin-audit/src/audit-writers.ts(viainstallAuditWriters) on anafterUpdatewhosesessionis exactly whatObjectQL.buildSessionemits for asudo()-shaped context — verified atengine.ts:3353, which copiesuserIdand addsisSystem: true. Three arms: the elevated shape, a bareisSystemcontrol, and a pre-change operator control.Readings.
Verdict: GOOD. The elevated write's ledger row is byte-identical in its attribution half to the pre-change operator write. Three findings behind that:
isSystemsuppression. The asymmetry therecordReferenceCheckElevationdocblock warns about is real and runs the safe way:read-audit.ts:556returns early onsession.isSystem === true, andaudit-writers.tshas no counterpart. A WRITE is indeed a different animal, and the difference favours us.sess.userId, which thesudo()-shaped spread preserves. The bare-isSystemcontrol shows the spread is load-bearing, not decorative — a bare{ isSystem: true }would have produced exactly the "vanish into system" outcome the ruling forbids.updated_byis stamped fromsession.userIdatplugin.ts:1028with noisSystemgate, so the cleared row itself still names the operator.This measurement is not what stops the card.
Gate (b) — RLS / sharing spillover: BAD READING
Question. What else does
isSystem: trueturn off on this write, beyond the object-level CRUD check the ruling intends to bypass?Mechanism.
security-plugin.ts:1560isif (opCtx.context?.isSystem) { return next(); }— the entire security middleware is short-circuited. Elevation is not granular; there is no narrower spelling of "system identity" in this codebase.Method. Real
SecurityPluginmiddleware over a realObjectQLengine (the same harness shape as the existing suite indelete-reference-cleanup-system-identity.test.ts), one fixture per guard, each run twice: as the plain operator and with thesudo()-shaped elevation.Readings.
PERMISSION_DENIEDop=updatePERMISSION_DENIEDop=update, FK unchangedusingrow scope on update (owner_id == current_user.id)PERMISSION_DENIEDop=update, FK unchangedcheck(product != null)PERMISSION_DENIEDop=update, FK unchangedvalidations, typescript)VALIDATION_FAILEDVALIDATION_FAILED, FK unchangedAlso bypassed on the engine side, by inspection with line cites: the
readonlystrip (engine.ts:10434single-row,:10596bulk) and lookup-target existence validation (engine.ts:5525). The ownership-anchor guard (#3004) is already exempt for this write through the #3023 marker and is unchanged either way.The good news first: declared validation rules are NOT bypassed —
evaluateValidationRulesatengine.ts:10453sits outside theisSystemguard that closes at:10446. That was the dispatch's first named worst case and it measured clean.But the dispatch's second named worst case measured dirty. The bad reading is defined as the elevation bypassing "a validation rule or sharing restriction that exists for reasons unrelated to referential integrity". An RLS
usingclause scoping which rows an operator may update, and an RLS post-imagecheckconstraining what shape a row may end up in, are both exactly that, and both are bypassed. The RLScheckcase is the one I find hardest to fit inside the ruling:checkis not a reach question at all, it is a data-shape constraint — semantically a validation rule wearing a different hat. The platform now honours "this FK must never be empty" when it is declared as avalidationsentry and silently ignores it when the same intent is declared as an RLScheckclause.The narrowing, measured rather than speculated
The dispatch says to propose a narrowing rather than ship the broad version, so I measured whether one is actually available instead of guessing. Probe: exempt contexts carrying
__referentialFieldClearfrom the CRUD check only (security-plugin.ts:1953), leaving every other guard in place. Reversible, restored and verified byte-clean against the HEAD blob.isSystem(as ruled)usingrow scopecheckThe narrowing fixes the reported defect and keeps all three guards. That is the whole reason this is a stop rather than a note in a PR body: the broad version is not the only way to satisfy the ruling's stated goal, and the alternative costs the reporting deployment nothing.
Does the #3023 marker already exempt the guard that was 403-ing?
No — measured. A context carrying
__referentialFieldClearbut notisSystemis still refused withPERMISSION_DENIEDop=update. The marker exempts only the ownership-anchor transfer guard at step 3.5 (security-plugin.ts:2474); the refusal in this card comes from the CRUD check at step 2 (:1953), which the marker does not reach. The card's diagnosis is complete and correct on this point — no correction needed there.The repo's own posture, which I did not expect to find
content/docs/permissions/system-context.mdxis the platform's stated authority on this flag, and it says:One demonstrably exists here, and the measurement above is what shows it.
Where I think this lands, on the four axes
SET NULLand Salesforce's lookup clearing both bypass row security, and the narrowing is stricter than both. The counterweight is that this platform, unlike either baseline, lets a deployment declare a post-imagecheckon the referencing object — a surface the FK analogy simply has no equivalent for, so the analogy stops carrying weight exactly where the spillover is.checkis honoured or ignored depending on which code path reaches the row, and nothing in the authoring surface says so. That is a rule that is declared but not enforced, which is the failure shape this project treats as most expensive.plugin-securitywhere the broad version adds no new mechanism at all — a real cost, and the one honest argument for the broad version on this axis.Recommendation: the marker-scoped narrowing, with the RLS
checkbypass being the single reading I would most want the maintainer to look at before deciding. Either way this is a permission-boundary semantics change and the choice is above my floor, so I have written no code and opened no PR.Out-of-scope finding filed unassigned while measuring: #12802.
Generated by Claude Code
Generated by Claude Code
claude commentedon Aug 27, 2026
os-dev-report
{ "issue": 12597, "status": "needs_decision", "branch": "claude/issue-12597-set-null-cleanup-system-identity", "pr": null, "premise_still_valid": true, "summary": "No implementation. Ruling constraint 2's two gate measurements were run first, as required; gate (a) reads clean and gate (b) reads badly in the sense the dispatch defines, so this is the stop-and-report the dispatch calls for rather than a PR. Gate (b): `isSystem` short-circuits the ENTIRE security middleware at security-plugin.ts:1560, and measurement shows the elevated set_null write would additionally bypass field-level security on the FK column, the RLS `using` row scope on update, and the RLS post-image `check` -- the last of which is a data-shape constraint unrelated to referential integrity, semantically a validation rule in different clothing. Declared validation rules themselves are NOT bypassed (engine.ts:10453 sits outside the isSystem guard), which was the dispatch's first named worst case and it measured clean. I then measured the narrowing the dispatch names instead of speculating about it: a marker-scoped exemption of the CRUD check alone fixes the card's exact fixture (role with full delete on A and NOTHING on B, non-empty referencing table) while keeping FLS, RLS `using` and RLS `check` all enforced. Both options fix the reported deployment; only one of them spends three extra guards to do it. That choice is a permission-boundary semantics change above my floor, so no code was written. The branch is pushed but carries NO commits.", "gate_a_audit_attribution": { "verdict": "GOOD -- attribution survives the elevation intact", "method": "Drove the real writeAudit (installAuditWriters, plugin-audit/src/audit-writers.ts) on an afterUpdate whose session is exactly what ObjectQL.buildSession (engine.ts:3353) emits for a sudo()-shaped context. Three arms: elevated, bare-isSystem control, pre-change operator control.", "readings": { "elevated": "sys_audit_log {action:update, user_id:'u_lead', actor:'u_lead', old_value:{product:'prod-1'}, new_value:{product:null}}; sys_activity {actor_id:'u_lead'}", "bare_isSystem_control": "user_id: null, actor: null -- proves the sudo() spread is load-bearing, not decorative", "pre_change_operator_control": "user_id: 'u_lead', actor: 'u_lead' -- byte-identical attribution half to the elevated arm" }, "why_it_holds": "The write path has NO isSystem suppression: read-audit.ts:556 returns early on session.isSystem, audit-writers.ts has no counterpart. The docblock's warning that a WRITE is a different animal is correct and the difference runs the safe way. Second channel intact too: updated_by is stamped from session.userId at plugin.ts:1028 with no isSystem gate." }, "gate_b_rls_sharing_spillover": { "verdict": "BAD READING -- the elevation bypasses guards unrelated to referential integrity", "mechanism": "security-plugin.ts:1560 -- `if (opCtx.context?.isSystem) { return next(); }`. Elevation is total; there is no narrower spelling of system identity in this codebase.", "method": "Real SecurityPlugin middleware over a real ObjectQL engine, one fixture per guard, each run as the plain operator and with the sudo()-shaped elevation.", "readings": [ "object-level CRUD (the card's 403): operator=PERMISSION_DENIED op=update | elevated=SUCCESS, FK cleared -- INTENDED", "field-level security on the FK column: operator=PERMISSION_DENIED op=update, FK unchanged | elevated=SUCCESS, FK cleared -- BYPASSED", "RLS `using` row scope on update (owner_id == current_user.id): operator=PERMISSION_DENIED op=update, FK unchanged | elevated=SUCCESS, FK cleared -- BYPASSED", "RLS post-image `check` (product != null): operator=PERMISSION_DENIED op=update, FK unchanged | elevated=SUCCESS, FK cleared -- BYPASSED", "declared validation rule (validations, type script): operator=VALIDATION_FAILED | elevated=VALIDATION_FAILED, FK unchanged -- NOT bypassed" ], "why_bad": "The dispatch defines a bad reading as the elevation bypassing 'a validation rule or sharing restriction that exists for reasons unrelated to referential integrity'. The validation-rule half measured clean; the sharing half did not. The RLS `check` case is the sharpest: `check` is not a reach question at all, so the platform would honour 'this FK must never be empty' when declared as a validations entry and silently ignore it when the same intent is declared as an RLS check clause." }, "guards_bypassed_enumerated": [ "plugin-security step 2, object-level CRUD check (security-plugin.ts:1953) -- MEASURED bypassed; this is the one the ruling intends", "plugin-security step 2.5, field-level security write enforcement (:2293) -- MEASURED bypassed", "plugin-security step 2.7, row-level WRITE authorization / RLS `using` pre-image (:2058) -- MEASURED bypassed", "plugin-security step 3, RLS filter injection (:2913) -- bypassed (same short-circuit)", "plugin-security step 3.6, RLS WRITE `check` post-image (:2581) -- MEASURED bypassed", "plugin-security step 3.5, ownership-anchor guard (:2432) -- already exempt via the #3023 marker; unchanged either way", "plugin-security step 3.7, Layer-0 tenant post-image check (:2667) -- bypassed (same short-circuit)", "plugin-security step 1.5, requiredPermissions AND-gate (:1891); step 2.8, controlled-by-parent write (:2259); step 2.9, field-predicate guard (:2861) -- bypassed (same short-circuit)", "objectql readonly-field strip, single-row (engine.ts:10434) and bulk (:10596) -- bypassed by inspection", "objectql assertReferencesResolve lookup-target existence validation (engine.ts:5525) -- bypassed by inspection", "NOT bypassed: declared validation rules -- evaluateValidationRules (engine.ts:10453) sits outside the isSystem guard closing at :10446. MEASURED." ], "marker_exemption_answer": "NO -- the #3023 __referentialFieldClear marker does NOT cover the guard that was 403-ing. MEASURED: a context carrying the marker but not isSystem is still refused with PERMISSION_DENIED op=update. The marker exempts only the ownership-anchor transfer guard at step 3.5 (security-plugin.ts:2474); the card's refusal comes from the CRUD check at step 2 (:1953), which the marker does not reach. The card's diagnosis is therefore COMPLETE and CORRECT on this point -- no correction needed.", "narrowing_probe": { "what": "Exempted contexts carrying __referentialFieldClear from the CRUD check ONLY (security-plugin.ts:1953), leaving every other guard in place. Reversible probe, not an implementation.", "mutation_proof_on_disk": "anchor count before=4, injected form count=1, original form count 4->3; file hash e129cc46 -> 6bfef4f4", "restore_proof": "post-restore hash-object == HEAD blob e129cc4664515d572124577dd7a50501bd65154f, and `git diff HEAD --name-only` on the file returned EMPTY", "marker_absent_on_pre_change_ref": "git show origin/main:...security-plugin.ts | grep -c INJECTED_FORM = 0, with a positive control on the same ref returning 4 -- so the marker is specific to the probe, not merely present in the right tree", "result": "The card's exact fixture SUCCEEDS under the narrowing, while FLS, RLS `using` and RLS `check` all still REFUSE. Measured, all three arms." }, "cascade_arm": "UNCHANGED and untouched -- no diff line to quote because there is no diff. The cascade arm (`await this.delete(childName, { where: { id: depId }, context } as any)` in cascadeDeleteRelations) was never edited; the worktree was verified clean and removed with zero commits on the branch. Option C remains excluded.", "bump_argument": "N/A -- no changeset, because there is no user-visible change and no implementation. If the maintainer rules for either option, the write-identity change on an existing operation argues for MINOR rather than patch: it changes who a write executes as, which is a behavioural contract change deployments can observe in their audit ledger and permission matrices, not a defect repair.", "tests": "No product tests -- nothing was implemented. Measurement runs only, all through scripts/pm/os-verify-lock.sh, all foreground. (1) `pnpm --filter @objectstack/plugin-audit exec vitest run --maxWorkers=2 src/zz-gate-measure-a.test.ts` -- first attempt was NOT MEASURED, not red: prerequisite refusal `Cannot find package '@objectstack/objectql/core'` (unbuilt dist). (2) `pnpm --workspace-concurrency=2 --filter '@objectstack/plugin-audit^...' --filter '@objectstack/plugin-security^...' build` -- lock VERDICT command-exit 0, held 196s. (3) gate (a) re-run -- lock VERDICT command-exit 0, `Tests 3 passed (3)`, readings captured to file. (4) `pnpm --filter @objectstack/plugin-security exec vitest run --maxWorkers=2 src/zz-gate-measure-b.test.ts` -- lock VERDICT command-exit 0, `Tests 7 passed (7)`. (5) narrowing probe (mutate/measure/restore with trap on EXIT INT TERM and absolute paths) -- lock VERDICT command-exit 0, `Tests 1 failed | 7 passed (8)`, the one failure being the probe's own expected flip (B2 asserted PERMISSION_DENIED and returned SUCCESS under the narrowing -- that flip IS the reading). Verdicts read from the lock's own VERDICT lines and vitest's own summary lines, never a bare $?. Ablation discipline applied to the probe: mutation proven on disk by anchored greps in BOTH directions plus a hash change, restore proven by hash-object equality against the HEAD blob plus an empty git diff. Measurement harnesses preserved at /tmp/claude-0/-home-user-objectstack/5b7dbdcd-321c-5e24-9c7d-c179ce356a6a/scratchpad/issue-12597/harness-gate-a.test.ts and harness-gate-b.test.ts; both were deleted from the worktree, which was verified clean before removal.", "mcp_calls": "2 -- one targeted search_issues for dedupe (REST /search/issues is blocked for this seat: 'sessions are bound to their configured repositories'), plus one positive-control search to make the zero-hit a reading. All issue/comment reads and both writes went over repo-scoped REST.", "open_questions": [ { "question": "The ruling says the set_null cleanup write 'runs under system identity'. Measurement shows that in this codebase system identity is total: it also bypasses field-level security, the RLS `using` row scope and the RLS post-image `check` on the referencing object. A marker-scoped exemption of the CRUD check alone was measured to fix the reported deployment while keeping all three. Which does the maintainer want?", "options": [ "A. Blanket `isSystem` as literally ruled. Matches the cited baselines exactly (PostgreSQL FK SET NULL and Salesforce lookup clearing both bypass row security). Adds no new mechanism. Cost: three guards bypassed that exist for reasons unrelated to referential integrity, most sharply the RLS post-image `check`, which is a data-shape constraint the FK analogy has no equivalent for -- so a deployment's declared `check` becomes declared-but-not-enforced depending on which code path reaches the row.", "B. Marker-scoped exemption of the object-level CRUD check only, keeping FLS / RLS `using` / RLS `check` enforced. MEASURED to fix the card's exact fixture (full delete on A, nothing at all on B, non-empty referencing table). Cost: stricter than both cited baselines; adds an exemption path to plugin-security rather than reusing the existing identity flag; a deployment that FLS-locks the FK column or RLS-scopes the referencing rows still gets a refused delete -- truthful and declared, but still refused.", "C. Blanket `isSystem` now, with the RLS `check` bypass filed as a separate card. Ships the ruling as written and keeps the sharpest edge visible. Cost: the gap is live in the meantime, and 'we will fix it later' on a permission boundary is the shape that does not get fixed later." ], "recommendation": "B. Both A and B fix the reported deployment -- that is measured, not argued -- so real business need does not discriminate between them and the tie breaks on what each spends. A spends three guards nobody ruled on; B spends none. On making AI-authored metadata hard to get wrong, B wins clearly: under A a declared RLS `check` is honoured or ignored depending on the code path, with nothing in the authoring surface saying so, which is exactly the declared-but-not-enforced shape this project treats as most expensive. The honest argument for A is that the ruling's own baselines bypass sharing and B is stricter than both -- but neither baseline lets a deployment declare a post-image `check` on the referencing object, so the analogy stops carrying weight precisely where the spillover is. The repo's own authority agrees with B in general terms: content/docs/permissions/system-context.mdx states 'Elevation is total, and it is not granular' and 'prefer a scoped user context whenever one exists' -- and one demonstrably exists here." } ], "out_of_scope_findings": [ "filed as #12802: the isSystem census page (content/docs/permissions/system-context.mdx) advertises itself as the authority 'built by census, not recall' but its file:line anchors have drifted -- all three rows I checked resolve to unrelated lines (row 18 cites engine.ts:3314, actual :5525; row 13 cites :6860, actual :10434; row 14 cites :7004, actual :10596). Filed unassigned with pm:queue as this dispatch instructs; flagging that my standing rules would have labelled a docs-drift observation `finding` rather than pm:queue, so retag if you prefer that convention." ] }Generated by Claude Code
os-zhuang commentedon Aug 27, 2026
STOP-AND-REPORT — ruling constraint 2 fired. No implementation, returned for a second ruling.
The dispatched seat ran the two gate measurements before any code, as constraint 2 required. Gate (a) reads clean; gate (b) reads badly in the sense the dispatch defined. Per that instruction it stopped rather than pushing through. Branch
claude/issue-12597-set-null-cleanup-system-identitywas pushed at base and carries zero commits; no PR exists; thecascadearm was never touched and ⛔ Option C remains excluded.State:
pm:dispatched→needs-user-decision, unassigned.I re-verified every load-bearing claim below against
origin/mainmyself rather than relaying the seat's report — a ruling will rest on these.Gate (a) — audit attribution: GOOD, the elevation is safe here
Driven against the real
writeAudit(plugin-audit/src/audit-writers.ts) on a session shaped exactly asObjectQL.buildSession(engine.ts:3353) emits for asudo()-shaped context. Three arms:user_id/actorset_nullwriteu_lead/u_lead—old {product:'prod-1'}→new {product:null}isSystemcontrolnull/nullu_lead/u_lead— byte-identical to the elevated armThe bare-
isSystemcontrol is the part that makes this a reading rather than a hope: it proves thesudo()spread carryinguserIdis load-bearing, not decorative. The write path has noisSystemsuppression —read-audit.ts:556returns early onsession.isSystemandaudit-writers.tshas no counterpart — so the asymmetry thereferenceCheckContextdocblock warns about (a WRITE is a different animal from the elevated READ) runs the safe way here.updated_byis stamped fromsession.userIdatplugin.ts:1028with noisSystemgate, so attribution has two intact channels.Gate (b) — RLS / sharing spillover: BAD
Mechanism, verified:
plugin-security/src/security-plugin.ts:1559-1561—That sits ahead of every later step, so the elevation is total. There is no narrower spelling of system identity in this codebase. The repo's own page says so in as many words —
content/docs/permissions/system-context.mdx:19: "Elevation is total, and it is not granular."Measured, one fixture per guard, each run as the plain operator and again with the
sudo()-shaped elevation:PERMISSION_DENIEDop=updatePERMISSION_DENIED, FK unchangedusingrow scope on updatePERMISSION_DENIED, FK unchangedcheckPERMISSION_DENIED, FK unchangedVALIDATION_FAILEDVALIDATION_FAILED, FK unchanged — ✅ not bypassedThe dispatch's first-named worst case — "it would let the FK-clear write through a validation rule" — measured clean. I confirmed why:
evaluateValidationRulesruns in the engine's own update path (engine.ts:10453), outside plugin-security's short-circuit entirely.The sharpest edge is the RLS post-image
check.checkis not a reach question at all; it is a data-shape constraint. So the platform would honour "this FK must never be empty" when a deployment declares it as avalidationsentry, and silently ignore the same intent declared as an RLScheckclause — enforcement depending on which spelling the author chose, with nothing in the authoring surface saying so.The #3023 marker question, answered: NO
The
__referentialFieldClearmarker does not cover the guard that was 403-ing. Measured: a context carrying the marker but notisSystemis still refused withPERMISSION_DENIEDop=update. I verified the structure — the marker gates a different block (:2474, the ownership-anchor transfer guard), while the card's refusal comes from the CRUD check at:1953, which the marker never reaches.⇒ the card's diagnosis is complete and correct on this point. No correction needed.
The narrowing — measured, not speculated
Rather than argue about a hypothetical, the seat probed it. Exempting
__referentialFieldClearcontexts from the CRUD check alone, leaving every other guard standing:Reversible probe under full ablation discipline: mutation proven on disk by anchored greps in both directions plus a file-hash change (
e129cc46→6bfef4f4); restore proven bygit hash-objectequality against the HEAD blob plus an emptygit diff; and the probe marker verified absent on the pre-change ref (grep -c= 0 with a positive control returning 4 on the same ref) — so the marker is specific to the probe, not merely present in the right tree.⇒ both A and B fix the reported deployment. That is measured, not argued.
裁定格式,四棱分析(中文):
一句话问题:上一裁把
set_null的清理写入判为「以系统身份执行」,但实测表明本仓的系统身份是全量的 —— 它同时绕过被引用表的字段级安全、RLS 行域using、以及 RLS 后像check。而一个只豁免对象级 CRUD 检查的收窄方案,实测同样修好了原报告的部署。裁决当时未必知道「系统身份」在本仓意味着这么多。选项 × 真实代价:
isSystem):与裁决援引的基线完全一致(PostgreSQL FKSET NULL与 Salesforce lookup 清空都绕过行安全),不新增任何机制。代价:三个与引用完整性无关的守卫被一并绕过,其中 RLS 后像check最刺眼 —— 它是数据形状约束,FK 类比里根本没有对应物,于是部署方声明的check变成「取决于走哪条代码路径才生效」。using/ RLScheck三者仍然拒绝。代价:比裁决援引的两个基线都更严;在 plugin-security 里新增一条豁免路径,而不是复用既有的身份标志;若部署方对 FK 列上了字段级安全或对引用行设了 RLS,删除仍会被拒 —— 拒得诚实、拒得有声明依据,但仍是拒。check外溢另开一卡):照裁决原样落地,把最锋利的那条边留在明面上。代价:缺口在此期间是活的,而「权限边界的问题以后再补」正是那种补不上的形状。四棱:
check,而两个基线都不允许 —— 类比恰好在外溢发生处失效。check到底生效与否取决于代码路径,而授权界面上看不出来 —— 这正是本项目视为最贵的 declared-but-enforced 不一致形状,和当初催生 ADR-0049 的是同一个族。派发席建议:B。⚠️ 这是受派 dev 席的建议,不是本 PM 的裁定 —— 权限边界语义在我的判定层之上,我只负责把测量如实上呈。仓内既有权威与 B 同向:
content/docs/permissions/system-context.mdx:19「Elevation is total, and it is not granular」,并主张「prefer a scoped user context whenever one exists」—— 而此处确实存在一个可用的收窄。支持 A 的诚实论点:裁决援引的基线确实绕过 sharing,B 比两者都严;若产品语义就是要与 Salesforce 对齐,A 才是忠于裁决的那一档。
置信缺口:仓外部署是否已依赖「引用清理受 FLS/RLS 约束」的现状,未测。B 新增的豁免路径本身是否会成为下一个被滥用的标记,未评估。
⛔ Not dispatchable until ruled. ⛔ Option C (cascade under system identity) stays excluded either way — untouched by this report.
Generated by Claude Code
os-zhuang commentedon Aug 28, 2026
Claim: PM seat for the
domain:enginelane, sessionsession_01LZbWd2jNV1FErXTPSS4Dry. Dispatching to anos-devseat on branchclaude/issue-12597-referential-field-clear-crud-exemption.claude/issue-12597-set-null-cleanup-system-identitystill exists atbc5156fcb, is an ancestor ofmainwith zero unique commits, and its name now describes the option the maintainer declined. It is history, not a base — do not build on it.⚖️ The ruled scope — round 2, option B
Authoritative record: comment 5448714931. Maintainer, 2026-08-28, live director session (summon #2), ruling verbatim: 「6272 A1 其他同意」(「其他同意」 adopts B on this card). Round 1's option-B framing (comment 5434623379) is superseded — it said blanket
isSystem, and the constraint-2 measurement is what changed the ruling.__referentialFieldClearmarker from the object-level CRUD check ONLY (plugin-security/src/security-plugin.ts:1953). This is exactly the shape the round-1 seat's reversible probe already measured working.isSystem.security-plugin.ts:1559-1561short-circuits the entire middleware; elevation there is total. The repo's own page says so —content/docs/permissions/system-context.mdx:19: "Elevation is total, and it is not granular."usingrow scope on update, RLS post-imagecheck.isSystemcontrol proved thesudo()spread load-bearing. Keep that shape.engine.ts:10453, outside the short-circuit). ⛔ Do not move them.cascadestays operator-identity and excluded — untouched for the third time.await this.delete(childName, { where: { id: depId }, context } as any)incascadeDeleteRelationsis out of scope regardless of how it is argued. Option C remains excluded.My earlier claim (comment 5444960966) dispatched at contract-review tier. That no longer applies, and the card face above is stale on this point — it still reads "
needs:contract-reviewattached, dispatch atCONTRACT_REVIEW_TIER".The maintainer removed the label (comment 5449316668), verbatim: 「我的意思是 12597 还挂着 label:"needs:contract-review"」. The recorded reasoning: clause ② covers the published contract's accept/reject face (path limb
packages/spec/src/**), which this diff will not touch; this card changes runtime permission-gate behaviour inplugin-security— the 安全/权限边界 manual floor, a different control, already exercised by two maintainer rulings.⇒ ⛔ Do not re-attach
needs:contract-review. Current labels arepm:queue+domain:engine, and that is correct. The dev still declaresClause-②: yes|noin its claim, self-assessed against the actual diff — deliberately, not inferred from "no spec files, therefore no."What the seat does not need to re-derive
The round-1 measurements are on this card and were re-verified by the PM against
origin/main; they are inputs, not homework:__referentialFieldClearmarker does not today reach the CRUD check (a context carrying the marker but notisSystemis stillPERMISSION_DENIEDop=update); it gates only the ownership-anchor guard at:2474;Generated by Claude Code
feat(security): marker-scoped CRUD exemption for the referential FK c…
test(security): ablation prediction for the #12597 CRUD exemption
docs(security): point the exemption comment at the suite that exists
claude commentedon Aug 28, 2026
os-dev-report
{ "issue": 12597, "status": "done", "branch": "claude/issue-12597-referential-field-clear-crud-exemption", "pr": "https://github.com/objectstack-ai/objectstack/pull/12941", "premise_still_valid": true, "summary": "Implemented the round-2 ruling (option B, comment 5448714931 + correction 5449316668): a write carrying the existing server-derived `__referentialFieldClear` marker is exempt from the object-level CRUD check, and from that check alone. The card's premise was re-verified rather than assumed -- my own ablation shows the exact fixture (full delete on A, nothing at all on B, non-empty referencing table) is still PERMISSION_DENIED op=update without the change and succeeds with it. Not `isSystem`: field-level security on the FK column, the RLS `using` row scope and the RLS post-image `check` all stay enforced and are each pinned. The cascade arm is untouched. The write is not elevated at all, so audit attribution keeps the measured operator-identity shape by construction.", "changed": [ "packages/plugins/plugin-security/src/security-plugin.ts:1953-2005 -- the object-level CRUD check (step 2, now at :2005) is guarded by `if (permissionSets.length > 0 && !referentialFieldClearWrite)`, where `referentialFieldClearWrite = opCtx.operation === 'update' && opCtx.context?.__referentialFieldClear === true` (:2001) is computed immediately above it with the ruling, the rejected `isSystem` alternative and both narrowings recorded in the comment. Nothing else in the middleware changed: steps 1.5 (requiredPermissions), 2.5 FLS (now :2344), 2.6 write-scope stamp, 2.7 RLS pre-image (now :2109), 2.8, 3.5, 3.6 RLS post-image check (now :2632) and 3.7 are byte-identical to origin/main -- the only shift is this comment's 50 lines.", "packages/plugins/plugin-security/src/delete-reference-cleanup-system-identity.test.ts -- the constraint-1 boundary pin INVERTED in place with a comment naming this card and the round-2 ruling; three new permission-set fixtures (FLS_LOCKED_LEAD, RLS_USING_LEAD, RLS_CHECK_LEAD); a new `#12597` describe with seven arms. 14 tests in the file, up from 7.", ".changeset/referential-field-clear-crud-exemption.md -- `@objectstack/plugin-security: minor`." ], "narrowings_inside_the_ruled_scope": [ "`update` only: the engine stamps the marker on the `set_null` UPDATE alone, so any other operation carrying it is context INHERITANCE (a hook doing its own writes), never the ruled write. Strictly narrower than the ruling; it cannot affect the ruled positive case, which is an update.", "the ADR-0090 D10 delegator half is skipped with the caller half, because it is the SAME object-level grant question (`checkObjectPermission` on `delegatorSets`, same object, same operation) asked of the second principal. 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. Flagged in open_questions as the one judgment call inside the ruled scope." ], "pins": { "positive -- the card's exact fixture (role with full delete on A, NOTHING on B, non-empty referencing table)": { "site": "security-plugin.ts:1953 (exempted)", "reading": "delete SUCCEEDS; parent row gone, referencing row SURVIVES with product cleared to null; the context reaching the engine for the cleanup write is userId 'u_lead', isSystem not true, __referentialFieldClear true" }, "guard 1 -- field-level security on the FK column": { "site": "security-plugin.ts:2293", "reading": "still PERMISSION_DENIED / 403, details.object='os_ehr_andon_record', details.forbiddenFields contains 'product', FK unchanged, parent row still present" }, "guard 2 -- RLS `using` row scope on update": { "site": "security-plugin.ts:2058", "reading": "still PERMISSION_DENIED / 403, details.recordId = the referencing row, developerMessage says 'row-level security', FK unchanged. CONTROL arm: same policy, row in scope => the write is admitted and the FK clears, so the refusal is the row scope and not the exemption failing to fire" }, "guard 3 -- RLS post-image `check` (`product != null`)": { "site": "security-plugin.ts:2581", "reading": "still PERMISSION_DENIED / 403, developerMessage says 'row-level CHECK', FK unchanged" }, "extra -- the converse": "a caller without delete rights on the TARGET is still refused (PERMISSION_DENIED, details.object='os_ehr_product', operation='delete'): the exemption widens which deletes succeed, never who may ask", "extra -- an ordinary unmarked update": "the same caller updating the referencing object directly is still PERMISSION_DENIED op=update -- the marker is the whole difference, which is what proves the exemption is scoped to it", "envelope discipline": "every refusal is asserted on its own gate's ADR-0112 envelope (forbiddenFields / recordId + 'row-level security' / 'row-level CHECK'), never on the bare fact of a refusal -- the ablation shows all three go red when the CRUD check answers instead" }, "inverted_pin": "`the elevation is the CHECK only -- a non-empty referencing table still needs the caller's own write authority (constraint 1)` in delete-reference-cleanup-system-identity.test.ts was INVERTED in place, not deleted and not silently re-expected: it now asserts the ruled success and carries a comment naming #12597 and the 2026-08-28 round-2 ruling as the reason, and stating that the assertion it replaces is what measured the residue this card was opened for.", "audit_attribution": "Preserved by construction: this change does not touch identity at all. The cleanup UPDATE still runs under the operator's own context (plus the marker), so both channels the round-1 gate-(a) measurement found intact are untouched -- writeAudit's user_id/actor keyed on session.userId, and the updated_by stamp at plugin.ts:1028. The positive pin asserts the context that actually reaches the engine: userId 'u_lead', isSystem not true. The bare-isSystem shape that produced user_id: null / actor: null is never constructed on this path.", "cascade_arm": "UNTOUCHED, verified mechanically: `git diff origin/main -- packages/objectql/src/engine.ts` is EMPTY, so `await this.delete(childName, { where: { id: depId }, context } as any)` in cascadeDeleteRelations is byte-identical to origin/main. The child-row DELETE carries no marker (the engine stamps referentialCtx on the set_null arm only), so the exemption cannot reach it even in principle, and it is additionally fenced by the `operation === 'update'` narrowing. Option C remains excluded.", "clause_2": { "declared": "no", "reasoning": "Self-assessed against the actual diff, not inferred from 'no spec files'. The diff is packages/plugins/plugin-security/src/** plus one changeset. Asked on the merits: does the accept/reject set of a PUBLISHED contract change, or does a public surface widen? No -- no packages/spec schema, no export added or removed, no authorable metadata key, no REST parameter set, no new public API. What changes is a RUNTIME permission-gate verdict, which is 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 (comment 5449316668); `needs:contract-review` was NOT re-attached." }, "bump": { "level": "minor", "package": "@objectstack/plugin-security", "argument": "This changes WHICH DELETES SUCCEED: the card's exact fixture went from a 403 to a completed delete with the FK cleared. That is an observable behavioural contract change on the delete path -- a deployment can stop granting write access on referencing tables purely to make deletes work -- not a patch-grade defect repair. Matches the ruling's MINOR. No ADR-0087 marker is owed (that gate asks only of declared-BREAKING changesets); check-adr-0087-registration, check-empty-changeset and check-changeset-no-major all exit 0." }, "tests": "All heavy runs through scripts/pm/os-verify-lock.sh; verdicts read off the lock's own VERDICT line and vitest's own summary line, never a bare $?. Union re-run AFTER the final commit, at HEAD aae820b00 (working tree clean). (1) `pnpm --filter @objectstack/plugin-security test` -- 'Test Files 87 passed (87) / Tests 1585 passed (1585)', lock VERDICT command-exit 0. (2) target suite alone -- 'Tests 14 passed (14)'. (3) `pnpm --filter @objectstack/plugin-security typecheck` -- exit 0, BUT NOT MEASURED for the test file: this package's tsconfig.json excludes **/*.test.ts, verified with `tsc --noEmit --listFiles` (0 hits for the edited file). I measured that half separately with a test-inclusive tsc program: 11 errors total, 0 of them in the file I edited, and 11 is exactly the number the shrink-only ledger records for this package (check-type-check-coverage.mjs TEST_DEBT '@objectstack/plugin-security': errors 11) -- so the ratchet does not move. (4) `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). (5) `pnpm lint` (full repo, `eslint . --no-inline-config`) -- exit 0, no findings; the full-repo scan was run, so no narrowing is claimed. (6) gate families derived with `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack` (it reads its own change set from the merge base; I did not hand it a diff) -- 21 matched + the convention-triggered set; all exit 0: check:where-matcher (309 matchers, 0 silently-wrong, no files added to the baseline), check:engine-double-contract, check:objectql-double-limit, check:cross-package-test-inputs ('OK: 20 package(s) read outside themselves, all declared'), 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 ('OK ... no raw ASCII control bytes'), check:changeset-gate-self-tests, check:objectui-changeset, check:pm-half-states, check:i18n-stale-fill, check-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. NOT MEASURED (not red): check:i18n refused with 'PREREQUISITE NOT MET -- the workspace CLI is not built'; by inspection its population here is closed and does not include my paths (scripts/i18n-extract.config.ts runs --objects-only over six src/objects/* definitions plus the generated bundles), and CI runs the gate anyway. check:type-check-debt --re-measure also needs the full built closure -- covered instead by the test-inclusive tsc measurement above. BASE FAILURE HIT DURING THIS RUN, SINCE RESOLVED: `pnpm check:adr-anchors` was red on origin/main while this branch was built ('ADR-0006 D4 is cited by 1 file(s), but ADR-0006 declares no D4' -> packages/spec/src/data/object.zod.ts), tracked as #12913 -- not mine, packages/spec is not this seat's surface, and check-adr-anchors was NOT touched to make it go away. #12917 has since landed (750fff559, in origin/main's history) and the base is green on that gate; re-measured myself at origin/main 8beb3deaf: `grep -c 'ADR-0006 D4'` on that file returns 0, positive control `grep -c namespace` on the same ref and file returns 8, so the zero is a real reading. Nothing in this report or the PR body asserts a current base failure. ABLATION (reverse verification): prediction committed FIRST as empty commit 141ba0680 naming the six tests predicted red; mutation = restore the bare `if (permissionSets.length > 0)`; mutation PROVEN ON DISK by a blob hash change cfbb035f -> 9acdc533 plus anchored greps in both directions (guarded spelling 1 -> 0, bare spelling 3 -> 4); reading was 'Tests 6 failed | 8 passed (14)' -- exactly the six predicted, including all three guard pins, which go red DESPITE still refusing because each asserts its own gate's envelope; restore PROVEN by git hash-object equality against the HEAD blob (cfbb035f), an empty `git diff HEAD` and an empty `git status --porcelain`; `trap ... EXIT INT TERM` with absolute paths throughout. NO REBUILD LEG APPLIES: the suite imports the mutated file relatively ('./security-plugin.js'), so vitest reads the source in this checkout, never dist/ -- and the dependency closure was built once up front (`pnpm --filter '@objectstack/plugin-security^...' build`, lock VERDICT command-exit 0) so the aliased/dist-resolved dependencies are current. REVIEW ROUND, at head 0dd9a3496 (working tree clean): the target suite re-run -- 'Test Files 1 passed (1) / Tests 14 passed (14)', lock VERDICT command-exit 0 -- plus check:where-matcher, check:engine-double-contract, check:query-options-erasure, check:nul-bytes and check-comment-mask-adoption, all exit 0. The comment-only edit cannot move a behavioural verdict, so the full package suite and the full-repo lint were not re-run at that head; their readings stand at aae820b00, whose only difference is 3 lines of comment text.", "mcp_calls": "0 -- the whole run used repo-scoped REST through the container (probe: GET /repos/objectstack-ai/objectstack -> 200), so no MCP GitHub call was made: issue + comments read via REST, the duplicate sweep via the REST list endpoint + local grep, the draft PR opened via POST /repos/.../pulls, its body updated via PATCH, the drift-bot finding posted via POST /repos/.../issues/12941/comments, and this report via POST /repos/.../issues/12597/comments.", "out_of_scope_findings": [ "NOT filed -- already open as #12511 (`[finding] A type error in a ledgered package's tests is invisible to its own typecheck ... 19 packages in that state`). Observed here from the inside: packages/plugins/plugin-security/tsconfig.json excludes **/*.test.ts with no sibling tsconfig.test.json, so the package's advertised `typecheck` renders a verdict over src only (--listFiles: 0 hits for the edited test file) while its 11 test-surface errors sit in the shrink-only TEST_DEBT ledger, discoverable only by a full-closure build. Same class, same package population, same proposed remedy as #12511 -- filing a second card would be a duplicate. Dedupe sweep: 347 open issues listed via `GET /repos/.../issues?state=open&per_page=100` (5 pages) and grepped locally for typecheck/type-check/tsconfig/exclude; #12511 and #12939 are the neighbours, and #12939 is a different shape (a package with NO typecheck script at all).", "NOT filed, offered for PM grading rather than acted on: no page in content/docs/permissions/ enumerates the layer-3 (object CRUD) exemptions, so this marker joins `isSystem` and ADR-0056's `publicFormGrant` in being documented only at its own site. Nothing today is FALSE (the hand-read above found no falsified sentence), so this is a documentation enhancement, not a defect or a declared-not-enforced gap -- filing it as a finding card looked like manufacturing work, and the reviewer-of-record's instruction on the docs question was to report rather than edit. Grade it if you want the page written." ], "open_questions": [ { "question": "Inside the ruled scope, should the ADR-0090 D10 delegator half of the object-level grant check be exempted along with the caller half? I implemented YES; recording it because the ruling names 'the object-level CRUD check' without splitting the two principals, so this is the one judgment call in the diff.", "options": [ "A -- exempt both halves (implemented): the D10 check is `checkObjectPermission(operation, object, delegatorSets)`, literally the same object-level grant question asked of the second principal. An agent whose delegator is exempt here does not act beyond that user's reach, and the delegated delete behaves like the direct one.", "B -- exempt only the caller half: strictly narrower, but a delete would then succeed directly and refuse on-behalf-of, which reintroduces exactly the invisible 'delete right is not delete right' coupling this card exists to remove, on the delegated path only, where it is hardest to diagnose." ], "recommendation": "A, because the two calls are one guard asked of two principals and B would re-create the defect on the delegated path. Reversible in one line if a reviewer reads the ruling more narrowly; the code comment states the reasoning at the site." } ], "head": "0dd9a3496", "review_round": { "defect found by the reviewer-of-record, fixed": "The exemption comment cited `delete-reference-cleanup-crud-exemption.test.ts`, a filename that resolves to nothing -- an artefact of an earlier plan to put the pins in their own file before they went into the existing suite. Corrected at commit 0dd9a3496 to this package's `delete-reference-cleanup-system-identity.test.ts`, naming its `#12597` describe. Verified: `git grep` for the dead name returns 0 files; the cited name resolves to a tracked file (`git ls-files`). Re-run at that head: 'Test Files 1 passed (1) / Tests 14 passed (14)', with check:where-matcher, check:engine-double-contract, check:query-options-erasure, check:nul-bytes and check-comment-mask-adoption all exit 0. The reviewer's framing is right and worth keeping: 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.", "docs-drift bot: 8 rows, all wrong -- reported on the PR": "Every row is anchored `via set_null (literal)` on a token this diff introduces only in PROSE. Measured independently: `grep -c set_null` on security-plugin.ts is 0 at merge base aef1b7e64 and 2 at head, and both occurrences are comment lines (:1958, :1988); no code line in the diff contains the token. Positive control on the same query shape: `grep -c permissionSets` at the merge base returns 117, so the zero is a real reading. Filed as a bot finding in a PR comment (anchoring on comment text over-lists) rather than acted on.", "docs hand-read -- wrong rows are not a clearance": "The bot lists no content/docs/permissions/ page at all, which is where its own declared blind spot (a page declaring a rule by its INPUTS shares no token with the emitter) would bite for a change to WHICH DELETES SUCCEED. One question was put to authorization.mdx, access-matrix.mdx and system-context.mdx: does any sentence state or imply that the reference-cleanup write is subject to the caller's authority on the referencing object? NO FALSIFIED SENTENCE FOUND. authorization.mdx's layer table describes the ladder for caller-initiated operations and never claims it is exemption-free -- it already coexists with two layer-3 exemptions it does not caveat either (the isSystem short-circuit, ADR-0056's publicFormGrant). system-context.mdx is about isSystem, which this diff deliberately does not use; its row 18 and its 'Elevation is total, and it is not granular' are untouched and still true -- this design obeys that sentence rather than contradicting it. access-matrix.mdx is the per (permission set x object) snapshot, unaffected. Repo-wide, no page asserts that deleting a record requires rights on the referencing object, and deployment/troubleshooting.mdx's 'Cannot delete record: delete restricted' entry is about `restrict` escalation and `multiple: true` member semantics with zero permission/403 content. Nothing in content/docs/ is edited by this PR.", "release-owned rows: audited, never edited": "content/docs/releases/v15.mdx:847 -- 'Engine-internal referential cascades (`set_null` on user deletion) are exempted via a server-side `__referentialFieldClear` marker that cannot be forged from a client (#3048)': STILL TRUE. It describes the ownership-anchor guard's exemption, unchanged and still keyed on that marker; this PR adds a SECOND consumer of the same marker and the sentence never claims to be the only one. Its un-forgeability half also still holds -- assembleExecutionContext builds the inbound envelope from a CLOSED field set that carries no `__` operation-private key, verified in source. content/docs/releases/v17.mdx:3819 -- the `multiple: true` set_null member-removal note (#9002): nothing about permissions, not falsified. Both read-only: a code PR does not edit content/docs/releases/.", "stale claim corrected in the PR body (body edit only, no push)": "The Verification section said the adr-anchors base failure's fix was 'in flight as #12917'. True when written, false by the time it was read: #12917 landed as 750fff559 and origin/main has moved to 8beb3deaf. The bullet now records that the failure was hit during the run, that check-adr-anchors was deliberately not touched for it, and that it is resolved -- re-measured independently (0 hits, positive control 8). The bullet was corrected rather than deleted, so the fact that this seat met it and declined to 'fix' someone else's surface stays on the record. No commit, no CI cycle." } }Generated by Claude Code
os-zhuang commentedon Aug 28, 2026
PM review — ACCEPT → driving to land
Verdict argues at the PR, not at its narrative: #12941.
Checklist conclusion: PASS. The diff implements the 2026-08-28 round-2 ruling (option B) and nothing wider. Read off the diff itself rather than the report: the guard is
opCtx.operation === 'update' && opCtx.context?.__referentialFieldClear === true, spent at exactly one site —if (permissionSets.length > 0 && !referentialFieldClearWrite). One gate, marker-scoped, notisSystem.Spot-checks I ran myself
docs/adr/**·.claude/**·skills/**·AGENTS.md·CLAUDE.md; PR file list viaget_files(⛔ not the report's self-description) = 3 files, none hit ⇒ queue-eligiblepackages/spec/src/**in the file list ⇒ not triggeredno;needs:contract-reviewabsent, per the maintainer's own 2026-08-28 correction — ⛔ not re-attachedcompleted: 36success+ 2skipped(Build Docs,Console Pin Gate), zero failures — read across every check, ⛔ not the required subsetmergeable_statecleantotal_countgrew 32 → 35 → 38 across three polls as the sharded jobs rolled up (Test Core (1/6)alone ran 10:02→10:19). Each intermediate poll was internally self-consistent — every rowcompleted, none failed — and each was still a partial list. A self-consistent page is not a terminal reading; only the count settling with every rowcompletedis. Cross-checked against thecheck_suite.completedwebhook, which by its own note does not cover this App's own suites.Deviations
checkObjectPermissionsits inside the veryifwhose condition changed, so it travels with the caller half mechanically — that is what the ruled sentence means in this code, not a second edit. B would have made the delete succeed directly and refuse on-behalf-of, re-creating this card's defect on the delegated path.set_nulltoken this diff introduces only in comment prose (grep -cat merge base 0 → head 2, both//lines; positive controlpermissionSets= 117). Reported as a bot finding rather than acted on — correct call, and the hand-read ofcontent/docs/permissions/**was done anyway because wrong rows are not a clearance. No falsified sentence found.check:adr-anchors, [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) was met and correctly refused — not this seat's surface, and the gate was ⛔ not touched to make it go away. It has since been fixed by fix(spec): namespace tombstone stops citing ADR-0006 D4 — unblocks the merge queue's check-adr-anchors red #12917; the stale "in flight" wording was corrected on both surfaces rather than deleted, so the refusal stays on the record.⇒ Path face clean, Clause-② not triggered, every check green ⇒ ready + enqueue, the queue being the only sanctioned landing path.
Generated by Claude Code
security(engine): exempt the referential FK-clear write from the obje…