fix(plugin-sharing): make package-seeded (org-null) sharing rules visible and addressable by name (#7760)

Rules seeded from an app or package are defined under the system context, so
they are stored with `organization_id = null` (platform-global). `listRules`
and the by-name fallback of `getRule` scoped their reads with a strict
`organization_id = <caller org>` equality, which such a row can never satisfy:
an org-scoped admin saw `GET /api/v1/sharing/rules` answer `{"data":[]}` over a
table of active seeded rules, and by-name GET/evaluate answered 404
`RULE_NOT_FOUND`. Only the by-id branch, which carries no org filter, worked.
Enforcement was unaffected — the boot reconcile also reads under the system
context — which is exactly why this stayed invisible.

Both admin reads now match "this organization OR platform-global", the same
predicate `sys_business_unit` approver expansion settled on in #3807 and the
one `sys_metadata`'s pending-draft listing uses. Another organization's row
still fails the match; only rows belonging to no organization become visible.

`defineRule` is deliberately NOT widened. Its existence lookup decides
upsert-vs-insert, so widening it would let one organization's admin rewrite a
row every other organization reads — a cross-tenant WRITE, a different act from
a cross-tenant read of a platform-global row. A same-named create therefore
still produces a row stamped with the caller's own organization, and
`findRuleRowByName` prefers that row over the platform-global one via two
sequenced lookups rather than one `$or` with `limit: 1`, so the preference is a
decision rather than whichever row a dialect happened to reach first.

The test fake engine's filter matcher short-circuited on `$or` and DROPPED its
sibling field keys, so `listRules`'s `{object_name, active, $or:[…]}` would have
matched the whole table there while driver-sql and driver-memory conjoin the
two. A fake looser than the contract it stands in for is how a green suite
ships a broken filter, so it now conjoins.

Fixes #7676


Claude-Session: https://claude.ai/code/session_01BVc1ekPpi6yaWywAUhfzfd

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

Author: os-help

.changeset/spotty-pans-visit.md

@@ -0,0 +1,14 @@
+---
+'@objectstack/plugin-sharing': patch
+---
+
+Make package-seeded sharing rules visible and addressable by name to org-scoped admins
+
+Sharing rules seeded from an app or package are defined under the system context, so they are stored with `organization_id = null` (platform-global). `SharingRuleService.listRules` and the by-name fallback of `getRule` scoped their reads with a strict `organization_id = <caller org>` equality, which such a row can never satisfy. An authenticated org-scoped admin therefore saw `GET /api/v1/sharing/rules` return an empty list over a table of active seeded rules, and by-name `GET` and `evaluate` answered 404 `RULE_NOT_FOUND`; only the by-id branch, which was never org-scoped, still worked.
+
+Both admin reads now match "this organization OR platform-global", mirroring how enforcement has always read these rows under the system context. Consequences worth knowing:
+
+- Seeded rules now appear in the admin rule list and can be fetched, evaluated and deleted by name. An org admin could already do all three **by row id** — the by-id branch carries no org filter — so this adds an address form and discoverability, not a new authority. Deleting a package-seeded rule remains reversible: the next boot reseeds it.
+- Rules belonging to a **different** organization remain invisible and unresolvable by name; only rows belonging to no organization at all become visible.
+- `defineRule` is deliberately **not** widened. Its existence lookup decides upsert-vs-insert, so widening it would let one organization's admin rewrite a row every other organization reads. A same-named create still produces a row stamped with the caller's own organization, and by-name lookups prefer that row over the platform-global one.
+- Callers passing a context with no organization (boot seeding, rule hooks, backfills, the boot reconcile) are unaffected — that path was already unfiltered and is unchanged.

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

@@ -221,6 +221,41 @@ export class SharingRuleService implements ISharingRuleService {
     return rowFromRule(newRow);
   }
 
+  /**
+   * [#7676] Tenant scope for a sharing-rule ADMIN read: "this org ∪
+   * platform-global".
+   *
+   * `organization_id = null` on `sys_sharing_rule` means "owned by no
+   * organization" — a row written by the package/app seeder
+   * (`bootstrapDeclaredSharingRules`, which defines under `SYSTEM_CTX` and so
+   * stamps `organization_id: null`) before any org id exists. A strict
+   * `organization_id = <request org>` equality made every such row invisible to
+   * the admin API while enforcement kept reading them under `SYSTEM_CTX`: on a
+   * stock boot `GET /api/v1/sharing/rules` answered `{data: []}` over four
+   * active seeded rules, by-name GET and evaluate 404'd `RULE_NOT_FOUND`, and
+   * only the org-unfiltered by-id branch of {@link getRule} still worked. Rules
+   * that grant access but cannot be listed, inspected or deactivated are the
+   * worst half of both properties.
+   *
+   * Widening the READ leaks nothing across tenants: another org's row still
+   * fails the match, and a null-org row is platform-global by construction —
+   * every org already receives the grants it materialises. This is the same
+   * predicate, for the same reason, that `sys_business_unit` approver expansion
+   * settled on in #3807 and that `sys_metadata`'s pending-draft listing uses.
+   *
+   * ⚠️ It is deliberately NOT applied to {@link defineRule}'s existence lookup.
+   * That lookup decides UPSERT-or-insert, so widening it would let one org's
+   * admin overwrite the label, criteria, recipient and access level of a row
+   * every OTHER org reads — a cross-tenant WRITE, which is a different act from
+   * a cross-tenant read of a platform-global row. A same-named POST therefore
+   * still creates an org-stamped row of the tenant's own, and
+   * {@link findRuleRowByName} prefers it.
+   */
+  private adminOrgScope(where: Record<string, unknown>, orgId: string | null | undefined): Record<string, unknown> {
+    if (!orgId) return where;
+    return { ...where, $or: [{ organization_id: orgId }, { organization_id: null }] };
+  }
+
   async listRules(
     filter: { object?: string; activeOnly?: boolean },
     context: ExecutionContext,
@@ -231,9 +266,8 @@ export class SharingRuleService implements ISharingRuleService {
     if (filter.activeOnly) where.active = true;
     // `organizationId` is not on the envelope — see defineRule().
     const orgId = (context as any)?.organizationId ?? context?.tenantId;
-    if (orgId) where.organization_id = orgId;
     const rows = await this.engine.find('sys_sharing_rule', {
-      where,
+      where: this.adminOrgScope(where, orgId),
       orderBy: [{ field: 'name', order: 'asc' }],
       limit: 1000,
       context: SYSTEM_CTX,
@@ -252,15 +286,34 @@ export class SharingRuleService implements ISharingRuleService {
       context: SYSTEM_CTX,
     });
     if (Array.isArray(byId) && byId[0]) return rowFromRule(byId[0]);
-    const byName = await this.engine.find('sys_sharing_rule', {
-      where: orgId ? { name: idOrName, organization_id: orgId } : { name: idOrName },
-      limit: 1,
-      context: SYSTEM_CTX,
-    });
-    if (Array.isArray(byName) && byName[0]) return rowFromRule(byName[0]);
+    const byName = await this.findRuleRowByName(idOrName, orgId);
+    if (byName) return rowFromRule(byName);
     return null;
   }
 
+  /**
+   * [#7676] Resolve a rule by NAME for an admin read: this org first, the
+   * platform-global (`organization_id IS NULL`) row second.
+   *
+   * Two sequenced lookups rather than one `$or` with `limit: 1`, because when
+   * BOTH rows exist the answer must be the caller's own: a single disjunctive
+   * query with a row cap picks whichever row the driver happened to reach
+   * first, so an org that had authored its own `share_red_projects_with_execs`
+   * could get the platform row back on one dialect and its own on another.
+   * Preference is a decision, so it is written as one.
+   *
+   * No `orgId` (SYSTEM_CTX — boot seeding, hooks, backfills) keeps the
+   * unfiltered by-name lookup it has always had.
+   */
+  private async findRuleRowByName(name: string, orgId: string | null | undefined): Promise<any | null> {
+    const first = async (where: Record<string, unknown>): Promise<any | null> => {
+      const rows = await this.engine.find('sys_sharing_rule', { where, limit: 1, context: SYSTEM_CTX });
+      return Array.isArray(rows) && rows[0] ? rows[0] : null;
+    };
+    if (!orgId) return first({ name });
+    return (await first({ name, organization_id: orgId })) ?? (await first({ name, organization_id: null }));
+  }
+
   async deleteRule(idOrName: string, context: ExecutionContext): Promise<void> {
     this.assertCanManageRules(context); // [ADR-0111 D6]
     const row = await this.getRule(idOrName, context);

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

@@ -23,8 +23,16 @@ function makeEngine() {
   const ensure = (n: string) => (tables[n] ??= []);
   function matches(row: Row, f: any): boolean {
     if (!f || typeof f !== 'object') return true;
-    if (Array.isArray(f.$or)) return f.$or.some((x: any) => matches(row, x));
-    if (Array.isArray(f.$and)) return f.$and.every((x: any) => matches(row, x));
+    // [#7676] A combinator is CONJOINED with its sibling field keys, never a
+    // short-circuit that returns before they are read. This used to
+    // `return f.$or.some(...)`, silently DROPPING every sibling — so
+    // `listRules`'s `{object_name, active, $or:[…org scope…]}` would have
+    // matched the whole table here while the real drivers (driver-sql's
+    // `applyFilterCondition`, driver-memory's mingo document) AND the two.
+    // A fake looser than the contract it stands in for is how a green suite
+    // ships a broken filter (the #4434 lesson, applied to reads).
+    if (Array.isArray(f.$or) && !f.$or.some((x: any) => matches(row, x))) return false;
+    if (Array.isArray(f.$and) && !f.$and.every((x: any) => matches(row, x))) return false;
     for (const [k, v] of Object.entries(f)) {
       if (k === '$or' || k === '$and') continue;
       const rv = row[k];
@@ -850,3 +858,164 @@ describe('[ADR-0111 D6] sharing-rule management gate', () => {
     expect(r.id).toBeTruthy();
   });
 });
+
+// ─────────────────────────────────────────────────────────────────────
+// [#7676] Package-seeded rules (`organization_id = null`) are visible and
+// addressable BY NAME to an org-scoped admin.
+//
+// The QA run's finding: on a stock boot `GET /api/v1/sharing/rules` answered
+// `{data: []}` over four active seeded rules, by-name GET/evaluate 404'd
+// `RULE_NOT_FOUND`, and only the org-unfiltered by-id branch still worked —
+// because the seeder defines under SYSTEM_CTX (`organization_id: null`) while
+// an authenticated admin's context carries `organizationId: 'org_…'`, and a
+// strict equality can never match. Enforcement was unaffected (boot reconcile
+// reads under SYSTEM_CTX, no org filter), which is exactly why it stayed
+// invisible: rules that grant access but cannot be listed or deactivated.
+// ─────────────────────────────────────────────────────────────────────
+
+describe('[#7676] admin visibility of package-seeded (org-null) sharing rules', () => {
+  let engine: ReturnType<typeof makeEngine>;
+  let rules: SharingRuleService;
+
+  /** An authenticated org-scoped sharing admin — the shape the REST layer builds. */
+  const ORG1_ADMIN = { userId: 'admin', organizationId: 'org1', systemPermissions: ['manage_sharing'] } as any;
+  const SEEDED = 'share_red_projects_with_execs';
+  /**
+   * The seeder's own context — `bootstrapDeclaredSharingRules` defines under
+   * plugin-sharing's `SYSTEM_CTX`, which carries NO organization, and that is
+   * precisely what stamps `organization_id: null` on the row. Not this file's
+   * older `SYS`, which is `{isSystem: true, organizationId: 'org1'}` — system
+   * only bypasses the ADR-0111 D6 capability gate, never the org scope.
+   */
+  const BOOT = { isSystem: true, positions: [], permissions: [] } as any;
+
+  beforeEach(async () => {
+    engine = makeEngine();
+    engine._tables.project = [
+      { id: 'p_red', status: 'red', owner_id: 'someone' },
+      { id: 'p_green', status: 'green', owner_id: 'someone' },
+    ];
+    rules = new SharingRuleService({ engine: engine as any, sharing: new SharingService({ engine: engine as any }) });
+
+    // Package seed — defined under SYSTEM_CTX exactly as
+    // `bootstrapDeclaredSharingRules` does, so `organization_id` lands null.
+    await rules.defineRule({
+      name: SEEDED, label: 'Red projects → execs', object: 'project',
+      criteria: { status: 'red' }, recipientType: 'user', recipientId: 'exec',
+      managedBy: 'package',
+    } as any, BOOT);
+    // A rule belonging to a DIFFERENT organization — the isolation pin.
+    await rules.defineRule({
+      name: 'other_org_rule', label: 'Other org', object: 'project',
+      criteria: { status: 'green' }, recipientType: 'user', recipientId: 'mallory',
+    } as any, { userId: 'other', organizationId: 'org2', systemPermissions: ['manage_sharing'] } as any);
+    // An API-created rule stamped with THIS org — must keep working as today.
+    await rules.defineRule({
+      name: 'org1_rule', label: 'Org1 own', object: 'project',
+      criteria: { status: 'red' }, recipientType: 'user', recipientId: 'alice',
+    } as any, ORG1_ADMIN);
+  });
+
+  it('the fixture really does store a null organization_id on the seeded row', () => {
+    const seeded = engine._tables.sys_sharing_rule.find((r) => r.name === SEEDED);
+    expect(seeded?.organization_id).toBeNull();
+    expect(seeded?.managed_by).toBe('package');
+    // …and the org-stamped rows really are stamped, or the pins below prove nothing.
+    expect(engine._tables.sys_sharing_rule.find((r) => r.name === 'org1_rule')?.organization_id).toBe('org1');
+    expect(engine._tables.sys_sharing_rule.find((r) => r.name === 'other_org_rule')?.organization_id).toBe('org2');
+  });
+
+  // ── visibility (the defect) ──────────────────────────────────────────
+
+  it('listRules shows the seeded rule to an org-scoped admin', async () => {
+    const names = (await rules.listRules({}, ORG1_ADMIN)).map((r) => r.name);
+    expect(names).toContain(SEEDED);
+    // Exact set — "this org ∪ platform-global", and nothing else. Kept HERE
+    // rather than on the isolation pins below so that each test has ONE
+    // predicted direction under the ablation: this one flips red, they do not.
+    expect(names.sort()).toEqual([SEEDED, 'org1_rule'].sort());
+  });
+
+  it('getRule resolves the seeded rule BY NAME for an org-scoped admin', async () => {
+    const row = await rules.getRule(SEEDED, ORG1_ADMIN);
+    expect(row?.name).toBe(SEEDED);
+    expect(row?.organization_id).toBeNull();
+  });
+
+  it('evaluateRule works BY NAME for the seeded rule (no RULE_NOT_FOUND)', async () => {
+    const res = await rules.evaluateRule(SEEDED, ORG1_ADMIN);
+    expect(res.matchedRecords).toBe(1);
+    expect(res.grantsCreated).toBe(1);
+    // The control the QA run used: by-id already worked, and must still agree.
+    const byId = await rules.getRule(SEEDED, ORG1_ADMIN);
+    expect((await rules.evaluateRule(byId!.id, ORG1_ADMIN)).ruleId).toBe(byId!.id);
+  });
+
+  // ── isolation pins (must stay green under the ablation) ──────────────
+
+  it('another organization’s rule stays invisible — list', async () => {
+    const names = (await rules.listRules({}, ORG1_ADMIN)).map((r) => r.name);
+    expect(names).not.toContain('other_org_rule');
+    // Positive half, so this cannot pass by listing nothing at all — and it is
+    // the org's OWN row deliberately, which survives the ablation.
+    expect(names).toContain('org1_rule');
+  });
+
+  it('another organization’s rule stays unresolvable — by name', async () => {
+    expect(await rules.getRule('other_org_rule', ORG1_ADMIN)).toBeNull();
+    await expect(rules.evaluateRule('other_org_rule', ORG1_ADMIN)).rejects.toThrow(/RULE_NOT_FOUND/);
+  });
+
+  it('the org scope is CONJOINED with the object/activeOnly filters, not substituted for them', async () => {
+    await rules.defineRule({
+      name: 'seeded_other_object', label: 'Other object', object: 'account',
+      criteria: { tier: 'gold' }, recipientType: 'user', recipientId: 'exec',
+      managedBy: 'package',
+    } as any, BOOT);
+    await rules.defineRule({
+      name: 'seeded_inactive', label: 'Inactive seed', object: 'project',
+      criteria: { status: 'red' }, recipientType: 'user', recipientId: 'exec',
+      managedBy: 'package', active: false,
+    } as any, BOOT);
+
+    // Both halves assert on the org's OWN row for the positive side, so this
+    // test stays GREEN under the ablation and goes red only for its own cause:
+    // an org scope that SUBSTITUTED for the object/activeOnly predicates
+    // instead of being conjoined with them (the exact way a top-level `$or`
+    // fails when a filter evaluator short-circuits on the combinator).
+    const projects = (await rules.listRules({ object: 'project' }, ORG1_ADMIN)).map((r) => r.name);
+    expect(projects).not.toContain('seeded_other_object');
+    expect(projects).toContain('org1_rule');
+    const active = (await rules.listRules({ object: 'project', activeOnly: true }, ORG1_ADMIN)).map((r) => r.name);
+    expect(active).not.toContain('seeded_inactive');
+    expect(active).toContain('org1_rule');
+  });
+
+  // ── unchanged behaviour ──────────────────────────────────────────────
+
+  it('an API-created org-stamped rule is still listed and gettable by name', async () => {
+    expect((await rules.getRule('org1_rule', ORG1_ADMIN))?.organization_id).toBe('org1');
+    expect((await rules.listRules({}, ORG1_ADMIN)).map((r) => r.name)).toContain('org1_rule');
+  });
+
+  it('a no-org (SYSTEM_CTX / boot) context still sees every rule, unfiltered', async () => {
+    const names = (await rules.listRules({}, BOOT)).map((r) => r.name).sort();
+    expect(names).toEqual([SEEDED, 'org1_rule', 'other_org_rule'].sort());
+    expect((await rules.getRule('other_org_rule', BOOT))?.organization_id).toBe('org2');
+  });
+
+  it('when both a platform-global and an own-org row share a name, by-name resolves the OWN row', async () => {
+    // `defineRule` is deliberately NOT widened, so a same-named POST from an
+    // org admin creates its own row instead of overwriting the shared seed.
+    const own = await rules.defineRule({
+      name: SEEDED, label: 'Org1 override', object: 'project',
+      criteria: { status: 'red' }, recipientType: 'user', recipientId: 'alice',
+    } as any, ORG1_ADMIN);
+    expect(engine._tables.sys_sharing_rule.filter((r) => r.name === SEEDED)).toHaveLength(2);
+    expect(engine._tables.sys_sharing_rule.find((r) => r.organization_id === null)?.label)
+      .toBe('Red projects → execs'); // the seed row is untouched
+    const resolved = await rules.getRule(SEEDED, ORG1_ADMIN);
+    expect(resolved?.id).toBe(own.id);
+    expect(resolved?.organization_id).toBe('org1');
+  });
+});