Skip to content

Commit 8e7955b

Browse files
os-zhuangclaude
andauthored
fix(plugin-sharing): require platform authority to delete a platform-global sharing rule (#7795) (#8156)
`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. Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73 Co-authored-by: Claude <noreply@anthropic.com>
1 parent cc63bc7 commit 8e7955b

3 files changed

Lines changed: 391 additions & 1 deletion

File tree

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
---
2+
'@objectstack/plugin-sharing': patch
3+
---
4+
5+
Refuse deletion of a platform-global sharing rule to org-scoped callers (#7795)
6+
7+
`SharingRuleService.deleteRule` now requires **platform** authority to delete a
8+
sharing rule whose `organization_id` is `null` — a row seeded from declared
9+
metadata that belongs to no organization. A caller holding only the org-scoped
10+
`manage_sharing` capability is refused with `PERMISSION_DENIED`, which the REST
11+
layer answers as **403**; the `manage_platform_settings` capability, the
12+
built-in `platform_admin` position, and system contexts are all still permitted.
13+
14+
Why: such a rule's criteria query runs unscoped, so deleting it purged **every**
15+
tenant's `sys_record_share` grants under it — a cross-tenant destructive act
16+
authorized by a capability declared `scope: 'org'`. Two measured facts made it
17+
worse: the boot seeder re-creates the rule on the next restart under a *new* id,
18+
so the delete was a revocation wearing removal's clothes rather than a removal;
19+
and the safe lever was unavailable while the destructive one was not — an org
20+
admin's `active: false` creates a second, org-stamped row and leaves the shared
21+
rule running, so deletion was the only lever an org admin had over it.
22+
23+
Deliberately **403, not 404**: the row is intentionally visible — listing,
24+
reading and evaluating platform-global rules stay open to org admins, exactly as
25+
shipped — so answering "no such rule" would contradict a read the same caller
26+
can perform one call earlier. Nothing on the read/evaluate surface changes.

packages/plugins/plugin-sharing/src/sharing-rule-service.ts

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@ import type {
1212
// has declared for every one of these context parameters since #6523 (the
1313
// #6206 ruling: no per-site subset contracts).
1414
import type { ExecutionContext } from '@objectstack/spec/kernel';
15+
// [#7795] The built-in platform-operator position (ADR-0068 D2) — one of the
16+
// two spellings of platform authority the ruling names; see
17+
// {@link SharingRuleService.assertCanDeletePlatformGlobalRule}.
18+
import { BUILTIN_IDENTITY_PLATFORM_ADMIN } from '@objectstack/spec/identity';
1519
import type { SharingEngine } from './sharing-service.js';
1620
import type { SharingService } from './sharing-service.js';
1721
import { normalizeAccessLevel, normalizeStoredAccessLevel } from './access-level.js';
@@ -116,6 +120,111 @@ export class SharingRuleService implements ISharingRuleService {
116120
);
117121
}
118122

123+
/**
124+
* [#7795] DELETING a platform-global (`organization_id = null`) rule requires
125+
* PLATFORM authority. Org-scoped `manage_sharing` does not authorize it.
126+
*
127+
* Maintainer ruling, 2026-08-12 (方向 B), quoted verbatim and untranslated:
128+
*
129+
* > **裁定:方向 B —— read/evaluate 保持开放,delete 需要平台级权限。**
130+
* >
131+
* > - `deleteRule` 对 `organization_id = null` 的行,要求调用者持有平台级权限
132+
* > (`manage_platform_settings` 或 `platform_admin` 位置);仅持 org 级
133+
* > `manage_sharing` 者拒绝。
134+
* > - 错误面用 **403 `PERMISSION_DENIED`**,不是 404 —— 该行是有意可见的,
135+
* > 404 会撒谎。
136+
* > - #7760 开放的能力(列出、查看、评估种子规则)全部保持不动。
137+
*
138+
* ## Why only DELETE, and only this row class
139+
*
140+
* `manage_sharing` is declared `scope: 'org'` in the spec's capability
141+
* registry, but a null-org rule belongs to no organization: its criteria
142+
* query runs unscoped under {@link SYSTEM_CTX}, so {@link deleteRule}'s
143+
* grant purge revokes EVERY tenant's `sys_record_share` rows, not just the
144+
* caller's. That is the one act on this surface an org-level capability
145+
* should not reach, and the two measurements the ruling rests on say why —
146+
* both re-verified against this build before the guard was written:
147+
*
148+
* 1. **The delete is a revocation wearing removal's clothes.**
149+
* `bootstrapDeclaredSharingRules` re-seeds declared rules on every boot,
150+
* and {@link defineRule}'s existence lookup under a null org is `{name}` —
151+
* which matches nothing once the row is deleted, so the insert branch
152+
* mints a fresh `uid('srule')`. Measured: the rule returns after a
153+
* restart under a DIFFERENT id, with its grants re-materialised. The
154+
* profile of an outage, not of an administrative change.
155+
* 2. **The safe lever is unavailable while the destructive one is not.**
156+
* An org admin cannot deactivate this row: `defineRule`'s existence
157+
* lookup is deliberately strict (`{name, organization_id: orgId}`, held
158+
* that way by #7676 so one org cannot upsert over a row other orgs read),
159+
* so `active: false` from an org admin creates a SECOND, org-stamped row
160+
* and leaves the shared one running. Measured: two rows, the null-org one
161+
* still `active: true`. Scoped + reversible refused, cross-tenant +
162+
* irreversible-until-reboot permitted — the inverse of the safe
163+
* arrangement, and closing the destructive lever is the structural fix.
164+
*
165+
* ## Two spellings of platform authority, and why BOTH are accepted
166+
*
167+
* They are not synonyms — they are two independent channels by which the
168+
* SAME underlying grant (an unscoped `admin_full_access`) reaches an
169+
* `ExecutionContext`:
170+
*
171+
* - `manage_platform_settings` — a `scope: 'platform'` CAPABILITY, arriving
172+
* on `context.systemPermissions`. `admin_full_access` carries it;
173+
* `organization_admin` deliberately withholds it (it gets only
174+
* `manage_org_users` / `setup.access` / `setup.write`), which is exactly
175+
* what makes it a discriminator between a platform operator and a tenant
176+
* admin — the same reasoning plugin-security's
177+
* `PLATFORM_ADMIN_ONLY_CAPABILITIES` probe encodes.
178+
* - `platform_admin` — a built-in POSITION (ADR-0068 D2), arriving on
179+
* `context.positions`, DERIVED by the shared resolver from the unscoped
180+
* `admin_full_access` user grant (never a stored boolean).
181+
*
182+
* A context built by the shared authz resolver carries both. A HAND-BUILT
183+
* context — the population ADR-0096 D3 is still eliminating, and which
184+
* plugin-security's probe comment names the sharing service as part of —
185+
* may carry only one. Accepting either is therefore the fail-safe reading of
186+
* a ruling that names both, and refusing on the absence of both cannot
187+
* silently over-refuse a genuine platform operator.
188+
*
189+
* ## The error surface is 403 `PERMISSION_DENIED`, deliberately not 404
190+
*
191+
* The row is DELIBERATELY visible: #7760 opened listing, reading and
192+
* evaluating seeded rules to org admins on purpose, and this guard leaves
193+
* all three untouched. A 404 here would be the platform lying about a row
194+
* the caller could list and read one call earlier. The message prefix is
195+
* what `rest-server.ts`'s sharing-rule `handleError` maps to HTTP 403 +
196+
* `{code: 'PERMISSION_DENIED'}`, which is also the pairing the spec's own
197+
* `HttpStatusErrorCodeMap[403]` records. ⛔ Do not "harden" this into a 404
198+
* or a silent no-op — both re-open the lie this shape exists to avoid.
199+
*
200+
* Placed AFTER {@link getRule} resolves, so a row the caller cannot see at
201+
* all keeps its existing silent-no-op behaviour rather than gaining a new
202+
* refusal that would disclose the row's existence.
203+
*
204+
* ⚠️ Recorded consequence, accepted by the ruling: with delete closed, an org
205+
* admin has NO lever at all over a platform-global rule. The ruling
206+
* explicitly declines to pre-build a per-org suppression mechanism
207+
* (「⛔ 不做 D」) absent measured demand — do not add one here.
208+
*/
209+
private assertCanDeletePlatformGlobalRule(row: SharingRuleRow, context: ExecutionContext): void {
210+
// Only the platform-global class is gated — an org's own rows are
211+
// untouched, and so is every read verb.
212+
if (row.organization_id != null) return;
213+
// Boot seeding, hooks, backfills and the plugin machinery, as everywhere else.
214+
if (context?.isSystem) return;
215+
const caps = Array.isArray(context?.systemPermissions) ? context.systemPermissions : [];
216+
if (caps.includes('manage_platform_settings')) return;
217+
const positions = Array.isArray(context?.positions) ? context.positions : [];
218+
if (positions.includes(BUILTIN_IDENTITY_PLATFORM_ADMIN)) return;
219+
throw new Error(
220+
'PERMISSION_DENIED: deleting a platform-global sharing rule requires platform authority — ' +
221+
'the manage_platform_settings capability or the platform_admin position. Org-scoped ' +
222+
'manage_sharing does not authorize it, because this rule belongs to no organization and ' +
223+
'deleting it revokes every tenant’s grants under it (#7795). It remains listable, ' +
224+
'readable and evaluable.',
225+
);
226+
}
227+
119228
async defineRule(input: DefineSharingRuleInput, context: ExecutionContext): Promise<SharingRuleRow> {
120229
this.assertCanManageRules(context);
121230
if (!input.name) throw new Error('VALIDATION_FAILED: name is required');
@@ -336,6 +445,9 @@ export class SharingRuleService implements ISharingRuleService {
336445
this.assertCanManageRules(context); // [ADR-0111 D6]
337446
const row = await this.getRule(idOrName, context);
338447
if (!row) return;
448+
// [#7795] A platform-global row is visible to an org admin by design
449+
// (#7760) but is NOT theirs to destroy — 403, never 404.
450+
this.assertCanDeletePlatformGlobalRule(row, context);
339451
// Drop materialised grants first so we don't orphan them.
340452
//
341453
// [#4434] This used to be a predicate-shaped `engine.delete` on

0 commit comments

Comments
 (0)