删除记录时「引用清理」用操作人身份查询引用表,读权不足即整体 403(应以系统身份执行) #12166
Description
Activity
Maintainer ruling recorded — A: delete-time reference cleanup runs under the SYSTEM identity
Provenance: maintainer, 2026-08-26, live PM chat (decision-inbox batch 1, presented with the global-platform comparison by the skills seat, session
session_01JANH3y7qe3MD8aLaLXci8N), verbatim: 「A」.The industry comparison that grounded the ruling, kept with it: referential-integrity actions (check / clear / cascade / restrict) are engine responsibility executed under system identity on every mainstream platform — the RDBMS FK baseline (RESTRICT/SET NULL/CASCADE run by the engine regardless of the deleter's SELECT grants), Salesforce (lookup clearing and cascade-delete documented as bypassing sharing; restrict errors name the blocking relationship), Dataverse (platform-side Remove Link/Restrict/Cascade, with
Cascade User-Ownedexisting precisely as the explicit NARROWING), ServiceNow and Odoo alike. The current caller-identity behaviour is the outlier and composes badly: it silently makes "delete permission" mean "delete + read on every referencing table", invisible in the permission UI.Four binding implementation constraints, accepted with the ruling — these go into the dispatch brief verbatim:
- Scope pinned: ONLY the pre-delete reference-cleanup/reference-check queries switch to system identity, following the in-repo
isSystemprecedent (packages/objectql/src/integrity/dangling-reference-audit.ts). Nothing else about the delete path changes identity. - The error names the referenced OBJECT, never record contents — one notch more conservative than Salesforce: the caller learns "blocked by / cleared references on object X", nothing about rows.
- Audit records both halves: triggered-by = the deleting operator, executed-as = system (the Salesforce/Dataverse ledger shape).
- Behaviour vs identity separation: if the spec later declares per-relationship on-delete behaviour (restrict / set-null), behaviour follows the declaration — the identity stays system, unconditionally.
Confidence gap recorded as presented: no measurement of deployments relying on the 403 as a de-facto delete gate; under the industry baseline that usage is itself non-standard and should be expressed as an explicit restrict, not a permission side effect.
State transition in the same stroke:
needs-user-decision→pm:queue(domain:enginestands — the engine seat dispatches; clause discipline per that lane's standing rules for a permission-semantics change, review from严).
Generated by Claude Code
- Scope pinned: ONLY the pre-delete reference-cleanup/reference-check queries switch to system identity, following the in-repo
已认领并派发
- 会话:
session_01W6HFzyH98W1YaQXhJUJt6o(席位单 [PM seat] domain:engine — ⏳ vacant #6367) - 分支:
claude/issue-12166-reference-cleanup-system-identity - 认领的同一动作里重读了全部评论:三条 —— 11:49Z 的初次 triage、23:51Z 的状态修复兼升级(明确推翻了前一条的「无开放设计问题」)、以及 02:05Z 记录的维护者裁决。没有任何其他会话的在先认领。
裁决为 A,维护者原话「A」,2026-08-26 决策箱第一批。随裁决接受的四条实现约束逐字进了派发令,一条不改写、不软化:
- 范围钉死——只有删除前的引用清理/引用检查查询切系统身份,照
packages/objectql/src/integrity/dangling-reference-audit.ts的isSystem先例;删除路径的其他任何部分身份不变。 - 报错点名被引用对象,绝不涉及记录内容——比 Salesforce 还保守一档:操作人只知道「被对象 X 的引用挡住/已清理对象 X 上的引用」,不知道任何行。
- 审计两半都记:triggered-by = 发起删除的操作人,executed-as = 系统。
- 行为与身份分离:将来 spec 若声明按关系的 on-delete 行为(restrict / set-null),行为跟声明走,身份无条件保持系统。
派发令里另加了三点,都是从今天别的卡上真金白银换来的:
⚠️ 前提在当前 ref 上自己重验,不许从卡片正文继承。我今天就在另一张卡上栽过——把一条早已解除十五天的冻结当作生效约束写进了派发令,全靠 dev 自己复查才没造成损失。所以 dev 被要求自己去验dangling-reference-audit.ts的isSystem先例仍在、日志里那条[Security] Access denied: operation 'find'仍是当前形状。- pin 要钉终态而不是调用:钉「以系统身份调了 find」的 pin,别人改个包装就绿了;要钉的是空引用表 + 无读权 + 有完整删除权 ⇒ 删除成功,也就是原始报告里那个 A/B 对照本身。
- 置信缺口原样带下去:没有测量过是否有现存部署把 403 当作变相删除闸。裁决里已写明,按行业基线那种用法本身就非标准、应当表达为显式 restrict 而不是权限副作用——但 dev 若在实现中撞见相反证据,停下上报,不要自行权衡。
needs:contract-review已挂:这是权限边界语义变更,条款②适用(原先 403 的配置现在放行),复核从严。串行检查:落点在
packages/objectql(删除流程的引用清理),可能涉及packages/drivers/driver-sql。当前在飞的两张——#11991 在sql-driver.ts、#12032 在health-monitor.ts——都不与之同文件;hook-wrappers.ts上的 #12397 已随 PR #12581 落地,那条串行也已释放。
Generated by Claude Code
- 会话:
{ "issue": 12166, "status": "done", "branch": "claude/issue-12166-reference-cleanup-system-identity", "pr": "https://github.com/objectstack-ai/objectstack/pull/12596", "premise_still_valid": true, "clause2": true, "clause2_evidence": "Relaxes a permission check: configurations that returned 403 now succeed. Reproduced on origin/main@7bd6447f41 through the REAL SecurityPlugin middleware over a REAL ObjectQL engine, three-part fixture from the report (A referenced by B's lookup; role with full delete on A and no grant on B; B EMPTY) -> PERMISSION_DENIED/403, developerMessage \"[Security] Access denied: operation 'find' on object 'os_ehr_andon_record' is not permitted for positions []\". After the fix the same fixture returns SUCCESS. git diff --stat vs merge-base: 5 files, 1078 insertions(+), 5 deletions(-) (.changeset 78; engine-reference-check-system-identity.test.ts 228; engine.ts 310; delete-reference-cleanup-system-identity.test.ts 427; operation-message.ts 40) - 655 of the added lines are the two new pin suites. UNCHANGED, and pinned: the caller's own delete authorisation on the target, the set_null UPDATE, the cascade child DELETE, and the target's own delete all still run as the caller. dispatch-gates independently flagged the surface as clause-2 SUSPECT via packages/spec/src/**.", "constraintsHonoured": { "scope": "Only the dependents probe switches identity, sudo()-shaped ({...context, isSystem: true}) per the dangling-reference-audit.ts:623 precedent - never a bare {isSystem:true}, so the caller's transaction handle, TENANT scope and userId survive (a bare system context would have widened the probe across the tenant wall). Pinned in engine-reference-check-system-identity.test.ts: probe carries isSystem AND tenantId/userId/timezone; the set_null UPDATE, cascade DELETE and the target's own delete are each asserted NOT elevated; the caller's context object is asserted un-mutated.", "errorNamesObjectOnly": "DELETE_RESTRICTED names dependentObject unconditionally. The row COUNT is disclosed only when the caller's own identity would have produced the same rows - compared on ROW IDENTITY, not length, so RLS narrowing counts too. Otherwise the count is withheld from message, developerMessage AND the dependentCount envelope field (absent, never 0 - 0 would be a false statement about the rows), via two new catalog keys delete_restricted_opaque / delete_restricted_required_opaque in all four bundled locales. Rationale: without it the refusal is an exact repeatable cardinality oracle over a table the caller may not read. Suppression is CONDITIONAL and both halves are pinned - a sighted caller still gets the count byte-for-byte as before, so no existing assertion, catalog entry or REST envelope field changed.", "auditBothHalves": "triggeredBy = the deleting operator, executedAs = 'system', plus referencedObject and relationField - never a row id, value or count. Filed BEFORE the probe, so a refused or failed check is recorded too. Pinned in both directions (record present on success; present on a DELETE_RESTRICTED refusal). DECLARED LIMIT, stated in code, changeset and PR body: this is an engine LOG record, not a sys_audit_log row - the elevated operation is a read, and plugin-audit's read-audit writer declares and pins that a system-elevated read produces no row. A durable row belongs to the plugin owning that shape.", "behaviourVsIdentity": "The elevation is unconditional and does not read `behavior`. The call site carries an explicit do-not-make-this-conditional note for the future per-relationship on-delete declaration." }, "premiseReverified": "Yes, on current origin/main (7bd6447f41), not inherited. (1) isSystem precedent live at packages/objectql/src/integrity/dangling-reference-audit.ts:623 -> `context: { isSystem: true }`. (2) The pre-delete probe was located BY SYMBOL (ObjectQL.cascadeDeleteRelations's dependents probe, engine.ts), never by the card's line numbers. (3) The 403 shape is current - REPRODUCED rather than quoted, output pasted in the PR body.", "gates": { "green": "Full suites: @objectstack/objectql 4202 passed / 239 files (sharded 3x); @objectstack/plugin-security 1548 passed / 84 files; @objectstack/spec 11476 passed / 432 files (sharded 2x); objectql + plugin-security typecheck exit 0. Gate union re-derived on final head 31645534b2 via `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack` (no paths; 5 paths, 32 matched families, unchanged after the fixes). All green: check:authorable-surface, check:liveness, check:empty-state, check:strictness-ledger, check:variant-docs, check:doc-authoring, check:doc-formula-expressions, check:changeset-gate-self-tests, check:objectui-changeset, check:adr-0087-registration, check:changeset-no-major, check:empty-changeset, check:cross-package-test-inputs, check:ci-filter-parity, check:published-files, check:page-declaration-shape, check:slot-lookup, check:spec-parsed-alias, check:stack-collection-maps, check:merge-driver, check:plugin-teardown-shape, check:release-rehearsal-selftest, check:test-source-alias, check:type-source-resolution, check:comment-mask-adoption, check:nul-bytes, check:engine-split-ratio, check:docs-affected, check:docs-drift-comment, check:i18n (CLI built first - it refuses to measure otherwise), check:i18n-stale-fill, check:engine-double-contract, check:where-matcher, check:objectql-double-limit, check:durability-log-level, check:query-options-erasure, check:type-check-coverage. Every exit code captured BEFORE any pipe (EXIT=$? on its own line, never after a `| tail`).", "red": "Three went red ON THIS DIFF FIRST and were REPAIRED, not baselined. (1) check:objectql-double-limit - both new find() doubles were limit-blind; they now apply the caller's bound by presence. (2) check:durability-read-invention - the disclosure probe's catch answered silently; it still WITHHOLDS rather than rethrows (propagating a DISCLOSURE probe's failure would turn a correct 409 into a 500) but now says so, which is the rule's own second remedy 'say something, or ask the error's type'. (3) check:query-options-erasure - my new find() call added an `as any`, growing engine.ts 8->9; it now carries the declared EngineQueryOptions type. All three re-run green on 31645534b2. NOTE: `pnpm check:i18n` and `check:dev-prereqs` initially reported red as PRECONDITION failures (nothing measured) - see notMeasured.", "notMeasured": "check:dev-prereqs - refuses outright because 50 of 67 workspace packages have no dist/ in this worktree; it names @objectstack/hono, account, setup, none of which this diff touches. A worktree precondition, not a finding; CI builds the workspace. check:type-check-debt --re-measure - needs the full workspace closure built, same reason; its STRUCTURAL half check:type-check-coverage is green and both touched packages typecheck clean. Separately: `pnpm check:i18n` was first run in a form that measured NOTHING - vitest/CLI arg parsing swallowed the target ('--silent' consumed the file pattern; the gate itself warns that piping it reports the PIPE's status). Caught by capturing the exit code directly, then re-run properly after building @objectstack/cli: green. One transient recorded honestly: @objectstack/rest:build failed inside a parallel turbo build with NO TypeScript error printed; rebuilt alone at a larger heap it exits 0 - resource contention in the shared container, not this change.", "channelDeviation": "Declared: the REST issue-list channel required for the out-of-scope dedupe search is UNAVAILABLE from this seat - api.github.com answers HTTP 403 'GitHub access is not enabled for this session' for reads as well as writes (probed both). The dedupe search and the filing therefore went through the MCP GitHub tools, and `gh` is not installed in this container either." }, "ablations": [ "A - revert the identity elevation (probe context back to the caller's). PREDICTION WRITTEN TO DISK BEFORE ANY MUTATION: direction RED, exactly 6 failures (5 plugin-security + 1 objectql), with THE CONVERSE green (the delete gate runs before the cascade) and the constraint-3 ledger pin green (the record is filed BEFORE the probe). OBSERVED: RED, exactly 6 - and precisely the six named tests, with exactly those two green. Mutation proven on disk BEFORE any result was read via anchored grep -cF (removed-text 1->0, injected-text 0->1). Restored under trap '...' EXIT INT TERM; restore verified by empty `git diff` (exit 0) and marker residue count 0.", "B - force discloseCount = true (revert the constraint-2 suppression). PREDICTION WRITTEN FIRST: RED, exactly 1 failure (the blind-caller test), sighted CONTROL stays green. OBSERVED: RED, exactly 1 - 'AssertionError: expected 2 to be undefined'. Same on-disk proof, same trap, restore verified empty.", "NO-REBUILD JUSTIFIED BY IMPORT FORM, not by assertion: the objectql suite imports './engine.js' (RELATIVE SOURCE); the plugin-security suite's '@objectstack/objectql' is aliased to '../../objectql/src/index.ts' by that package's own vitest.config.ts, so it reads SOURCE too. '@objectstack/spec' DOES resolve to dist/ there - measured, not assumed: the first pin run returned the bare key 'delete_restricted_required_opaque' against a stale dist, which is why `pnpm --filter @objectstack/spec build` came first. Dependency closure was built before any test ran (the new-worktree trap: the very first run failed with 'Failed to resolve entry for package @objectstack/core')." ], "openQuestions": [], "out_of_scope_findings": [ "filed as #12597: the WRITE half of reference cleanup (the set_null UPDATE and the cascade child DELETE) still runs as the caller per constraint 1, so a role with full delete on the target and no grant on the referencing object succeeds only while that table is EMPTY - a non-empty one still 403s, now on 'update' instead of 'find'. This is the ruling's DELIBERATE boundary, not an implementation defect, and I did not renegotiate it: it is pinned as an assertion in the PR. Filed unassigned and unlabelled for triage because its practical reach was never measured - it decides how many of the reporting deployment's 17 role x object pairs are actually fixed. The issue lays out three options (keep as-is / elevate set_null only / elevate both) and flags that cascade-as-system is a much larger permission change than set_null-as-system, so the two likely need separate rulings. Clause 2 applies to any of them." ], "confidenceGap": "Carried forward UNWEIGHED, as instructed. I found NO contrary evidence that any deployment relies on the 403 as a de-facto delete gate: no spec or doc declares the coupling as contract, and no test in this repo pinned the 403 as intended (nothing was edited or deleted to make this pass). Per the ruling, such usage is itself non-standard and belongs in an explicit deleteBehavior:'restrict'.", "assertionsChanged": "None existing. No test in the repo pinned the 403. The only correction was to an assertion I wrote myself this session: it asserted the refusal contained no row id at all, which was wrong - the TARGET's own id appears in developerMessage and is the caller's own input, not a disclosure. Now scoped to the referencing object's row ids, with the reason on the line, and declared in the PR body.", "changesetGrade": "minor on @objectstack/objectql and @objectstack/spec, argued in the changeset rather than defaulted. Not patch: 'records a role could never delete are now deletable' is a security-surface accept-set change an upgrader must find in a release-notes heading, and patch is exactly where it would not be looked for. Not major: nothing gets more restrictive and no API changes shape. The spec half rides minor because the message catalog gains two keys.", "summary": "The pre-delete reference check now runs under the system identity, sudo()-shaped so the caller's transaction, tenant scope and userId survive the elevation. Reproduced the deployment's exact 403 first (empty referencing table included) against the real SecurityPlugin over a real engine, then pinned the TERMINAL STATE rather than the call: empty referencing table + no read grant + full delete rights => the delete SUCCEEDS, and the converse, a caller without delete rights on the target is still refused about the TARGET. All four ruling constraints implemented as written; none needed deviation. Because the elevated probe now sees rows the caller cannot, DELETE_RESTRICTED withholds the dependent count unless the caller's own identity would have produced the same rows (compared on row identity, so RLS narrowing counts), via two new opaque catalog keys - otherwise the refusal would be a cardinality oracle over a table the caller may not read; the referenced object is named either way. Report delivered at draft-PR time; CI convergence is the PM's to read." }
Generated by Claude Code
Generated by Claude Code
os-dev-report
Re-posted with the marker as literal text: in the previous comment (5427498688) the GitHub body sanitizer ate the
<!-- os-dev-report -->HTML comment, which makes that comment invisible to the PM's scan. Same payload, byte for byte. No edit path was available from this seat —api.github.comanswers 403 for reads and writes alike,ghis not installed, and the MCP surface can add comments but not update them. This comment supersedes 5427498688; ignore that one.{ "issue": 12166, "status": "done", "branch": "claude/issue-12166-reference-cleanup-system-identity", "pr": "https://github.com/objectstack-ai/objectstack/pull/12596", "premise_still_valid": true, "clause2": true, "clause2_evidence": "Relaxes a permission check: configurations that returned 403 now succeed. Reproduced on origin/main@7bd6447f41 through the REAL SecurityPlugin middleware over a REAL ObjectQL engine, three-part fixture from the report (A referenced by B's lookup; role with full delete on A and no grant on B; B EMPTY) -> PERMISSION_DENIED/403, developerMessage \"[Security] Access denied: operation 'find' on object 'os_ehr_andon_record' is not permitted for positions []\". After the fix the same fixture returns SUCCESS. git diff --stat vs merge-base: 5 files, 1078 insertions(+), 5 deletions(-) (.changeset 78; engine-reference-check-system-identity.test.ts 228; engine.ts 310; delete-reference-cleanup-system-identity.test.ts 427; operation-message.ts 40) - 655 of the added lines are the two new pin suites. UNCHANGED, and pinned: the caller's own delete authorisation on the target, the set_null UPDATE, the cascade child DELETE, and the target's own delete all still run as the caller. dispatch-gates independently flagged the surface as clause-2 SUSPECT via packages/spec/src/**.", "constraintsHonoured": { "scope": "Only the dependents probe switches identity, sudo()-shaped ({...context, isSystem: true}) per the dangling-reference-audit.ts:623 precedent - never a bare {isSystem:true}, so the caller's transaction handle, TENANT scope and userId survive (a bare system context would have widened the probe across the tenant wall). Pinned in engine-reference-check-system-identity.test.ts: probe carries isSystem AND tenantId/userId/timezone; the set_null UPDATE, cascade DELETE and the target's own delete are each asserted NOT elevated; the caller's context object is asserted un-mutated.", "errorNamesObjectOnly": "DELETE_RESTRICTED names dependentObject unconditionally. The row COUNT is disclosed only when the caller's own identity would have produced the same rows - compared on ROW IDENTITY, not length, so RLS narrowing counts too. Otherwise the count is withheld from message, developerMessage AND the dependentCount envelope field (absent, never 0 - 0 would be a false statement about the rows), via two new catalog keys delete_restricted_opaque / delete_restricted_required_opaque in all four bundled locales. Rationale: without it the refusal is an exact repeatable cardinality oracle over a table the caller may not read. Suppression is CONDITIONAL and both halves are pinned - a sighted caller still gets the count byte-for-byte as before, so no existing assertion, catalog entry or REST envelope field changed.", "auditBothHalves": "triggeredBy = the deleting operator, executedAs = 'system', plus referencedObject and relationField - never a row id, value or count. Filed BEFORE the probe, so a refused or failed check is recorded too. Pinned in both directions (record present on success; present on a DELETE_RESTRICTED refusal). DECLARED LIMIT, stated in code, changeset and PR body: this is an engine LOG record, not a sys_audit_log row - the elevated operation is a read, and plugin-audit's read-audit writer declares and pins that a system-elevated read produces no row. A durable row belongs to the plugin owning that shape.", "behaviourVsIdentity": "The elevation is unconditional and does not read `behavior`. The call site carries an explicit do-not-make-this-conditional note for the future per-relationship on-delete declaration." }, "premiseReverified": "Yes, on current origin/main (7bd6447f41), not inherited. (1) isSystem precedent live at packages/objectql/src/integrity/dangling-reference-audit.ts:623 -> `context: { isSystem: true }`. (2) The pre-delete probe was located BY SYMBOL (ObjectQL.cascadeDeleteRelations's dependents probe, engine.ts), never by the card's line numbers. (3) The 403 shape is current - REPRODUCED rather than quoted, output pasted in the PR body.", "gates": { "green": "Full suites: @objectstack/objectql 4202 passed / 239 files (sharded 3x); @objectstack/plugin-security 1548 passed / 84 files; @objectstack/spec 11476 passed / 432 files (sharded 2x); objectql + plugin-security typecheck exit 0. Gate union re-derived on final head 31645534b2 via `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack` (no paths; 5 paths, 32 matched families, unchanged after the fixes). All green: check:authorable-surface, check:liveness, check:empty-state, check:strictness-ledger, check:variant-docs, check:doc-authoring, check:doc-formula-expressions, check:changeset-gate-self-tests, check:objectui-changeset, check:adr-0087-registration, check:changeset-no-major, check:empty-changeset, check:cross-package-test-inputs, check:ci-filter-parity, check:published-files, check:page-declaration-shape, check:slot-lookup, check:spec-parsed-alias, check:stack-collection-maps, check:merge-driver, check:plugin-teardown-shape, check:release-rehearsal-selftest, check:test-source-alias, check:type-source-resolution, check:comment-mask-adoption, check:nul-bytes, check:engine-split-ratio, check:docs-affected, check:docs-drift-comment, check:i18n (CLI built first - it refuses to measure otherwise), check:i18n-stale-fill, check:engine-double-contract, check:where-matcher, check:objectql-double-limit, check:durability-log-level, check:query-options-erasure, check:type-check-coverage. Every exit code captured BEFORE any pipe (EXIT=$? on its own line, never after a `| tail`).", "red": "Three went red ON THIS DIFF FIRST and were REPAIRED, not baselined. (1) check:objectql-double-limit - both new find() doubles were limit-blind; they now apply the caller's bound by presence. (2) check:durability-read-invention - the disclosure probe's catch answered silently; it still WITHHOLDS rather than rethrows (propagating a DISCLOSURE probe's failure would turn a correct 409 into a 500) but now says so, which is the rule's own second remedy 'say something, or ask the error's type'. (3) check:query-options-erasure - my new find() call added an `as any`, growing engine.ts 8->9; it now carries the declared EngineQueryOptions type. All three re-run green on 31645534b2. NOTE: `pnpm check:i18n` and `check:dev-prereqs` initially reported red as PRECONDITION failures (nothing measured) - see notMeasured.", "notMeasured": "check:dev-prereqs - refuses outright because 50 of 67 workspace packages have no dist/ in this worktree; it names @objectstack/hono, account, setup, none of which this diff touches. A worktree precondition, not a finding; CI builds the workspace. check:type-check-debt --re-measure - needs the full workspace closure built, same reason; its STRUCTURAL half check:type-check-coverage is green and both touched packages typecheck clean. Separately: `pnpm check:i18n` was first run in a form that measured NOTHING - vitest/CLI arg parsing swallowed the target ('--silent' consumed the file pattern; the gate itself warns that piping it reports the PIPE's status). Caught by capturing the exit code directly, then re-run properly after building @objectstack/cli: green. One transient recorded honestly: @objectstack/rest:build failed inside a parallel turbo build with NO TypeScript error printed; rebuilt alone at a larger heap it exits 0 - resource contention in the shared container, not this change.", "channelDeviation": "Declared: the REST issue-list channel required for the out-of-scope dedupe search is UNAVAILABLE from this seat - api.github.com answers HTTP 403 'GitHub access is not enabled for this session' for reads as well as writes (probed both). The dedupe search and the filing therefore went through the MCP GitHub tools, and `gh` is not installed in this container either. The same outage is why this report had to be re-posted rather than edited when the sanitizer ate the HTML marker." }, "ablations": [ "A - revert the identity elevation (probe context back to the caller's). PREDICTION WRITTEN TO DISK BEFORE ANY MUTATION: direction RED, exactly 6 failures (5 plugin-security + 1 objectql), with THE CONVERSE green (the delete gate runs before the cascade) and the constraint-3 ledger pin green (the record is filed BEFORE the probe). OBSERVED: RED, exactly 6 - and precisely the six named tests, with exactly those two green. Mutation proven on disk BEFORE any result was read via anchored grep -cF (removed-text 1->0, injected-text 0->1). Restored under trap '...' EXIT INT TERM; restore verified by empty `git diff` (exit 0) and marker residue count 0.", "B - force discloseCount = true (revert the constraint-2 suppression). PREDICTION WRITTEN FIRST: RED, exactly 1 failure (the blind-caller test), sighted CONTROL stays green. OBSERVED: RED, exactly 1 - 'AssertionError: expected 2 to be undefined'. Same on-disk proof, same trap, restore verified empty.", "NO-REBUILD JUSTIFIED BY IMPORT FORM, not by assertion: the objectql suite imports './engine.js' (RELATIVE SOURCE); the plugin-security suite's '@objectstack/objectql' is aliased to '../../objectql/src/index.ts' by that package's own vitest.config.ts, so it reads SOURCE too. '@objectstack/spec' DOES resolve to dist/ there - measured, not assumed: the first pin run returned the bare key 'delete_restricted_required_opaque' against a stale dist, which is why `pnpm --filter @objectstack/spec build` came first. Dependency closure was built before any test ran (the new-worktree trap: the very first run failed with 'Failed to resolve entry for package @objectstack/core')." ], "openQuestions": [], "out_of_scope_findings": [ "filed as #12597: the WRITE half of reference cleanup (the set_null UPDATE and the cascade child DELETE) still runs as the caller per constraint 1, so a role with full delete on the target and no grant on the referencing object succeeds only while that table is EMPTY - a non-empty one still 403s, now on 'update' instead of 'find'. This is the ruling's DELIBERATE boundary, not an implementation defect, and I did not renegotiate it: it is pinned as an assertion in the PR. Filed unassigned and unlabelled for triage because its practical reach was never measured - it decides how many of the reporting deployment's 17 role x object pairs are actually fixed. The issue lays out three options (keep as-is / elevate set_null only / elevate both) and flags that cascade-as-system is a much larger permission change than set_null-as-system, so the two likely need separate rulings. Clause 2 applies to any of them." ], "confidenceGap": "Carried forward UNWEIGHED, as instructed. I found NO contrary evidence that any deployment relies on the 403 as a de-facto delete gate: no spec or doc declares the coupling as contract, and no test in this repo pinned the 403 as intended (nothing was edited or deleted to make this pass). Per the ruling, such usage is itself non-standard and belongs in an explicit deleteBehavior:'restrict'.", "assertionsChanged": "None existing. No test in the repo pinned the 403. The only correction was to an assertion I wrote myself this session: it asserted the refusal contained no row id at all, which was wrong - the TARGET's own id appears in developerMessage and is the caller's own input, not a disclosure. Now scoped to the referencing object's row ids, with the reason on the line, and declared in the PR body.", "changesetGrade": "minor on @objectstack/objectql and @objectstack/spec, argued in the changeset rather than defaulted. Not patch: 'records a role could never delete are now deletable' is a security-surface accept-set change an upgrader must find in a release-notes heading, and patch is exactly where it would not be looked for. Not major: nothing gets more restrictive and no API changes shape. The spec half rides minor because the message catalog gains two keys.", "summary": "The pre-delete reference check now runs under the system identity, sudo()-shaped so the caller's transaction, tenant scope and userId survive the elevation. Reproduced the deployment's exact 403 first (empty referencing table included) against the real SecurityPlugin over a real engine, then pinned the TERMINAL STATE rather than the call: empty referencing table + no read grant + full delete rights => the delete SUCCEEDS, and the converse, a caller without delete rights on the target is still refused about the TARGET. All four ruling constraints implemented as written; none needed deviation. Because the elevated probe now sees rows the caller cannot, DELETE_RESTRICTED withholds the dependent count unless the caller's own identity would have produced the same rows (compared on row identity, so RLS narrowing counts), via two new opaque catalog keys - otherwise the refusal would be a cardinality oracle over a table the caller may not read; the referenced object is named either way. Report delivered at draft-PR time; CI convergence is the PM's to read." }
Generated by Claude Code
Generated by Claude Code
修复已提交(PR #12596),但你报的 17 组不一定全好 —— 边界说在前面
裁决 A 已按四条约束原样实现,你报告里那个精确的 403 先被复现了(真
SecurityPlugin走真 ObjectQL 引擎,三段夹具照你写的来,含空引用表),修完同一夹具返回成功。但这次修的是「读」那一半。 删除前的引用检查(
find)现在以系统身份跑;而引用清理的写那一半——set_null的 UPDATE 和级联子删除——按裁决约束 1 仍以操作人身份执行。后果,说清楚而不是让你自己撞上:
引用表状态 修复前 修复后 空(零引用) 403 find✅ 删除成功 非空(真有引用要清) 403 find仍 403,改在 update上所以你盘出的 17 组「角色×对象」里,有多少真的被这次修好,取决于其中多少组的引用表是非空的 —— 这个数没有人测过。你那边的实测环境比我们任何一个容器都更接近真相:如果方便,按同样的 A/B 方式回一下哪些组还在 403、以及报错里的 operation 是
find还是update,那就直接把这个数字定下来了。这不是这个 PR 的缺陷:约束 1 明写「只有删除前的引用清理/引用检查查询切系统身份,删除路径的其他任何部分身份不变」,dev 没有擅自扩大范围,而且把这条边界写成了断言 pin 住,不是留成隐含。已另开 #12597 追踪写那一半,未认领、未定级,正因为它的实际覆盖面从没被测量——而那个测量恰恰决定它值不值得做。
顺带两点你可能关心的:
- 报错文案你提的诉求部分满足了:拒绝时会点名是哪个被引用对象挡住的。但引用行数只在「以你自己的身份查也会得到同样的行」时才告诉你——否则那个数字会变成一张你无权读的表上的精确计数探针。被引用对象的名字是无条件给的。
- 多租户没有被顺带放宽:提权用的是
{...context, isSystem: true}而不是裸的{isSystem: true},你的租户范围和用户身份都跟着走。照抄仓里既有先例的写法会把这个探针捅穿租户墙,dev 没有照抄。
Generated by Claude Code
Contract review: PASS (skills seat, session
session_01MnijPVVDakqK2J335JoJtq; downgrade-fuse machine reading this sub-round:external_metadata.last_served_model = claude-fable-5).Reviewed the contract increment of PR #12596 against the tree, not the report: ① public-surface delta is exactly two
_opaquecatalog twins (delete_restricted_opaque/delete_restricted_required_opaque, all four locales, same sentence minus the count placeholder) — the wire code stays the single ADR-0112 memberDELETE_RESTRICTED, no schema shape moves, and the contract-side comment block records who owns the counted-vs-opaque choice and forbids cosmetic use; ② the accept-set relaxation is the maintainer-ruled one (2026-08-26 option A) and the four binding constraints are visible in the diff itself: sudo-shaped elevation ({ ...context, isSystem: true }— transaction, tenant, userId survive; a bare system context is named a defect at the call site), scope pinned to the dependents probe alone with set_null/cascade/target-delete asserted un-elevated, both-halves audit record filed before the probe, and the elevation explicitly unconditional with a do-not-make-conditional note against futurebehaviordeclarations; ③ the disclosure gate closes the oracle the elevation would otherwise open — count only when the caller's own identity yields the same rows, compared on row identity; object named unconditionally per the ruling; ④ changeset grademinor×2 correctly argued (an accept-set security change must surface in a release-notes heading). Clause-② declaration on the report (clause2: true) matches the path limb (packages/spec/src/**) — both limbs reviewed.needs:contract-reviewcleared on this card in the same stroke (the PR side was never hung — noting rather than churning a hang-and-clear).Landing NOT executed by this chain — handed back with a named reader: PR #12596's head has two red checks (
Type Check · debt ledgerfailure at 02:40:49Z, with the aggregateTypeScript Type Checkred on its account; the other 31 runs green/skipped). This is the family the dev's report declared unmeasurable in its worktree (needs the full workspace closure); the queue-entry bar is every-check-green, so the enqueue waits. Reader: thedomain:engineseat's collection loop (its dispatch, its rework round) — likely a ledger re-measure commit. On the next green head this card is enqueue-eligible without further contract review (this PASS covers the reviewed increment; a semantic change to the contract face after this comment re-hangs the label per the standing rule).
Generated by Claude Code
Landing takeover (supersedes the "handed back to the engine seat" line in the PASS comment above — a prose handback is not anyone's inbox, and the maintainer called it): the red on PR #12596 is now diagnosed from the job log and is the author-remedy mechanical class, so the review chain takes it rather than waiting on a seat that may be off shift. Reading, verbatim from
check:type-check-debt --re-measureon the head:@objectstack/objectqlTEST_DEBT records 354, tsc reports 356 (+2);@objectstack/plugin-securityrecords 11, tsc reports 12 (+1) — three raw tsc errors introduced by this PR's two new test suites (the layer the dev's worktree could not measure without the full closure; CI measured it). The gate's own text names the one self-serve remedy: fix the new errors; ⛔ raising a shrink-only ledger is maintainer-only and not taken.Execution: a patch-round dev is dispatched onto the EXISTING branch
claude/issue-12166-reference-cleanup-system-identityunder the interrupted-dev takeover protocol — assignee, prior claims and branch untouched; scope = type-level repair of the three new test-file errors only, stop-and-report if any fix would touch the reviewed contract face (the contract PASS above covers the reviewed increment and stays valid only while that face is unchanged). On the next all-green head this card is enqueue-eligible per the PASS. Skills seat, sessionsession_01MnijPVVDakqK2J335JoJtq.
Generated by Claude Code
- added a commit that references this issue
on Aug 27, 2026 Takeover patch round claimed
- Session:
session_01MnijPVVDakqK2J335JoJtq(patch-round dev, dispatched by the skills seat under the interrupted-dev takeover protocol) - Branch:
claude/issue-12166-reference-cleanup-system-identity— the EXISTING branch, continued. No new branch, no assignee change (stays os-warren), no prior claim replaced or edited. - Scope: type-level repair of the three raw tsc errors the two new pin suites introduced into the TEST_DEBT layer, and nothing else. The reviewed contract face (
packages/spec/src/system/operation-message.ts, the engine's operation semantics) is not touched — if a fix had needed it I would have stopped and reported instead, since the 2026-08-27 contract PASS covers the reviewed increment only. - Not taken: raising the shrink-only ledger. That is maintainer-only; the gate's own self-serve remedy — fix the new errors — is the one executed here.
Report to follow on this card with the
os-dev-reportmarker.
Generated by Claude Code
Generated by Claude Code
- Session:
os-dev-report
{ "issue": 12166, "status": "done", "round": "takeover patch round on the EXISTING branch — no new branch, assignee untouched (os-warren), no prior claim comment edited or replaced, PR draft/ready state not touched", "branch": "claude/issue-12166-reference-cleanup-system-identity", "pr": "https://github.com/objectstack-ai/objectstack/pull/12596", "head": "16d9a430569523a82afb363130a7d1a7aa230c9a — pushed to the same branch, so the PR updated itself", "premise_still_valid": true, "premise_check": "The takeover note's reading was REPRODUCED locally before anything was edited, to the unit and in the same shape as the CI log: @objectstack/objectql TEST_DEBT records 354, tsc reports 356 (+2); @objectstack/plugin-security records 11, tsc reports 12 (+1). All three raw errors sit in the two suites this PR added and nowhere else.", "merge": "origin/main (7af4a20ef) merged into the branch FIRST — clean, zero conflicts (merge commit eb41c41c07). The fix commit sits on top. Every reading below is on the merged tree.", "errors_fixed": [ { "raw": "packages/objectql/src/engine-reference-check-system-identity.test.ts(125,61): error TS2554: Expected 2-5 arguments, but got 1.", "fix": "the beforeEach fixture registration now passes the declared packageId: registerObject(o as any, 'test')" }, { "raw": "packages/objectql/src/engine-reference-check-system-identity.test.ts(176,61): error TS2554: Expected 2-5 arguments, but got 1.", "fix": "same call in the constraint-1 describe block's beforeEach — same repair" }, { "raw": "packages/plugins/plugin-security/src/delete-reference-cleanup-system-identity.test.ts(233,60): error TS2554: Expected 2-5 arguments, but got 1.", "fix": "the boot() helper's fixture registration — same repair" } ], "diagnosis": "ObjectRegistry.registerObject(schema, packageId, namespace?, ownership?, priority?) declares packageId as REQUIRED; the three fixture registrations passed the schema alone. 'test' is the spelling 30 other suites in this workspace already use for that argument (next: 'test-package' at 13), so this is the house convention rather than an invention. Runtime reach of the added argument was checked before choosing it, not after: inside registerObject packageId feeds (a) registerNamespace, gated on a namespace argument these calls do not pass, (b) the owner-conflict branch, which needs a prior owner of the same FQN and cannot fire since each beforeEach builds a fresh engine, and (c) applyProtection, which stamps _packageId and _provenance: 'package'. Only (c) fires. No consumer in either package reads _packageId here, and the two branches that read _provenance test it against 'org', which is false both before and after. Both suites re-run green, which is the measurement rather than the argument.", "scope_honoured": "Type-level only, and the whole diff is three lines in the two test files: 3 insertions, 3 deletions, zero net lines. No source file touched. The reviewed contract face is byte-identical — packages/spec/src/system/operation-message.ts and packages/objectql/src/engine.ts carry NO change from this round, so the 2026-08-27 contract PASS still covers exactly what it reviewed. Confirmed mechanically: `git diff --stat` of the PR against its merge base is still 5 files, 1078 insertions, 5 deletions — the same totals as before this round, because the change is line-neutral. The ledger was NOT raised (maintainer-only) and the runtime -1 informational was NOT lowered (someone else's improvement space, deliberately left).", "tests": "GATE'S OWN COMMAND, BOTH DIRECTIONS, both on the merged tree with the full workspace closure built (`pnpm exec turbo run build --filter='./packages/*' --filter='./packages/*/*'` — 70 tasks successful — which --re-measure refuses to run without). AFTER (fix in place): `pnpm check:type-check-debt` EXIT 0, quoting its own verdict lines — '✓ check:type-check-coverage --self-test — 47 semantic case(s) + 65 observation case(s) + 29 re-measure case(s) + 28 built-closure case(s) + 19 auto-lowering case(s) hold.' and 'check-type-check-coverage --re-measure: OK — 31 ledger entr(ies) re-measured in 250.7s, 1687 raw tsc error(s) total, none above its recorded number.' || BEFORE, taken as an ABLATION so the before-reading comes from the gate itself rather than only from the CI log: the fix was COMMITTED first, then reverted in the worktree, gate re-run, then restored. Prediction written to disk before any mutation (direction RED, exactly the two drift lines with those exact numbers, runtime's informational unchanged). Mutation proven on disk before any result was read, anchored on the exact text (objectql injected 2 / removed-form left 0; plugin-security injected 1 / left 0), under `trap restore EXIT INT TERM` with absolute paths and `git checkout HEAD -- ...` (never a bare checkout). OBSERVED, matching the prediction exactly: EXIT 1, 'check-type-check-coverage --re-measure: 2 ledger entr(ies) drifted upward' / '@objectstack/objectql: TEST_DEBT records 354 raw tsc error(s), `tsc --noEmit` now reports 356 (+2)' / '@objectstack/plugin-security: TEST_DEBT records 11 raw tsc error(s), `tsc --noEmit` now reports 12 (+1)'. Restore proven by an EMPTY `git diff HEAD` on both files plus zero residue of the mutated form and the fixed form present at 2 + 1 sites. || PER-PACKAGE TEST_DEBT numbers, measured with a faithful replication of the gate's own generated re-measure project (same extends, same exclude-minus-test-globs, same include, same default typeRoots, same `tsc --noEmit --pretty false`, same TS6059 drop): plugin-security 12 to 11, objectql 356 to 354 — both landing exactly on the recorded ceilings, with ZERO diagnostics left in either new test file. || SUITES, full, all green: `pnpm --filter @objectstack/plugin-security test` EXIT 0 — 86 test files passed, 1561 tests passed. `pnpm --filter @objectstack/objectql exec vitest run --shard=N/3 --maxWorkers=2` EXIT 0 on each of 3 shards — 80 + 80 + 80 = 240 files passed, 1577 + 1331 + 1300 = 4208 tests passed. || GATE FAMILIES re-derived on the final head via `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack` (no paths — the script takes its own change set; it confirmed 'gate list derived from the tree of objectstack-ai/objectstack at commit 16d9a4305'). The families a test-file edit can move were re-run on this head, exit codes captured before any pipe: check:nul-bytes 0, check:objectql-double-limit 0, check:query-options-erasure 0, check:test-source-alias 0, check:cross-package-test-inputs 0, check:durability-log-level 0, check:page-declaration-shape 0, check:slot-lookup 0, check:type-source-resolution 0, check:published-files 0, check:type-check-coverage 0, node scripts/check-comment-mask-adoption.mjs 0, node scripts/check-plugin-teardown-shape.mjs 0. || ESLINT, narrowed and the narrowing MEASURED rather than asserted: 2 files linted (count read from `--format json`), 0 errors, 0 warnings. The universe is read from eslint's own config, which declares no type-aware program ('no `parserOptions.project`, no typed @typescript-eslint rules', eslint.config line 328) — so a two-file edit cannot move the verdict on any file it does not touch. Three-part evidence complete; this is a measurement, not a skip. || CONTROL-CHARACTER self-scan on both edited files: `grep -naP '[\\x00-\\x08\\x0b\\x0c\\x0e-\\x1f\\x7f]'` — no matches.", "not_measured": "node scripts/check-engine-split-ratio.mjs — REFUSES on this checkout, exit 2, and its own text says why: 'cannot compute the ADR-0076 D7 trigger metric — this clone is shallow and its oldest visible commit on HEAD is 2026-08-01, which sits INSIDE the window'. A checkout precondition, not a finding; CI has the history. It also cannot move on this diff: engine.ts is untouched and the change is line-neutral. || Two more entries were first run under a WRONG COMMAND NAME and reported exit 254 — 'ERR_PNPM_RECURSIVE_EXEC_FIRST_FAIL Command check:comment-mask-adoption not found'. That is a typo of mine, not a red gate; both were re-run by their real invocation (node scripts/...mjs) and are counted green above. Recorded rather than quietly dropped.", "mcp_calls": "5 — issue comment read (1), PR read (1), takeover claim comment (1), this report comment (1), read-back of this comment (1). Card and PR context came through MCP because this seat has no `gh` and REST is 403 for this session; no bulk or wide-table read was taken.", "open_questions": [], "out_of_scope_findings": [], "summary": "CI's `Type Check · debt ledger` red on PR 12596 was three raw tsc TS2554 errors — 'Expected 2-5 arguments, but got 1' — introduced into the TEST_DEBT layer by the two pin suites this PR added: the fixture registrations called ObjectRegistry.registerObject with the schema alone while the signature declares packageId as required. Reproduced locally first, to the unit and in CI's shape (+2 objectql, +1 plugin-security), then repaired by passing the declared packageId at the three call sites and nothing else — 3 lines, zero net lines, no source file, no contract face, so the standing contract PASS still covers exactly the increment it reviewed. The gate's own command now exits 0 ('none above its recorded number'), and an ablation that reverts just those three lines puts back exactly the two drift lines CI reported, which is what makes the fix the cause rather than a coincidence. Both suites remain fully green (objectql 4208 tests over 240 files, plugin-security 1561 over 86). origin/main was merged first, cleanly. Ledger not raised; the runtime -1 informational left alone." }
Generated by Claude Code
Generated by Claude Code
Contract-review verdict refresh — PASS stands on the new head; PR-side label clearing + landing now.
The head of PR #12596 moved after the recorded PASS (takeover patch round: 3 tsc TS2554 fixes in the PR's own two new test files + a clean merge of origin/main; head now
16d9a430). Refresh evidence, tree-verified on the current head:- Contract face byte-identical to what the PASS reviewed:
packages/spec/src/system/operation-message.ts+40/−0 andpackages/objectql/src/engine.ts+305/−5 — same file-level stats as at review time; the patch round was 3 in-place lines in the two test files only (line-neutral, whole-PR stats unchanged at 5 files, +1078/−5). - Reviewer tier machine-read this sub-round:
get_session.external_metadata.last_served_model = claude-fable-5= CONTRACT_REVIEW_TIER. - CI on
16d9a430: 33/33 check runs completed, all success (2 path-filtered skips: Build Docs, Console Pin Gate) — including the previously redType Check · debt ledger, now green with the gate's own verdict line reproduced locally by the patch dev plus a 3-line ablation restoring exactly the two CI drift lines.
Card-side label was already clear (
pm:dispatched+domain:engineonly); completing the split stroke by clearingneeds:contract-reviewon the PR side now, then landing per the review-chain rule (un-governed code PR: pre-landing checks → auto-merge). PR is already ready (draft:false, untouched by the patch round).Fixes #12166closes this card on merge.
Generated by Claude Code
- Contract face byte-identical to what the PASS reviewed:
- added a commit that references this issue
on Sep 1, 2026 - added a commit that references this issue
on Oct 9, 2026
一句话
删除一条记录时,平台的「删除前引用清理」用当前操作人的身份去
find每一张引用表;操作人只要对任何一张引用表没有读权限,整个删除就 403 —— 与记录是否真的被引用无关(空库也 403)。期望这一步用系统身份执行。版本
@objectstack/*@17.2.0(driver-sql / objectql)最小复现
access: private,未授 read);DELETE /api/v1/data/<A>/<id>→ 403PERMISSION_DENIED;前端提示「您没有执行此操作的权限」。服务端日志(删除时刻)
为什么这是问题
期望行为
删除前的引用清理查询以系统身份(isSystem context)执行;操作人的权限只应决定「能不能删这条 A」,不应外溢到引用表的可读性。
出处
应用项目实测记录(含 12 张 UI 截图、A/B 根因验证):steedos-labs/os-project-titanwind-ehr#1809