fix(security): an organization-less permission-set row grants again — #11121 revoked standing access silently

.changeset/permission-set-residue-fallback.md

@@ -0,0 +1,38 @@
+---
+'@objectstack/plugin-security': patch
+---
+
+An organization-less `sys_permission_set` row grants again — #11121 revoked standing access silently
+
+#11121 made the request-time permission-set loader tenant-scoped so two
+organizations holding a row for the same name stop answering each other's
+requests. It shipped the second half as a COMMENT — "an organization-less
+leftover only where it does not [have its own]" — and the code read `.own`
+alone, which by `resolveOwnOrganizationRow`'s own documented contract is never
+a residue once an organization is supplied.
+
+That helper is written for SEEDERS, where refusing to read a residue as
+"already seeded" is the entire point. Enforcement wants the opposite reading: an
+organization-less row is still a row the principal was granted, and dropping it
+revokes standing access with no signal at the moment of loss — the failure this
+catalog's own header, and `resolve-authz-context`'s `sys_position` read, both
+name as the thing not to do.
+
+The asymmetry was observable on a single row: its `system_permissions` and
+`tab_permissions` kept applying, because that read is unscoped and by id, while
+its `object_permissions` and `admin_scope` stopped. One row, two enforcement
+planes, opposite verdicts. Every walled deployment carrying pre-#11121 rows —
+or any row authored without a tenant, which includes admin-UI-authored sets —
+lost those grants on upgrade, reported only as a boot WARN about "leftovers"
+that states the catalog is complete.
+
+Found by cloud's `apps/ee-group-showcase` dogfood suites, which had been failing
+four ADR-0111 / ADR-0105 assertions on cloud main while turbo replayed them from
+cache.
+
+Preference order is unchanged, so the cross-tenant bleed #11121 closed stays
+closed: this organization's own row still WINS wherever it exists, and a
+leftover is consulted only in its absence. #11121's suite covers seeding and the
+`sys_position` sweep; the three cases added here cover the loader path it did
+not — residue resolves, own beats residue, and the single-posture carve-out is
+untouched. Reverting the one-line fix reddens exactly the first of them.

packages/plugins/plugin-security/src/resolve-permission-sets-for-context.pin.test.ts

@@ -186,3 +186,86 @@ describe('[#7616] resolvePermissionSetsForContext is reachable through the servi
  expect(sets).toEqual([]);
  });
 });
