Skip to content

Decision: may an org-level sharing admin DELETE a platform-global (organization_id = null) sharing rule? #7795

Description

@os-help

Raised from #7761 (PR fixing getRule's by-id org filter). #7761's triage explicitly reserved this second axis for a maintainer ruling rather than a dev's judgment, so it is filed here so the question survives #7761 closing. No behaviour was changed for it — #7761's PR implements only the unambiguous half (another organization's row is unreachable by id).

The question

After #7760 and #7761, a platform-global sharing rule — organization_id = null, written by the boot seeder from declared metadata — is deliberately reachable by an org-scoped admin, by name and by id, for symmetry between the two address forms. Reachable currently means all three verbs, because getRule, evaluateRule and deleteRule share one resolution path.

So an admin holding manage_sharing in one organization may delete a rule that belongs to none, and deleteRule purges every sys_record_share grant it materialised — across every tenant, since a null-org rule's criteria query runs unscoped under SYSTEM_CTX.

Should read/evaluate/delete split for the null-org case?

Why this is not a dev call

manage_sharing is declared scope: 'org' in packages/spec/src/security/capabilities.ts:59. The capability vocabulary already distinguishes org from platform authority — manage_platform_settings and manage_metadata are scope: 'platform', and there is a built-in platform_admin position (ADR-0068 D2) — but SharingRuleService.assertCanManageRules accepts manage_sharing or manage_platform_settings interchangeably and never consults scope. Deciding that an org-scoped capability does not authorize a cross-tenant destructive act, and choosing which capability does, is a contract decision about the authority model, not an implementation detail.

Two measured facts that bear on it

1. The delete is impermanent, so it is a revocation rather than a removal. bootstrapDeclaredSharingRules re-seeds declared rules on every boot (sharing-plugin.ts:568), and the boot backfill re-materialises grants. Deleting a seeded rule therefore does not remove it — it revokes every tenant's grants under it until the next restart, then the rule returns under a new id. That is the profile of an outage, not of an administrative change.

2. The safe lever is unavailable while the destructive one is not. An org admin cannot deactivate a platform-global rule: defineRule's existence lookup is deliberately strict ({name, organization_id: orgId}, held that way by #7676 so one org cannot upsert over a row other orgs read), so active: false from an org admin creates a new org-stamped row and leaves the shared one running. The only lever an org admin has over a platform-global rule today is deleting it. The scoped, reversible operation is refused and the cross-tenant, irreversible-until-reboot one is permitted — the inverse of the safe arrangement.

Options

A. Status quo — read, evaluate and delete all permitted on null-org rows. Cheapest; keeps one resolution path. Leaves an org-level capability authorizing a cross-tenant grant purge, and leaves fact 2's inversion in place.

B. Split: read and evaluate permitted, delete refused unless the caller carries platform authority (manage_platform_settings / the platform_admin position). Preserves everything #7760 shipped — listing, inspection, evaluation of seeded rules — and removes only the cross-tenant destructive act from org-scoped callers. Costs a real split in deleteRule, plus a decision on the error surface (403 PERMISSION_DENIED reads more honestly than a 404 here, since the row is deliberately visible).

C. Refuse delete of null-org rows for every non-system caller. Simplest to reason about and matches fact 1 — the row is owned by the deployment's declared metadata, so the API is not where it should be removed. Strictly less flexible than B, and a genuine platform admin then has no API path to remove a seeded rule.

D. B or C, plus give org admins a scoped opt-out — an org-level "suppress this platform-global rule for my organization" that does not touch the shared row. Fixes fact 2 properly instead of only removing the dangerous lever. Materially more design and a new stored concept, so this needs its own business-need judgment.

Analysis on the three axes

Real business need. Measured, not asserted: nothing in this repo deletes a platform-global rule through the API. The only in-repo delete callers are the REST route and tests; the boot seeder only ever defines. The demand for "an org admin removes a shared seeded rule" is not evidenced anywhere in the example apps, and fact 1 shows it would not even work as intended. The demand that is evidenced is #7676's — admins need to see and evaluate seeded rules — and B and C both keep that whole. Under the startup-focus principle this argues against preserving an unexercised destructive capability, and against D until someone actually asks for per-org suppression.

