fix(plugin-security): the declared-positions seeder reads through the security catalog read (ADR-0131 C2 stage S2b)

.changeset/15196-declared-positions-catalog-read.md

@@ -0,0 +1,17 @@
+---
+'@objectstack/plugin-security': patch
+---
+
+A position saved through the metadata door no longer stops an app's declared positions from being seeded
+
+Clause-②: no
+
+Every seeding pass copies the app's declared positions into the `sys_position` catalog: the boot's pass and, on a walled deployment, the pass each new organization gets. That seeder read one source or the other: the engine registry alone whenever it held any position besides the six built-ins, otherwise the metadata service. A position a platform administrator saves with `PUT /api/v1/meta/position/:name` lives in the registry. So after one such save, every later pass seeded that position and none of the app's own. Measured on the showcase, walled: an organization created after the save held 7 positions (the six built-ins and the saved one) instead of 17.
+
+The seeder now reads through the security catalog read (`createSecurityCatalogReader`). It reads both sources on every pass, and for a name both hold, the registry answers first.
+
+- **New organizations** get the built-ins, every declared position and every door-saved one. Measured on the same walled showcase: 17 positions.
+- **Rows are written as before.** The door-saved position's row is unchanged: same name, label and description, `active: true`, `is_default: false`, no provenance stamp. For a name both sources hold, the registry's definition is still the one written.
+- **The six built-in positions** are still written only by the built-in seeder, with `managed_by: 'platform'`. Some deployments saved a definition under a built-in name before the six were declared, and that definition still shadows the declaration when the catalog is read. This seeder skips it by name, so it never reaches a row.
+- **A catalog read that fails** now writes nothing for that pass, and the boot reports it once as `[security] declared-position seeding failed`. That covers a source that cannot be read and a metadata list that lost a loader. Before, the seeder seeded whatever part it could read, without saying so.
+- **No grant changes.** A principal's permissions move only if it holds a position that is newly seeded into its organization.

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

@@ -13,17 +13,47 @@
  */
 
 import { describe, it, expect } from 'vitest';
+import { SchemaRegistry } from '@objectstack/objectql';
 import { bootstrapDeclaredPositions } from './bootstrap-declared-positions.js';
-import { securityBuiltinPositions } from './builtin-positions.js';
+import { registerBuiltinPositions, securityBuiltinPositions } from './builtin-positions.js';
+import { SECURITY_PLUGIN_ID } from './manifest.js';
 
