fix(plugin-security): a cloned permission set is not reported as an unowned declaration on boot (#21669)

.changeset/21669-cloned-permission-set-not-unowned.md

@@ -0,0 +1,13 @@
+---
+'@objectstack/plugin-security': patch
+---
+
+fix(plugin-security): a permission set the environment cloned no longer logs `permission_set_declaration_unowned` on every boot (#21669)
+
+Clause-②: no
+
+The declared-permission seeding pass walks every `permission` item in the engine registry. That registry also holds the permission sets an environment authored itself, which boot hydration loads from their `sys_metadata` rows. A set made with Setup's **Clone** action is one of them, and it carries no package id because it has none. The pass judged "no owning package" before it looked at the set's row, so on every boot, and on every `metadata:reloaded`, it logged one warning per cloned set: `[permission_set_declaration_unowned] declared permission set "…" has no owning package — not materialized … the Setup admin surface reads sys_permission_set and cannot see this set`. That is false for a clone. Its row exists (`managed_by: admin`), and Setup lists it and edits it.
+
+The pass now checks the row first. A registry item with no package id whose `sys_permission_set` row the environment owns (`managed_by` other than `package`) is the environment's own set. It is counted as `skippedEnvAuthored`, the count a package declaration over an environment row already gets, and no warning is logged. The pass reads that row from the existence read it already makes, so no query is added. On a per-organization pass, the environment door's organization-less row counts too.
+
+Unchanged: a declaration with no owning package and no environment row still logs `permission_set_declaration_unowned`, with the same text, and still counts as `skippedUnowned`. If the row could not be read, the warning still fires, because an unreadable row does not prove the environment owns the set. The publish-time materializer is unchanged. Nothing is written or granted differently: only the false warning stops, and the clone moves from the `skippedUnowned` count to the `skippedEnvAuthored` count in the pass's summary line.

packages/plugins/plugin-security/src/bootstrap-declared-permissions.test.ts

@@ -500,3 +500,156 @@ describe('[#18571] the unowned permission-set refusal moves a counter', () => {
     expect(ql.rows.some((r) => r.name === 'crm_orphan')).toBe(false);
   });
 });