+
+/**
+ * [#11121 residue] An ORGANIZATION-LESS `sys_permission_set` row still grants.
+ *
+ * #11121 made this loader tenant-scoped so two organizations holding a row for
+ * the same name stop answering each other's requests. Its comment promised the
+ * other half — "an organization-less leftover only where it does not [have its
+ * own]" — and the code read `.own` alone, which by
+ * `resolveOwnOrganizationRow`'s documented contract is NEVER a residue once an
+ * organization is supplied. That helper is written for SEEDERS, where refusing
+ * to see a residue as "already seeded" is the entire point; enforcement wants
+ * the opposite reading.
+ *
+ * The consequence was a silent revocation on upgrade: every walled deployment
+ * carrying pre-#11121 rows (or any row authored without a tenant — the admin
+ * UI, a hand-run seed) kept its `system_permissions` and `tab_permissions`,
+ * because that read is unscoped and by id, while its `object_permissions` and
+ * `admin_scope` stopped applying. One row, two enforcement planes, opposite
+ * verdicts, no signal at the moment of loss.
+ */
+const RESIDUE_ROW = {
+ name: 'legacy_auditor',
+ label: 'Legacy Auditor',
+ organization_id: null,
+ object_permissions: JSON.stringify({ deal: { allowRead: true, allowDelete: true } }),
+ field_permissions: JSON.stringify({}),
+ system_permissions: JSON.stringify([]),
+ tab_permissions: JSON.stringify({}),
+};
+
+/** The SAME name, stamped with the caller's organization — this one must win. */
+const OWN_ROW = {
+ name: 'legacy_auditor',
+ label: 'Legacy Auditor (this organization)',
+ organization_id: 'org_a',
+ object_permissions: JSON.stringify({ deal: { allowRead: true, allowDelete: false } }),
+ field_permissions: JSON.stringify({}),
+ system_permissions: JSON.stringify([]),
+ tab_permissions: JSON.stringify({}),
+};
+
+describe('[#11121] the per-organization loader does not silently revoke organization-less grants', () => {
+ it('resolves an organization-LESS row for a caller who has an organization', async () => {
+ const svc = await locateSecurityService([RESIDUE_ROW]);
+
+ const sets = await svc.resolvePermissionSetsForContext?.({
+ userId: 'u1',
+ organizationId: 'org_a',
+ permissions: ['legacy_auditor'],
+ } as any);
+
+ const found = (sets ?? []).find((s) => s.name === 'legacy_auditor');
+ expect(found, 'the grant vanished for a caller in an organization — this is the silent revocation').toBeTruthy();
+ // Whole, not merely present: the columns that went missing are the point.
+ expect((found as any)?.objects?.deal?.allowDelete).toBe(true);
+ });
+
+ it('still prefers THIS organization\'s own row when both exist — the cross-tenant bleed stays closed', async () => {
+ const svc = await locateSecurityService([RESIDUE_ROW, OWN_ROW]);
+
+ const sets = await svc.resolvePermissionSetsForContext?.({
+ userId: 'u1',
+ organizationId: 'org_a',
+ permissions: ['legacy_auditor'],
+ } as any);
+
+ const found = (sets ?? []).find((s) => s.name === 'legacy_auditor');
+ expect(found?.label).toBe('Legacy Auditor (this organization)');
+ // The residue's broader grant must NOT leak in behind the own row.
+ expect((found as any)?.objects?.deal?.allowDelete).toBe(false);
+ });
+
+ it('resolves the same row for a caller with NO organization (single posture, unchanged)', async () => {
+ const svc = await locateSecurityService([RESIDUE_ROW]);
+
+ const sets = await svc.resolvePermissionSetsForContext?.({
+ userId: 'u1',
+ permissions: ['legacy_auditor'],
+ } as any);
+
+ expect((sets ?? []).some((s) => s.name === 'legacy_auditor')).toBe(true);
+ });
+});

packages/plugins/plugin-security/src/security-plugin.ts

@@ -1088,13 +1088,32 @@ export class SecurityPlugin implements Plugin {
  const fetched = Array.isArray(rows) ? rows : rows?.records ?? [];
  // One row per NAME: this organization's own where it has one, an
  // organization-less leftover only where it does not.
+ //
+ // The residue arm is load-bearing and was missing (#11121 shipped the
+ // comment without it, and its suite covers seeding rather than this
+ // loader). `resolveOwnOrganizationRow` is written for SEEDERS, where
+ // refusing to see a residue as "already seeded" is the whole point —
+ // so it never returns one as `own`. ENFORCEMENT wants the opposite
+ // reading: an organization-less row is still a row this principal was
+ // granted, and dropping it revokes standing access with no signal at
+ // the moment of loss — the failure mode this catalog's own header,
+ // and `resolve-authz-context`'s `sys_position` read, both call out as
+ // the thing not to do. The asymmetry was observable on a single row:
+ // its `system_permissions` and `tab_permissions` kept applying (that
+ // read is unscoped by id) while its `object_permissions` and
+ // `admin_scope` silently stopped.
+ //
+ // Preference order is unchanged and still closes the cross-tenant
+ // bleed #11121 fixed: this organization's own row WINS wherever it
+ // exists, and a leftover is consulted only in its absence.
  const byName = new Map<string, any>();
  for (const name of new Set(names)) {
- const own = resolveOwnOrganizationRow(
+ const { own, organizationLessResidue } = resolveOwnOrganizationRow(
  fetched.filter((r: any) => r?.name === name),
  organizationId,
- ).own;
- if (own) byName.set(name, own);
+ );
+ const row = own ?? organizationLessResidue;
+ if (row) byName.set(name, row);
  }
  const all = Array.from(byName.values());
  // [ADR-0049] A DEACTIVATED set grants nothing. Not defence in depth