-/** Minimal in-memory ql for sys_position seeding. */
-function makeQl(declared: any[] = []) {
+/**
+ * The metadata service of a deployment that declares no position in it — the
+ * catalog read takes both sources and refuses to be built over a missing one.
+ */
+const NO_METADATA_POSITIONS = { get: async () => undefined, list: async () => [] };
+
+/**
+ * A registry over a fixed list, as the catalog read uses one: its list, its
+ * by-name read (first match), and no disabled package.
+ */
+function listRegistry(items: () => any[]) {
+  return {
+    listItems: (type: string) => (type === 'position' ? [...items()] : []),
+    getItem: (type: string, name: string) => (type === 'position' ? items().find((i) => i?.name === name) : undefined),
+    isPackageDisabled: () => false,
+  };
+}
+
+/** A metadata service holding `items` as its declared positions. */
+const metadataListing = (items: any[]) => ({
+  get: async (type: string, name: string) => (type === 'position' ? items.find((i) => i.name === name) : undefined),
+  list: async (type: string) => (type === 'position' ? items.map((i) => ({ ...i })) : []),
+});
+
+/**
+ * Minimal in-memory ql for sys_position seeding. `registry` replaces the
+ * list-backed one when a case needs the real engine registry's by-name
+ * precedence.
+ */
+function makeQl(declared: any[] = [], registry?: unknown) {
   const rows: any[] = [];
   return {
     rows,
     // [#8378] Items are surfaced as the real engine surfaces them — the
     // document itself, not a `{ content: <item> }` box that nothing produces.
-    registry: { listItems: (type: string) => (type === 'position' ? [...declared] : []) },
+    registry: registry ?? listRegistry(() => declared),
     async find(object: string, q: any) {
       if (object !== 'sys_position') return [];
       const where = q?.where ?? {};
@@ -57,7 +87,7 @@ function makeQl(declared: any[] = []) {
 describe('bootstrapDeclaredPositions (#2909 T2 — seed-only semantics locked)', () => {
   it('inserts new declared positions with identity + display fields only', async () => {
     const ql = makeQl([{ name: 'contributor', label: 'Contributor', description: 'Does work' }]);
-    const r = await bootstrapDeclaredPositions(ql, null);
+    const r = await bootstrapDeclaredPositions(ql, NO_METADATA_POSITIONS);
     expect(r.seeded).toBe(1);
     const row = ql.rows[0];
     expect(row).toMatchObject({ name: 'contributor', label: 'Contributor', active: true, is_default: false });
@@ -72,7 +102,7 @@ describe('bootstrapDeclaredPositions (#2909 T2 — seed-only semantics locked)',
       id: 'pos_1', name: 'contributor', label: 'Contributor', description: 'old',
       active: true, is_default: false,
     });
-    const r = await bootstrapDeclaredPositions(ql, null);
+    const r = await bootstrapDeclaredPositions(ql, NO_METADATA_POSITIONS);
     expect(r.updated).toBe(1);
     expect(ql.rows[0].label).toBe('Contributor v2');
     expect(ql.rows[0].description).toBe('new text');
@@ -87,7 +117,7 @@ describe('bootstrapDeclaredPositions (#2909 T2 — seed-only semantics locked)',
       active: false, is_default: true, delegatable: true, managed_by: 'package',
       permissions: ['something_admin_set'],
     });
-    await bootstrapDeclaredPositions(ql, null);
+    await bootstrapDeclaredPositions(ql, NO_METADATA_POSITIONS);
     const row = ql.rows[0];
     expect(row.active).toBe(false);
     expect(row.is_default).toBe(true);
@@ -100,32 +130,23 @@ describe('bootstrapDeclaredPositions (#2909 T2 — seed-only semantics locked)',
 
   it('is idempotent — a re-run inserts nothing new', async () => {
     const ql = makeQl([{ name: 'a', label: 'A' }, { name: 'b', label: 'B' }]);
-    const r1 = await bootstrapDeclaredPositions(ql, null);
+    const r1 = await bootstrapDeclaredPositions(ql, NO_METADATA_POSITIONS);
     expect(r1.seeded).toBe(2);
-    const r2 = await bootstrapDeclaredPositions(ql, null);
+    const r2 = await bootstrapDeclaredPositions(ql, NO_METADATA_POSITIONS);
     expect(r2.seeded).toBe(0);
     expect(ql.rows).toHaveLength(2);
   });
 });
 
 /**
- * [ADR-0131 C2 S2] What the seeder reads now that the six built-in positions
- * are declared metadata: the same registry-first two-step as before, with the
- * six taken out of both its decision and its result.
- *
- * The plugin registers the six with the engine registry on every boot. Two
- * things must survive that: the stack-declared positions the metadata service
- * holds still seed (six registered names must not make the registry "hold a
- * position" and silence them), and the six stay `bootstrapBuiltinRoles`'s rows
- * — this seeder writes no copy of them and refreshes none of their columns.
+ * [ADR-0131 C2 S2] The six built-in positions are declared metadata, which the
+ * catalog read lists; they stay `bootstrapBuiltinRoles`'s rows — this seeder
+ * writes no copy of them and refreshes none of their columns.
  */
 describe('bootstrapDeclaredPositions — the six built-in positions are not this seeder’s', () => {
   const builtins = () => securityBuiltinPositions.map((p) => ({ ...p }));
-  const metadataListing = (items: any[]) => ({
-    list: async (type: string) => (type === 'position' ? items.map((i) => ({ ...i })) : []),
-  });
 
-  it('reads the metadata service when the registry holds the six and nothing else', async () => {
+  it('seeds the metadata service’s positions while the registry holds the six', async () => {
     const ql = makeQl(builtins());
     const r = await bootstrapDeclaredPositions(
       ql,
@@ -135,13 +156,6 @@ describe('bootstrapDeclaredPositions — the six built-in positions are not this
     expect(r).toEqual({ seeded: 2, updated: 0, unchanged: 0, unreadable: 0 });
   });
 
-  it('keeps the two-step otherwise: a registry holding another position answers alone, minus the six', async () => {
-    const ql = makeQl([...builtins(), { name: 'door_authored', label: 'Door Authored' }]);
-    const r = await bootstrapDeclaredPositions(ql, metadataListing([{ name: 'field_rep', label: 'Field Rep' }]));
-    expect(ql.rows.map((row) => row.name)).toEqual(['door_authored']);
-    expect(r).toEqual({ seeded: 1, updated: 0, unchanged: 0, unreadable: 0 });
-  });
-
   it('writes nothing for the six on a fresh organization — no copy ahead of the built-in pass', async () => {
     const ql = makeQl(builtins());
     const r = await bootstrapDeclaredPositions(ql, metadataListing(builtins()), { organizationId: 'org_a' });
@@ -164,3 +178,122 @@ describe('bootstrapDeclaredPositions — the six built-in positions are not this
     expect(r).toEqual({ seeded: 1, updated: 0, unchanged: 0, unreadable: 0 });
   });
 });
+
+/**
+ * [ADR-0131 C2 S2b] The seeder reads through the security catalog read — the
+ * engine registry and the metadata service, in its one read order — where it
+ * used to take the registry ALONE whenever the registry held any position
+ * besides the six.
+ *
+ * The registry here is the real `SchemaRegistry`, so a name's answer is the
+ * registry's own by-name precedence, not a fixture's: the six registered the
+ * way the plugin registers them (packaged, composite keys), and a definition a
+ * metadata author saved hydrated the way the door hydrates one (no package, the
+ * bare slot).
+ */
+describe('bootstrapDeclaredPositions — one catalog read, both sources (ADR-0131 C2 S2b)', () => {
+  const declaredRegistry = (...authored: Array<Record<string, unknown>>) => {
+    const registry = new SchemaRegistry();
+    for (const item of authored) registry.registerItem('position', { ...item }, 'name');
+    registerBuiltinPositions(registry, SECURITY_PLUGIN_ID);
+    return registry;
+  };
+  const byName = (rows: any[]) => Object.fromEntries(rows.map((row) => [row.name, row]));
+
+  it('a door-authored position no longer silences the stack’s declared positions', async () => {
+    const ql = makeQl([], declaredRegistry({ name: 'door_authored', label: 'Door Authored' }));
+    const r = await bootstrapDeclaredPositions(
+      ql,
+      metadataListing([{ name: 'field_rep', label: 'Field Rep' }, { name: 'auditor', label: 'Auditor' }]),
+      { organizationId: 'org_late' },
+    );
+    expect(ql.rows.map((row) => row.name).sort()).toEqual(['auditor', 'door_authored', 'field_rep']);
+    expect(r).toEqual({ seeded: 3, updated: 0, unchanged: 0, unreadable: 0 });
+  });
+
+  it('writes the door-authored position’s row as the registry-only read wrote it', async () => {
+    const ql = makeQl([], declaredRegistry({ name: 'door_authored', label: 'Door Authored' }));
+    await bootstrapDeclaredPositions(ql, metadataListing([{ name: 'field_rep', label: 'Field Rep' }]));
+    const { id, ...row } = byName(ql.rows).door_authored;
+    expect(id).toMatch(/^position_/);
+    expect(row).toEqual({ name: 'door_authored', label: 'Door Authored', description: null, active: true, is_default: false });
+  });
+
+  // The read order, measured: for a name BOTH sources hold, the registry's body
+  // is the one written — what the either-or wrote too, because the registry
+  // holding that name was itself what made the registry answer.
+  it('a name both sources hold is written from the registry’s body', async () => {
+    const ql = makeQl([], declaredRegistry({ name: 'field_rep', label: 'Field Rep (saved at the door)', description: 'Door text' }));
+    const r = await bootstrapDeclaredPositions(
+      ql,
+      metadataListing([{ name: 'field_rep', label: 'Field Rep', description: 'Declared text' }]),
+    );
+    expect(ql.rows.map((row) => [row.name, row.label, row.description])).toEqual([
+      ['field_rep', 'Field Rep (saved at the door)', 'Door text'],
+    ]);
+    expect(r).toEqual({ seeded: 1, updated: 0, unchanged: 0, unreadable: 0 });
+  });
+
+  // A definition saved under a built-in name before the six were declared is
+  // hydrated into the bare slot and shadows the declaration at read. Positive
+  // control first: the registry really answers the stored body for the name.
+  it('a stored definition shadowing a built-in name is neither seeded nor restamped', async () => {
+    // Both hydration shapes: stated tenant-authored only (hydrated before the
+    // six were registered), and with the declaration's envelope grafted on
+    // (hydrated after) — which names THIS plugin's package.
+    const shadows = [
+      { name: 'org_admin', label: 'Repurposed Org Admin', description: 'Saved at the door', _provenance: 'org' },
+      { name: 'everyone', label: 'Repurposed Everyone', description: 'Saved at the door', _provenance: 'org', _packageId: SECURITY_PLUGIN_ID },
+    ];
+    const registry = declaredRegistry(...shadows);
+    expect((registry.getItem('position', 'org_admin') as any)?.label).toBe('Repurposed Org Admin');
+    expect((registry.getItem('position', 'everyone') as any)?.label).toBe('Repurposed Everyone');
+
+    const seeded = securityBuiltinPositions.map((p, i) => ({
+      id: `pos_builtin_${i}`, name: p.name, label: p.label, description: p.description,
+      managed_by: 'platform', active: true, is_default: false,
+    }));
+    const fresh = makeQl([], registry);
+    const onFresh = await bootstrapDeclaredPositions(fresh, NO_METADATA_POSITIONS, { organizationId: 'org_a' });
+    expect(fresh.rows).toEqual([]);
+    expect(onFresh).toEqual({ seeded: 0, updated: 0, unchanged: 0, unreadable: 0 });
+
+    const existing = makeQl([], registry);
+    existing.rows.push(...seeded.map((row) => ({ ...row })));
+    const onExisting = await bootstrapDeclaredPositions(existing, NO_METADATA_POSITIONS);
+    expect(existing.rows).toEqual(seeded);
+    expect(onExisting).toEqual({ seeded: 0, updated: 0, unchanged: 0, unreadable: 0 });
+  });
+
+  // A read that did not happen is not "nothing declared": the seeder writes
+  // nothing and the failure reaches its caller, which reports it.
+  it('writes nothing when a catalog source cannot be read, and says so', async () => {
+    const ql = makeQl([{ name: 'field_rep', label: 'Field Rep' }]);
+    (ql.registry as any).listItems = () => { throw new Error('registry unreachable'); };
+    const failure = await bootstrapDeclaredPositions(ql, NO_METADATA_POSITIONS).then(
+      () => undefined,
+      (e: any) => ({ code: e?.code, status: e?.status }),
+    );
+    expect(failure).toEqual({ code: 'SERVICE_UNAVAILABLE', status: 503 });
+    expect(ql.rows).toEqual([]);
+
+    const degraded = makeQl([{ name: 'field_rep', label: 'Field Rep' }]);
+    const lostLoader = {
+      ...NO_METADATA_POSITIONS,
+      listDiagnosed: async () => ({ items: [{ name: 'auditor', label: 'Auditor' }], degraded: true, errors: ['loader down'] }),
+    };
+    const partial = await bootstrapDeclaredPositions(degraded, lostLoader).then(
+      () => undefined,
+      (e: any) => ({ code: e?.code, status: e?.status }),
+    );
+    expect(partial).toEqual({ code: 'SERVICE_UNAVAILABLE', status: 503 });
+    expect(degraded.rows).toEqual([]);
+  });
+
+  it('refuses a composition with no metadata service rather than reading the registry alone', async () => {
+    const ql = makeQl([{ name: 'field_rep', label: 'Field Rep' }]);
+    const failure = await bootstrapDeclaredPositions(ql, null).then(() => undefined, (e: unknown) => e);
+    expect(failure).toBeInstanceOf(TypeError);
+    expect(ql.rows).toEqual([]);
+  });
+});