Skip to content

fix(plugin-sharing): require platform authority to delete a platform-global sharing rule (#7795) - #8156

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-7795-platform-global-sharing-rule-delete-gate
Aug 12, 2026
Merged

fix(plugin-sharing): require platform authority to delete a platform-global sharing rule (#7795)#8156
os-zhuang merged 2 commits into
mainfrom
claude/issue-7795-platform-global-sharing-rule-delete-gate

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes#7795

Implements the maintainer ruling of 2026-08-12 — 方向 B: read/evaluate stay open, delete requires platform authority.

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

What changed

SharingRuleService.deleteRule resolves the row as before, then — for a row whose organization_id is null — requires the caller to carry platform authority. Everything else on the surface is untouched.

callerdelete a platform-global rule
org admin with manage_sharing onlyrefused, PERMISSION_DENIED (HTTP 403)
manage_platform_settings capabilitypermitted
platform_admin positionpermitted
system context (boot, hooks, backfills)permitted
org admin deleting its own org's rulepermitted, unchanged

The guard sits aftergetRule resolves, so a row the caller cannot see at all keeps its existing silent no-op rather than gaining a refusal that would disclose the row's existence.

Both premises re-verified before building

The card was ruled at 03:04Z and plugin-sharing has taken two merges since (#8097, #8120 — neither touched deleteRule; everything here was located by symbol, not by line). Both measurements the ruling rests on were re-run against this build first:

  1. The delete is a revocation wearing removal's clothes. Measured: an org admin deletes the seeded rule, grants drop to 0, then the boot seeder's own defineRule call revives it under a different id (srule_4472403f…srule_050a3107…) with its grants re-materialised. Mechanism: defineRule's existence lookup under a null org is {name}, which matches nothing after the delete, so the insert branch mints a fresh uid('srule').
  2. The safe lever is unavailable while the destructive one is not. Measured: an org admin's active: false produced two rows — the platform-global one still active: true, plus a new org1-stamped row active: false — while deleteRule on the shared row succeeded and purged its grants. The scoped, reversible operation refused; the cross-tenant, irreversible-until-reboot one permitted.

Both reproduce exactly as the card describes, so the ruling's basis is intact.

Which platform-permission mechanism, and why both

The ruling names two, and they are not synonyms — they are two independent channels by which the same underlying grant (an unscoped admin_full_access) reaches an ExecutionContext:

  • manage_platform_settings — a scope: 'platform' capability on context.systemPermissions. admin_full_access carries it; organization_admin deliberately withholds it (it gets only manage_org_users / setup.access / setup.write), which is exactly what makes it a platform-vs-tenant discriminator — the same reasoning plugin-security's PLATFORM_ADMIN_ONLY_CAPABILITIES probe already encodes.
  • platform_admin — a built-in position (ADR-0068 D2) on context.positions, derived by the shared authz resolver from that unscoped grant, never a stored boolean.

A context built by the shared resolver carries both; a hand-built one (the population ADR-0096 D3 is still eliminating, and which plugin-security's probe comment names the sharing service as part of) may carry only one. Accepting either is therefore the fail-safe reading of a ruling that names both, and cannot silently over-refuse a genuine platform operator. The check deliberately does not widen to the whole PLATFORM_ADMIN_ONLY_CAPABILITIES set — that would authorize more than was ruled.

403, not 404 — asserted as two separate facts

The row is deliberately visible (that is what #7760 opened), so a 404 would be the platform lying about a row the caller can list and read one call earlier. The tests assert the ADR-0112 code token exactly, then pair it against the spec's own HttpStatusErrorCodeMap — it must be the code the platform pairs with 403, and must not be either 404-shaped code (RESOURCE_NOT_FOUND or this route's RULE_NOT_FOUND). A 404 regression therefore fails as a named, different assertion instead of sliding through a loose "it threw" check. One test makes the ruling's reason itself executable: the same refused caller can still read the row it may not delete.

Ablation — direction predicted before the fix existed

Predicted: the four refusal tests flip red, every permitted-side and #7760 read-surface pin stays green; plain red, not "more diagnostics" and not inverted. Measured by removing the guard call from deleteRule and re-running:

× refused — 403 PERMISSION_DENIED, by id → expected the call to be REFUSED, but it resolved
× refused — by NAME as well as by id → expected the call to be REFUSED, but it resolved
× refusal leaves the row AND grants intact → promise resolved "undefined" instead of rejecting
× the 404 would be a LIE — caller can still READ → expected the call to be REFUSED, but it resolved
✓ manage_platform_settings authorizes the delete
✓ platform_admin position authorizes the delete
✓ system context still deletes
✓ org admin can still delete its OWN org's rule
✓ #7760 stays whole: LIST / GET / EVALUATE
Tests 4 failed | 79 passed (83)

Red for the right reason — the delete succeeds instead of being refused — and every read-surface pin held. Restored from the commit (git checkout branch -- path), back to 514/514.

One pre-existing fixture re-triaged, not batch re-spelled

[ADR-0111 D6] manage_sharing authorizes the full surface went red. Its RULE_ADMIN persona carried noorganizationId, 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 at all. Disposition: add the missing declaration (the REST layer never builds a tenant admin without an org), so the test keeps measuring its own subject. Not re-spelled away, and not silenced.

Swept the rule's consumption radius rather than the edited package: deleteRule has no other service-level caller — packages/client's test drives a mocked HTTP endpoint, and the ISharingRuleService signature is unchanged.

Verification

  • pnpm --filter @objectstack/plugin-sharing test514 passed (20 files), 15 new
  • pnpm --filter @objectstack/plugin-sharing typecheck — clean
  • Gates re-derived with scripts/pm/dispatch-gates.mjs: check:engine-double-contract (OK — no new fake engine, the existing one 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: check:query-options-erasure (ratchet holds, no files added, test surface at the ceiling) and check:type-check-debt (OK, none above its recorded number — ledger untouched), both run after a full pnpm build closure
  • eslint clean on both changed files; control-byte self-scan clean

Merged origin/main (5 commits) before opening — none touched plugin-sharing, packages/spec did not move, suite re-run green after the merge.

Recorded, not built

The ruling explicitly declines direction D (a per-org suppression mechanism) absent measured demand, and accepts the consequence it names: after this lands, an org admin has no lever at all over a platform-global rule. That consequence is real and is recorded in the guard's docblock — not worked around here.


Generated by Claude Code

…global sharing rule (#7795)
`deleteRule` on a row with `organization_id = null` now refuses a caller who
holds only the org-scoped `manage_sharing` capability, with a
`PERMISSION_DENIED` the REST layer answers as 403. `manage_platform_settings`,
the built-in `platform_admin` position and system contexts stay permitted.
Such a rule belongs to no organization and its criteria query runs unscoped, so
the delete purged every tenant's `sys_record_share` grants under it — a
cross-tenant destructive act authorized by a `scope: 'org'` capability. Both
measurements the ruling rests on were re-verified against this build first: the
boot seeder revives the rule under a NEW id on the next restart (a revocation
wearing removal's clothes), and an org admin's `active: false` creates a second
org-stamped row while the shared rule keeps running (the safe lever refused,
the destructive one permitted).
403 and not 404 is deliberate: #7760 opened listing, reading and evaluating
seeded rules to org admins on purpose, and all three are untouched here, so a
404 would contradict a read the same caller can perform one call earlier.
Maintainer ruling 2026-08-12 (方向 B), quoted verbatim in the guard's docblock.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73
@vercel

vercelBot commented Aug 12, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 12, 2026 6:17pm

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-sharing.

7 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/kernel/runtime-services/examples.mdx(via @objectstack/plugin-sharing)
  • content/docs/kernel/runtime-services/sharing-service.mdx(via @objectstack/plugin-sharing)
  • content/docs/kernel/services-checklist.mdx(via @objectstack/plugin-sharing)
  • content/docs/permissions/authorization.mdx(via packages/plugins/plugin-sharing)
  • content/docs/permissions/permissions-matrix.mdx(via packages/plugins/plugin-sharing)
  • content/docs/plugins/packages.mdx(via @objectstack/plugin-sharing)
  • content/docs/protocol/objectql/security.mdx(via packages/plugins/plugin-sharing)

1 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/implementation-status.mdx(via @objectstack/plugin-sharing)

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 12, 2026
@os-zhuang
os-zhuang marked this pull request as ready for review August 12, 2026 18:39
@os-zhuang
os-zhuang added this pull request to the merge queueAug 12, 2026
Merged via the queue into main with commit 8e7955bAug 12, 2026
26 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-7795-platform-global-sharing-rule-delete-gate branch August 12, 2026 18:54
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

2 participants

@os-zhuang@claude