fix(sharing): 停用规则时撤回已物化授权,并修复 DELETE 500 (#4433, #4434) (#4495)

* wip: fix sharing-rule withdrawal (#4433) and DELETE 500 (#4434)

An over-granting sharing rule had no withdrawal path on the API surface:
deactivating it never withdrew its materialized grants, and the DELETE
route answered 500 for both address forms. RC-exit blockers, since v17
advertises "switching a rule off actually withdraws access".

#4434 — deleteRule purged sys_record_share with a predicate-shaped
engine.delete carrying neither a scalar id nor multi:true, the one shape
the engine's dispatch refuses. It threw before ever reaching the rule
row, so every DELETE /sharing/rules/:idOrName 500'd. Routed through
purgeRuleGrants instead of adding multi:true, so a rule's grants retire
exactly one way — SharingService.revoke, as every other withdrawal path
already does (AGENTS.md PD #5).

#4433 — three independent gaps, one per path the issue walked:
  - deactivation: the rule-write trigger skipped reconcile on every
    isSystem write. defineRule (the only implementation behind
    POST /sharing/rules) writes with SYSTEM_CTX unconditionally, so the
    skip caught 100% of REST authoring. Now gated on boot phase, the
    question the skip actually meant to ask.
  - record touch: evaluateAllForRecord listed activeOnly rules, so a
    deactivated rule was absent from the loop and its grants were never
    examined. Lists every rule; inactive ones desire nothing and take
    reconcileForRecord's existing revoke branch.
  - boot: the backfill was handed activeOnly rules, making it
    structurally incapable of revoking. Walks every rule, plus a new
    sweepOrphanedRuleGrants for grants whose rule row is gone entirely
    (unreachable by rule iteration).

Test fakes: makeEngine().delete now mirrors the real engine's dispatch
guard. The looser fake is why #4434 shipped green — the pre-existing
"deleteRule drops rule + all its grants" test passed against a delete
the running server always rejected.

Progress: service + plugin fixes done, regression tests added for both
issues. Remaining: rule-rebind.test.ts still pins the old isSystem skip
and needs updating; full gates + live-server verification pending.

Refs #4433, #4434

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017gEHJN2NFpS9VMeURvakgD

* test(sharing): pin the boot-phase reconcile contract (#4433)

rule-rebind.test.ts asserted the defect: 'skips system-context writes'
passed a mocked session the real REST path never sends, then asserted
the reconcile did NOT happen. Replaced with the corrected contract —
deferral is about WHEN a write happens (before the kernel:bootstrapped
backfill), not WHO made it — plus cases for both session kinds after
boot and the seeding deferral during it.

pnpm --filter @objectstack/plugin-sharing test: 243 passed (11 files).

Refs #4433

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017gEHJN2NFpS9VMeURvakgD

* docs(sharing): document the withdrawal lifecycle + add changeset

Live verification on a persistent SQLite datastore confirmed all three
withdrawal moments and both DELETE address forms, so the guarantee is now
worth stating: an over-granting sharing rule is always recoverable from
the API surface.

The CI docs-drift advisory flagged six hand-written pages against this
change. None asserted anything false — the withdrawal timing was simply
undocumented, which is how the behaviour drifted unnoticed in the first
place. Added the missing section to permissions/sharing-rules.mdx (the
three reconcile moments, delete-by-id-or-name, the boot sweep for grants
whose rule row is gone) and sharpened permissions-matrix.mdx, which said
rules re-evaluate on record insert/update and stopped there — true but
partial now that rule writes and boot also reconcile. The other four
flagged pages only list the plugin or its layer and needed no change.
content/docs/references/ is generated and was not touched.

Changeset: minor on @objectstack/plugin-sharing. Not patch — a
`source: 'rule'` grant whose rule is inactive or gone now disappears on
upgrade, and DELETE starts succeeding where it used to 500, so callers
that treated that 500 as "unsupported" will now really delete.

Refs #4433, #4434

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017gEHJN2NFpS9VMeURvakgD

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: os-zhuang

.changeset/sharing-rule-withdrawal-and-delete.md

@@ -0,0 +1,55 @@
+---
+"@objectstack/plugin-sharing": minor
+---
+
+fix(plugin-sharing): deactivating or deleting a sharing rule actually withdraws its grants (#4433, #4434)
+
+An over-granting sharing rule had no withdrawal path on the product's API
+surface. Deactivating it left every grant it had materialised in place — not on
+the next record touch, not after a full restart — and the DELETE route answered
+500 for both address forms it advertises, so the rule could not be removed
+either. Together that made a too-broad rule unrecoverable short of hand-editing
+`sys_record_share`, against a v17 release note that advertises the opposite
+("switching a rule off actually withdraws access").
+
+`minor`, not `patch`: this changes an observable runtime behaviour that
+deployments may have adapted to. A `source: 'rule'` grant whose rule is
+inactive — or whose rule row is gone — now disappears, on the deactivating
+write, on the next touch of the record, and on the next boot. Anything relying
+on those rows surviving deactivation (including data repaired by hand around
+the old behaviour) will see them revoked on upgrade. `DELETE
+/api/v1/sharing/rules/:idOrName` also starts succeeding where it used to 500,
+so callers that treated that 500 as "unsupported" will now really delete.
+
+#4433 — three independent gaps, one per path the report walked:
+
+- **The deactivating write.** The `sys_sharing_rule` reconcile trigger skipped
+  every `isSystem` write, on the theory that those were boot seeding.
+  `SharingRuleService.defineRule` — the only implementation behind
+  `POST /sharing/rules`, and the documented way to deactivate a rule — writes
+  with SYSTEM_CTX unconditionally, because it must reach a platform table the
+  sharing middleware otherwise gates. So the skip caught 100% of REST
+  authoring: the withdrawal path built by #3821 existed, had tests (against a
+  mocked session the real path never sends), and was unreachable in production.
+  Now gated on boot phase, which is the question the skip actually meant to
+  ask.
+- **The record touch.** `evaluateAllForRecord` listed only active rules, so a
+  deactivated rule was absent from the loop entirely and its grants were never
+  examined. It now reconciles every rule; an inactive one desires nothing and
+  takes the existing revoke-the-remainder branch.
+- **The boot pass.** `backfillRuleGrants` was handed an `activeOnly` list,
+  making it structurally incapable of revoking anything. It now walks every
+  rule, and a new `sweepOrphanedRuleGrants` retires grants whose rule row is
+  gone entirely — unreachable by rule iteration, so they need their own sweep.
+
+#4434 — `deleteRule` purged `sys_record_share` with a predicate-shaped
+`engine.delete` carrying neither a scalar id nor `multi: true`, the one shape
+the engine's dispatch refuses; it threw before ever reaching the rule row.
+Fixed by routing through the same `SharingService.revoke` path every other
+withdrawal already uses, rather than adding `multi: true` — a rule's grants now
+retire exactly one way instead of two divergent ones.
+
+The unit fakes are part of the fix: `makeEngine().delete` accepted any `where`,
+which is why #4434 shipped green — the pre-existing "deleteRule drops rule and
+all its grants" test asserted success against a delete the running server
+always rejected. The fakes now mirror the real engine's dispatch guard.

content/docs/permissions/permissions-matrix.mdx

@@ -161,7 +161,7 @@ Sharing rules extend access beyond ownership and the depth axis. The declarative
 </Callout>
 
 <Callout type="info">
-**Beyond declarative rules:** Two other sharing mechanisms exist but are **not** `SharingRule` types. **Manual sharing** is a runtime grant — `sys_record_share` rows created with `source: 'manual'` (see `packages/plugins/plugin-sharing/src/sharing-service.ts`). **Matrix / territory-shaped access** is served today by multi-position assignment anchored on business units, plus pre-resolved membership sets: a registered `rls-membership-resolver` (ADR-0105 D11) stages e.g. `territory_account_ids` into `ExecutionContext.rlsMembership`, and a policy references it as `account_id IN (current_user.territory_account_ids)`. The aspirational `TerritorySchema` was removed in ADR-0105 D11 — it had no runtime object, stack field or resolver; a generalized dimension-security module will arrive with its own ADR. Owner/criteria rules are re-evaluated on insert/update via internal sharing rule hooks.
+**Beyond declarative rules:** Two other sharing mechanisms exist but are **not** `SharingRule` types. **Manual sharing** is a runtime grant — `sys_record_share` rows created with `source: 'manual'` (see `packages/plugins/plugin-sharing/src/sharing-service.ts`). **Matrix / territory-shaped access** is served today by multi-position assignment anchored on business units, plus pre-resolved membership sets: a registered `rls-membership-resolver` (ADR-0105 D11) stages e.g. `territory_account_ids` into `ExecutionContext.rlsMembership`, and a policy references it as `account_id IN (current_user.territory_account_ids)`. The aspirational `TerritorySchema` was removed in ADR-0105 D11 — it had no runtime object, stack field or resolver; a generalized dimension-security module will arrive with its own ADR. Criteria rules are re-evaluated on record insert/update via internal sharing rule hooks, on every write to the rule itself, and once per boot — so deactivating or deleting a rule withdraws the grants it materialized (see [Sharing Rules](/docs/permissions/sharing-rules#switching-a-rule-off-withdraws-the-access-it-granted)).
 </Callout>
 
 ### Configuration Example

content/docs/permissions/sharing-rules.mdx

@@ -161,6 +161,38 @@ covered; an unauthorized call fails with `403 PERMISSION_DENIED`. Boot
 seeding, lifecycle hooks, and backfills run as system context and are
 unaffected.
 
+### Switching a rule off withdraws the access it granted
+
+A sharing rule's grants are **materialized** — evaluating a rule writes real
+`sys_record_share` rows with `source: 'rule'` and `source_id` set to the rule.
+Because those rows outlive the evaluation that produced them, withdrawal has to
+be an explicit act, and it happens at **three** moments:
+
+| When | What is reconciled |
+|:--|:--|
+| **The write that deactivates or edits the rule** | That rule's grants, immediately — deactivating with `active: false` (or `POST {basePath}/sharing/rules` with the same name) revokes them before the call returns |
+| **The next insert/update of a matching record** | That record's grants for every rule on the object — an inactive rule desires nothing, so its rows are revoked |
+| **Every boot** | All rules, plus a sweep of `source: 'rule'` rows whose `source_id` no longer resolves to any rule |
+
+Deleting a rule withdraws its grants too, whether you delete it through
+`DELETE {basePath}/sharing/rules/:idOrName` (by id **or** by name) or through
+the plain data API in Setup.
+
+The practical guarantee: **an over-granting rule is always recoverable from the
+API surface.** Switch it off or delete it, and the access it materialized is
+gone — not on the next time somebody happens to touch the record, and not only
+after a restart (objectstack#4433, #4434). A grant whose rule row has vanished
+entirely is retired by the boot sweep, so a database repaired by hand — or
+upgraded from a build that leaked these rows — converges on the next start.
+
+<Callout type="info">
+  Because withdrawal is materialized rather than computed at read time, the
+  revocation is visible in `sys_record_share` itself. `GET
+  {basePath}/data/:object/:id/shares` is the fastest way to confirm a rule's
+  access is really gone, and `POST {basePath}/sharing/rules/:idOrName/evaluate`
+  forces a reconcile on demand.
+</Callout>
+
 ### There is no "share every record" rule
 
 The predicate is **mandatory on every authoring path**, whether you declare

packages/plugins/plugin-sharing/src/boot-backfill.test.ts

@@ -58,6 +58,13 @@ function makeEngine() {
       return t[i];
     },
     async delete(o: string, opts?: any) {
+      // Mirror `ObjectQLEngine.delete`'s dispatch guard (#4434) — see the same
+      // note in sharing-rule.test.ts. A fake looser than the contract it
+      // stands in for is how a green suite ships a dead route.
+      const whereId = opts?.where && typeof opts.where === 'object' ? (opts.where as any).id : undefined;
+      const t0 = typeof whereId;
+      const scalarId = whereId != null && (t0 === 'string' || t0 === 'number' || t0 === 'bigint');
+      if (!scalarId && !opts?.multi) throw new Error('Delete requires an ID or options.multi=true');
       const t = ensure(o); const where = opts?.where ?? {};
       for (let i = t.length - 1; i >= 0; i--) if (matches(t[i], where)) t.splice(i, 1);
       return { ok: true };
@@ -127,6 +134,94 @@ describe('backfillRuleGrants (#2926 ③ — seed rows materialize at boot)', ()
   });
 });
 
+/**
+ * objectstack#4433 (restart half) — "not at boot".
+ *
+ * The boot pass was handed `listRules({ activeOnly: true })`, so a deactivated
+ * rule was never evaluated and the grants it had materialised survived every
+ * restart: the rule read `active: false` while the orphaned `source: 'rule'`
+ * row kept answering. The pass is the last line of defence for withdrawal, so
+ * it has to walk EVERY rule — `evaluateRule` purges the ones it finds inactive.
+ */
+describe('boot rule backfill withdraws deactivated rules (#4433)', () => {
+  let engine: ReturnType<typeof makeEngine>;
+  let sharing: SharingService;
+  let rules: SharingRuleService;
+
+  beforeEach(async () => {
+    engine = makeEngine();
+    sharing = new SharingService({ engine: engine as any });
+    rules = new SharingRuleService({ engine: engine as any, sharing });
+    engine._tables.showcase_private_note = [
+      { id: 'note_n', title: 'rc1 rule target', owner_id: 'admin' },
+    ];
+    await rules.defineRule({
+      name: 'rc1_rule_livetest', label: 'RC1 live test', object: 'showcase_private_note',
+      criteria: { title: 'rc1 rule target' },
+      recipientType: 'user', recipientId: 'member_b', accessLevel: 'read',
+    }, SYS);
+    // Boot 1 materialises the grant — the issue's step 3 baseline.
+    await backfillRuleGrants(rules, await rules.listRules({}, SYS));
+    expect(engine._tables.sys_record_share ?? []).toHaveLength(1);
+  });
+
+  it('revokes a deactivated rule\'s grants on the next boot', async () => {
+    // Admin switches the rule off. Nothing else touches the record.
+    await rules.defineRule({
+      name: 'rc1_rule_livetest', label: 'RC1 live test', object: 'showcase_private_note',
+      criteria: { title: 'rc1 rule target' },
+      recipientType: 'user', recipientId: 'member_b', accessLevel: 'read', active: false,
+    }, SYS);
+
+    // Restart: the pass must see the inactive rule to be able to purge it.
+    await backfillRuleGrants(rules, await rules.listRules({}, SYS));
+
+    expect(engine._tables.sys_record_share ?? []).toHaveLength(0);
+  });
+
+  it('an activeOnly rule list can never withdraw — the #4433 boot repro', async () => {
+    await rules.defineRule({
+      name: 'rc1_rule_livetest', label: 'RC1 live test', object: 'showcase_private_note',
+      criteria: { title: 'rc1 rule target' },
+      recipientType: 'user', recipientId: 'member_b', accessLevel: 'read', active: false,
+    }, SYS);
+
+    // The old call site, pinned as the defect it was: an inactive rule is
+    // absent from the list, so the pass has nothing to purge with.
+    const activeOnly = await rules.listRules({ activeOnly: true }, SYS);
+    expect(activeOnly).toHaveLength(0);
+    await backfillRuleGrants(rules, activeOnly);
+    expect(engine._tables.sys_record_share ?? []).toHaveLength(1); // still granted
+
+    // Reconciling every rule is what repairs it.
+    await backfillRuleGrants(rules, await rules.listRules({}, SYS));
+    expect(engine._tables.sys_record_share ?? []).toHaveLength(0);
+  });
+
+  it('still materialises active rules (the #2926 behaviour is untouched)', async () => {
+    engine._tables.showcase_private_note.push({ id: 'note_2', title: 'rc1 rule target', owner_id: 'admin' });
+    await backfillRuleGrants(rules, await rules.listRules({}, SYS));
+    expect((engine._tables.sys_record_share ?? []).map((s) => s.record_id).sort()).toEqual(['note_2', 'note_n']);
+  });
+
+  it('sweeps grants whose rule row vanished before the restart', async () => {
+    // A rule removed by a path that never reached deleteRule (data-API delete
+    // with the hook unbound, a migration, a crash mid-delete).
+    engine._tables.sys_sharing_rule = [];
+    await backfillRuleGrants(rules, await rules.listRules({}, SYS));
+    expect(engine._tables.sys_record_share ?? []).toHaveLength(1); // unreachable by rule iteration
+
+    expect(await rules.sweepOrphanedRuleGrants()).toBe(1);
+    expect(engine._tables.sys_record_share ?? []).toHaveLength(0);
+  });
+
+  it('the sweep is idempotent across repeated boots', async () => {
+    expect(await rules.sweepOrphanedRuleGrants()).toBe(0);
+    expect(await rules.sweepOrphanedRuleGrants()).toBe(0);
+    expect(engine._tables.sys_record_share ?? []).toHaveLength(1);
+  });
+});
+
 describe("backfillRetiredAccessLevels (#3865 — stored 'full' normalises to 'edit')", () => {
   let engine: ReturnType<typeof makeEngine>;
 

packages/plugins/plugin-sharing/src/rule-rebind.test.ts

@@ -171,11 +171,16 @@ describe('SharingServicePlugin reconciles grants on rule writes (#3821)', () =>
       listRules: vi.fn(async () => []),
       evaluateRule: vi.fn(async () => ({ ruleId: 'r1', matchedRecords: 2, expandedUsers: 1, grantsCreated: 2, grantsUpdated: 0, grantsRevoked: 0 })),
       revokeRuleGrants: vi.fn(async () => 2),
+      sweepOrphanedRuleGrants: vi.fn(async () => 0),
     };
     (plugin as any).ruleService = ruleService;
     const ctx = makeCtx();
     logger = ctx.logger;
     (plugin as any).bindRuleRebindTriggers(engine, ctx);
+    // [#4433] Boot is over — `kernel:bootstrapped` has run its backfill, so
+    // runtime rule writes own reconciliation from here. Before this point the
+    // trigger defers to that pass (see the boot-phase describe below).
+    (plugin as any).ruleGrantsBootReconciled = true;
   });
 
   it('backfills existing records when a rule is created', async () => {
@@ -196,12 +201,26 @@ describe('SharingServicePlugin reconciles grants on rule writes (#3821)', () =>
     expect(ruleService.evaluateRule).not.toHaveBeenCalled();
   });
 
-  it('skips system-context writes — boot backfill owns those', async () => {
-    await engine.fire('afterInsert', 'sys_sharing_rule', {
-      result: { id: 'r1' },
+  /**
+   * objectstack#4433 — this is the defect, and it hid behind the test that
+   * used to live here ("skips system-context writes").
+   *
+   * `SharingRuleService.defineRule` — the ONLY implementation behind
+   * `POST /sharing/rules`, the documented way to deactivate a rule — writes
+   * `sys_sharing_rule` with SYSTEM_CTX unconditionally, because it has to
+   * reach a platform table the sharing middleware otherwise gates. So the old
+   * `session.isSystem` skip did not filter out "boot seeding"; it filtered out
+   * every REST authoring write there is. The old test passed a mocked
+   * `session: { isSystem: true }` that the real REST path never sends, and
+   * asserted the reconcile did NOT happen — pinning the bug as the contract.
+   */
+  it('reconciles a SYSTEM_CTX authoring write once boot is done (#4433)', async () => {
+    // Exactly what `POST /sharing/rules` with `active: false` produces.
+    await engine.fire('afterUpdate', 'sys_sharing_rule', {
+      result: { id: 'r1', active: false },
       session: { isSystem: true },
     });
-    expect(ruleService.evaluateRule).not.toHaveBeenCalled();
+    expect(ruleService.evaluateRule).toHaveBeenCalledWith('r1', expect.objectContaining({ isSystem: true }));
   });
 
   it('never fails the authoring write when reconciliation throws', async () => {
@@ -228,3 +247,71 @@ describe('SharingServicePlugin reconciles grants on rule writes (#3821)', () =>
     expect(order).toEqual(['rebind', 'reconcile']);
   });
 });
+
+/**
+ * [#4433] Boot phase — the predicate that replaced the `isSystem` skip.
+ *
+ * The skip exists to avoid duplicating work: declared-rule seeding and package
+ * bootstrap write `sys_sharing_rule` before `kernel:bootstrapped`, and that
+ * pass reconciles every rule anyway. That is a statement about WHEN a write
+ * happens, not about WHO made it — so it is gated on boot phase, which is true
+ * for exactly the writes the backfill covers and false for every runtime one.
+ */
+describe('SharingServicePlugin defers reconciliation to the boot backfill (#4433)', () => {
+  let engine: ReturnType<typeof makeEngine>;
+  let plugin: SharingServicePlugin;
+  let ruleService: AnyRecord;
+
+  beforeEach(() => {
+    engine = makeEngine();
+    plugin = new SharingServicePlugin();
+    ruleService = {
+      listRules: vi.fn(async () => []),
+      evaluateRule: vi.fn(async () => ({ ruleId: 'r1', matchedRecords: 0, expandedUsers: 0, grantsCreated: 0, grantsUpdated: 0, grantsRevoked: 0 })),
+      revokeRuleGrants: vi.fn(async () => 0),
+      sweepOrphanedRuleGrants: vi.fn(async () => 0),
+    };
+    (plugin as any).ruleService = ruleService;
+    (plugin as any).bindRuleRebindTriggers(engine, makeCtx());
+    // Boot still in flight — `ruleGrantsBootReconciled` defaults to false.
+  });
+
+  it('skips the per-write reconcile while boot is still in flight', async () => {
+    await engine.fire('afterInsert', 'sys_sharing_rule', { result: { id: 'seeded' } });
+    expect(ruleService.evaluateRule).not.toHaveBeenCalled();
+  });
+
+  it('still rebinds the lifecycle hooks during boot', async () => {
+    // Deferring the reconcile must not defer the binding — a rule seeded at
+    // boot has to be enforceable for records written straight afterwards.
+    await engine.fire('afterInsert', 'sys_sharing_rule', { result: { id: 'seeded' } });
+    expect(ruleService.listRules).toHaveBeenCalled();
+  });
+
+  it('reconciles every write once the backfill has run — user session', async () => {
+    (plugin as any).ruleGrantsBootReconciled = true;
+    await engine.fire('afterUpdate', 'sys_sharing_rule', {
+      result: { id: 'r1', active: false },
+      session: { userId: 'admin' },
+    });
+    expect(ruleService.evaluateRule).toHaveBeenCalledWith('r1', expect.objectContaining({ isSystem: true }));
+  });
+
+  it('reconciles every write once the backfill has run — system session', async () => {
+    (plugin as any).ruleGrantsBootReconciled = true;
+    await engine.fire('afterUpdate', 'sys_sharing_rule', {
+      result: { id: 'r1', active: false },
+      session: { isSystem: true },
+    });
+    expect(ruleService.evaluateRule).toHaveBeenCalledWith('r1', expect.objectContaining({ isSystem: true }));
+  });
+
+  it('purges on a post-boot delete regardless of session kind', async () => {
+    (plugin as any).ruleGrantsBootReconciled = true;
+    await engine.fire('afterDelete', 'sys_sharing_rule', {
+      input: { id: 'r1' },
+      session: { isSystem: true },
+    });
+    expect(ruleService.revokeRuleGrants).toHaveBeenCalledWith('r1');
+  });
+});