+
+// ───────────────────────────────────────────────────────────────────────────
+// [#21669] An ENVIRONMENT-owned set in the registry walk is not an unowned
+// declaration. The engine registry the boot loop walks also holds every
+// env-wide `sys_metadata` `permission` row `loadMetaFromDb` hydrates — a set
+// made by Setup's Clone action among them — and such an item carries no
+// package id because it has none. The loop judged "unowned" before it read
+// the row, so a set whose `managed_by:'admin'` row Setup lists and edits was
+// reported, on every boot, as one Setup "cannot see".
+//
+// The item below is the shape measured on a real showcase cold boot after a
+// Clone (the boot-level pin is
+// `packages/qa/dogfood/test/permission-set-clone-boot-unowned-warning.dogfood.test.ts`):
+// no `_packageId`, no `packageId`, `_provenance: 'org'`.
+// ───────────────────────────────────────────────────────────────────────────
+
+const CLONE_NAME = 'crm_sales_rep_local';
+
+/** The registry item a hydrated clone is — measured, not invented. */
+const clonedItem = () => ({
+  name: CLONE_NAME,
+  label: 'Sales Rep (local)',
+  objects: { crm_lead: { allowRead: true, allowCreate: true } },
+  fields: {},
+  systemPermissions: [],
+  rowLevelSecurity: [],
+  tabPermissions: {},
+  _provenance: 'org',
+});
+
+/** The row the Clone action leaves: the environment's own set. */
+const clonedRow = (over: Record<string, any> = {}) => ({
+  id: 'ps_clone', name: CLONE_NAME, managed_by: 'admin', package_id: null, active: true,
+  object_permissions: '{"crm_lead":{"allowRead":true,"allowCreate":true}}',
+  ...over,
+});
+
+/** The unowned refusal's first sentence — the text an operator reads, unchanged by this card. */
+const unownedFirstSentence = (name: string) =>
+  `declared permission set "${name}" has no owning package — not materialized.`;
+
+describe('[#21669] an environment-owned set is told apart from an unowned declaration', () => {
+  it('a CLONED set (row managed_by admin) boots with no warning on any channel, and its row is untouched', async () => {
+    const ql = makeQl([clonedItem()]);
+    ql.rows.push(clonedRow());
+    const before = JSON.stringify(ql.rows);
+
+    const cap = captureAllConsole();
+    let r: Awaited<ReturnType<typeof bootstrapDeclaredPermissions>>;
+    try {
+      r = await bootstrapDeclaredPermissions(ql, undefined);
+    } finally {
+      cap.restore();
+    }
+
+    expect(cap.seen.filter((l) => l.includes(PERMISSION_SET_DECLARATION_UNOWNED))).toEqual([]);
+    expect(cap.seen).toEqual([]);
+    // Where a package declaration over an environment row already lands —
+    // never clobbered, and counted, so CONSERVATION still holds.
+    expect(r).toMatchObject({ skippedEnvAuthored: 1, skippedUnowned: 0, seeded: 0, updated: 0, unchanged: 0 });
+    expect(JSON.stringify(ql.rows)).toBe(before);
+  });
+
+  it('the same, on a per-organization pass: the environment door\'s organization-less row is the environment\'s', async () => {
+    // `permission-set-projection.ts` (the ENVIRONMENT door) is deliberately not
+    // per-organization, so a clone's row carries no organization_id and a
+    // walled pass sees it only beside its own rows.
+    const ql = makeQl([clonedItem()]);
+    ql.rows.push(clonedRow());
+
+    const warn = vi.fn();
+    const r = await bootstrapDeclaredPermissions(ql, undefined, {
+      logger: { info: () => {}, warn }, organizationId: 'org_a', platformBucketNames: [],
+    });
+
+    const events = warn.mock.calls.map((c) => (c[1] as any)?.event).filter(Boolean);
+    expect(events).not.toContain(PERMISSION_SET_DECLARATION_UNOWNED);
+    expect(r).toMatchObject({ skippedEnvAuthored: 1, skippedUnowned: 0, seeded: 0 });
+    expect(ql.rows).toHaveLength(1);
+  });
+
+  it('⛔ a TRULY unowned declaration — no package id, no row — still warns, with the same code and text', async () => {
+    const ql = makeQl([declaredSet({ name: 'crm_orphan', _packageId: undefined })]);
+
+    const cap = captureAllConsole();
+    let r: Awaited<ReturnType<typeof bootstrapDeclaredPermissions>>;
+    try {
+      r = await bootstrapDeclaredPermissions(ql, undefined);
+    } finally {
+      cap.restore();
+    }
+
+    expect(cap.seen).toHaveLength(1);
+    expect(cap.seen[0]).toContain(`[${PERMISSION_SET_DECLARATION_UNOWNED}]`);
+    expect(cap.seen[0]).toContain(unownedFirstSentence('crm_orphan'));
+    expect(r).toMatchObject({ skippedUnowned: 1, skippedEnvAuthored: 0 });
+    expect(ql.rows).toHaveLength(0);
+  });
+
+  it('⛔ the same on a per-organization pass with no row at all', async () => {
+    const ql = makeQl([declaredSet({ name: 'crm_orphan', _packageId: undefined })]);
+    const warn = vi.fn();
+
+    const r = await bootstrapDeclaredPermissions(ql, undefined, {
+      logger: { info: () => {}, warn }, organizationId: 'org_a', platformBucketNames: [],
+    });
+
+    const unowned = warn.mock.calls.filter((c) => (c[1] as any)?.event === PERMISSION_SET_DECLARATION_UNOWNED);
+    expect(unowned).toHaveLength(1);
+    expect(String(unowned[0]![0])).toContain(unownedFirstSentence('crm_orphan'));
+    expect(r).toMatchObject({ skippedUnowned: 1, skippedEnvAuthored: 0 });
+  });
+
+  it('⛔ a row the PACKAGE door owns is not the environment\'s: an unowned item over it still warns', async () => {
+    // The guard asks who owns the ROW. A `managed_by:'package'` row under the
+    // name is a package's record, so it proves nothing about an item that
+    // carries no owner of its own.
+    const ql = makeQl([declaredSet({ _packageId: undefined })]);
+    ql.rows.push({ id: 'ps_pkg', name: 'crm_sales_rep', managed_by: 'package', package_id: 'com.example.crm' });
+
+    const warn = vi.fn();
+    const r = await bootstrapDeclaredPermissions(ql, undefined, { logger: { info: () => {}, warn } });
+
+    const events = warn.mock.calls.map((c) => (c[1] as any)?.event).filter(Boolean);
+    expect(events).toEqual([PERMISSION_SET_DECLARATION_UNOWNED]);
+    expect(r).toMatchObject({ skippedUnowned: 1, skippedEnvAuthored: 0 });
+  });
+
+  it('⛔ a read that cannot answer proves nothing: the unowned refusal stands, exactly as before', async () => {
+    const ql = unreadableQl([clonedItem()]);
+    const warn = vi.fn();
+
+    const r = await bootstrapDeclaredPermissions(ql, undefined, { logger: { info: () => {}, warn } });
+
+    const events = warn.mock.calls.map((c) => (c[1] as any)?.event).filter(Boolean);
+    expect(events).toContain(PERMISSION_SET_DECLARATION_UNOWNED);
+    expect(r).toMatchObject({ skippedUnowned: 1, skippedEnvAuthored: 0 });
+  });
+
+  it('⭐ CONSERVATION, with a clone in the walk: every named item lands in exactly one counter', async () => {
+    const owned = declaredSet({ name: 'crm_new', _packageId: 'com.a' });
+    const orphan = declaredSet({ name: 'crm_orphan', _packageId: undefined });
+    const ql = makeQl([owned, clonedItem(), orphan]);
+    ql.rows.push(clonedRow());
+
+    const out = await bootstrapDeclaredPermissions(ql, undefined, { logger: mute() });
+
+    expect(out).toMatchObject({ seeded: 1, skippedEnvAuthored: 1, skippedUnowned: 1, unreadable: 0 });
+    const counted = out.seeded + out.updated + out.unchanged
+      + out.skippedEnvAuthored + out.skippedForeign + out.skippedUnowned + out.unreadable;
+    expect(counted).toBe(3);
+  });
+});

packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts

@@ -182,6 +182,48 @@ export function readDeclared(engine: any, type: string): any[] {
   return [];
 }
 
+/**
+ * ADR-0086 two-doors — is this `sys_permission_set` row ENVIRONMENT-owned?
+ *
+ * The one spelling of the fact this seeder already acted on: a row the PACKAGE
+ * door does not own — `managed_by` `admin`/`platform`, the legacy `user`, or
+ * absent (pre-provenance rows, including `bootstrapPlatformAdmin`'s platform
+ * defaults) — is environment-authored config that package materialization
+ * never clobbers. {@link upsertPackagePermissionSet}'s last branch and the boot
+ * loop's unowned-vs-environment split both ask THIS predicate, so the two can
+ * never disagree about which rows are the environment's.
+ */
+function isEnvironmentOwnedRow(row: any): boolean {
+  return !!row && typeof row === 'object' && row.managed_by !== 'package';
+}
+
+/**
+ * [#21669] Does the environment already own a row under this name?
+ *
+ * The engine registry the boot loop walks holds MORE than package
+ * declarations: `loadMetaFromDb` hydrates every env-wide `sys_metadata`
+ * `permission` row into the same collection, and a set an environment
+ * authored itself — the Setup Clone action, a Setup "New", a metadata-door
+ * save — carries no package id there, because none exists. Read as a
+ * declaration, such an item is "unowned"; it is not a declaration at all. Its
+ * row stands, the environment door owns it, and Setup lists it.
+ *
+ * Answered from the batched oracle the loop already holds — no extra read.
+ * The row is this pass's own, or — on a per-organization pass — the
+ * organization-less row the ENVIRONMENT door writes (`permission-set-
+ * projection.ts` is deliberately not per-organization; see this file's
+ * header). `unknown` is NOT an answer: a read that failed cannot prove the
+ * environment owns the name, so the caller falls through to the unowned
+ * refusal exactly as before.
+ */
+async function environmentOwnsName(existingByName: ExistingByNameIndex, name: string): Promise<boolean> {
+  const lookup = await existingByName.get(name);
+  const row = lookup.status === 'present'
+    ? lookup.row
+    : lookup.status === 'absent' ? lookup.organizationLessResidue : undefined;
+  return isEnvironmentOwnedRow(row);
+}
+
 /**
  * Upsert ONE declared/published PermissionSet body into `sys_permission_set`
  * under the owning `packageId`, applying the ADR-0086 provenance rules
@@ -297,7 +339,7 @@ export async function upsertPackagePermissionSet(
     return out;
   }
 
-  if (existing.managed_by === 'package') {
+  if (!isEnvironmentOwnedRow(existing)) {
     if (!permissionSetNameIsForeign(existing.package_id, packageId)) {
       // Our own row — re-seed so the record always reflects the shipped/published
       // declaration (idempotent; covers version bumps without bookkeeping).
@@ -359,8 +401,9 @@ export async function upsertPackagePermissionSet(
     return out;
   }
 
-  // `platform`/`user` — or absent (legacy rows, incl. bootstrapPlatformAdmin
-  // defaults): env-authored config. Never clobbered by package materialization.
+  // {@link isEnvironmentOwnedRow}: `admin`/`platform`/`user` — or absent
+  // (legacy rows, incl. bootstrapPlatformAdmin defaults): env-authored config.
+  // Never clobbered by package materialization.
   out.skippedEnvAuthored += 1;
   return out;
 }
@@ -417,6 +460,20 @@ export async function bootstrapDeclaredPermissions(
     // Registry provenance first (ADR-0010 `_packageId`), author-declared
     // spec `packageId` (ADR-0086 D3) as fallback.
     const packageId: string | undefined = ps._packageId ?? ps.packageId ?? undefined;
+    // [#21669] Tell an ENVIRONMENT-owned set apart from a package declaration
+    // BEFORE judging ownership. A registry item with no package id whose row
+    // the environment already owns (a Setup clone: `managed_by:'admin'`, no
+    // `package_id`) is not an unowned declaration — it is not a declaration —
+    // and the `permission_set_declaration_unowned` warning told the operator,
+    // on every boot and every `metadata:reloaded`, that Setup could not see a
+    // set Setup lists and edits. It lands where every package declaration
+    // over an environment row lands: `skippedEnvAuthored`, never clobbered.
+    // ⛔ A declaration with no owner AND no environment row still reaches the
+    // unowned refusal below, unchanged — that warning is true.
+    if (!packageId && await environmentOwnsName(existingByName, String(ps.name))) {
+      out.skippedEnvAuthored += 1;
+      continue;
+    }
     const r = await upsertPackagePermissionSet(ql, ps, packageId, options.logger, { existingByName, organizationId, residue, refusals, collisions });
     out.seeded += r.seeded;
     out.updated += r.updated;