Long-term soundness. B or C aligns with the direction #7676/#7760 already took: scope reads generously, keep writes strictly owned. Option A is the workaround-shaped choice — it leaves the authority model saying one thing (scope: 'org') while the enforcement seam does another, which is the class of drift ADR-0111 D6 exists to prevent.

Making AI-written / metadata-authored apps hard to get wrong. A platform-global rule is a declared metadata artifact. Letting a tenant-level API call delete a declared artifact — impermanently, so it silently returns at the next boot with a new id — is exactly the kind of surface where an automated admin action produces a confusing half-state. Refusing it loudly at the service boundary is the structural prevention.

Recommendation: B, with C as the acceptable simpler ruling. B keeps every capability #7760 deliberately unlocked, removes only the cross-tenant destructive one from org-scoped callers, and expresses the platform/org distinction the capability registry already declares. I would not do D without evidence of demand for per-org suppression. Whatever is chosen, fact 2 is worth recording: if delete is closed off, an org admin has no lever at all over a platform-global rule, which may itself argue for D later.

Filed unassigned.


Generated by Claude Code

Activity

  1. huangyiirene commented on Aug 12, 2026

    @huangyiirene
    Collaborator

    Maintainer ruling — 2026-08-12

    裁定:方向 B —— read/evaluate 保持开放,delete 需要平台级权限。

    要点:

    • deleteRule 对 organization_id = null 的行,要求调用者持有平台级权限(manage_platform_settings 或 platform_admin 位置);仅持 org 级 manage_sharing 者拒绝。
    • 错误面用 403 PERMISSION_DENIED,不是 404 —— 该行是有意可见的,404 会撒谎。
    • fix(plugin-sharing): make package-seeded (org-null) sharing rules visible and addressable by name for org-scoped admins #7760 开放的能力(列出、查看、评估种子规则)全部保持不动。
    • 依据卡内两条实测:删除是「披着 removal 外衣的 revocation」(重启即复活、换 id),且安全杠杆(停用)对 org 管理员不可用而破坏杠杆反而可用 —— 这是反向的安全排布,收掉破坏杠杆是结构性修复。
    • ⛔ 不做 D(per-org 抑制机制):无任何实测需求。记录在案:本裁定落地后 org 管理员对平台级规则将没有任何杠杆;若未来出现真实的 per-org 抑制需求,另立卡评估,不预建。

    裁定人:维护者 huangyiirene(2026-08-12,接受 PM 综合分析后批准);由 PM 会话 session_01GZKbx4xyF7U5WXj6ch49BM 代笔落卡。转 pm:queue。


    Generated by Claude Code

  2. self-assigned this
    on Aug 12, 2026
  3. os-zhuang commented on Aug 12, 2026

    @os-zhuang
    Contributor

    Claim: PM loop round 5 (domain:identity seat #6022)
    Session: session_01PEVB6w7D7uCszR9Mw1BL73
    Branch: claude/issue-7795-platform-global-sharing-rule-delete-gate
    Worktree: objectstack-issue-7795
    Domain: domain:identity
    File surface: packages/plugins/plugin-sharing/src/sharing-rule-service.ts — deleteRule and its permission check — plus tests and a changeset.
    Container & model: M, mode:subagent, model: opus — small diff, but it adds a refusal on a destructive path, and the error-surface choice is part of the ruling.
    Serial constraints cleared: plugin-sharing is free. PR #8120 (#7858) MERGED and landing-verified; PR #8097 (#7807) merged earlier this shift. Zero dev agents in flight in this lane.

    ⚠️ The title says "Decision" but this card is RULED — it carries pm:queue, not needs-user-decision. A successor scanning titles would misread it as pending; the ruling is the 2026-08-12 comment above.

    The ruling this executes (维护者 2026-08-12) — quoted verbatim, untranslated

    裁定:方向 B —— read/evaluate 保持开放,delete 需要平台级权限。

    • deleteRule 对 organization_id = null 的行,要求调用者持有平台级权限(manage_platform_settings 或 platform_admin 位置);仅持 org 级 manage_sharing 者拒绝。
    • 错误面用 403 PERMISSION_DENIED,不是 404 —— 该行是有意可见的,404 会撒谎。
    • fix(plugin-sharing): make package-seeded (org-null) sharing rules visible and addressable by name for org-scoped admins #7760 开放的能力(列出、查看、评估种子规则)全部保持不动。
    • 依据卡内两条实测:删除是「披着 removal 外衣的 revocation」(重启即复活、换 id),且安全杠杆(停用)对 org 管理员不可用而破坏杠杆反而可用 —— 这是反向的安全排布,收掉破坏杠杆是结构性修复。
    • ⛔ 不做 D(per-org 抑制机制):无任何实测需求。记录在案:本裁定落地后 org 管理员对平台级规则将没有任何杠杆;若未来出现真实的 per-org 抑制需求,另立卡评估,不预建。

    Three things in that ruling that are easy to under-implement

    1. 403, not 404 — and the reason is load-bearing. The row is deliberately visible (that is what fix(plugin-sharing): make package-seeded (org-null) sharing rules visible and addressable by name for org-scoped admins #7760 opened), so a 404 would be a lie the platform tells about a row the caller can list and read one call earlier. ⛔ Do not "harden" it into a 404.
    2. fix(plugin-sharing): make package-seeded (org-null) sharing rules visible and addressable by name for org-scoped admins #7760's read/evaluate surface stays open — listing, viewing and evaluating seed rules are the regression surface, not collateral. A fix that gates the whole organization_id = null class rather than just delete reverses a shipped decision.
    3. ⛔ Do not build direction D. The ruling names the consequence it is knowingly accepting — after this lands, org admins have no lever over platform-level rules — and rules out pre-building a suppression mechanism. If the dev feels the absence of a lever is a gap, that is a report, not a rider.

    Premise re-verification is the dev's first action

    This card was ruled at 03:04Z; plugin-sharing has taken two merges from this lane since (#8097 rewrote expandRecipient and business-unit-graph.ts; #8120 added federated-phantom-anchors.ts and guarded both filters). Neither should have touched deleteRule, but line anchors have drifted twice in this package today — locate by symbol, and confirm the two measurements the ruling rests on (delete-is-revocation: restart revives, id changes; and the disable lever being unavailable to org admins while the destructive one is available) still hold before building on them.


    Generated by Claude Code

  4. os-zhuang commented on Aug 12, 2026

    @os-zhuang
    Contributor
    {
      "issue": 7795,
      "status": "done",
      "branch": "claude/issue-7795-platform-global-sharing-rule-delete-gate",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/8156",
      "premise_still_valid": true,
      "summary": "Implemented the 2026-08-12 ruling (方向 B): SharingRuleService.deleteRule now refuses an organization_id = null row unless the caller carries platform authority — the manage_platform_settings capability or the platform_admin position — while system contexts and org admins deleting their OWN org's rows are unaffected. The refusal is PERMISSION_DENIED, which rest-server.ts's sharing-rule handleError answers as 403; the guard sits AFTER getRule resolves, so a row the caller cannot see keeps its existing silent no-op instead of gaining a refusal that would disclose it. #7760's read surface (list/get/evaluate of seeded rules) is untouched and pinned as a regression surface. Direction D was NOT built, per the ruling. Both premises re-verified against this build BEFORE implementing (see tests): the delete is a revocation wearing removal's clothes (restart revives the rule under a new id with grants re-materialised), and the safe lever is unavailable while the destructive one is not (an org admin's active:false creates a second org-stamped row while the shared rule keeps running). On PM assumption #3 — the ruling's two spellings are NOT synonyms but two independent channels carrying the same unscoped admin_full_access grant onto ExecutionContext: manage_platform_settings arrives on systemPermissions (admin_full_access carries it, organization_admin deliberately withholds it), platform_admin arrives on positions as a resolver-derived built-in. A resolver-built context carries both; a hand-built one may carry only one, so the check accepts EITHER — the fail-safe reading, and deliberately not widened to plugin-security's whole PLATFORM_ADMIN_ONLY_CAPABILITIES set, which would authorize more than was ruled. Everything was located by symbol, not by line; confirmed #8097 and #8120 did not touch deleteRule.",
      "tests": "pnpm --filter @objectstack/plugin-sharing test => 'Test Files 20 passed (20) / Tests 514 passed (514)' (15 new). typecheck (tsc --noEmit) clean. PREMISE PROBE (throwaway file, deleted after; predictions written first): premise (a) '[PROBE a] id1= srule_4472403f-5c07-447a-9d9f-f758a0ab623d  id2= srule_050a3107-a647-472b-b4b2-72f6de89ec1c  same? false  grants after reboot= 1'; premise (b) '[PROBE b] rows named SEEDED after org-admin active:false = [{\"org\":null,\"active\":true},{\"org\":\"org1\",\"active\":false}]' and '[PROBE b] org admin deleted platform-global row. survives? false  grants= 0'. Both reproduce exactly as the card describes. ABLATION (direction predicted before the fix existed: 4 refusal tests RED, all permitted-side and #7760 pins GREEN; plain red, not inverted, not more-diagnostics) — measured by removing the guard call: 'Tests 4 failed | 79 passed (83)', the four being the by-id refusal, the by-name refusal, the row+grants-survive pin and the 404-would-be-a-lie pin, each failing with 'expected the call to be REFUSED, but it resolved successfully' / 'promise resolved \"undefined\" instead of rejecting' — red for the right reason. Restored via git checkout branch -- path (never stash), back to 514/514. Rejection cases assert code AND status distinctly: the ADR-0112 code token is parsed exactly and paired against the spec's own HttpStatusErrorCodeMap[403], and asserted NOT equal to either 404-shaped code (RESOURCE_NOT_FOUND and this route's RULE_NOT_FOUND), so a 404 regression fails as a named different assertion. The refused persona is an ORG-LEVEL admin holding manage_sharing — the exact caller the ruling refuses and the only one that can fail against the pre-fix build. GATES: dispatch-gates.mjs re-derived for the final surface — check:engine-double-contract OK (no new fake engine; the existing makeEngine is reused), check:test-source-alias, check:changeset-gate-self-tests, check:objectui-changeset, check:docs-audit-scope, check:nul-bytes, check-changeset-no-major all green. The two that do NOT appear in that list, run after a full pnpm build closure: check:query-options-erasure => 'ratchet holds: 67 unswept non-test site(s) in 17 file(s), none new ... baseline key set verified against 37b82ed: no files added' (test surface at the ceiling, unchanged — no new erasure sites since the rule only fires on find/findOne/count/aggregate); check:type-check-debt => 'OK — 33 ledger entr(ies) re-measured, 1966 raw tsc error(s) total, none above its recorded number' — ledger NOT raised (the surplus notes it prints are pre-existing entries in other packages, none in plugin-sharing). eslint clean on both changed files; control-byte self-scan clean. Merged origin/main (5 commits) before opening the PR — none touched plugin-sharing and packages/spec did not move, so the re-check was scoped to the overlap per AGENTS.md §10; suite re-run green after the merge. CI gate conclusions on the PR are in_progress at report time, per the report-at-draft-PR-time contract.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #8158: plugin-sharing's adminOrgScope falls open when the caller has no org id (`if (!orgId) return where`) — that branch exists for SYSTEM_CTX but is reached on CAPABILITY, so an authenticated holder of org-scoped manage_sharing whose session carries no activeOrganizationId reads (and can evaluate, i.e. write grants for) every tenant's sharing rules. resolve-authz-context only stamps tenantId when truthy, so such a context is constructible. Same class as #7761 through a different door. Reachability in a real deployment is explicitly NOT measured — flagged in the issue as the first thing triage should settle. Unassigned, unlabeled.",
        "not filed, folded into this PR as a test-fixture correction: the pre-existing [ADR-0111 D6] 'manage_sharing authorizes the full surface' fixture built its RULE_ADMIN persona with no organizationId, so defineRule stamped organization_id: null and the rule it created to exercise the CAPABILITY gate was incidentally platform-global — a shape no org admin can author through the API. Disposition was 'add the missing declaration' (the REST layer never builds a tenant admin without an org), keeping the test on its own subject rather than re-spelling it away."
      ]
    }

    Notes for the PM that do not fit the schema:

    • The card's title still begins with "Decision". It is ruled and now implemented; a successor scanning titles would still misread it. Worth retitling on close.
    • The ruling's recorded consequence is now live: with delete closed, an org admin has no lever at all over a platform-level rule. That is recorded verbatim in the guard's docblock, not worked around. Direction D was not built.
    • PR fix(plugin-sharing): require platform authority to delete a platform-global sharing rule (#7795) #8156 is a draft and carries a changeset, so no skip-changeset label applies. I did not flip it to ready or arm auto-merge.

    Generated by Claude Code

  5. os-zhuang commented on Aug 12, 2026

    @os-zhuang
    Contributor

    PM review: ACCEPT — PR #8156. Seat #6022, session session_01PEVB6w7D7uCszR9Mw1BL73. CI convergence is mine.

    Verified against GitHub

    check result
    Path face ✅ 3 files, zero fork triggers
    403 PERMISSION_DENIED, not 404 ✅ — and the test asserts the code is not equal to either 404-shaped code (RESOURCE_NOT_FOUND, RULE_NOT_FOUND), so a 404 regression fails as a named different assertion
    ⛔ direction D absent ✅ — the one "suppression" hit in the diff is a comment declining it: "The ruling explicitly declines to pre-build a per-org suppression mechanism (「⛔ 不做 D」) absent measured demand — do not add one here."
    #7760's read surface ✅ untouched and pinned as a regression surface
    Changeset ✅

    You measured both of the ruling's premises before building on them

    The ruling rested on two claims from the card. You probed both, first, with predictions written down:

    • (a) delete is revocation wearing removal's clothes — id1 ≠ id2 after restart, grants after reboot = 1. The rule comes back under a new id with grants re-materialised.
    • (b) the safe lever is unavailable while the destructive one is not — an org admin's active: false produced [{org: null, active: true}, {org: "org1", active: false}], i.e. a second, org-stamped row while the shared rule keeps running; the delete, by contrast, worked.

    ⚠️ A ruling whose premises have quietly expired is worse than no ruling, because it still reads as authoritative. Confirming them costs one probe file and you deleted it after. That is the right shape.

    Assumption #3 — you didn't just answer it, you found the check it implies

    I asked which mechanism to use, since the ruling names both manage_platform_settings and platform_admin. Your finding:

    they are NOT synonyms but two independent channels carrying the same unscoped admin_full_access grant onto ExecutionContext — the capability arrives on systemPermissions, the position on positions as a resolver-derived built-in. A resolver-built context carries both; a hand-built one may carry only one.

    So the check accepts either — the fail-safe reading, since requiring both would refuse a legitimately-authorized caller whose context was assembled differently. And you declined to widen to PLATFORM_ADMIN_ONLY_CAPABILITIES, which would have authorized more than was ruled. That restraint is the part I would have had to catch, and you got there first.

    The guard's placement is a disclosure decision, and you made the right one

    the guard sits AFTER getRule resolves, so a row the caller cannot see keeps its existing silent no-op instead of gaining a refusal that would disclose it

    Placing it before the lookup would have turned "this row does not exist for you" into "this row exists and you may not touch it" — leaking the existence of rows outside the caller's visibility, in a PR whose entire purpose is tightening a security boundary. Not mentioned in the ruling, correctly decided anyway.

    The docs question — no change, and the reason is a phrase collision worth recording

    I flagged that a card adding a refusal has an inverted docs risk: if any page promises org admins this capability, the prose keeps promising a lever the ruling knowingly removes.

    Three pages do contain "platform-global" — sharing-rules.mdx:63, permissions-matrix.mdx:47, rls.mdx:84. ⚠️ All three use it in a different sense: tenancy.enabled: false, an object's tenancy posture, not a sharing rule with organization_id = null. Nothing documents org-admin deletion of platform-global rules, so there is nothing to update.

    Recording it because the collision is a live trap: the same phrase means two things in this repo, and a grep-and-glance review would have "found" a docs conflict that does not exist — the mirror image of the count-based false alarms this lane hit three times today.

    #8158

    Filed unassigned and unlabelled, with reachability explicitly not measured and flagged as the first thing triage should settle. adminOrgScope falling open when the caller has no org id — reached on CAPABILITY, not just SYSTEM_CTX — is a cross-tenant read (and evaluate) of every tenant's sharing rules if constructible. ⛔ Grading is triage's channel; I am not pre-empting it, but I will name it in the round report because "same class as #7761 through a different door" is the kind of pattern that deserves to be seen next to its sibling.

    The folded-in fixture correction is right too: an ADR-0111 D6 fixture whose RULE_ADMIN had no organizationId was incidentally authoring platform-global rules — a shape no org admin can create through the API. Adding the missing declaration keeps that test on its own subject instead of re-spelling it away.

    Next (mine)

    Per-job gate conclusions, then flip ready → arm, in that order.


    Generated by Claude Code

  6. os-zhuang commented on Aug 12, 2026

    @os-zhuang
    Contributor

    Armed — PR #8156, 18:40Z, and in the merge queue. Seat #6022.

    All jobs completed, zero failures, read per job: Test Core (1/3) ✅ 18:24Z · (2/3) ✅ 18:25Z · (3/3) ✅ 18:22Z + rollup ✅ · TypeScript Type Check ✅ 18:27Z · ESLint ✅ 18:20Z · Dogfood Regression Gate (1–3/3) + rollup ✅ · Temporal Conformance (live PG + MySQL) ✅ · Build Core · Dogfood Verify CLI · Check Changeset ✅.

    Pre-arm: head confirmed 5967b569 (the commit CI ran on), path face zero fork triggers, 4 commits behind main so a read-only merge-tree dry run — clean. Then draft: false → enable_pr_auto_merge.

    Held until MERGED. The landing check will verify content on origin/main, and this card's negative space is unusually load-bearing — three things that must be true and are each satisfiable by a worse implementation:

    1. the refusal exists for organization_id = null rows;
    2. ⛔ the code is 403 PERMISSION_DENIED, not any 404 shape — the row is deliberately visible, so a 404 would be the platform lying about something it just listed;
    3. ⛔ the guard is still after getRule — moving it earlier turns "does not exist for you" into "exists, hands off", leaking row existence from inside a PR that tightens a boundary;
    4. ⛔ no per-org suppression mechanism (direction D), which the ruling declined by name.

    Generated by Claude Code

  7. os-zhuang commented on Aug 12, 2026

    @os-zhuang
    Contributor

    落地确认 —— PR #8156 已合并(8e7955b7e)。裁定的四项逐条在合并后的 origin/main 上核过,读的是命中处本身,不是 PR 描述:

    裁定项 核验结果
    平台级权限方可删除 organization_id = null 的行 ✅ assertCanDeletePlatformGlobalRule:row.organization_id != null 直接放行(org 自己的行不受影响),isSystem 放行,manage_platform_settings 能力放行,BUILTIN_IDENTITY_PLATFORM_ADMIN 位置放行,其余抛出
    错误面 403 PERMISSION_DENIED,不是 404 ✅ 抛的是 PERMISSION_DENIED: 开头。同文件里确实另有 RULE_NOT_FOUND(:480),但那是 evaluateRule 的既有分支,不在本路径上
    guard 在 getRule 之后 ✅ deleteRule:assertCanManageRules → getRule → if (!row) return; → 然后才是本 guard
    ⛔ 不做方向 D ✅ 无 per-org 抑制机制。docblock 里出现的 "suppression" 是解释为何不建的散文,不是实现

    最后一行值得单独说,因为它正是本班反复踩到的形状:被禁用项的字面出现,可能恰恰是解释「为何不用它」的散文。 只按关键词命中判断,这里会得出"方向 D 被建了"的相反结论。本班五次险些误报,全部发生在正确的合并上——所以判据一律是去读那一处命中,而不是数命中。

    裁定接受的那个后果也被如实记进了 guard 的 docblock,没有被绕过:本 PR 之后,org 管理员对平台级规则不再有任何操作杠杆。read / list / evaluate(#7760 开放的面)完整保留,并有 pin 守着。


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions