diff --git a/.changeset/23430-grant-permission-set-id-retired.md b/.changeset/23430-grant-permission-set-id-retired.md new file mode 100644 index 00000000000..4ab6d2854e1 --- /dev/null +++ b/.changeset/23430-grant-permission-set-id-retired.md @@ -0,0 +1,36 @@ +--- +'@objectstack/plugin-security': minor +'@objectstack/cli': minor +'@objectstack/spec': minor +--- + +feat(plugin-security,cli)!: `sys_user_permission_set.permission_set_id` is retired, the boot-time grant-name backfill is deleted, and a grant is unique per user, set and organization (ADR-0131 D4/D10) + +Clause-②: yes (narrowing) + + + +**BREAKING**, shipped as `minor` under the repo's launch-window convention for breaking changes. + +A permission-set grant names its set by `permission_set`, its one reference (ADR-0131 D4). The legacy id column is no longer declared, and the grant's uniqueness now keys the name. + +FROM → TO: + +- `sys_user_permission_set.permission_set_id` → `permission_set`, the machine name of the set the grant holds. A seed row or data-door write that still carries `permission_set_id` is refused with `400 INVALID_FIELD`. Fix: delete the key and name the set, `{ user_id, permission_set: '', organization_id }`. +- The unique index (`user_id`, `permission_set_id`, `organization_id`) and the (`permission_set_id`) index → one unique index on (`user_id`, `permission_set`, `organization_id`), declared `unique: 'organization'`: its organization key part is NULL-safe (ADR-0120 D3), so it refuses a second grant of one set to one user in one organization AND a second platform-wide grant (organization NULL), for every writer and every write path — batches, multi-row updates, concurrent writes and raw SQL included. The data doors answer `409 UNIQUE_VIOLATION` (the engine throws `DuplicateRecordError`, `DUPLICATE_RECORD`). Fix: keep the grant that exists; to change it, update that row. +- The boot-time grant-name backfill (`runOneTimeGrantPermissionSetNameBackfill`, the `sys_migration` entry it recorded at `kernel:bootstrapped`) → `os migrate organization-ownership --apply`. Its verified rewrite names every grant from the physical `permission_set_id` column by raw SQL, verifies each name against the security catalog, and its `column-drops` step then drops the column through the schema-drift ceremony (schema sync is additive, so an undeclared column stays until it is dropped). Fix: run the ceremony on every database upgraded from a release before the `permission_set` column existed, then restart the server: the boot's organization-administrator sweep revokes, by name, a newly named grant whose pair no longer qualifies, and a server left running across `--apply` serves it until then. + +On a 17.x database every grant names nothing until `--apply` names it, so between the first v18 boot and `--apply` no grant confers anything, the platform administrator's included. + +What else changes: + +- `os migrate organization-ownership --plan` reports a `grants` block on `sys_user_permission_set`, keyed like the new index (a NULL organization keyed with NULL): + - `duplicates`: groups of named grants holding one set for one user in one organization. `--apply`'s `column-drops` step does not drop `permission_set_id` while one group remains. + - `rewriteCollisions`: legacy grants whose derived name their user already holds in the same organization. `--apply`'s rewrite names none of them and blocks, naming both ids. + - On a database without the `permission_set` column it counts zero and names the remedy (boot this release once), instead of refusing the plan. + + The ceremony deletes no grant: remove the one you do not keep, then re-run. +- The organization-administrator reconcile and its boot sweep write no grant for a pair holding an unnamed grant in that organization: the ceremony's rewrite names that grant, and a named grant beside it would hold the key the rename needs. +- `--apply` drops the two retired indexes before the column, in a separate earlier schema-drift apply: MySQL refuses to drop a column a key still uses. +- The organization-administrator reconcile and its boot sweep, the platform-admin bootstrap's unnamed-grant leg (Leg C) and the catalog reference report read grants by name only. A legacy grant that names nothing confers nothing, still withholds the platform-admin promotion while it is unscoped, and is reached by no revocation until the ceremony's rewrite names it; once named, the boot sweep revokes it where its pair no longer qualifies. The report's `UnnamedGrant` entries lose `permissionSetId`. +- `GET /meta/object/sys_user_permission_set` no longer serves the hidden `permission_set_id` field. diff --git a/content/docs/permissions/tenant-audit-census.mdx b/content/docs/permissions/tenant-audit-census.mdx index 81ad4c8c715..a0d13072c4a 100644 --- a/content/docs/permissions/tenant-audit-census.mdx +++ b/content/docs/permissions/tenant-audit-census.mdx @@ -122,7 +122,7 @@ are reported as `undecidable` rather than assumed either way. The same holds twice over for the context. An options argument spelled as a literal can be read; one spelled `options`, `{ ...opts }`, or handed through a -forwarding shim cannot, and **53 of the 227 sites are spelled that way**. A +forwarding shim cannot, and **53 of the 225 sites are spelled that way**. A context resolved from an inline literal or a local `const` can be tested for `isSystem`; one arriving from a helper call cannot. @@ -187,10 +187,10 @@ reproduce them. Where it disagrees, it disagrees on the page: | carried figure | where it survives | this census | | :--- | :--- | ---: | -| 175 write call sites | quoted in the merged changeset | **227** | +| 175 write call sites | quoted in the merged changeset | **225** | | 24 carrying no tenant context | quoted in the merged changeset | **2** provable and tenancy-enabled; **32** more whose options argument is unreadable | -| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **154 of 227** decidable, **73** undecidable | -| 135 (77%) silenced by the `isSystem` guard before the posture gate | the lost issue body — **no surviving corroboration** | **not reproduced**: 122 decidably elevated, 0 decidably not, 97 undecidable | +| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **154 of 225** decidable, **71** undecidable | +| 135 (77%) silenced by the `isSystem` guard before the posture gate | the lost issue body — **no surviving corroboration** | **not reproduced**: 121 decidably elevated, 0 decidably not, 96 undecidable | | 141 and 132, two independent re-derivations | the card that filed this work | — | **The differences are not reconciled, and deliberately so.** The old census's @@ -207,11 +207,11 @@ would report a smaller number and would not say so. The fourth row is the one worth flagging to anyone citing it. **The 135 / 77% figure has no surviving corroboration anywhere in the tree.** This census reads -122 of 227 (54%) as decidably elevated, with 97 more whose elevation is a +121 of 225 (54%) as decidably elevated, with 96 more whose elevation is a run-time fact — so the claim is neither confirmed nor refuted, and the honest answer is that a static reading cannot settle it. -⇒ **Cite `2 / 227`, and say what it is**: the sites whose options argument was +⇒ **Cite `2 / 225`, and say what it is**: the sites whose options argument was READ and holds no tenant context, against a decidably tenancy-enabled object. That is the control's provable yield surface. ⛔ Do not cite it as "the sites without tenant context" — **32 further sites** have an options argument this @@ -223,23 +223,23 @@ cannot read, and they are neither in nor out. | what | count | | :--- | ---: | -| write call sites on the application surface | **227** | +| write call sites on the application surface | **225** | | …whose object name is statically decidable | 154 | -| …whose object name is chosen at run time | 73 | +| …whose object name is chosen at run time | 71 | | …against an object with tenancy ENABLED | 153 | | …against an object that declares tenancy off | 1 | -| threading a tenant context | 166 | +| threading a tenant context | 164 | | PROVABLY carrying none (options read, no context key) | **8** | | …of those, against a decidably tenancy-enabled object | **2** | | options argument UNREADABLE — may or may not carry one | 53 | | …of those, against a decidably tenancy-enabled object | 32 | -| threading a decidably ELEVATED (`isSystem`) context | 122 | +| threading a decidably ELEVATED (`isSystem`) context | 121 | | threading a context that is decidably NOT elevated | 0 | -| threading a context whose elevation is a run-time fact | 97 | +| threading a context whose elevation is a run-time fact | 96 | | how the instrument reached the site | count | | :--- | ---: | -| receiver carried a readable engine type | 189 | +| receiver carried a readable engine type | 187 | | receiver erased, placed by the object NAME | 28 | | receiver erased, placed by an `object: string` PARAMETER | 5 | | receiver erased, placed by an `UNTYPED_RECEIVERS` row | 5 | @@ -247,7 +247,7 @@ cannot read, and they are neither in nor out. | object name spelled inline | 104 | | object name spelled through a `const` | 50 | | object name is an `object: string` parameter | 7 | -| object name is some other run-time expression | 66 | +| object name is some other run-time expression | 64 | ### Subtractions the census could NOT defend — enforced @@ -297,12 +297,12 @@ holds still. They are required to be HERE and to say WHEN they were true; their values are not compared. The reasoning, and the measurement behind it, are in `scripts/check-tenant-audit-census.mjs`. -Measured on 2026-10-14 at `f98ac45c5`. +Measured on 2026-10-14 at `95898bba1`. | corpus scale (not enforced) | count | | :--- | ---: | -| tracked non-test sources scanned | 650 | -| engine-shaped types recognised | 76 | +| tracked non-test sources scanned | 649 | +| engine-shaped types recognised | 75 | | declared objects in the registry | 116 | | same-named calls subtracted as non-engine | 164 | diff --git a/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md b/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md index c08f7aab950..996b49762e8 100644 --- a/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md +++ b/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md @@ -33,19 +33,19 @@ silent, and `node scripts/tenant-audit-census.mjs --write` is the resolution. | Measure | Value | |---|---:| -| Write call sites | 227 | +| Write call sites | 225 | | Object name statically decidable | 154 | -| Object name chosen at run time | 73 | +| Object name chosen at run time | 71 | | Against a tenancy-enabled object | 153 | | Against an object declaring tenancy off | 1 | -| Threading a tenant context | 166 | +| Threading a tenant context | 164 | | Provably carrying none | 8 | | …and decidably tenancy-enabled | 2 | | Options argument unreadable | 53 | | …and decidably tenancy-enabled | 32 | -| Threading a decidably elevated context | 122 | +| Threading a decidably elevated context | 121 | | Threading a decidably non-elevated context | 0 | -| Threading a context of undecidable elevation | 97 | +| Threading a context of undecidable elevation | 96 | ## Subtractions the census could NOT defend — enforced @@ -90,12 +90,12 @@ holds still. They are required to be HERE and to say WHEN they were true; their values are not compared. The reasoning, and the measurement behind it, are in `scripts/check-tenant-audit-census.mjs`. -Measured on 2026-10-14 at `f98ac45c5`. +Measured on 2026-10-14 at `95898bba1`. | corpus scale (not enforced) | count | | :--- | ---: | -| tracked non-test sources scanned | 650 | -| engine-shaped types recognised | 76 | +| tracked non-test sources scanned | 649 | +| engine-shaped types recognised | 75 | | declared objects in the registry | 116 | | same-named calls subtracted as non-engine | 164 | @@ -160,8 +160,6 @@ Measured on 2026-10-14 at `f98ac45c5`. | `packages/plugins/plugin-security/src/bootstrap-platform-admin.ts` | `insert` | `object` | undecidable | elevated | 1 | | `packages/plugins/plugin-security/src/claim-seed-ownership.ts` | `update` | `schema.name` | undecidable | elevated | 1 | | `packages/plugins/plugin-security/src/cleanup-package-permissions.ts` | `delete` | `object` | undecidable | elevated | 1 | -| `packages/plugins/plugin-security/src/grant-permission-set-name-backfill.ts` | `insert` | `DATA_MIGRATION_FLAG_OBJECT` | undecidable | elevated | 1 | -| `packages/plugins/plugin-security/src/grant-permission-set-name-backfill.ts` | `update` | `GRANT_OBJECT` | undecidable | context, elevation undecidable | 1 | | `packages/plugins/plugin-security/src/invitation-placement.ts` | `insert` | `sys_user_position` | enabled | elevated | 1 | | `packages/plugins/plugin-security/src/suggested-audience-bindings.ts` | `delete` | `sys_audience_binding_suggestion` | enabled | context, elevation undecidable | 1 | | `packages/plugins/plugin-security/src/suggested-audience-bindings.ts` | `insert` | `sys_audience_binding_suggestion` | enabled | context, elevation undecidable | 1 | diff --git a/packages/cli/src/commands/migrate/organization-ownership.ts b/packages/cli/src/commands/migrate/organization-ownership.ts index 76382ed2e7d..875d77acde2 100644 --- a/packages/cli/src/commands/migrate/organization-ownership.ts +++ b/packages/cli/src/commands/migrate/organization-ownership.ts @@ -37,7 +37,6 @@ import { TemplateChoiceRefusal, type ApplyExecutor, type CeremonyLedgerPort, - type ColumnDropOutcome, type OrganizationOwnershipApplyReport, } from '../../utils/organization-ownership-apply.js'; @@ -482,43 +481,29 @@ export default class MigrateOrganizationOwnership extends Command { }, }, columnDrops: { - drop: async (tables) => { - const drift = await liveStack.detectManagedDrift(); - const wanted = new Set(tables); - const entries = drift.filter((d) => { - const op = (d as { op?: { type?: string; column?: string } }).op; - return op?.type === 'drop_column' && op.column === 'organization_id' && wanted.has(String(d.table)); - }); - const outcomes: ColumnDropOutcome[] = []; - const proposed = new Set(entries.map((d) => String(d.table))); + drop: async (columns) => { // The tables this booted composition declares, keyed as the driver // keys its managed set. A table outside it can never be proposed, // whatever a release declares: `not-declared`, and `not-proposed` // only for a declared one. A rotation shard is judged by its base. const { StorageNameMapping } = await import('@objectstack/spec/system'); const { rotationBaseOf } = await import('../../utils/unmanaged-tables.js'); + const { buildIndexName } = await import('@objectstack/driver-sql'); + const { dropColumnsThroughDrift } = await import('../../utils/organization-ownership-column-drops.js'); const declared = new Set( (liveStack.allObjects() as Array<{ name?: unknown } | null>) .map((o) => o?.name) .filter((name): name is string => typeof name === 'string' && name.length > 0) .map((name) => StorageNameMapping.resolveTableName({ name })), ); - for (const table of tables) { - if (!proposed.has(table)) { - outcomes.push( - declared.has(rotationBaseOf(table) ?? table) - ? { table, outcome: 'not-proposed', detail: 'the drift differ proposed no drop for it' } - : { table, outcome: 'not-declared' }, - ); - } - } - if (entries.length === 0) return outcomes; - const { applied, skipped } = await liveStack.applyMigrationEntries(entries, { allowDestructive: true }); - for (const d of applied) outcomes.push({ table: String(d.table), outcome: 'dropped' }); - for (const d of skipped) { - outcomes.push({ table: String(d.table), outcome: 'failed', detail: String((d as { message?: unknown }).message ?? 'skipped by the schema-drift ceremony') }); - } - return outcomes; + return dropColumnsThroughDrift( + { + drift: liveStack, + isDeclared: (table) => declared.has(rotationBaseOf(table) ?? table), + indexName: (table, indexColumns, unique) => buildIndexName(table, [...indexColumns], unique), + }, + columns, + ); }, }, ledger, diff --git a/packages/cli/src/utils/organization-ownership-apply.served-catalog.integration.test.ts b/packages/cli/src/utils/organization-ownership-apply.served-catalog.integration.test.ts index 008f535b31d..bc3cd9a9c12 100644 --- a/packages/cli/src/utils/organization-ownership-apply.served-catalog.integration.test.ts +++ b/packages/cli/src/utils/organization-ownership-apply.served-catalog.integration.test.ts @@ -129,7 +129,7 @@ async function ceremony(posture: string, db: Database.Database): Promise { throw new Error(`the door was asked to save ${type} ${name}`); } }, templates: { find: async () => undefined }, - columnDrops: { drop: async (tables) => tables.map((table) => ({ table, outcome: 'not-proposed' as const })) }, + columnDrops: { drop: async (columns) => columns.map((c) => ({ table: c.table, column: c.column, outcome: 'not-proposed' as const })) }, ledger: { read: async () => checkpoint, write: async (cp) => { checkpoint = cp; } }, }); } diff --git a/packages/cli/src/utils/organization-ownership-apply.test.ts b/packages/cli/src/utils/organization-ownership-apply.test.ts index ca33a92c9cd..4e4b90b79a4 100644 --- a/packages/cli/src/utils/organization-ownership-apply.test.ts +++ b/packages/cli/src/utils/organization-ownership-apply.test.ts @@ -34,6 +34,9 @@ import { type OrganizationOwnershipApplyReport, type TemplatePort, } from './organization-ownership-apply.js'; +import { buildOrganizationOwnershipPlan } from './organization-ownership-plan.js'; +import { backfillOrgAdminGrants } from '@objectstack/plugin-security'; +import { assertEngineDeleteDispatch } from '@objectstack/metadata-core'; let dir: string; let file: string; @@ -125,11 +128,11 @@ function ledger(): CeremonyLedgerPort & { checkpoint: CeremonyCheckpoint | null; /** The schema-drift ceremony, as SQLite answers it: the column goes. */ const columnDrops: ColumnDropPort = { - async drop(tables) { - return tables.map((table) => { - writes.push(`ALTER TABLE ${table} DROP COLUMN organization_id`); - db.exec(`ALTER TABLE "${table}" DROP COLUMN organization_id`); - return { table, outcome: 'dropped' as const }; + async drop(columns) { + return columns.map(({ table, column }) => { + writes.push(`ALTER TABLE ${table} DROP COLUMN ${column}`); + db.exec(`ALTER TABLE "${table}" DROP COLUMN "${column}"`); + return { table, column, outcome: 'dropped' as const }; }); }, }; @@ -393,13 +396,15 @@ describe('ADR-0131 D10 --apply — the acceptance fixture', () => { { id: 'set_secret', namespace: 'ai', key: 'api_key', value: null, encrypted: 1, locked: 0, locked_reason: null, value_enc: 'secret:h_1' }, ]); - // Every grant names its set; the id column is still there (C7b's later drop). - expect(rows('SELECT id, permission_set, permission_set_id FROM sys_user_permission_set ORDER BY id')).toEqual([ - { id: 'grant_1', permission_set: 'member_default', permission_set_id: 'ps_org' }, - { id: 'grant_2', permission_set: 'member_default', permission_set_id: 'ps_null' }, - { id: 'grant_3', permission_set: 'field_auditor', permission_set_id: 'ps_auth' }, - { id: 'grant_4', permission_set: 'member_default', permission_set_id: 'ps_null' }, + // Every grant names its set, and the id column the rewrite read is gone (C7b). + expect(rows('SELECT id, permission_set FROM sys_user_permission_set ORDER BY id')).toEqual([ + { id: 'grant_1', permission_set: 'member_default' }, + { id: 'grant_2', permission_set: 'member_default' }, + { id: 'grant_3', permission_set: 'field_auditor' }, + { id: 'grant_4', permission_set: 'member_default' }, ]); + expect(rows(`SELECT name FROM pragma_table_info('sys_user_permission_set') WHERE name = 'permission_set_id'`)).toEqual([]); + expect(writes).toContain('ALTER TABLE sys_user_permission_set DROP COLUMN permission_set_id'); // Attributed rows carry their organization — the anchor's, else the Default Organization. expect(rows(`SELECT organization_id FROM sys_file WHERE id = 'file_1'`)).toEqual([{ organization_id: 'org_b' }]); @@ -698,9 +703,11 @@ describe('ADR-0131 D10 --apply — idempotent, resumable, gated', () => { const l = ledger(); const answering = async (outcome: 'not-declared' | 'not-proposed'): Promise => { const real = columnDrops.drop; - columnDrops.drop = async (tables) => [ - ...(await real(tables.filter((t) => t !== 'sys_flow_dispatch'))), - ...(tables.includes('sys_flow_dispatch') ? [{ table: 'sys_flow_dispatch', outcome }] : []), + columnDrops.drop = async (columns) => [ + ...(await real(columns.filter((c) => c.table !== 'sys_flow_dispatch'))), + ...(columns.some((c) => c.table === 'sys_flow_dispatch') + ? [{ table: 'sys_flow_dispatch', column: 'organization_id', outcome }] + : []), ]; try { return await apply(c, l); @@ -714,7 +721,8 @@ describe('ADR-0131 D10 --apply — idempotent, resumable, gated', () => { const notDeclared = await answering('not-declared'); const drops = notDeclared.steps.find((s) => s.step === 'column-drops')!; expect(drops.status).toBe('blocked'); - expect(drops.summary).toEqual({ dropped: 1, due: 2, notDeclared: 1 }); + // sys_job.organization_id and the grant's retired permission_set_id drop; sys_flow_dispatch is kept. + expect(drops.summary).toEqual({ dropped: 2, due: 3, notDeclared: 1 }); expect(flowDispatchBlocker(notDeclared)).toHaveLength(1); expect(rows(`SELECT name FROM pragma_table_info('sys_flow_dispatch') WHERE name = 'organization_id'`)).toHaveLength(1); expect(rows(`SELECT name FROM pragma_table_info('sys_job') WHERE name = 'organization_id'`)).toHaveLength(0); @@ -744,3 +752,147 @@ describe('ADR-0131 D10 --apply — idempotent, resumable, gated', () => { expect(rows(`SELECT name FROM pragma_table_info('sys_job') WHERE name = 'organization_id'`)).toHaveLength(0); }); }); + +describe('ADR-0131 D10 --apply — the grant\'s retired id column (C7b, seat answers 6162538253 Q5 A)', () => { + const plan = () => buildOrganizationOwnershipPlan({ reader: executor(), posture: 'single', database: file }); + const grantsPlan = async () => (await plan()).tables.find((t) => t.object === 'sys_user_permission_set')!.grants!; + const idColumn = () => rows(`SELECT name FROM pragma_table_info('sys_user_permission_set') WHERE name = 'permission_set_id'`); + + /** + * The engine surface the organization-administrator sweep reads and writes, + * over the fixture database. An insert keeps the columns the 17.x table has. + */ + const sqliteQl = { + async find(object: string, query: { where?: Record; limit?: number }) { + const where = query?.where ?? {}; + const keys = Object.keys(where); + const text = keys.map((k) => (where[k] === null ? `"${k}" IS NULL` : `"${k}" = ?`)).join(' AND '); + return db.prepare(`SELECT * FROM "${object}"${keys.length ? ` WHERE ${text}` : ''}${query?.limit ? ` LIMIT ${query.limit}` : ''}`) + .all(...keys.filter((k) => where[k] !== null).map((k) => where[k] as never)) as Record[]; + }, + async insert(object: string, data: Record) { + const columns = new Set(rows(`SELECT name FROM pragma_table_info('${object}')`).map((r) => String(r.name))); + const keys = Object.keys(data).filter((k) => columns.has(k)); + db.prepare(`INSERT INTO "${object}" (${keys.map((k) => `"${k}"`).join(', ')}) VALUES (${keys.map(() => '?').join(', ')})`) + .run(...keys.map((k) => data[k] as never)); + return data; + }, + async delete(object: string, options: { where?: { id?: unknown } }) { + assertEngineDeleteDispatch(options as never); + db.prepare(`DELETE FROM "${object}" WHERE id = ?`).run(options?.where?.id as never); + return 1; + }, + }; + + /** A 17.x organization owner: a membership, and a legacy grant of the organization-administrator set by id. */ + function seedOrgAdminLegacyGrant(): void { + db.exec(` + CREATE UNIQUE INDEX uniq_grant_key ON sys_user_permission_set (user_id, permission_set, COALESCE(organization_id, '__global__')); + CREATE TABLE sys_member (id TEXT PRIMARY KEY, organization_id TEXT, user_id TEXT, role TEXT); + INSERT INTO sys_member VALUES ('mem_oa', 'org_default', 'usr_oa', 'owner'); + INSERT INTO sys_permission_set VALUES ('ps_oa', NULL, 'organization_admin_no_bypass', 'Org Admin', 'platform', 1, '{}', '[]'); + INSERT INTO sys_user_permission_set VALUES ('grant_oa', 'org_default', 'usr_oa', 'ps_oa', NULL); + `); + } + const withOrgAdminSet = (c: ReturnType): ReturnType => { + const resolve = c.resolve.bind(c); + c.resolve = async (type, name) => + type === 'permission' && name === 'organization_admin_no_bypass' ? { definition: { name, label: 'Org Admin' } } : resolve(type, name); + return c; + }; + + it('[Q-S3b (ii)] on a 17.x grant table carrying the declared index, the first boot\'s organization-administrator sweep writes NO row beside the owner\'s legacy grant, so --plan counts no collision and the rewrite names it and completes', async () => { + seedOrgAdminLegacyGrant(); + const swept = await backfillOrgAdminGrants(sqliteQl, { posture: 'single' }); + expect(swept).toMatchObject({ granted: 0, skipped: 1 }); + expect(rows(`SELECT id FROM sys_user_permission_set WHERE user_id = 'usr_oa'`)).toEqual([{ id: 'grant_oa' }]); + expect((await grantsPlan()).rewriteCollisions).toEqual([]); + + const report = await apply(withOrgAdminSet(catalog()), ledger()); + const rewrite = report.steps.find((s) => s.step === 'rewrite')!; + expect(rewrite.status).toBe('done'); + expect(rewrite.summary.collisions).toBe(0); + expect(rows(`SELECT id, permission_set FROM sys_user_permission_set WHERE user_id = 'usr_oa'`)) + .toEqual([{ id: 'grant_oa', permission_set: 'organization_admin_no_bypass' }]); + // …and once named, the next sweep finds it and writes nothing either. + expect(await backfillOrgAdminGrants(sqliteQl, { posture: 'single' })).toMatchObject({ granted: 0 }); + }); + + it('the safety net — a legacy grant whose name its user already holds in its organization (written by any other writer) is not named, on a database carrying the declared index: --plan counts the pair, --apply blocks the rewrite naming both ids, deletes neither, and finishes once the operator removes one', async () => { + db.exec(` + CREATE UNIQUE INDEX uniq_grant_key ON sys_user_permission_set (user_id, permission_set, COALESCE(organization_id, '__global__')); + INSERT INTO sys_user_permission_set VALUES ('grant_named', 'org_default', 'usr_1', NULL, 'member_default'); + `); + expect((await grantsPlan()).rewriteCollisions).toEqual([{ id: 'grant_1', name: 'member_default', heldBy: 'grant_named' }]); + + const report = await apply(catalog(), ledger()); + const rewrite = report.steps.find((s) => s.step === 'rewrite')!; + expect(rewrite.status).toBe('blocked'); + expect(rewrite.summary.collisions).toBe(1); + expect(rewrite.blockers.join('\n')).toMatch(/1 legacy grant\(s\) would name a permission set their user already holds in the same organization \(grant_1 \(member_default, held by grant_named\)\)/); + expect(rows(`SELECT id, permission_set FROM sys_user_permission_set WHERE user_id = 'usr_1' ORDER BY id`)) + .toEqual([{ id: 'grant_1', permission_set: null }, { id: 'grant_named', permission_set: 'member_default' }]); + expect(writes.some((w) => /DELETE FROM "sys_user_permission_set"/.test(w))).toBe(false); + expect(idColumn()).toHaveLength(1); + + // The operator keeps the named grant; the re-run names the rest and drops the column. + db.exec(`DELETE FROM sys_user_permission_set WHERE id = 'grant_1'`); + const again = await apply(catalog(), ledger()); + expect(again.steps.find((s) => s.step === 'rewrite')!.status).toBe('done'); + expect(again.steps.find((s) => s.step === 'column-drops')!.status).toBe('done'); + expect(idColumn()).toEqual([]); + }); + + it('two NAMED grants of one set to one user in one organization (an index that never built) block the id column\'s drop; neither is deleted', async () => { + db.exec(`INSERT INTO sys_user_permission_set VALUES + ('grant_d1', 'org_default', 'usr_7', NULL, 'member_default'), + ('grant_d2', 'org_default', 'usr_7', NULL, 'member_default')`); + expect((await grantsPlan()).duplicates).toEqual({ groups: 1, rows: 2 }); + + const report = await apply(catalog(), ledger()); + expect(report.steps.find((s) => s.step === 'rewrite')!.status).toBe('done'); + const drops = report.steps.find((s) => s.step === 'column-drops')!; + expect(drops.status).toBe('blocked'); + expect(drops.blockers.join('\n')).toMatch(/sys_user_permission_set\.permission_set_id is not dropped: 2 grant\(s\) in 1 group\(s\)/); + expect(rows(`SELECT id FROM sys_user_permission_set WHERE user_id = 'usr_7' ORDER BY id`)).toEqual([{ id: 'grant_d1' }, { id: 'grant_d2' }]); + expect(idColumn()).toHaveLength(1); + expect(writes.some((w) => /DELETE FROM "sys_user_permission_set"/.test(w))).toBe(false); + + db.exec(`DELETE FROM sys_user_permission_set WHERE id = 'grant_d2'`); + const again = await apply(catalog(), ledger()); + expect(again.steps.find((s) => s.step === 'column-drops')!.status).toBe('done'); + expect(idColumn()).toEqual([]); + }); + + it('a second platform-wide grant (organization NULL) is a duplicate too: GROUP BY keys NULLs together, unlike the index', async () => { + await onlyTemplates(`('tpl_none', NULL, 'crm.none', 'en-US', 'x', 0, 'package')`, async () => { + db.exec(` + CREATE TABLE sys_user_permission_set (id TEXT PRIMARY KEY, organization_id TEXT, user_id TEXT, + permission_set_id TEXT, permission_set TEXT); + INSERT INTO sys_user_permission_set VALUES + ('g1', NULL, 'usr_p', NULL, 'member_default'), + ('g2', NULL, 'usr_p', NULL, 'member_default'); + `); + const report = await apply(catalog(), ledger(), { posture: 'isolated' }); + const drops = report.steps.find((s) => s.step === 'column-drops')!; + expect(drops.blockers.join('\n')).toMatch(/permission_set_id is not dropped: 2 grant\(s\) in 1 group\(s\)/); + }); + }); + + it('a 17.x database this release has not booted against (no permission_set column): --plan counts zero and names the boot, --apply blocks the rewrite with the same remedy', async () => { + await onlyTemplates(`('tpl_none', NULL, 'crm.none', 'en-US', 'x', 0, 'package')`, async () => { + db.exec(` + CREATE TABLE sys_user_permission_set (id TEXT PRIMARY KEY, organization_id TEXT, user_id TEXT, permission_set_id TEXT); + INSERT INTO sys_user_permission_set VALUES ('g1', NULL, 'usr_p', 'ps_x'); + `); + const grants = await grantsPlan(); + expect(grants).toMatchObject({ nameColumn: 'absent', duplicates: { groups: 0, rows: 0 }, rewriteCollisions: [] }); + expect(grants.note).toMatch(/Boot this release once against the database/); + + const report = await apply(catalog(), ledger(), { posture: 'isolated' }); + const rewrite = report.steps.find((s) => s.step === 'rewrite')!; + expect(rewrite.status).toBe('blocked'); + expect(rewrite.blockers.join('\n')).toMatch(/has no permission_set column.*Boot this release once against the database/); + }); + }); +}); diff --git a/packages/cli/src/utils/organization-ownership-apply.ts b/packages/cli/src/utils/organization-ownership-apply.ts index 258e2c5e648..872282c0a35 100644 --- a/packages/cli/src/utils/organization-ownership-apply.ts +++ b/packages/cli/src/utils/organization-ownership-apply.ts @@ -29,6 +29,12 @@ import { type TablePlan, type UnattributableRow, } from './organization-ownership-plan.js'; +import { + grantCollisionBlocker, + planGrantRewrite, + type LegacySetRow, + type UnnamedReason, +} from './organization-ownership-grants.js'; /** * The ADR-0131 D10 ceremony's apply step — ceremony items 2 to 4: the backup @@ -64,7 +70,8 @@ import { * 3. `rewrite` — the verified id→name rewrite (D4): * `sys_user_permission_set.permission_set` is named from the set row its * `permission_set_id` points at, then EVERY grant is verified to name a set - * the security catalog resolves. The id column stays (C7b drops it). + * the security catalog resolves. The id column is read here, by raw SQL, + * and dropped by column-drops. * 4. `mirror-deletion` (fate 2) — gated on the rewrite's verification, which * is read again immediately before the first delete. Only rows of a * `mirror-deletion` object that match its mirror selector and none of its @@ -73,7 +80,10 @@ import { * pass verified are deleted, by id. * 5. `column-drops` (fate 1) — `organization_id` leaves the D7 objects * through the schema-drift ceremony ({@link ColumnDropPort}), never a hand - * `ALTER`. + * `ALTER`; so does the retired `sys_user_permission_set.permission_set_id` + * (C7b), once the rewrite has read it and no two grants hold one set for + * one user in one organization (the unique index that replaces the old + * one would refuse them; they are reported, never deleted). * * Then the post-check re-runs the plan and reports, per table, what remains. * @@ -129,23 +139,36 @@ export interface TemplatePort { find(name: string, locale: string): Promise<{ definition: Readonly>; environment: boolean } | undefined>; } +/** An index a retired column keyed, named by its declaration (the driver derives the physical name). */ +export interface RetiredIndex { + columns: readonly string[]; + unique: boolean; +} + +/** One column drop: `organization_id` of a fate-1 table, or a retired column the ceremony owns. */ +export interface ColumnDrop { + table: string; + column: string; + /** Indexes the column keyed, dropped before it: the port drops them in an earlier, separate apply. */ + retiredIndexes?: readonly RetiredIndex[]; +} + /** - * One table's column drop, as the schema-drift ceremony answered it. + * One column drop, as the schema-drift ceremony answered it. * * `not-declared`: the composition this run booted declares no object for the * table, so the ceremony has no declaration to drop the column against. That * is a fact about the composition the ceremony was run in, never about the * release. `not-proposed` is only for a table it DOES declare. */ -export interface ColumnDropOutcome { - table: string; +export interface ColumnDropOutcome extends ColumnDrop { outcome: 'dropped' | 'not-proposed' | 'not-declared' | 'failed'; detail?: string; } -/** Drops `organization_id` on the named tables through the schema-drift ceremony (ADR-0120 D4). */ +/** Drops the named columns through the schema-drift ceremony (ADR-0120 D4). */ export interface ColumnDropPort { - drop(tables: readonly string[]): Promise; + drop(columns: readonly ColumnDrop[]): Promise; } /** What the ceremony records after each finished step. */ @@ -901,61 +924,72 @@ async function verifyReferences(run: Run): Promise { } async function runRewrite(run: Run): Promise { - const summary: Record = { named: 0, grants: 0, verified: 0 }; + const summary: Record = { named: 0, grants: 0, verified: 0, collisions: 0 }; const notes: string[] = []; + const blockers: string[] = []; const cols = await columnsOf(run, GRANTS); if (cols?.has('permission_set') && cols.has('permission_set_id')) { - const sets = new Map(); + const sets = new Map(); if (await columnsOf(run, SETS)) { const setCols = (await columnsOf(run, SETS))!; for (const row of await readAll(run, SETS, ['name', ...(setCols.has(ORG) ? [ORG] : [])])) { - sets.set(String(row.id), { name: text(row.name), organization: text(row[ORG]) }); + sets.set(String(row.id), { name: text(row.name), organizationId: text(row[ORG]) }); } } const t = run.sql.q(GRANTS, GRANTS); const nameCol = run.sql.q('permission_set', GRANTS); - const unnamed = await readAll(run, GRANTS, ['permission_set_id', ...(cols.has(ORG) ? [ORG] : [])], { - text: `${nameCol} IS NULL OR ${nameCol} = ''`, + const keyCols = ['user_id', ...(cols.has(ORG) ? [ORG] : [])]; + const unnamedWhere = { text: `${nameCol} IS NULL OR ${nameCol} = ''`, params: [] }; + const legacy = (await readAll(run, GRANTS, ['permission_set_id', ...keyCols], unnamedWhere)).map((g) => ({ + id: String(g.id), + userId: text(g.user_id), + organizationId: text(g[ORG]), + setId: text(g.permission_set_id), + })); + const named = (await readAll(run, GRANTS, ['permission_set', ...keyCols], { + text: `${nameCol} IS NOT NULL AND ${nameCol} <> ''`, params: [], - }); - const reasons = { noId: [] as string[], dangling: [] as string[], crossOrganization: [] as string[], unresolved: [] as string[] }; - for (const grant of unnamed) { - const gid = String(grant.id); - const setId = text(grant.permission_set_id); - if (!setId) { reasons.noId.push(gid); continue; } - const set = sets.get(setId); - if (!set?.name) { reasons.dangling.push(gid); continue; } - // A name carries no organization: an organization's set row may be named - // only by that organization's grants (the grant rule the resolver applies). - if (set.organization !== null && set.organization !== text(grant[ORG])) { reasons.crossOrganization.push(gid); continue; } - if (!(await resolveName(run, 'permission', set.name))) { reasons.unresolved.push(gid); continue; } + })).map((g) => ({ id: String(g.id), userId: text(g.user_id), organizationId: text(g[ORG]), name: String(g.permission_set) })); + const resolvable = new Map(); + for (const legacyGrant of legacy) { + const name = legacyGrant.setId ? sets.get(legacyGrant.setId)?.name : null; + if (name && !resolvable.has(name)) resolvable.set(name, (await resolveName(run, 'permission', name)) !== undefined); + } + // A legacy grant whose name the user already holds in its organization is + // never named: the unique index would refuse it (organization-ownership-grants.ts). + const decided = planGrantRewrite(legacy, named, sets, (name) => resolvable.get(name) === true); + for (const { id, name } of decided.name) { await execute( run, GRANTS, `UPDATE ${t} SET ${nameCol} = ? WHERE ${run.sql.q('id', GRANTS)} = ? AND (${nameCol} IS NULL OR ${nameCol} = '')`, - [set.name, gid], + [name, id], ); summary.named++; } - const describe: Record = { + summary.collisions = decided.collisions.length; + if (decided.collisions.length) blockers.push(grantCollisionBlocker(GRANTS, decided.collisions)); + const describe: Record = { noId: 'carry neither a name nor a permission_set_id', dangling: 'point at a set row that does not exist', crossOrganization: 'point at another organization\'s set row — a name carries no organization, so it is never written', unresolved: 'point at a set row whose name the security catalog does not resolve', }; - for (const key of Object.keys(reasons) as (keyof typeof reasons)[]) { - if (reasons[key].length) notes.push(`${reasons[key].length} unnamed grant(s) ${describe[key]}: ${reasons[key].slice(0, 20).join(', ')}.`); + for (const key of Object.keys(decided.reasons) as UnnamedReason[]) { + const ids = decided.reasons[key]; + if (ids.length) notes.push(`${ids.length} unnamed grant(s) ${describe[key]}: ${ids.slice(0, 20).join(', ')}.`); } if (summary.named) run.opts.onProgress?.(`rewrite: ${summary.named} grant(s) now name their permission set`); } const verification = await verifyReferences(run); summary.grants = verification.grants; summary.verified = verification.verified; + blockers.push(...verification.blockers); return { step: 'rewrite', - status: verification.blockers.length ? 'blocked' : 'done', + status: blockers.length ? 'blocked' : 'done', summary, - blockers: verification.blockers, + blockers, notes: [...notes, ...verification.notes], }; } @@ -1092,10 +1126,23 @@ async function deletePromotedTemplates(run: Run, tables: readonly string[]): Pro // ── Step 5: column drops (fate 1) ──────────────────────────────────────────── +/** The retired id column of a grant (ADR-0131 D4, C7b), which the rewrite reads before column-drops drops it. */ +const GRANT_SET_ID_COLUMN = 'permission_set_id'; +/** + * The two indexes the retired column keyed (C7b). Schema sync is additive, so + * they outlive their declaration, and they go BEFORE the column: MySQL refuses + * to drop a column a key still uses, or keeps the key minus that column — a + * unique (user_id, organization_id) that would refuse any second grant. + */ +const GRANT_SET_ID_INDEXES: readonly RetiredIndex[] = [ + { columns: ['user_id', 'permission_set_id', 'organization_id'], unique: true }, + { columns: ['permission_set_id'], unique: false }, +]; + async function runColumnDrops(run: Run): Promise { const plan = await planFor(run); const blockers: string[] = []; - const due: string[] = []; + const due: ColumnDrop[] = []; for (const table of plan.tables) { if (table.fate !== 'column-drop' || table.physical !== 'present' || table.organizationColumn !== 'present') continue; const held = table.categories.filter((c) => c.rows > 0); @@ -1106,7 +1153,24 @@ async function runColumnDrops(run: Run): Promise { ); continue; } - due.push(...table.tables); + due.push(...table.tables.map((t) => ({ table: t, column: ORG }))); + } + // [C7b] The grant's retired id column: the rewrite (step 3) has read it and + // verified every grant by name. It goes once no two grants hold one set for + // one user in one organization — the unique index that replaces the old one + // refuses them, and which to keep is the operator's call, never the ceremony's. + if ((await columnsOf(run, GRANTS))?.has(GRANT_SET_ID_COLUMN)) { + const duplicates = plan.tables.find((t) => t.object === GRANTS)?.grants?.duplicates; + if (duplicates && duplicates.rows > 0) { + blockers.push( + `${GRANTS}.${GRANT_SET_ID_COLUMN} is not dropped: ${duplicates.rows} grant(s) in ${duplicates.groups} group(s) ` + + 'hold one permission set for one user in one organization, which the unique index on (user_id, ' + + 'permission_set, organization_id) refuses, and a revoke of one of them would leave the other in force. ' + + 'Which grant stays is never guessed and none is deleted here: remove all but one grant per group, then re-run.', + ); + } else { + due.push({ table: GRANTS, column: GRANT_SET_ID_COLUMN, retiredIndexes: GRANT_SET_ID_INDEXES }); + } } let dropped = 0; let notDeclared = 0; @@ -1119,7 +1183,7 @@ async function runColumnDrops(run: Run): Promise { if (outcome.outcome === 'not-declared') { notDeclared++; blockers.push( - `${outcome.table}.${ORG} was not dropped: the composition this run booted declares no object for ` + + `${outcome.table}.${outcome.column} was not dropped: the composition this run booted declares no object for ` + `${outcome.table}, so the schema-drift ceremony has no declaration to drop it against, and no release ` + 'changes that. Re-run the ceremony where the deployment\'s composition is declared: its project directory ' + 'and environment (the requires capability whose provider declares this object; OS_AUTH_SECRET for the ' + @@ -1129,10 +1193,10 @@ async function runColumnDrops(run: Run): Promise { } blockers.push( outcome.outcome === 'not-proposed' - ? `${outcome.table}.${ORG} was not proposed for dropping by the schema-drift ceremony: the object this ` + - `runtime declares still carries the column${outcome.detail ? ` (${outcome.detail})` : ''}. Run the ceremony ` + - 'with the release whose declaration drops it.' - : `Dropping ${outcome.table}.${ORG} failed: ${outcome.detail ?? 'no detail'}. Re-run once the cause is fixed.`, + ? `${outcome.table}.${outcome.column} was not proposed for dropping by the schema-drift ceremony: the object ` + + `this runtime declares still carries the column${outcome.detail ? ` (${outcome.detail})` : ''}. Run the ` + + 'ceremony with the release whose declaration drops it.' + : `Dropping ${outcome.table}.${outcome.column} failed: ${outcome.detail ?? 'no detail'}. Re-run once the cause is fixed.`, ); } } diff --git a/packages/cli/src/utils/organization-ownership-column-drops.test.ts b/packages/cli/src/utils/organization-ownership-column-drops.test.ts new file mode 100644 index 00000000000..9dc21e9d78b --- /dev/null +++ b/packages/cli/src/utils/organization-ownership-column-drops.test.ts @@ -0,0 +1,172 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The ceremony's `columnDrops` port (`dropColumnsThroughDrift`) on a REAL SQL + * driver: the grant's retired `permission_set_id` goes only after the two + * indexes it keyed (C7b), each dropped through the schema-drift ceremony. + * + * The database is a 17.x grant table carrying both legacy indexes under the + * names the driver gave them, re-synced against today's declaration: schema + * sync is additive, so the column and both indexes outlive it, and the + * differ proposes their drops. + * + * ⚠️ `@objectstack/driver-sql` resolves to its built `dist/`, so a stale build + * makes this a verdict about build state. The value import puts the file in + * the `integration` tier (`vitest-tiers.ts`, KERNEL). + */ + +import { afterEach, describe, expect, it } from 'vitest'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { SqlDriver, buildIndexName } from '@objectstack/driver-sql'; +import { dropColumnsThroughDrift, type ColumnDropTarget } from './organization-ownership-column-drops.js'; +import type { ColumnDrop } from './organization-ownership-apply.js'; + +const T = 'sys_user_permission_set'; +const LEGACY_UNIQUE = buildIndexName(T, ['user_id', 'permission_set_id', 'organization_id'], true); +const LEGACY_ID_INDEX = buildIndexName(T, ['permission_set_id'], false); + +const fields = { + user_id: { type: 'text', maxLength: 255 }, + permission_set: { type: 'text', maxLength: 100 }, + organization_id: { type: 'text', maxLength: 255 }, +}; +/** The 17.x declaration: the id column and its two indexes. */ +const LEGACY = { + name: T, + tenancy: { enabled: false }, + fields: { ...fields, permission_set_id: { type: 'text', maxLength: 255 } }, + indexes: [ + { fields: ['user_id', 'permission_set_id', 'organization_id'], unique: true }, + { fields: ['permission_set_id'] }, + ], +}; +/** Today's declaration: the column and both indexes retired, the key on the name. */ +const CURRENT = { + name: T, + tenancy: { enabled: false }, + fields, + indexes: [{ fields: ['user_id', 'permission_set', 'organization_id'], unique: true }], +}; +const DROP: ColumnDrop = { + table: T, + column: 'permission_set_id', + retiredIndexes: [ + { columns: ['user_id', 'permission_set_id', 'organization_id'], unique: true }, + { columns: ['permission_set_id'], unique: false }, + ], +}; + +let dir: string | undefined; +const drivers: SqlDriver[] = []; +afterEach(async () => { + while (drivers.length) await drivers.pop()!.disconnect().catch(() => {}); + if (dir) rmSync(dir, { recursive: true, force: true }); + dir = undefined; +}); + +async function driverOn(file: string, object: typeof LEGACY | typeof CURRENT): Promise { + const driver = new SqlDriver({ client: 'better-sqlite3', connection: { filename: file }, useNullAsDefault: true }); + (driver as any).logger = { debug() {}, info() {}, warn() {}, error() {} }; + drivers.push(driver); + await driver.initObjects([object]); + return driver; +} + +/** A 17.x database with one grant, booted by today's declaration. */ +async function upgraded(): Promise { + dir = mkdtempSync(join(tmpdir(), 'os-c7b-column-drops-')); + const file = join(dir, 'grants.db'); + const legacy = await driverOn(file, LEGACY); + await (legacy as any).knex(T).insert({ id: 'g1', user_id: 'u1', permission_set_id: 'ps_1', permission_set: 'member_default', organization_id: 'org_a' }); + await legacy.disconnect(); + drivers.pop(); + return driverOn(file, CURRENT); +} + +const target = (driver: SqlDriver, calls: string[][]): ColumnDropTarget => ({ + drift: { + detectManagedDrift: () => driver.detectManagedDrift(), + applyMigrationEntries: (entries, opts) => { + calls.push(entries.map((d) => `${String((d as any).op?.type)}:${String((d as any).op?.indexName ?? (d as any).op?.column)}`)); + return driver.applyMigrationEntries(entries, opts); + }, + }, + isDeclared: (table) => table === T, + indexName: (table, columns, unique) => buildIndexName(table, [...columns], unique), +}); + +const indexNames = async (driver: SqlDriver): Promise => + ((await (driver as any).knex.raw(`SELECT name FROM sqlite_master WHERE type = 'index' AND tbl_name = ?`, [T])) as Array<{ name: string }>) + .map((r) => r.name) + .filter((n) => !n.startsWith('sqlite_autoindex')) + .sort(); +const columnNames = async (driver: SqlDriver): Promise => + ((await (driver as any).knex.raw(`SELECT name FROM pragma_table_info(?)`, [T])) as Array<{ name: string }>).map((r) => r.name); + +describe('C7b — the retired grant id column drops after the indexes it keyed', () => { + it('the fixture is real: the column and both legacy indexes outlive the declaration, and the differ proposes their drops', async () => { + const driver = await upgraded(); + expect(await columnNames(driver)).toContain('permission_set_id'); + expect(await indexNames(driver)).toEqual(expect.arrayContaining([LEGACY_UNIQUE, LEGACY_ID_INDEX])); + const ops = (await driver.detectManagedDrift()).map((d) => `${(d as any).op?.type}:${(d as any).op?.indexName ?? (d as any).op?.column}`); + expect(ops).toEqual(expect.arrayContaining([`drop_index:${LEGACY_UNIQUE}`, `drop_index:${LEGACY_ID_INDEX}`, 'drop_column:permission_set_id'])); + }); + + it('drops both indexes in one apply, then the column in a later one; the grant and the new key survive', async () => { + const driver = await upgraded(); + const calls: string[][] = []; + expect(await dropColumnsThroughDrift(target(driver, calls), [DROP])).toEqual([ + { table: T, column: 'permission_set_id', outcome: 'dropped' }, + ]); + expect(calls).toHaveLength(2); + expect([...calls[0]!].sort()).toEqual([`drop_index:${LEGACY_UNIQUE}`, `drop_index:${LEGACY_ID_INDEX}`].sort()); + expect(calls[1]).toEqual(['drop_column:permission_set_id']); + expect(await columnNames(driver)).not.toContain('permission_set_id'); + const left = await indexNames(driver); + expect(left).not.toContain(LEGACY_UNIQUE); + expect(left).not.toContain(LEGACY_ID_INDEX); + expect(left).toContain(buildIndexName(T, ['user_id', 'permission_set', 'organization_id'], true)); + expect(await (driver as any).knex(T).select('id', 'permission_set')).toEqual([{ id: 'g1', permission_set: 'member_default' }]); + }); + + it('a column the differ proposes no drop for is not-proposed when declared, not-declared when not, and nothing is applied', async () => { + const driver = await upgraded(); + const calls: string[][] = []; + const t = target(driver, calls); + const outcomes = await dropColumnsThroughDrift( + { ...t, isDeclared: (table) => table === T }, + [{ table: T, column: 'user_id', retiredIndexes: [{ columns: ['user_id'], unique: false }] }, { table: 'sys_gone', column: 'organization_id' }], + ); + expect(outcomes).toEqual([ + { table: T, column: 'user_id', outcome: 'not-proposed', detail: 'the drift differ proposed no drop for it' }, + { table: 'sys_gone', column: 'organization_id', outcome: 'not-declared' }, + ]); + expect(calls).toEqual([]); + }); + + it('an index drop the ceremony skips keeps its column: failed, naming the index, and the column is not dropped', async () => { + const driver = await upgraded(); + const calls: string[][] = []; + const t = target(driver, calls); + const refusing: ColumnDropTarget = { + ...t, + drift: { + detectManagedDrift: t.drift.detectManagedDrift, + applyMigrationEntries: async (entries, opts) => { + if (entries.some((d) => (d as any).op?.type === 'drop_index')) { + calls.push(['refused']); + return { applied: [], skipped: entries.map((d) => ({ ...d, message: 'refused by the test' })) }; + } + return t.drift.applyMigrationEntries(entries, opts); + }, + }, + }; + const [outcome] = await dropColumnsThroughDrift(refusing, [DROP]); + expect(outcome).toMatchObject({ table: T, column: 'permission_set_id', outcome: 'failed' }); + expect(outcome!.detail).toMatch(/could not be dropped first \(refused by the test\)/); + expect(calls).toEqual([['refused']]); + expect(await columnNames(driver)).toContain('permission_set_id'); + }); +}); diff --git a/packages/cli/src/utils/organization-ownership-column-drops.ts b/packages/cli/src/utils/organization-ownership-column-drops.ts new file mode 100644 index 00000000000..a674684c3f5 --- /dev/null +++ b/packages/cli/src/utils/organization-ownership-column-drops.ts @@ -0,0 +1,85 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { ColumnDrop, ColumnDropOutcome } from './organization-ownership-apply.js'; +import type { SqlDriverLike } from './schema-migrate.js'; + +type DriftEntry = Awaited>[number]; + +export interface ColumnDropTarget { + /** The booted stack's schema-drift ceremony (ADR-0120 D4). */ + drift: Pick; + /** Whether the booted composition declares an object for this table (a rotation shard judged by its base). */ + isDeclared(table: string): boolean; + /** The physical name the driver gives a declared index — `@objectstack/driver-sql`'s `buildIndexName`. */ + indexName(table: string, columns: readonly string[], unique: boolean): string; +} + +const opOf = (d: DriftEntry): { type?: string; column?: string; indexName?: string } => + ((d as { op?: unknown }).op ?? {}) as { type?: string; column?: string; indexName?: string }; +const key = (table: string, column: string): string => `${table}.${column}`; +const messageOf = (d: DriftEntry): string => String((d as { message?: unknown }).message ?? 'skipped by the schema-drift ceremony'); + +/** + * The ceremony's `columnDrops` port: each named column goes through the + * schema-drift ceremony, never a hand `ALTER`. + * + * A column's retired indexes ({@link ColumnDrop.retiredIndexes}) are dropped + * FIRST, in their own `applyMigrationEntries` call: that call runs column ops + * before index ops, so one call would drop the column under a key that still + * uses it (MySQL refuses it, or keeps the key minus the column). A column + * whose retired index the ceremony could not drop is not dropped; its outcome + * is `failed`, naming the index. + */ +export async function dropColumnsThroughDrift(target: ColumnDropTarget, columns: readonly ColumnDrop[]): Promise { + const drift = await target.drift.detectManagedDrift(); + const wanted = new Set(columns.map((c) => key(c.table, c.column))); + let columnEntries = drift.filter((d) => { + const op = opOf(d); + return op.type === 'drop_column' && wanted.has(key(String(d.table), String(op.column))); + }); + const outcomes: ColumnDropOutcome[] = []; + const proposed = new Set(columnEntries.map((d) => key(String(d.table), String(opOf(d).column)))); + for (const c of columns) { + if (proposed.has(key(c.table, c.column))) continue; + outcomes.push( + target.isDeclared(c.table) + ? { table: c.table, column: c.column, outcome: 'not-proposed', detail: 'the drift differ proposed no drop for it' } + : { table: c.table, column: c.column, outcome: 'not-declared' }, + ); + } + + // The retired indexes of every column that IS proposed, by physical name. + const indexOwner = new Map(); + for (const c of columns) { + if (!proposed.has(key(c.table, c.column))) continue; + for (const index of c.retiredIndexes ?? []) indexOwner.set(key(c.table, target.indexName(c.table, index.columns, index.unique)), c); + } + const indexEntries = drift.filter((d) => { + const op = opOf(d); + return op.type === 'drop_index' && indexOwner.has(key(String(d.table), String(op.indexName))); + }); + if (indexEntries.length) { + const { skipped } = await target.drift.applyMigrationEntries(indexEntries, { allowDestructive: true }); + const blocked = new Set(); + for (const d of skipped) { + const owner = indexOwner.get(key(String(d.table), String(opOf(d).indexName)))!; + if (blocked.has(key(owner.table, owner.column))) continue; + blocked.add(key(owner.table, owner.column)); + outcomes.push({ + table: owner.table, + column: owner.column, + outcome: 'failed', + detail: `the index ${String(opOf(d).indexName)} it keyed could not be dropped first (${messageOf(d)})`, + }); + } + columnEntries = columnEntries.filter((d) => !blocked.has(key(String(d.table), String(opOf(d).column)))); + } + + if (columnEntries.length === 0) return outcomes; + const { applied, skipped } = await target.drift.applyMigrationEntries(columnEntries, { allowDestructive: true }); + for (const d of applied) outcomes.push({ table: String(d.table), column: String(opOf(d).column), outcome: 'dropped' }); + for (const d of skipped) { + outcomes.push({ table: String(d.table), column: String(opOf(d).column), outcome: 'failed', detail: messageOf(d) }); + } + return outcomes; +} diff --git a/packages/cli/src/utils/organization-ownership-grants.ts b/packages/cli/src/utils/organization-ownership-grants.ts new file mode 100644 index 00000000000..a20c0ee8f39 --- /dev/null +++ b/packages/cli/src/utils/organization-ownership-grants.ts @@ -0,0 +1,120 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The grant half of the ADR-0131 D10 ceremony, as one pure decision both + * `--plan` and `--apply` take: which legacy grant (`permission_set` empty, + * `permission_set_id` set) the verified rewrite names, and which it may not. + * + * ## Why a legacy grant can collide + * + * The grant's unique index is (`user_id`, `permission_set`, + * `organization_id`), NULL-safe on the organization (C7b), and a 17.x + * database's first v18 boot creates it. A NAMED grant written beside a legacy + * one of the same user and organization — by Setup, a seed, or any writer + * other than the organization-administrator sweep, which writes nothing beside + * an unnamed grant — holds the key the legacy grant's rename needs, and the + * index refuses that rename. So the rewrite names a legacy grant only when no + * stored named grant, and no grant it names earlier in the same run, already + * holds that user, set and organization (a NULL organization keyed with NULL, + * as the index keys it). The others are listed, both ids named, and kept: + * which grant stays is the operator's call, and the ceremony deletes no grant. + */ + +/** A grant written before the name column existed: it carries the legacy id only. */ +export interface LegacyGrant { + id: string; + userId: string | null; + organizationId: string | null; + setId: string | null; +} + +/** A grant that already names its set. */ +export interface NamedGrant { + id: string; + userId: string | null; + organizationId: string | null; + name: string; +} + +/** A legacy `sys_permission_set` row, as the rewrite reads it. */ +export interface LegacySetRow { + name: string | null; + organizationId: string | null; +} + +/** Why a legacy grant is not named. */ +export type UnnamedReason = 'noId' | 'dangling' | 'crossOrganization' | 'unresolved'; + +/** A legacy grant the rewrite may not name: its name would duplicate `heldBy`. */ +export interface GrantCollision { + id: string; + name: string; + heldBy: string; +} + +export interface GrantRewritePlan { + /** The legacy grants to name, in id order. */ + name: Array<{ id: string; name: string }>; + collisions: GrantCollision[]; + reasons: Record; +} + +/** The unique key of a grant; a NULL organization is one key, as the write door keys it. */ +export function grantKey(userId: string | null, name: string, organizationId: string | null): string { + return JSON.stringify([userId, name, organizationId]); +} + +/** + * Decide the rewrite. `resolves` answers whether the security catalog resolves + * a name; `--plan`, which reads no catalog, passes none and counts every + * derivable name. + */ +export function planGrantRewrite( + legacy: readonly LegacyGrant[], + named: readonly NamedGrant[], + sets: ReadonlyMap, + resolves: (name: string) => boolean = () => true, +): GrantRewritePlan { + const out: GrantRewritePlan = { + name: [], + collisions: [], + reasons: { noId: [], dangling: [], crossOrganization: [], unresolved: [] }, + }; + const held = new Map(); + for (const g of [...named].sort((a, b) => a.id.localeCompare(b.id))) { + const key = grantKey(g.userId, g.name, g.organizationId); + if (!held.has(key)) held.set(key, g.id); + } + for (const grant of [...legacy].sort((a, b) => a.id.localeCompare(b.id))) { + if (!grant.setId) { out.reasons.noId.push(grant.id); continue; } + const set = sets.get(grant.setId); + if (!set?.name) { out.reasons.dangling.push(grant.id); continue; } + // A name carries no organization: an organization's set row may be named + // only by that organization's grants (the grant rule the resolver applies). + if (set.organizationId !== null && set.organizationId !== grant.organizationId) { + out.reasons.crossOrganization.push(grant.id); + continue; + } + if (!resolves(set.name)) { out.reasons.unresolved.push(grant.id); continue; } + const key = grantKey(grant.userId, set.name, grant.organizationId); + const holder = held.get(key); + if (holder !== undefined) { + out.collisions.push({ id: grant.id, name: set.name, heldBy: holder }); + continue; + } + held.set(key, grant.id); + out.name.push({ id: grant.id, name: set.name }); + } + return out; +} + +/** The blocker a collision list reads as: both ids, the reason, the remedy. */ +export function grantCollisionBlocker(table: string, collisions: readonly GrantCollision[]): string { + const listed = collisions.slice(0, 20).map((c) => `${c.id} (${c.name}, held by ${c.heldBy})`).join(', '); + return ( + `${collisions.length} legacy grant(s) would name a permission set their user already holds in the same organization ` + + `(${listed}${collisions.length > 20 ? ', …' : ''}). The unique index on ${table} (user_id, permission_set, ` + + 'organization_id) refuses the second row, so each is kept unnamed: it confers nothing, and none is deleted here. ' + + 'Which grant stays is yours: remove the legacy grant (or the one that holds the set), then re-run.' + ); +} diff --git a/packages/cli/src/utils/organization-ownership-plan.ts b/packages/cli/src/utils/organization-ownership-plan.ts index 2fc20598904..435f67a0027 100644 --- a/packages/cli/src/utils/organization-ownership-plan.ts +++ b/packages/cli/src/utils/organization-ownership-plan.ts @@ -14,6 +14,7 @@ import { type RowPredicate, } from './organization-ownership-inventory.js'; import { physicalTableListSql, rotationBaseOf } from './unmanaged-tables.js'; +import { planGrantRewrite, type GrantCollision, type LegacySetRow } from './organization-ownership-grants.js'; /** * The ADR-0131 D10 ceremony's preflight — ceremony item 1, read-only. @@ -154,6 +155,24 @@ export interface TablePlan { * until the operator chooses one row per group; nothing is guessed. */ templateConflicts?: TemplateConflict[]; + /** `sys_user_permission_set` only: what the grant's unique key holds ({@link GrantKeyPlan}). */ + grants?: GrantKeyPlan; +} + +/** + * `sys_user_permission_set` (C7b): the populations the unique index on + * (`user_id`, `permission_set`, `organization_id`) refuses, which the ceremony + * reports and never deletes. A NULL organization is keyed with NULL, as the + * write door keys it. + */ +export interface GrantKeyPlan { + /** `absent` on a 17.x database this release has not booted against yet: nothing can be counted. */ + nameColumn: 'present' | 'absent'; + /** Named grants holding one set for one user in one organization: they block the legacy id column's drop. */ + duplicates: { groups: number; rows: number }; + /** Legacy grants the rewrite may not name, each with the grant that already holds its key: they block the rewrite. */ + rewriteCollisions: GrantCollision[]; + note?: string; } /** One `(name, locale)` the promotion cannot carry without the operator's choice. */ @@ -165,6 +184,9 @@ export interface TemplateConflict { /** The table whose customized and authored rows the ceremony promotes (ruling C on ADR-0131 §6 Q1). */ export const TEMPLATE_OBJECT = 'sys_email_template'; +/** The permission-set grant table whose unique key the plan reads ({@link GrantKeyPlan}). */ +export const GRANT_OBJECT = 'sys_user_permission_set'; +const LEGACY_SET_OBJECT = 'sys_permission_set'; /** Provenance the seeders stamped: only such a row can be a copy of code. */ export const SEEDED_BY: readonly string[] = ['package', 'platform', 'system']; @@ -642,6 +664,53 @@ async function planCategory(sql: Sql, table: string, columns: ReadonlySet): Promise { + const { sql } = ctx; + if (!columns.has('permission_set') || !columns.has('user_id')) { + return { + nameColumn: 'absent', + duplicates: { groups: 0, rows: 0 }, + rewriteCollisions: [], + note: + `${table} has no permission_set column yet, so no grant names its set and nothing here can be counted. ` + + 'Boot this release once against the database (its schema sync adds the column), then re-run the plan.', + }; + } + const keyColumns = ['user_id', 'permission_set', ...(columns.has(ORG) ? [ORG] : [])]; + const duplicates = await planCategory(sql, table, columns, { + id: 'duplicate-grants', + label: 'grants holding one permission set for one user in one organization', + citation: 'seat answers 6162538253, Q5 A', + where: [{ column: 'permission_set', isNull: false }], + conflictBy: { groupBy: keyColumns }, + }); + const rewriteCollisions: GrantCollision[] = []; + if (columns.has('permission_set_id')) { + const t = sql.q(table, table); + const nameCol = sql.q('permission_set', table); + const select = ['id', 'user_id', 'permission_set', 'permission_set_id', ...(columns.has(ORG) ? [ORG] : [])] + .map((c) => sql.q(c, table)).join(', '); + const rows = await sql.read(table, `SELECT ${select} FROM ${t}`, []); + const named = rows.filter((r) => nonEmptyId(r.permission_set) !== null); + const legacy = rows.filter((r) => nonEmptyId(r.permission_set) === null); + const sets = new Map(); + const setColumns = ctx.catalog.columns.get(LEGACY_SET_OBJECT); + if (legacy.length && setColumns?.has('name')) { + const setSelect = ['id', 'name', ...(setColumns.has(ORG) ? [ORG] : [])].map((c) => sql.q(c, LEGACY_SET_OBJECT)).join(', '); + for (const r of await sql.read(LEGACY_SET_OBJECT, `SELECT ${setSelect} FROM ${sql.q(LEGACY_SET_OBJECT, LEGACY_SET_OBJECT)}`, [])) { + sets.set(String(r.id), { name: nonEmptyId(r.name), organizationId: nonEmptyId(r[ORG]) }); + } + } + rewriteCollisions.push(...planGrantRewrite( + legacy.map((r) => ({ id: String(r.id), userId: nonEmptyId(r.user_id), organizationId: nonEmptyId(r[ORG]), setId: nonEmptyId(r.permission_set_id) })), + named.map((r) => ({ id: String(r.id), userId: nonEmptyId(r.user_id), organizationId: nonEmptyId(r[ORG]), name: String(r.permission_set) })), + sets, + ).collisions); + } + return { nameColumn: 'present', duplicates: { groups: duplicates.groups ?? 0, rows: duplicates.rows }, rewriteCollisions }; +} + async function planPresentTable(ctx: PlanContext, entry: InventoryEntry, source: TablePlan['source'], tables: string[]): Promise { const { sql } = ctx; const columns = ctx.catalog.columns.get(tables[0]!)!; @@ -722,6 +791,7 @@ async function planPresentTable(ctx: PlanContext, entry: InventoryEntry, source: for (const table of tables) conflicts.push(...(await readTemplateConflicts(sql, table, ctx.catalog.columns.get(table)!))); plan.templateConflicts = conflicts; } + if (entry.object === GRANT_OBJECT) plan.grants = await planGrantKeys(ctx, tables[0]!, columns); const rows = plan.rows!; const departing = plan.departures.reduce((n, d) => n + d.rows, 0); diff --git a/packages/core/src/security/resolve-authz-context.ts b/packages/core/src/security/resolve-authz-context.ts index 3590c6ab680..73c8cecf913 100644 --- a/packages/core/src/security/resolve-authz-context.ts +++ b/packages/core/src/security/resolve-authz-context.ts @@ -808,8 +808,8 @@ function grantAppliesInTenant(organizationId: unknown, tenantId: string | undefi * [ADR-0131 D4] The permission set a stored `sys_user_permission_set` grant * names: its `permission_set`, or `undefined` for a grant that names none — * `NULL` or blank, which is a grant written before the column existed that the - * one-time backfill has not named yet, or one it could not name (an id with no - * set row, or a set row of another organization). + * v18 ceremony's verified rewrite (`os migrate organization-ownership + * --apply`, ADR-0131 D10) has not named yet, or one it could not name. * * A grant that names nothing confers nothing through this resolver. The same * rule the `plugin-security` and `plugin-auth` grant readers apply, each diff --git a/packages/plugins/plugin-auth/src/ensure-default-organization.ts b/packages/plugins/plugin-auth/src/ensure-default-organization.ts index df86ce7f9b1..ca28891052d 100644 --- a/packages/plugins/plugin-auth/src/ensure-default-organization.ts +++ b/packages/plugins/plugin-auth/src/ensure-default-organization.ts @@ -483,8 +483,8 @@ export async function ensureDefaultOrganization( } // [ADR-0131 D4] The grants NAMING the set (`permission_set`), not the // grants carrying the row's id. A grant that names nothing yet — an - // upgraded deployment's, before the one-time backfill names it at - // `kernel:bootstrapped` — is not read as the admin: this helper CONFERS + // legacy one the v18 ceremony's rewrite has not named (ADR-0131 D10) — + // is not read as the admin: this helper CONFERS // (an owner membership and the seeded rows), and a grant that names no set // confers nothing. The answer is then `no_admin`, which decides nothing // (`default-org-bootstrap-once.ts` records no decision on it), so a later diff --git a/packages/plugins/plugin-auth/src/grant-readers-unnamed-grant.test.ts b/packages/plugins/plugin-auth/src/grant-readers-unnamed-grant.test.ts index 45c854f1744..45ca5f60f00 100644 --- a/packages/plugins/plugin-auth/src/grant-readers-unnamed-grant.test.ts +++ b/packages/plugins/plugin-auth/src/grant-readers-unnamed-grant.test.ts @@ -4,12 +4,11 @@ * [ADR-0131 D4] The grant that names nothing, as each `plugin-auth` grant * reader treats it once the readers key on `sys_user_permission_set.permission_set`. * - * A grant written before the name column existed carries `NULL` until the - * one-time backfill (`plugin-security`, `kernel:bootstrapped`) names it; on an - * upgraded deployment's first boot every grant is unnamed when the - * `kernel:ready` passes run, and the backfill leaves some unnamed for good (an - * id with no set row, or a set row of another organization). Each reader takes - * its fail-closed direction: + * A grant written before the name column existed carries `NULL` until the v18 + * ceremony's verified rewrite (`os migrate organization-ownership --apply`, + * ADR-0131 D10) names it from the legacy `permission_set_id`, a column no + * runtime declares any more (C7b). Each reader takes its fail-closed + * direction: * * - **the last-administrator guard** counts no administrator through it, and * reads it as evidence that the environment is NOT fresh — so the bootstrap @@ -18,13 +17,12 @@ * administrator: it confers an owner membership, and an unnamed grant * confers nothing. * - * And the guard's simulation of a write that re-points a grant's id without - * its name: the platform derives that name after the guard runs, so the guard - * reads it as taking the standing away — while an id echoed unchanged keeps it. + * And a write naming the retired id is refused as an undeclared field before + * it moves anything. * * Real ObjectQL over the SQL driver with the real `SecurityPlugin` (its objects * and its name hooks); a grant is unnamed by writing `NULL` with the name hooks - * unbound, the state the backfill leaves. + * unbound, the state the ceremony's rewrite finds. */ import { describe, it, expect, afterEach, vi } from 'vitest'; @@ -137,7 +135,7 @@ const adminGrant = async (engine: ObjectQL): Promise => ((await engine.find('sys_user_permission_set', { where: { user_id: 'usr_admin' }, ...SYS })) as any[])[0]; /** - * The state the backfill leaves on a grant it has not named. Written past both + * The state of a legacy grant the ceremony's rewrite has not named. Written past both * hooks that would refuse it: the name hooks, and the guard itself — clearing * the sole administrator's grant name takes the standing away, which the guard * refuses like any other revocation. The guard is bound again afterwards. @@ -175,12 +173,18 @@ describe('[ADR-0131 D4] a grant that names nothing — the last-administrator gu expect((await adminGrant(engine)).permission_set).toBe('admin_full_access'); }); - it('writing the legacy id moves no standing: nothing derives a name from it, so the write is permitted', async () => { + it('writing the retired id is refused 400 INVALID_FIELD (C7b) and moves no standing', async () => { const { engine } = await soleGrantAnchoredAdmin(); const grant = await adminGrant(engine); - const [viewer] = (await engine.find('sys_permission_set', { where: { name: 'viewer_readonly' }, ...SYS })) as any[]; - await engine.update('sys_user_permission_set', { id: grant.id, permission_set_id: viewer.id, reason: 'renewed' }, SYS); - expect(await adminGrant(engine)).toMatchObject({ permission_set: 'admin_full_access', reason: 'renewed' }); + let refused: any; + try { + await engine.update('sys_user_permission_set', { id: grant.id, permission_set_id: 'ps_viewer', reason: 'renewed' }, SYS); + } catch (e) { + refused = e; + } + expect([refused?.code, refused?.status]).toEqual(['INVALID_FIELD', 400]); + expect(await adminGrant(engine)).toMatchObject({ permission_set: 'admin_full_access' }); + expect((await adminGrant(engine)).reason ?? null).not.toBe('renewed'); }); }); diff --git a/packages/plugins/plugin-auth/src/last-admin-guard.ts b/packages/plugins/plugin-auth/src/last-admin-guard.ts index 27d31ba715c..ebfaa0ca4e7 100644 --- a/packages/plugins/plugin-auth/src/last-admin-guard.ts +++ b/packages/plugins/plugin-auth/src/last-admin-guard.ts @@ -641,8 +641,8 @@ export const MEMBER_STANDING_KEYS = ['role', 'user_id', 'userId'] as const; * snake_case and camelCase spellings the readers already tolerate. * * [ADR-0131 D4] The enumeration reads WHICH set by the grant's name, - * `permission_set`, and only by it: `permission_set_id` moves nothing, so a - * write touching it alone is skipped like any other non-standing column. + * `permission_set`, and only by it (the legacy `permission_set_id` is retired, + * C7b, and a write naming it is refused as an undeclared field). */ export const GRANT_STANDING_KEYS = [ 'permission_set', @@ -1111,7 +1111,7 @@ export function registerLastAdminGuard( * install every grant names a set the platform ships, so the fresh answer is * unchanged; such a grant is left behind by a set definition that went away * (an uninstalled package, a removed environment definition), or is an - * upgraded deployment's grant before the one-time backfill names it. + * legacy grant the v18 ceremony's rewrite has not named (ADR-0131 D10). * * Deliberately an OVER-approximation of "the `admin_full_access` definition * went away": a dangling unscoped grant to some other set trips it too. The diff --git a/packages/plugins/plugin-security/src/auto-org-admin-grant.test.ts b/packages/plugins/plugin-security/src/auto-org-admin-grant.test.ts index 8d7a6651a76..ff3a85bca33 100644 --- a/packages/plugins/plugin-security/src/auto-org-admin-grant.test.ts +++ b/packages/plugins/plugin-security/src/auto-org-admin-grant.test.ts @@ -291,7 +291,7 @@ describe('reconcileOrgAdminGrant', () => { id: 'ups1', user_id: 'u1', organization_id: 'o1', - permission_set_id: 'ps_org_admin', + permission_set: 'organization_admin', }, ]; const res = await reconcileOrgAdminGrant(stub, 'u1', 'o1', WALLED); @@ -305,7 +305,7 @@ describe('reconcileOrgAdminGrant', () => { id: 'ups1', user_id: 'u1', organization_id: 'o1', - permission_set_id: 'ps_org_admin', + permission_set: 'organization_admin', }, ]; const res = await reconcileOrgAdminGrant(stub, 'u1', 'o1', WALLED); @@ -365,7 +365,7 @@ describe('backfillOrgAdminGrants', () => { id: 'ups_orphan', user_id: 'u4', organization_id: 'o2', - permission_set_id: 'ps_org_admin_o2', + permission_set: 'organization_admin', }, ], }); @@ -458,8 +458,8 @@ describe('[ADR-0105 D4] posture selects the org-admin variant', () => { ], // Pre-existing unbounded grants from a previously-walled boot. sys_user_permission_set: [ - { id: 'ups1', user_id: 'u1', organization_id: 'o1', permission_set_id: 'ps_org_admin' }, - { id: 'ups2', user_id: 'u2', organization_id: 'o1', permission_set_id: 'ps_org_admin' }, + { id: 'ups1', user_id: 'u1', organization_id: 'o1', permission_set: 'organization_admin' }, + { id: 'ups2', user_id: 'u2', organization_id: 'o1', permission_set: 'organization_admin' }, ], }); @@ -553,9 +553,9 @@ describe('[#12699] suppressUnboundedOrgAdminGrant', () => { // match every copy of the name. (⛔ Nothing re-points them — a row still // held by someone who qualifies is left exactly as it is.) sys_user_permission_set: [ - { id: 'ups1', user_id: 'u1', organization_id: 'o1', permission_set_id: 'ps_org_admin' }, - { id: 'ups2', user_id: 'u2', organization_id: 'o1', permission_set_id: 'ps_org_admin' }, - { id: 'ups3', user_id: 'u9', organization_id: 'o1', permission_set_id: 'ps_org_admin' }, + { id: 'ups1', user_id: 'u1', organization_id: 'o1', permission_set: 'organization_admin' }, + { id: 'ups2', user_id: 'u2', organization_id: 'o1', permission_set: 'organization_admin' }, + { id: 'ups3', user_id: 'u9', organization_id: 'o1', permission_set: 'organization_admin' }, ], }); @@ -683,7 +683,7 @@ describe('[#4640] revoke speaks the engine\'s delete signature', () => { sys_permission_set: catalogFor('o1'), sys_member: [{ id: 'm1', user_id: 'u1', organization_id: 'o1', role: 'member' }], sys_user_permission_set: [ - { id: 'ups1', user_id: 'u1', organization_id: 'o1', permission_set_id: 'ps_org_admin' }, + { id: 'ups1', user_id: 'u1', organization_id: 'o1', permission_set: 'organization_admin' }, ], }); @@ -752,7 +752,7 @@ describe('[#4640] revoke speaks the engine\'s delete signature', () => { sys_permission_set: catalogFor('o1'), sys_member: [], sys_user_permission_set: [ - { id: 'ups1', user_id: 'u1', organization_id: 'o1', permission_set_id: 'ps_org_admin' }, + { id: 'ups1', user_id: 'u1', organization_id: 'o1', permission_set: 'organization_admin' }, ], }); const res = await reconcileOrgAdminGrant(stub, 'u1', 'o1', WALLED); @@ -763,67 +763,45 @@ describe('[#4640] revoke speaks the engine\'s delete signature', () => { }); // --------------------------------------------------------------------------- -// [ADR-0131 D4] The grant names its set; legacy row ids only ever REVOKE. +// [ADR-0131 D4/D10] Grants are read and written by name alone. // // A grant is written by name with no `sys_permission_set` read, in every -// posture. The catalog rows' ids are read only to reach a grant written before -// the name column existed (it names nothing until the one-time backfill), and -// only to take it away — wide across every organization's copy (#11670), so a -// demotion or a posture flip never leaves such a grant in force. +// posture, and no catalog row id reaches a grant any more: the legacy +// `permission_set_id` is retired (C7b). A grant written before the name column +// existed names nothing until the v18 ceremony's verified rewrite names it; +// until then it confers nothing and no reconcile reaches it, and once named, +// the boot sweep revokes it by name where its pair no longer qualifies. // --------------------------------------------------------------------------- -describe('[ADR-0131 D4] by-name grants; the legacy id reach restricts only', () => { - it('a legacy unnamed grant pointing at ANY organization’s copy is still revoked on demotion', async () => { +describe('[ADR-0131 D4/D10] by-name grants; no id reaches a grant', () => { + it('an unnamed legacy grant is reached by neither a demotion nor the sweep: it confers nothing, and it is the ceremony\'s to name', async () => { const stub = makeStub({ sys_permission_set: [...PLATFORM_BUCKET], sys_member: [], - sys_user_permission_set: [ - { id: 'ups_old', user_id: 'u1', organization_id: 'o3', permission_set_id: 'ps_org_admin' }, - ], + sys_user_permission_set: [{ id: 'ups_old', user_id: 'u1', organization_id: 'o3' }], }); - const res = await reconcileOrgAdminGrant(stub, 'u1', 'o3', WALLED); - expect(res.action).toBe('revoked'); - expect(stub.tables.sys_user_permission_set).toHaveLength(0); + expect((await reconcileOrgAdminGrant(stub, 'u1', 'o3', WALLED)).action).toBe('noop'); + expect((await backfillOrgAdminGrants(stub, WALLED)).revoked).toBe(0); + expect(stub.tables.sys_user_permission_set.map((r) => r.id)).toEqual(['ups_old']); }); - it('a legacy unnamed grant the backfill will name stands for the pair: no duplicate is written', async () => { - const stub = makeStub({ - sys_permission_set: catalogFor('o1'), - sys_member: [{ id: 'm1', user_id: 'u1', organization_id: 'o1', role: 'owner' }], - sys_user_permission_set: [ - { id: 'ups_pre', user_id: 'u1', organization_id: 'o1', permission_set_id: 'ps_org_admin' }, - ], - }); - expect(await reconcileOrgAdminGrant(stub, 'u1', 'o1', WALLED)).toEqual({ action: 'noop' }); - expect(stub.tables.sys_user_permission_set.map((r) => r.id)).toEqual(['ups_pre']); - }); - - it('a legacy unnamed grant on ANOTHER organization’s copy confers nothing and does not stand for the pair: it is left, and the pair is granted by name', async () => { + it('[Q-S3b (ii)] a qualifying pair holding an unnamed legacy grant in that organization is written nothing: the ceremony names that grant', async () => { const stub = makeStub({ sys_permission_set: catalogFor('o1', 'o2'), sys_member: [{ id: 'm1', user_id: 'u1', organization_id: 'o1', role: 'owner' }], - sys_user_permission_set: [ - { id: 'ups_cross', user_id: 'u1', organization_id: 'o1', permission_set_id: 'ps_org_admin_o2' }, - ], - }); - expect((await reconcileOrgAdminGrant(stub, 'u1', 'o1', WALLED)).action).toBe('granted'); - const rows = stub.tables.sys_user_permission_set; - expect(rows.find((r) => r.id === 'ups_cross')).toEqual({ - id: 'ups_cross', user_id: 'u1', organization_id: 'o1', permission_set_id: 'ps_org_admin_o2', + sys_user_permission_set: [{ id: 'ups_pre', user_id: 'u1', organization_id: 'o1', permission_set: null }], }); - expect(rows.filter((r) => r.permission_set === 'organization_admin')).toHaveLength(1); + expect(await reconcileOrgAdminGrant(stub, 'u1', 'o1', WALLED)).toEqual({ action: 'skipped', reason: 'unnamed_grant_awaits_ceremony' }); + expect(stub.tables.sys_user_permission_set).toEqual([{ id: 'ups_pre', user_id: 'u1', organization_id: 'o1', permission_set: null }]); }); - it('the backfill sweep reaches an orphan grant pointing at ANY organization’s copy', async () => { + it('CONTROL — an unnamed grant in ANOTHER organization does not hold back this one: the pair is granted by name', async () => { const stub = makeStub({ sys_permission_set: catalogFor('o1', 'o2'), sys_member: [{ id: 'm1', user_id: 'u1', organization_id: 'o1', role: 'owner' }], - sys_user_permission_set: [ - { id: 'ups_o2', user_id: 'u9', organization_id: 'o2', permission_set_id: 'ps_org_admin_o2' }, - ], + sys_user_permission_set: [{ id: 'ups_o2', user_id: 'u1', organization_id: 'o2', permission_set: null }], }); - const summary = await backfillOrgAdminGrants(stub, WALLED); - expect(summary.revoked).toBe(1); - expect(stub.tables.sys_user_permission_set.map((g) => g.user_id)).toEqual(['u1']); + expect((await reconcileOrgAdminGrant(stub, 'u1', 'o1', WALLED)).action).toBe('granted'); + expect(stub.tables.sys_user_permission_set.filter((r) => r.permission_set === 'organization_admin')).toHaveLength(1); }); it('the backfill grants with ZERO sys_permission_set rows, and an orphan by name is swept', async () => { @@ -853,7 +831,7 @@ describe('[ADR-0131 D4] by-name grants; the legacy id reach restricts only', () } }); - it('the read ORDER of a grant: catalog rows are read only for the legacy reach, never to write it', async () => { + it('the read ORDER of a grant: no catalog row is read, to revoke or to write', async () => { const stub = makeStub({ sys_permission_set: catalogFor('o1'), sys_member: [{ id: 'm1', user_id: 'u1', organization_id: 'o1', role: 'owner' }], @@ -861,18 +839,14 @@ describe('[ADR-0131 D4] by-name grants; the legacy id reach restricts only', () await reconcileOrgAdminGrant(stub, 'u1', 'o1', { posture: 'single' }); expect(stub.findCalls.map((c) => c.object)).toEqual([ 'sys_member', // does the pair qualify - 'sys_permission_set', // superseded-variant legacy ids (revoke reach) 'sys_user_permission_set', // superseded grants for the pair, by name - 'sys_user_permission_set', // …and the pair's unnamed legacy ones, by every copy's id - 'sys_permission_set', // legacy ids a grant of this organization may name (duplicate check) 'sys_user_permission_set', // does the grant already exist, by name - 'sys_user_permission_set', // …or as a legacy unnamed grant the backfill will name + 'sys_user_permission_set', // does the pair hold an unnamed legacy grant (Q-S3b) ]); const grantReads = stub.findCalls.filter((c) => c.object === 'sys_user_permission_set'); expect(grantReads.map((c) => c.where?.permission_set ?? null)).toEqual([ - 'organization_admin', null, 'organization_admin_no_bypass', null, + 'organization_admin', 'organization_admin_no_bypass', null, ]); - expect(grantReads[1].where.permission_set_id).toEqual({ $in: ['ps_org_admin', 'ps_org_admin_o1'] }); - expect(grantReads[3].where.permission_set_id).toEqual({ $in: ['ps_org_admin_nb', 'ps_org_admin_nb_o1'] }); + expect(grantReads.every((c) => !('permission_set_id' in (c.where ?? {})))).toBe(true); }); }); diff --git a/packages/plugins/plugin-security/src/auto-org-admin-grant.ts b/packages/plugins/plugin-security/src/auto-org-admin-grant.ts index 119d88e8d86..01db4f91c52 100644 --- a/packages/plugins/plugin-security/src/auto-org-admin-grant.ts +++ b/packages/plugins/plugin-security/src/auto-org-admin-grant.ts @@ -22,9 +22,9 @@ * * [ADR-0131 D4] A grant names `organization_admin` (or its no-bypass * variant) by NAME, and the set is the security catalog's definition of it - * (D3): no `sys_permission_set` row is read to write a grant, and none needs - * to exist. The legacy row ids are read only to REVOKE a grant written before - * the name column existed ({@link resolvePermissionSetIdsForName}). + * (D3): no `sys_permission_set` row is read, to write a grant or to revoke + * one. A grant written before the name column existed is named by the v18 + * ceremony's verified rewrite before its id column is dropped (D10, C7b). * * **Why this isn't done by the better-auth org plugin directly:** * better-auth does not know about ObjectStack permission sets — it @@ -47,8 +47,7 @@ import { postureEnforcesWall, type TenancyPosture } from '@objectstack/spec/secu import { securityCatalogReaderOf } from '@objectstack/core'; // [#11670] The per-organization catalog's scan budget, which the legacy // revoke reach below sizes itself by. -import { SEED_ORGANIZATION_SCAN_LIMIT } from './organization-scope.js'; -import { GRANT_SET_ID_FIELD, GRANT_SET_NAME_FIELD, grantSetNameOf } from './grant-permission-set-name.js'; +import { GRANT_SET_NAME_FIELD } from './grant-permission-set-name.js'; const SYSTEM_CTX = { isSystem: true } as const; @@ -315,60 +314,6 @@ export function autoOrgAdminGrantReason( ); } -/** - * [#11670] Rows a REVOKE is willing to hold for ONE org-admin set name under a - * walled posture: one per organization the catalog sweep covers, plus the - * organization-less platform-bucket row. - * - * A BUDGET rather than a bound — nothing caps rows-per-name, since the name is - * unique per organization (ADR-0120 D3). Exceeding it is therefore DETECTED and - * reported rather than silently truncated: a grant pointing at a row past the - * budget is a grant no revoke here can reach. - */ -const ORG_ADMIN_SET_COPY_SCAN_LIMIT = SEED_ORGANIZATION_SCAN_LIMIT + 1; - -/** - * EVERY legacy `sys_permission_set.id` bearing `name`, across the installation - * — the REVOKE reach of {@link grantsHoldingName} for a grant written before - * the name column existed (ADR-0131 D4), which names nothing until the - * one-time backfill names it. - * - * Wide, and never posture-keyed (#11670): the grant a revoke has to reach may - * point at any organization's copy, written under either posture, and a narrow - * match would leave a capability the platform decided to take away in force - * (ADR-0105 D4 F2). ⛔ Wide to REVOKE only — nothing is granted through it. A - * database with no such row answers `[]`, and the revoke then reads by name - * alone. The leg goes when ADR-0131 D10 drops the id column. - */ -async function resolvePermissionSetIdsForName( - ql: any, - name: string, - logger?: MaybeLogger, - /** - * Keep only the rows a grant of this organization may name — organization- - * less, or this organization's own — the rule the one-time backfill names a - * legacy grant by. Absent: every copy (the revoke reach). - */ - appliesToOrganization?: string, -): Promise { - const limit = ORG_ADMIN_SET_COPY_SCAN_LIMIT; - const rows = await tryFind(ql, 'sys_permission_set', { name }, limit, logger); - if (rows.length >= limit) { - logger?.warn?.( - '[security] the org-admin permission-set scan hit its bound — a grant pointing at a row past ' + - 'it is NOT reached by this revoke; the next boot sweep asks again', - { object: 'sys_permission_set', name, limit }, - ); - } - const ids: string[] = []; - for (const row of rows) { - if (appliesToOrganization !== undefined && row?.organization_id && row.organization_id !== appliesToOrganization) continue; - const id = row?.id; - if (typeof id === 'string' && id.length > 0 && !ids.includes(id)) ids.push(id); - } - return ids; -} - /** * [ADR-0131 D3] Why an org-admin grant of `name` cannot be written, or * `undefined` when it can: the security catalog — the one place the resolver @@ -397,51 +342,17 @@ async function orgAdminSetMissingFromCatalog( /** * [ADR-0131 D4] The grant rows of `where` that hold the set `name` — read BY - * NAME (`sys_user_permission_set.permission_set`), which reaches every copy of - * the set at once: a name is the reference, whichever organization's row the - * grant's id was written against. - * - * Plus — to RESTRICT only — the rows of `where` that name nothing yet - * (`grantSetNameOf` is empty) and whose id is one of `unnamedIds`: a grant - * written before the name column existed, on an upgraded deployment's first - * boot before the backfill names it (`kernel:ready` runs ahead of the - * backfill's `kernel:bootstrapped`), or one the backfill could not name (an - * id on another organization's row). A revocation reaches it, and so does - * the duplicate check a grant insert makes; nothing is ever GRANTED through - * it. The id leg goes when ADR-0131 C8 counts the unnamed grants and drops - * the column. - * - * Rows come back once each, named ones first. + * NAME (`sys_user_permission_set.permission_set`), the grant's one reference + * to its set. */ async function grantsHoldingName( ql: any, where: Record, name: string, - unnamedIds: readonly string[], limit: number, logger?: MaybeLogger, ): Promise { - const named = await tryFind(ql, 'sys_user_permission_set', { ...where, [GRANT_SET_NAME_FIELD]: name }, limit, logger); - const unnamed = unnamedIds.length > 0 - ? (await tryFind( - ql, - 'sys_user_permission_set', - { ...where, [GRANT_SET_ID_FIELD]: { $in: [...unnamedIds] } }, - limit, - logger, - )).filter((row) => grantSetNameOf(row) === undefined) - : []; - const seen = new Set(); - const out: any[] = []; - for (const row of [...named, ...unnamed]) { - const key = row?.id === undefined || row?.id === null ? undefined : String(row.id); - if (key !== undefined) { - if (seen.has(key)) continue; - seen.add(key); - } - out.push(row); - } - return out; + return tryFind(ql, 'sys_user_permission_set', { ...where, [GRANT_SET_NAME_FIELD]: name }, limit, logger); } @@ -560,15 +471,12 @@ export async function reconcileOrgAdminGrant( // nothing and leave the superseded bits in force, which is the F2 outcome // this leg is the close-out for. // - // [ADR-0131 D4] Matched BY NAME, which is every copy at once; the copies' - // ids still reach a grant that names nothing yet ({@link grantsHoldingName}). - const supersededSetIds = await resolvePermissionSetIdsForName(ql, supersededSetName, logger); + // [ADR-0131 D4] Matched BY NAME, which is every copy at once. { const stale = await grantsHoldingName( ql, { user_id: userId, organization_id: orgId }, supersededSetName, - supersededSetIds, 5, logger, ); @@ -589,12 +497,17 @@ export async function reconcileOrgAdminGrant( // [ADR-0131 D4] Granting asks "does the pair already hold the set" BY NAME: // a grant holds its set by name, and the new grant names it alone — no // `sys_permission_set` row is read to write it, and none needs to exist. - // Revoking asks "does the pair hold ANY grant of that name", by name plus the - // legacy id reach of {@link grantsHoldingName}, which only ever restricts. - // The existence check reaches a legacy unnamed grant too — a duplicate it - // must not insert — but only one the backfill will name (its set row is - // organization-less or this organization's own): a grant it cannot name - // confers nothing, and must not stand in for the one written here. + // Revoking asks "does the pair hold ANY grant of that name", by name. A + // second grant of one set to one user in one organization cannot be written + // (the declared unique index on user, set and organization); one already + // stored is the v18 ceremony's to report, never deleted here. + // + // [C7b, seat answers on #15211 Q-S3b (ii)] A pair holding ANY grant that + // names nothing in this organization — a 17.x grant the v18 ceremony's + // rewrite has not named yet — is written nothing: the rewrite will name + // that grant, and a named grant written beside it would hold the very key + // the rewrite needs (the unique index would refuse its rename). The unnamed + // grant confers nothing meanwhile, as every 17.x grant does until --apply. if (shouldGrant) { const missing = await orgAdminSetMissingFromCatalog(ql, grantSetName, logger); if (missing) return { action: 'skipped', reason: missing }; @@ -602,26 +515,18 @@ export async function reconcileOrgAdminGrant( ql, { user_id: userId, organization_id: orgId }, grantSetName, - await resolvePermissionSetIdsForName(ql, grantSetName, logger, orgId), 5, logger, ); - if (existingGrants.length > 0) { - // Deduplicate stale duplicates if any slipped through — rows repeating - // an earlier row's own legacy id (a grant written by name carries none). - // A grant against another legacy copy is a different row and is never - // deleted here (#11670). - const kept = new Set(); - for (const row of existingGrants) { - const rowSetId = String(row?.[GRANT_SET_ID_FIELD] ?? ''); - if (!kept.has(rowSetId)) { - kept.add(rowSetId); - continue; - } - if (row?.id) await tryDelete(ql, 'sys_user_permission_set', String(row.id), logger); - } - return { action: 'noop' }; - } + if (existingGrants.length > 0) return { action: 'noop' }; + const unnamed = await tryFind( + ql, + 'sys_user_permission_set', + { user_id: userId, organization_id: orgId, [GRANT_SET_NAME_FIELD]: null }, + 1, + logger, + ); + if (unnamed.length > 0) return { action: 'skipped', reason: 'unnamed_grant_awaits_ceremony' }; const created = await tryInsert( ql, 'sys_user_permission_set', @@ -662,15 +567,11 @@ export async function reconcileOrgAdminGrant( // revokes; it never re-points or adopts a row for someone who still // qualifies. // - // [ADR-0131 D4] By NAME, every copy at once; the copies' ids still reach a - // grant that names nothing yet, so no demotion leaves one standing - // ({@link grantsHoldingName}). - const revocableSetIds = await resolvePermissionSetIdsForName(ql, grantSetName, logger); + // [ADR-0131 D4] By NAME, every copy at once. const existingGrants = await grantsHoldingName( ql, { user_id: userId, organization_id: orgId }, grantSetName, - revocableSetIds, 5, logger, ); @@ -730,22 +631,7 @@ export async function backfillOrgAdminGrants( // [#11670] The sweep is installation-wide by construction — it scans every // `sys_member` row and every org-admin grant, across organizations. [ADR-0131 - // D4] Grants are read by name; the legacy row ids resolved here only extend - // the orphan sweep's reach to grants that name nothing yet (restrict-only), - // so a database with no `sys_permission_set` row is swept all the same. - const permSetIds = await resolvePermissionSetIdsForName( - ql, - orgAdminSetNameForPosture(posture, suppressUnbounded), - logger, - ); - // [ADR-0105 D4] The orphan sweep below must see BOTH variants: a boot that - // changed posture leaves grants of the superseded set behind, and those are - // exactly the rows whose bits must stop applying. - const supersededIds = await resolvePermissionSetIdsForName( - ql, - supersededOrgAdminSetName(posture, suppressUnbounded), - logger, - ); + // D4] Grants are read by name. const members = await tryFind(ql, 'sys_member', {}, limit, logger); // De-duplicate by (user_id, organization_id) pair — a user with two @@ -773,17 +659,12 @@ export async function backfillOrgAdminGrants( // pair with NO membership row left (orphaned grants from deletes // that fired before this hook existed). // - // [ADR-0131 D4] The grants of either variant, by NAME — plus, through the - // copies' ids, the ones that name nothing yet (this sweep runs at - // `kernel:ready`, ahead of the name backfill), so an orphan is reached - // either way ({@link grantsHoldingName}). + // [ADR-0131 D4] The grants of either variant, by NAME. [ADR-0105 D4] Both + // variants: a boot that changed posture leaves grants of the superseded set + // behind, and those are exactly the rows whose bits must stop applying. const allGrants = [ - ...(await grantsHoldingName( - ql, {}, orgAdminSetNameForPosture(posture, suppressUnbounded), permSetIds, limit, logger, - )), - ...(await grantsHoldingName( - ql, {}, supersededOrgAdminSetName(posture, suppressUnbounded), supersededIds, limit, logger, - )), + ...(await grantsHoldingName(ql, {}, orgAdminSetNameForPosture(posture, suppressUnbounded), limit, logger)), + ...(await grantsHoldingName(ql, {}, supersededOrgAdminSetName(posture, suppressUnbounded), limit, logger)), ]; for (const g of allGrants) { const userId = String(g?.user_id ?? ''); diff --git a/packages/plugins/plugin-security/src/bootstrap-platform-admin-authenticable-target.test.ts b/packages/plugins/plugin-security/src/bootstrap-platform-admin-authenticable-target.test.ts index ae1f9191b88..79634affc8f 100644 --- a/packages/plugins/plugin-security/src/bootstrap-platform-admin-authenticable-target.test.ts +++ b/packages/plugins/plugin-security/src/bootstrap-platform-admin-authenticable-target.test.ts @@ -334,7 +334,6 @@ describe('#14348 — the promotion target is the oldest human that can AUTHENTIC { id: 'ups_legacy', user_id: 'usr_person0', - permission_set_id: adminPsId, permission_set: 'admin_full_access', organization_id: null, }, diff --git a/packages/plugins/plugin-security/src/bootstrap-platform-admin-existing-holder-scan.test.ts b/packages/plugins/plugin-security/src/bootstrap-platform-admin-existing-holder-scan.test.ts index 61b13fcb7f4..bb4b4dd463f 100644 --- a/packages/plugins/plugin-security/src/bootstrap-platform-admin-existing-holder-scan.test.ts +++ b/packages/plugins/plugin-security/src/bootstrap-platform-admin-existing-holder-scan.test.ts @@ -238,7 +238,6 @@ async function seedTenant( { id: 'ups_zzz_founder', user_id: 'usr_founder', - permission_set_id: ADMIN_PS_ID, permission_set: 'admin_full_access', organization_id: unscopedOrganizationId, }, @@ -259,7 +258,6 @@ async function seedTenant( { id: `ups_org_${n}`, user_id: `usr_orgadmin_${n}`, - permission_set_id: ADMIN_PS_ID, permission_set: 'admin_full_access', organization_id: `org_${n}`, }, @@ -275,7 +273,7 @@ async function findRows(engine: ObjectQL, object: string, where: any = {}): Prom /** Every `admin_full_access` grant row that no organization scopes. */ async function unscopedGrants(engine: ObjectQL): Promise { - const links = await findRows(engine, 'sys_user_permission_set', { permission_set_id: ADMIN_PS_ID }); + const links = await findRows(engine, 'sys_user_permission_set', { permission_set: 'admin_full_access' }); expect(links.length, 'ANTI-VACUITY: the fixture must have written grant rows').toBeGreaterThan(0); return links.filter((r) => !r.organization_id); } @@ -307,13 +305,14 @@ describe('#16861 — an existing platform admin is found, not sampled for', () = const engine = await boot(); await seedTenant(engine as any, 60); - const all = await findRows(engine, 'sys_user_permission_set', { permission_set_id: ADMIN_PS_ID }); + const all = await findRows(engine, 'sys_user_permission_set', { permission_set: 'admin_full_access' }); expect(all).toHaveLength(61); - // The exact read the defect shipped: same object, same where, same cap, no order. + // The exact read the defect shipped: same object, the same population (every + // grant of the admin set, read by name now that the id is retired), same cap, no order. const window50 = await (engine as any).find( 'sys_user_permission_set', - { where: { permission_set_id: ADMIN_PS_ID }, limit: 50 }, + { where: { permission_set: 'admin_full_access' }, limit: 50 }, { context: SYSTEM_CTX }, ); expect(window50).toHaveLength(50); @@ -421,7 +420,7 @@ describe('#16861 — an existing platform admin is found, not sampled for', () = ); await ql.insert( 'sys_user_permission_set', - { id: 'ups_system', user_id: 'usr_system', permission_set_id: ADMIN_PS_ID, permission_set: 'admin_full_access', organization_id: null }, + { id: 'ups_system', user_id: 'usr_system', permission_set: 'admin_full_access', organization_id: null }, { context: SYSTEM_CTX }, ); await seedUser(ql, 'usr_real', 'real@tenant.example', '2026-03-01T00:00:00.000Z', true); @@ -480,7 +479,7 @@ describe('#16861 — an existing platform admin is found, not sampled for', () = out.push({ id: `ups_${String(i).padStart(6, '0')}`, user_id: `usr_${String(i).padStart(6, '0')}`, - permission_set_id: permissionSets.find((p) => p.name === 'admin_full_access')?.id, + permission_set: 'admin_full_access', organization_id: `org_${i}`, }); } diff --git a/packages/plugins/plugin-security/src/bootstrap-platform-admin-promotion-selection.test.ts b/packages/plugins/plugin-security/src/bootstrap-platform-admin-promotion-selection.test.ts index 224650c766f..c899905cd2a 100644 --- a/packages/plugins/plugin-security/src/bootstrap-platform-admin-promotion-selection.test.ts +++ b/packages/plugins/plugin-security/src/bootstrap-platform-admin-promotion-selection.test.ts @@ -749,13 +749,11 @@ describe('#16682 — the promotion target is chosen, not sampled', () => { await seedLegacyPlatformBucket(engine as any, defaultPermissionSets); const first = await bootstrapPlatformAdmin(engine as any, defaultPermissionSets, {}); expect(first.reason).toBe('no_authenticable_user'); - const sets = await findRows(engine, 'sys_permission_set', { name: 'admin_full_access' }); await (engine as any).insert( 'sys_user_permission_set', { id: 'ups_legacy', user_id: 'usr_person0', - permission_set_id: sets[0].id, permission_set: 'admin_full_access', organization_id: null, }, diff --git a/packages/plugins/plugin-security/src/bootstrap-platform-admin.ts b/packages/plugins/plugin-security/src/bootstrap-platform-admin.ts index 56166db7690..b8141cc432b 100644 --- a/packages/plugins/plugin-security/src/bootstrap-platform-admin.ts +++ b/packages/plugins/plugin-security/src/bootstrap-platform-admin.ts @@ -449,12 +449,6 @@ export function shouldReplayBootstrapFor(opCtx: { */ const PLATFORM_ADMIN_PERMISSION_SET_NAME = 'admin_full_access'; -/** Whether a grant still carries the legacy id a release before the name column wrote (ADR-0131 D10). */ -function hasLegacySetId(row: any): boolean { - const id = row?.permission_set_id; - return id !== undefined && id !== null && String(id) !== ''; -} - /** * Who ALREADY holds the unscoped platform-admin grant (commit 1c83ca226) — the * one rule both {@link bootstrapPlatformAdmin}'s `already_have_admin` guard and @@ -526,33 +520,29 @@ function hasLegacySetId(row: any): boolean { * (`sys_user_permission_set.permission_set`), not for grants carrying the set * row's id: a holder is a grant of the set by name. * - * A grant written before that column existed carries no name until the - * one-time backfill names it — and the backfill runs at `kernel:bootstrapped`, - * AFTER the `kernel:ready` bootstrap that asks this question. On the first boot - * of an upgraded deployment, then, its platform administrator's grant names - * nothing yet, and a by-name scan alone would answer "nobody", promote the - * oldest user and hand them the seeded records: the second grant this function - * exists to refuse. So leg C, only when A and B found nobody, reaches the - * unscoped human grants that name NOTHING and still carry a legacy - * `permission_set_id`, and one comes back as `unnamedHolder`. It only ever - * RESTRICTS: the bootstrap withholds the promotion on it, and it is never a - * holder — never an `adminUserId`, never a claim target - * ({@link findExistingPlatformAdmin}). + * A grant written before that column existed carries no name until the v18 + * ceremony's verified rewrite names it (`os migrate organization-ownership + * --apply`, ADR-0131 D10). A by-name scan alone would answer "nobody" for such + * a grant, promote the oldest user and hand them the seeded records: the + * second grant this function exists to refuse. So leg C, only when A and B + * found nobody, reaches the unscoped human grants that name NOTHING, and one + * comes back as `unnamedHolder`. It only ever RESTRICTS: the bootstrap + * withholds the promotion on it, and it is never a holder — never an + * `adminUserId`, never a claim target ({@link findExistingPlatformAdmin}). * - * [C3 stage 8] Leg C asks for ANY such grant, not for grants of the admin set's - * row: the catalog row object is retired, so no read can say which set a legacy - * id pointed at. An unnamed grant of another set therefore withholds the - * promotion too, until the organization-ownership migration's verified - * id-to-name rewrite (ADR-0131 D10) names it — the stricter reading, since the - * one it cannot tell apart may be the administrator's. A database that holds no - * unnamed grant (every fresh one) answers by name alone. + * Leg C asks for ANY such grant, not for grants of the admin set: no read can + * say which set an unnamed grant held (the legacy id column is retired, C7b). + * An unnamed grant of another set therefore withholds the promotion too — the + * stricter reading, since the one it cannot tell apart may be the + * administrator's. A database that holds no unnamed grant (every fresh one, + * and every one the ceremony has run on) answers by name alone. */ async function findPlatformAdminGrantHolder( ql: any, reachUnnamed: boolean, ): Promise<{ holder: any | undefined; - /** Leg C: an unscoped human grant that names no set yet and carries a legacy id (restricts only). */ + /** Leg C: an unscoped human grant that names no set yet (restricts only). */ unnamedHolder: any | undefined; adminGrantRowsExamined: number; truncated: boolean; @@ -610,13 +600,13 @@ async function findPlatformAdminGrantHolder( holder = await scan({ [GRANT_SET_NAME_FIELD]: PLATFORM_ADMIN_PERMISSION_SET_NAME }, isUnscopedHumanHolder); } - // Leg C — the grants that name nothing yet and carry a legacy id; it - // restricts only (see the function note). + // Leg C — the grants that name nothing yet; it restricts only (see the + // function note). let unnamedHolder: any | undefined; if (!holder && reachUnnamed) { unnamedHolder = await scan( { [GRANT_SET_NAME_FIELD]: null }, - (r) => grantSetNameOf(r) === undefined && hasLegacySetId(r) && isUnscopedHumanHolder(r), + (r) => grantSetNameOf(r) === undefined && isUnscopedHumanHolder(r), ); } diff --git a/packages/plugins/plugin-security/src/catalog-reference-report.test.ts b/packages/plugins/plugin-security/src/catalog-reference-report.test.ts index 5e672dc30dd..008f5abb8a0 100644 --- a/packages/plugins/plugin-security/src/catalog-reference-report.test.ts +++ b/packages/plugins/plugin-security/src/catalog-reference-report.test.ts @@ -5,8 +5,8 @@ * whose name the security catalog does not resolve, per organization. * * Measured on a REAL `ObjectQL` engine over the SQL driver with the REAL - * `SecurityPlugin` started on it (so the two `kernel:bootstrapped` backfills - * the report runs after are the real ones), and the REAL metadata door + * `SecurityPlugin` started on it (so the `kernel:bootstrapped` handler that + * runs the report is the real one), and the REAL metadata door * (`ObjectStackProtocolImplementation`) for the definition saves. The catalog * is S1's read (`createSecurityCatalogReader`) over the engine registry and a * metadata service whose declarations a test can remove and re-add — the way @@ -228,7 +228,7 @@ const findings = (report: CatalogReferenceReport) => organizationId: o.organizationId, positionAssignments: o.positionAssignments.map((r) => `${r.id}:${r.name}`), permissionSetGrants: o.permissionSetGrants.map((r) => `${r.id}:${r.name}`), - unnamedGrants: o.unnamedGrants.map((g) => `${g.id}:${g.permissionSetId}`), + unnamedGrants: o.unnamedGrants.map((g) => `${g.id}:${g.userId}`), })); describe('[ADR-0131 D4] the catalog reference report — references whose names resolve nowhere, per organization', () => { @@ -264,7 +264,7 @@ describe('[ADR-0131 D4] the catalog reference report — references whose names // a grant whose permission set has gone. await insertRows(b, 'sys_permission_set', [{ id: 'ps_auditor', name: AUDITOR.name, label: 'Auditor' }]); await insertRows(b, 'sys_user_permission_set', [ - { id: 'g_ghost', user_id: 'usr_a', permission_set_id: 'ps_gone', permission_set: 'ghost_set', organization_id: ORG }, + { id: 'g_ghost', user_id: 'usr_a', permission_set: 'ghost_set', organization_id: ORG }, { id: 'g_ok', user_id: 'usr_a', permission_set: AUDITOR.name, organization_id: ORG }, ]); @@ -290,28 +290,28 @@ describe('[ADR-0131 D4] the catalog reference report — references whose names expect(report.organizations[1].positionAssignments[0].userId).toBe('usr_a'); }); - it('a grant on a row-only set stays unnamed after the name backfill, confers nothing, and is listed', async () => { + it('an unnamed grant stays unnamed through the boot (no boot names it, ADR-0131 D10), confers nothing, and is listed', async () => { const b = await boot(); await insertUser(b, 'usr_row_only'); - // A set row whose name no declaration carries: the backfill cannot show - // the name names a definition, so the grant keeps no name. + // A set row whose name no declaration carries, beside a grant that names + // nothing: only the v18 ceremony's rewrite names a legacy grant. await insertRows(b, 'sys_permission_set', [ { id: 'ps_row_only', name: 's3_row_only', label: 'Row only', system_permissions: ['s3_row_only_bit'] }, ]); await insertUnnamedGrants(b, [ - { id: 'g_row_only', user_id: 'usr_row_only', permission_set_id: 'ps_row_only', organization_id: ORG }, + { id: 'g_row_only', user_id: 'usr_row_only', organization_id: ORG }, ]); await b.bootstrapped(); const [grant] = (await b.engine.find('sys_user_permission_set', { where: { id: 'g_row_only' }, context: SYS })) as any[]; - expect(grant.permission_set ?? null, 'the name backfill left it unnamed').toBeNull(); + expect(grant.permission_set ?? null, 'no boot names it').toBeNull(); const resolved = await resolveUserAuthzGrants(b.engine as any, 'usr_row_only', { tenantId: ORG }); expect({ permissions: resolved.permissions.includes('s3_row_only'), bit: resolved.systemPermissions.includes('s3_row_only_bit') }) .toEqual({ permissions: false, bit: false }); expect(findings(await b.report())).toEqual([{ organizationId: ORG, positionAssignments: [], permissionSetGrants: [], - unnamedGrants: ['g_row_only:ps_row_only'], + unnamedGrants: ['g_row_only:usr_row_only'], }]); }); }); @@ -527,22 +527,18 @@ describe('[ADR-0131 D4] a read that did not happen is never "resolves nowhere"', }); describe('[ADR-0131 D3/D4] SecurityPlugin runs the report at kernel:bootstrapped', () => { - it('after the grant-name backfill — a name it resolved this boot is not reported — and the report writes nothing', async () => { + it('as the last kernel:bootstrapped handler, and the report writes nothing', async () => { const b = await boot(); - // An unnamed grant whose set row's name the catalog resolves, which the - // grant-name backfill names this boot: it may not be reported. (The - // row-only position backfill ran here too until C3 stage 8 unwired it.) + // (The grant-name backfill ran ahead of it until the v18 ceremony's + // rewrite replaced it, ADR-0131 D10; the row-only position backfill until + // C3 stage 8 unwired it.) await insertRows(b, 'sys_user_position', [ { id: 'up_ghost', user_id: 'usr_a', position: 'ghost_lead', organization_id: ORG }, ]); - await insertRows(b, 'sys_permission_set', [{ id: 'ps_auditor', name: AUDITOR.name, label: 'Auditor' }]); - await insertUnnamedGrants(b, [ - { id: 'g_backfilled', user_id: 'usr_a', permission_set_id: 'ps_auditor', organization_id: ORG }, - ]); const handlers = b.hooks.get('kernel:bootstrapped') ?? []; - // The grant-name backfill, then this report (ADR-0131 D4). - expect(handlers).toHaveLength(2); + // This report alone: the grant-name backfill that ran ahead of it is retired (ADR-0131 D10). + expect(handlers).toHaveLength(1); for (const handler of handlers.slice(0, -1)) await handler(); const writes = [ vi.spyOn(b.engine, 'insert'), vi.spyOn(b.engine, 'update'), vi.spyOn(b.engine, 'delete'), diff --git a/packages/plugins/plugin-security/src/catalog-reference-report.ts b/packages/plugins/plugin-security/src/catalog-reference-report.ts index 9923a24957b..66a31baf526 100644 --- a/packages/plugins/plugin-security/src/catalog-reference-report.ts +++ b/packages/plugins/plugin-security/src/catalog-reference-report.ts @@ -20,12 +20,9 @@ * 2. **Grants naming no permission set** — `sys_user_permission_set` rows * whose `permission_set` the catalog does not resolve. * 3. **Grants that name nothing** — `sys_user_permission_set` rows whose - * `permission_set` is empty. The grant-name backfill - * (`grant-permission-set-name-backfill.ts`) names every grant whose set row - * carries a name the catalog resolves; the rest — an id with no set row, - * another organization's set row, a set row whose name the catalog does not - * resolve (a row-only set) — stay unnamed, and an unnamed grant confers - * nothing. + * `permission_set` is empty. The v18 ceremony's verified rewrite names + * every grant it can before the legacy id column is dropped, and blocks on + * the rest; an unnamed grant confers nothing. * * And it lists PER POSITION, since a definition belongs to the environment and * to no organization: @@ -65,18 +62,16 @@ * * ## When it runs * - * At `kernel:bootstrapped`, from `SecurityPlugin.start`, AFTER the grant-name - * backfill registered beside it (handlers run in registration order): every - * `kernel:ready` handler has settled, so every code, package and environment - * definition this boot registers is in the catalog, and a name the backfill - * gives a grant this boot is not reported. A package installed into the running process later is not + * At `kernel:bootstrapped`, from `SecurityPlugin.start`: every `kernel:ready` + * handler has settled, so every code, package and environment definition this + * boot registers is in the catalog. A package installed into the running process later is not * re-judged until the next boot, which is why every line says "at this boot". * * ADR anchors: ADR-0131 D3 (one catalog home), D4 (references by name). */ import type { SecurityCatalogEntry, SecurityCatalogReader, SecurityCatalogType } from '@objectstack/core'; -import { GRANT_OBJECT, GRANT_SET_ID_FIELD, GRANT_SET_NAME_FIELD } from './grant-permission-set-name.js'; +import { GRANT_OBJECT, GRANT_SET_NAME_FIELD } from './grant-permission-set-name.js'; /** The user ↔ position assignment object (ADR-0057 D4). */ export const POSITION_ASSIGNMENT_OBJECT = 'sys_user_position'; @@ -122,8 +117,6 @@ export interface UnresolvedReference { export interface UnnamedGrant { id: string; userId: string | null; - /** The grant's id reference, when it carries one — what the name backfill could not turn into a name. */ - permissionSetId: string | null; } /** Everything the report found for one organization; `null` is the organization-less bucket. */ @@ -312,13 +305,13 @@ export async function collectCatalogReferences( if (readGrants) { const rows = await scanAll(engine, GRANT_OBJECT, [ - 'id', 'user_id', GRANT_SET_ID_FIELD, GRANT_SET_NAME_FIELD, 'organization_id', + 'id', 'user_id', GRANT_SET_NAME_FIELD, 'organization_id', ]); for (const row of rows) { const name = nameOf(row[GRANT_SET_NAME_FIELD]); if (name === undefined) { bucket(organizationOf(row)).unnamedGrants.push({ - id: String(row.id), userId: stringOrNull(row.user_id), permissionSetId: stringOrNull(row[GRANT_SET_ID_FIELD]), + id: String(row.id), userId: stringOrNull(row.user_id), }); continue; } @@ -440,10 +433,10 @@ function logReport(report: CatalogReferenceReport, logger?: CatalogReferenceRepo logError( logger, `[security] ${unnamed.count} ${GRANT_OBJECT} grant(s) name no permission set (${GRANT_SET_NAME_FIELD} is ` + - 'empty), so each confers nothing (ADR-0131 D4). The one-time name backfill names a grant whose set row ' + - 'carries a name the catalog resolves; these did not get one — the set row is missing, belongs to another ' + - 'organization, or carries a name the catalog does not resolve, and the backfill logs which. Nothing was ' + - 'changed. Fix: grant the intended permission set again by name, or remove these grants.', + 'empty), so each confers nothing (ADR-0131 D4). Nothing was changed. Fix: run the v18 ceremony, ' + + 'os migrate organization-ownership --apply, which names each legacy grant from its retired id and lists ' + + 'the ones it cannot name; then restart. Granting the set again by name before it runs makes a second ' + + 'grant the ceremony will report, not name.', { count: unnamed.count, organizations: unnamed.organizations }, ); } diff --git a/packages/plugins/plugin-security/src/claim-seed-ownership-warm-boot.test.ts b/packages/plugins/plugin-security/src/claim-seed-ownership-warm-boot.test.ts index 2f0e5b4516e..05756224b36 100644 --- a/packages/plugins/plugin-security/src/claim-seed-ownership-warm-boot.test.ts +++ b/packages/plugins/plugin-security/src/claim-seed-ownership-warm-boot.test.ts @@ -308,8 +308,8 @@ describe('the claim target is the existing platform admin, by the bootstrap rule await engine.insert('sys_user', { id, email: `${id}@example.test`, name: id, created_at: createdAt }, SYS); await engine.insert('sys_account', { id: `acc_${id}`, user_id: id, account_id: id, provider_id: 'credential' }, SYS); } - await engine.insert('sys_user_permission_set', { id: 'ups_1', user_id: 'usr_zed', permission_set_id: 'ps_admin', permission_set: 'admin_full_access', organization_id: null }, SYS); - await engine.insert('sys_user_permission_set', { id: 'ups_2', user_id: 'usr_amy', permission_set_id: 'ps_admin', permission_set: 'admin_full_access', organization_id: null }, SYS); + await engine.insert('sys_user_permission_set', { id: 'ups_1', user_id: 'usr_zed', permission_set: 'admin_full_access', organization_id: null }, SYS); + await engine.insert('sys_user_permission_set', { id: 'ups_2', user_id: 'usr_amy', permission_set: 'admin_full_access', organization_id: null }, SYS); return engine; } diff --git a/packages/plugins/plugin-security/src/explain-asker-gate.test.ts b/packages/plugins/plugin-security/src/explain-asker-gate.test.ts index 10093625b7b..a165eb05ce6 100644 --- a/packages/plugins/plugin-security/src/explain-asker-gate.test.ts +++ b/packages/plugins/plugin-security/src/explain-asker-gate.test.ts @@ -176,14 +176,14 @@ beforeAll(async () => { active: true, object_permissions: JSON.stringify({}), system_permissions: JSON.stringify([]), }, SYS); await e.insert('sys_user_permission_set', { - id: 'ups_ag_member', user_id: MEMBER, permission_set_id: 'ps_ag_member', permission_set: MEMBER_SET.name, + id: 'ups_ag_member', user_id: MEMBER, permission_set: MEMBER_SET.name, }, SYS); await e.insert('sys_permission_set', { id: 'ps_ag_edit_only', name: EDIT_ONLY_SET.name, label: EDIT_ONLY_SET.name, organization_id: null, managed_by: 'admin', active: true, object_permissions: JSON.stringify({}), system_permissions: JSON.stringify([]), }, SYS); await e.insert('sys_user_permission_set', { - id: 'ups_ag_edit_only', user_id: EDIT_ONLY_USER, permission_set_id: 'ps_ag_edit_only', permission_set: EDIT_ONLY_SET.name, + id: 'ups_ag_edit_only', user_id: EDIT_ONLY_USER, permission_set: EDIT_ONLY_SET.name, }, SYS); await e.insert(BOARD, [ { id: HIDDEN, name: 'hidden', owner_id: 'usr_ag_owner' }, diff --git a/packages/plugins/plugin-security/src/explain-enforce-parity.test.ts b/packages/plugins/plugin-security/src/explain-enforce-parity.test.ts index b2f54e68fc7..90fe6ed580f 100644 --- a/packages/plugins/plugin-security/src/explain-enforce-parity.test.ts +++ b/packages/plugins/plugin-security/src/explain-enforce-parity.test.ts @@ -431,7 +431,7 @@ async function bootPrincipal(source: PostureSource) { // Each grant carries the set's name beside its id, as every platform writer stores it. const grant = (user: string, set: string, name: string) => e.insert('sys_user_permission_set', { - id: `ups_${user}_${set}_${n}`, user_id: user, permission_set_id: set, permission_set: name, organization_id: ALPHA, + id: `ups_${user}_${set}_${n}`, user_id: user, permission_set: name, organization_id: ALPHA, }, SYS); await grant(USER_ADMIN, `ps_user_admin_${n}`, 'qa_user_admin'); await grant(USER_ADMIN, `ps_reader_${n}`, READER_SET); diff --git a/packages/plugins/plugin-security/src/explain-principal-person.test.ts b/packages/plugins/plugin-security/src/explain-principal-person.test.ts index 78bf8fba4af..2e1d348e0a4 100644 --- a/packages/plugins/plugin-security/src/explain-principal-person.test.ts +++ b/packages/plugins/plugin-security/src/explain-principal-person.test.ts @@ -134,7 +134,7 @@ beforeAll(async () => { active: true, object_permissions: JSON.stringify({}), system_permissions: JSON.stringify([]), }, SYS); await e.insert('sys_user_permission_set', { - id: 'ups_pp_grace', user_id: GRACE, permission_set_id: 'ps_pp_directory', permission_set: DIRECTORY_SET.name, + id: 'ups_pp_grace', user_id: GRACE, permission_set: DIRECTORY_SET.name, }, SYS); // A position Lin holds because Grace delegated it, until a date far ahead. await e.insert('sys_user_position', { diff --git a/packages/plugins/plugin-security/src/explain-removed-member-principal.test.ts b/packages/plugins/plugin-security/src/explain-removed-member-principal.test.ts index ab25ba84650..70663904a2f 100644 --- a/packages/plugins/plugin-security/src/explain-removed-member-principal.test.ts +++ b/packages/plugins/plugin-security/src/explain-removed-member-principal.test.ts @@ -156,12 +156,12 @@ async function boot(makeDriver: () => Driver, posture: TenancyPosture) { // Grant rows, each scoped to org_alpha. Removing a member deletes their // `sys_member` row, not their grants. await e.insert('sys_user_permission_set', { - id: `ups_admin_${n}`, user_id: USER_ADMIN, permission_set_id: `ps_user_admin_${n}`, permission_set: 'qa_user_admin', + id: `ups_admin_${n}`, user_id: USER_ADMIN, permission_set: 'qa_user_admin', organization_id: ALPHA, }, SYS); for (const u of [USER_MEMBER, USER_REMOVED]) { await e.insert('sys_user_permission_set', { - id: `ups_${u}_${n}`, user_id: u, permission_set_id: `ps_probe_${n}`, permission_set: PROBE_SET, organization_id: ALPHA, + id: `ups_${u}_${n}`, user_id: u, permission_set: PROBE_SET, organization_id: ALPHA, }, SYS); } await e.insert(PROBE, { id: 'p1', name: 'probe row' }, SYS); diff --git a/packages/plugins/plugin-security/src/grant-permission-set-name-backfill.test.ts b/packages/plugins/plugin-security/src/grant-permission-set-name-backfill.test.ts deleted file mode 100644 index db54796705f..00000000000 --- a/packages/plugins/plugin-security/src/grant-permission-set-name-backfill.test.ts +++ /dev/null @@ -1,783 +0,0 @@ -// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. - -/** - * [ADR-0131 D4] The one-time backfill of `sys_user_permission_set.permission_set` - * on grants written before the column existed. - * - * Measured on a REAL `ObjectQL` engine over a real SQL driver with the REAL - * `SecurityPlugin` started on it, so the backfill's writes pass the same - * engine, driver wall and name hooks a deployment's do. A grant "written - * before the column existed" is made by writing it with the name hooks - * unbound and clearing the name, which is the stored shape such a grant has. - * - * The catalog the names are verified through is S1's read - * (`createSecurityCatalogReader`) over the engine registry, which holds the - * permission-set definitions here as a package manifest's `permissions` put - * them there in a deployment. - */ - -import { describe, it, expect, afterEach, vi } from 'vitest'; -import { ObjectQL } from '@objectstack/objectql'; -import { SqlDriver } from '@objectstack/driver-sql'; -import { - LiteKernel, - assembleExecutionContext, - createSecurityCatalogReader, - resetPlatformAdminEmailMemo, - resolveUserAuthzGrants, - type SecurityCatalogReader, -} from '@objectstack/core'; -import type { PermissionSet } from '@objectstack/spec/security'; -import { DATA_MIGRATION_FLAG_OBJECT } from '@objectstack/spec/system'; -import { SysUser, SysAccount, SysMember, SysOrganization } from '@objectstack/platform-objects/identity'; -import { SysMigration } from '@objectstack/platform-objects/system'; - -import { SecurityPlugin } from './security-plugin.js'; -import { bootstrapPlatformAdmin } from './bootstrap-platform-admin.js'; -import { seedLegacyPlatformBucket } from './__tests__/legacy-catalog-rows.testkit.js'; -import { reconcileOrgAdminGrant } from './auto-org-admin-grant.js'; -import { SysPosition } from './__tests__/legacy-catalog-objects.testkit.js'; -import { SysUserPosition } from './objects/sys-user-position.object.js'; -import { SysPermissionSet } from './__tests__/legacy-catalog-objects.testkit.js'; -import { SysPositionPermissionSet } from './__tests__/legacy-catalog-objects.testkit.js'; -import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.js'; -import { defaultPermissionSets } from './objects/default-permission-sets.js'; -import { - GRANT_SET_NAME_BACKFILL_MIGRATION_ID, - runOneTimeGrantPermissionSetNameBackfill, -} from './grant-permission-set-name-backfill.js'; - -// --------------------------------------------------------------------------- -// Fixture -// --------------------------------------------------------------------------- - -const SYS = { isSystem: true } as const; -const POSTURE_ENV = 'OS_TENANCY_POSTURE'; -const OWNER_ENV = 'OS_PLATFORM_OWNER_EMAIL'; -const ORG = 'org_eq'; -const OTHER_ORG = 'org_other'; -const CATALOG_PACKAGE = 'com.objectstack.qa.grant-name-backfill'; - -/** The non-system administrator whose writes are the data door's (superuser wildcard). */ -const QA_ADMIN = { - name: 'qa_admin', - label: 'QA Admin', - objects: { - '*': { - allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true, - viewAllRecords: true, modifyAllRecords: true, - }, - }, -} as unknown as PermissionSet; - -/** A tenant's own set, and the other organization's set of another name. */ -const OWN_SET = { name: 'eq_reviewer', label: 'Reviewer', objects: {} } as unknown as PermissionSet; -const OTHER_SET = { name: 'other_auditor', label: 'Auditor', objects: {} } as unknown as PermissionSet; - -type Posture = 'single' | 'isolated'; -type Engine = ObjectQL; -const engines: Engine[] = []; - -afterEach(async () => { - vi.restoreAllMocks(); - delete process.env[POSTURE_ENV]; - delete process.env[OWNER_ENV]; - resetPlatformAdminEmailMemo(); - while (engines.length) { - try { await engines.pop()?.destroy(); } catch { /* noop */ } - } -}); - -const newLogger = () => ({ info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }); - -interface World { - engine: Engine; - catalog: SecurityCatalogReader; - /** Kernel lifecycle hooks the plugin registered, by event — collected, never fired by the fixture. */ - hooks: Map Promise>>; - pluginLogger: ReturnType; -} - -/** - * A booted engine with the RBAC objects, the deployment ledger and the started - * `SecurityPlugin`, in `posture`; the platform catalog rows seeded by - * `bootstrapPlatformAdmin` (and, in `single`, its promotion), and both - * organizations present. - */ -async function boot(posture: Posture, opts: { ledger?: boolean } = {}): Promise { - if (posture === 'isolated') { - process.env[POSTURE_ENV] = 'isolated'; - process.env[OWNER_ENV] = 'admin@eq.example'; - } - resetPlatformAdminEmailMemo(); - - const engine = new ObjectQL(); - engine.registerDriver( - new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }), - true, - ); - await engine.init(); - engine.registerApp({ - id: 'com.objectstack.qa.grant-name-backfill', - name: 'Grant name backfill', - version: '1.0.0', - type: 'plugin', - scope: 'system', - objects: [ - SysUser, SysAccount, SysMember, SysOrganization, - SysPosition, SysUserPosition, SysPermissionSet, SysPositionPermissionSet, SysUserPermissionSet, - ...(opts.ledger === false ? [] : [SysMigration]), - ], - } as any); - await engine.syncSchemas(); - engines.push(engine); - - const services: Record = { - manifest: { register: vi.fn() }, - objectql: engine, - metadata: { - get: async (type: string, name: string) => - type === 'permission' - ? ([...defaultPermissionSets, QA_ADMIN] as Array<{ name?: string }>).find((s) => s?.name === name) ?? null - : engine.getSchema(name) ?? null, - list: async () => [...defaultPermissionSets, QA_ADMIN], - }, - ...(posture === 'isolated' - ? { 'org-scoping': { name: 'com.objectstack.org-scoping' }, tenancy: { posture: 'isolated' } } - : {}), - }; - const hooks = new Map Promise>>(); - const pluginLogger = newLogger(); - const ctx: any = { - logger: pluginLogger, - hook: (event: string, handler: () => Promise) => { - if (!hooks.has(event)) hooks.set(event, []); - hooks.get(event)!.push(handler); - }, - registerService: vi.fn(), - getService: (name: string) => { - if (!(name in services)) throw new Error(`service not registered: ${name}`); - return services[name]; - }, - }; - const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' }); - await plugin.init(ctx); - await plugin.start(ctx); - vi.spyOn((engine as any).logger, 'warn').mockImplementation(() => undefined); - - for (const [id, slug] of [[ORG, 'eq'], [OTHER_ORG, 'other']]) { - await engine.insert('sys_organization', { id, name: id, slug }, { context: SYS } as any); - } - // The catalog definitions, where a package manifest's `permissions` put them - // — registered once the organizations exist, so the plugin's - // organization-creation catalog seeding (not this suite's subject) seeds - // exactly what it seeds in the S4a equivalence world. - // Copies: the registry stamps its item in place, and these are shared module objects. - for (const ps of [...defaultPermissionSets, QA_ADMIN, OWN_SET, OTHER_SET]) { - engine.registry.registerItem('permission', structuredClone(ps) as any, 'name' as any, CATALOG_PACKAGE); - } - // An existing database: the platform-bucket rows an earlier release seeded. - await seedLegacyPlatformBucket(engine, defaultPermissionSets); - const seeded = await bootstrapPlatformAdmin(engine, defaultPermissionSets); - expect(seeded.adminPromoted).toBe(false); // no user yet — the golden world promotes after its users exist - - const catalog = createSecurityCatalogReader({ registry: engine.registry, metadata: services.metadata as any }); - return { engine, catalog, hooks, pluginLogger }; -} - -const orgCtx = (organizationId: string) => ({ isSystem: true, tenantId: organizationId }); - -async function insertSet(engine: Engine, row: Record, organizationId?: string): Promise { - await engine.insert('sys_permission_set', row, { context: organizationId ? orgCtx(organizationId) : SYS } as any); -} - -async function insertUser(engine: Engine, id: string, email: string, createdAt: string): Promise { - await engine.insert('sys_user', { id, email, name: id, created_at: createdAt, email_verified: true }, { context: SYS } as any); - await engine.insert( - 'sys_account', { id: `acc_${id}`, user_id: id, account_id: email, provider_id: 'credential' }, { context: SYS } as any, - ); -} - -/** Write grants the way they were stored before the name column existed: id only, name NULL. */ -async function insertUnnamedGrants(engine: Engine, rows: Array>): Promise { - // [ADR-0131 D4] The engine refuses a grant naming nothing (`permission_set` - // is required), so the shape a grant written before the column carries is - // written beneath it, through the driver's own connection. - const driver: any = (engine as any).getDriver('sys_user_permission_set'); - for (const row of rows) { - await driver.knex('sys_user_permission_set').insert({ permission_set: null, ...row }); - } -} - -/** - * Turn every grant into what a grant written before the name column carries: - * the id of the set row it names (its own organization's copy, else the - * organization-less one) and NO name. Grants are written by name today - * (ADR-0131 D4), so the legacy id is put back beneath the engine. - */ -async function clearAllNames(engine: Engine): Promise { - const knex = ((engine as any).getDriver('sys_user_permission_set') as any).knex; - const sets = await knex('sys_permission_set').select('id', 'name', 'organization_id'); - for (const g of await knex('sys_user_permission_set').select('id', 'permission_set', 'permission_set_id', 'organization_id')) { - const named = sets.filter((r: any) => r.name === g.permission_set); - const row = named.find((r: any) => (r.organization_id ?? null) === (g.organization_id ?? null) && r.organization_id) - ?? named.find((r: any) => !r.organization_id); - await knex('sys_user_permission_set').where({ id: g.id }) - .update({ permission_set: null, permission_set_id: g.permission_set_id ?? row?.id ?? null }); - } -} - -async function grants(engine: Engine): Promise>> { - const rows = (await engine.find('sys_user_permission_set', { - fields: ['id', 'user_id', 'permission_set_id', 'permission_set', 'organization_id'], - orderBy: [{ field: 'id', order: 'asc' }], - context: SYS, - })) as any[]; - return rows; -} - -async function grant(engine: Engine, id: string): Promise | null> { - return (await engine.findOne('sys_user_permission_set', { where: { id }, context: SYS })) as any; -} - -async function ledgerRows(engine: Engine): Promise>> { - return (await engine.find(DATA_MIGRATION_FLAG_OBJECT, { - where: { id: GRANT_SET_NAME_BACKFILL_MIGRATION_ID }, - context: SYS, - })) as any[]; -} - -/** Every string a logger mock was handed — message and meta — for disclosure checks. */ -const loggedText = (logger: ReturnType): string => - JSON.stringify([...logger.info.mock.calls, ...logger.warn.mock.calls, ...logger.error.mock.calls]); - -// --------------------------------------------------------------------------- -// The four principals (the grant-equivalence world) -// --------------------------------------------------------------------------- - -const sortDeep = (value: unknown): unknown => { - if (Array.isArray(value)) return value.map(sortDeep).sort((a, b) => JSON.stringify(a).localeCompare(JSON.stringify(b))); - if (value && typeof value === 'object') { - return Object.fromEntries(Object.keys(value as object).sort().map((k) => [k, sortDeep((value as any)[k])])); - } - return value; -}; - -/** - * The platform administrator, organization administrator and member written by - * the REAL writers, as the S4a equivalence suite writes them; the organization's - * own catalog copies in the walled posture. - */ -async function seedPrincipals(world: World, posture: Posture): Promise { - const { engine } = world; - await insertUser(engine, 'usr_admin', 'admin@eq.example', '2025-01-01T00:00:00.000Z'); - await insertUser(engine, 'usr_orgadmin', 'orgadmin@eq.example', '2025-02-01T00:00:00.000Z'); - await insertUser(engine, 'usr_member', 'member@eq.example', '2025-03-01T00:00:00.000Z'); - const promoted = await bootstrapPlatformAdmin(engine, defaultPermissionSets); - expect(promoted.adminPromoted).toBe(posture === 'single'); - - if (posture === 'isolated') { - for (const name of ['organization_admin', 'organization_admin_no_bypass', 'viewer_readonly']) { - const [bucket] = (await engine.find('sys_permission_set', { where: { name }, context: SYS })) as any[]; - const { id: _id, created_at: _c, updated_at: _u, ...rest } = bucket; - await insertSet(engine, { ...rest, id: `ps_${name}_${ORG}`, organization_id: ORG }, ORG); - } - } - - await engine.insert('sys_member', [ - { id: 'm_owner', user_id: 'usr_orgadmin', organization_id: ORG, role: 'owner', created_at: '2025-02-01T00:00:00.000Z' }, - { id: 'm_member', user_id: 'usr_member', organization_id: ORG, role: 'member', created_at: '2025-03-01T00:00:00.000Z' }, - ], { context: orgCtx(ORG) } as any); - const reconciled = await reconcileOrgAdminGrant(engine, 'usr_orgadmin', ORG, { posture }); - expect(reconciled.action).toBe('granted'); - - const [viewer] = (await engine.find('sys_permission_set', { - where: { name: 'viewer_readonly', ...(posture === 'isolated' ? { organization_id: ORG } : {}) }, - context: SYS, - })) as any[]; - await engine.insert( - 'sys_user_permission_set', - // The shape a grant written before this stage carries: its legacy id - // beside the name, which `clearAllNames` then takes away. - { user_id: 'usr_member', permission_set_id: viewer.id, permission_set: viewer.name }, - { context: { userId: 'usr_orgadmin', positions: [], permissions: ['qa_admin'], tenantId: ORG } } as any, - ); -} - -async function grantsByPrincipal(engine: Engine): Promise> { - const resolve = (userId: string) => resolveUserAuthzGrants(engine, userId, { tenantId: ORG }); - const admin = await resolve('usr_admin'); - const orgAdmin = await resolve('usr_orgadmin'); - const member = await resolve('usr_member'); - const agentCtx = assembleExecutionContext({ - authz: { ...orgAdmin, userId: 'usr_orgadmin', tenantId: ORG } as any, - oauth: { - userId: 'usr_orgadmin', - scopes: ['data:read', 'actions:execute'], - clientId: 'agent_client_eq', - scopePermissions: ['mcp_agent_data_read'], - delegatesActions: true, - } as any, - localization: undefined, - requestLocale: undefined, - accessToken: undefined, - authGate: undefined, - }); - const agent = { - principalKind: (agentCtx as any)?.principalKind, - positions: (agentCtx as any)?.positions, - permissions: (agentCtx as any)?.permissions, - systemPermissions: (agentCtx as any)?.systemPermissions, - onBehalfOf: (agentCtx as any)?.onBehalfOf, - }; - return sortDeep({ platformAdmin: admin, organizationAdmin: orgAdmin, member, agent }) as Record; -} - -// --------------------------------------------------------------------------- -// Pins -// --------------------------------------------------------------------------- - -describe('[ADR-0131 D4] grant name backfill — every name agrees with the id and resolves through the catalog read', () => { - for (const posture of ['single', 'isolated'] as const) { - it(`${posture}: each unnamed grant gets the name of the set row its id points at, verified through the catalog read`, async () => { - const world = await boot(posture); - const { engine, catalog } = world; - await seedPrincipals(world, posture); - // An organization's grant pointing at the organization-less platform row, - // the shape grants written before the per-organization catalog carry. - const [bucketViewer] = (await engine.find('sys_permission_set', { - where: { name: 'viewer_readonly', organization_id: null }, context: SYS, - })) as any[]; - await insertUnnamedGrants(engine, [ - { id: 'g_bucket', user_id: 'usr_orgadmin', permission_set_id: bucketViewer.id, organization_id: ORG }, - ]); - await clearAllNames(engine); - const before = await grants(engine); - expect(before.length).toBe(posture === 'single' ? 4 : 3); - expect(before.every((g) => g.permission_set === null)).toBe(true); - - const logger = newLogger(); - const outcome = await runOneTimeGrantPermissionSetNameBackfill(engine as any, { catalog, logger }); - expect(outcome.status).toBe('ran'); - expect(outcome.backfill?.named.length).toBe(before.length); - - for (const g of await grants(engine)) { - const [set] = (await engine.find('sys_permission_set', { where: { id: g.permission_set_id }, context: SYS })) as any[]; - expect(g.permission_set, g.id).toBe(set.name); - const entry = await catalog.resolve('permission', g.permission_set); - expect(entry?.name, g.id).toBe(g.permission_set); - } - expect(logger.error).not.toHaveBeenCalled(); - }); - } -}); - -describe('[ADR-0131 D4] grant name backfill — every principal’s grants are back once the names are', () => { - for (const posture of ['single', 'isolated'] as const) { - it(`${posture}: an unnamed grant confers nothing through the resolver; after the backfill every principal resolves to the golden`, async () => { - const world = await boot(posture); - await seedPrincipals(world, posture); - await clearAllNames(world.engine); - - // The resolver reads a grant's set by its name (ADR-0131 C2 stage S5a), - // so a grant the backfill has not named yet confers nothing: what is - // left is what `everyone` names, the deployment's baseline. - const unnamed = (await grantsByPrincipal(world.engine)) as Record; - expect(unnamed.member.permissions).not.toContain('viewer_readonly'); - expect(unnamed.organizationAdmin.permissions).toEqual(['member_default']); - expect(unnamed.organizationAdmin.posture).toBe('MEMBER'); - if (posture === 'single') { - // The promoted first user's grant row is the anchor; walled, the - // declared owner is, and no grant row carries it. - expect(unnamed.platformAdmin.permissions).not.toContain('admin_full_access'); - expect(unnamed.platformAdmin.posture).not.toBe('PLATFORM_ADMIN'); - } - const outcome = await runOneTimeGrantPermissionSetNameBackfill(world.engine as any, { - catalog: world.catalog, logger: newLogger(), - }); - expect(outcome.status).toBe('ran'); - expect((await grants(world.engine)).every((g) => typeof g.permission_set === 'string')).toBe(true); - expect(await grantsByPrincipal(world.engine)).toEqual(GOLDEN[posture]); - }); - } -}); - -describe('[ADR-0131 D4] grant name backfill — report, never guess', () => { - it('an id that names no set row stays unnamed and is reported by count and grant id; the verdict is recorded', async () => { - const { engine, catalog } = await boot('single'); - await insertUser(engine, 'usr_member', 'member@eq.example', '2025-03-01T00:00:00.000Z'); - await insertUnnamedGrants(engine, [{ id: 'g_gone', user_id: 'usr_member', permission_set_id: 'ps_gone' }]); - - const logger = newLogger(); - const outcome = await runOneTimeGrantPermissionSetNameBackfill(engine as any, { catalog, logger }); - - expect((await grant(engine, 'g_gone'))?.permission_set).toBeNull(); - expect(outcome.backfill?.dangling).toEqual(['g_gone']); - const [message, meta] = logger.warn.mock.calls.find(([m]) => String(m).includes('names no sys_permission_set row'))!; - expect(String(message)).toMatch(/^\[security\] 1 sys_user_permission_set grant\(s\)/); - expect(meta).toMatchObject({ count: 1, grants: { ids: ['g_gone'] } }); - expect(outcome.status).toBe('ran'); - expect(await ledgerRows(engine)).toHaveLength(1); - }); - - it('a catalog read that answers nothing: no name is written, the refusal is loud, and the verdict is not recorded', async () => { - const world = await boot('single'); - await seedPrincipals(world, 'single'); - await clearAllNames(world.engine); - const silent: SecurityCatalogReader = { resolve: async () => undefined, list: async () => [] }; - - const logger = newLogger(); - const outcome = await runOneTimeGrantPermissionSetNameBackfill(world.engine as any, { catalog: silent, logger }); - - const after = await grants(world.engine); - expect(after.length).toBe(3); - expect(after.every((g) => g.permission_set === null)).toBe(true); - expect(outcome.backfill?.named).toEqual([]); - expect([...(outcome.backfill?.unresolved ?? [])].sort()).toEqual(after.map((g) => g.id).sort()); - expect(logger.error).toHaveBeenCalledTimes(1); - const [message, , meta] = logger.error.mock.calls[0]; - expect(String(message)).toMatch(/^\[security\] 3 sys_user_permission_set grant\(s\) were NOT given a permission_set/); - expect(meta).toMatchObject({ count: 3 }); - expect(outcome.status).toBe('undecided'); - expect(await ledgerRows(world.engine)).toHaveLength(0); - }); - - it('a name the catalog does not resolve YET leaves the verdict unrecorded; once it is registered, the next pass names it', async () => { - const { engine, catalog } = await boot('single'); - await insertUser(engine, 'usr_member', 'member@eq.example', '2025-03-01T00:00:00.000Z'); - await insertSet(engine, { id: 'ps_late', name: 'late_reviewer', label: 'Late' }); - await insertUnnamedGrants(engine, [{ id: 'g_late', user_id: 'usr_member', permission_set_id: 'ps_late' }]); - - const first = await runOneTimeGrantPermissionSetNameBackfill(engine as any, { catalog, logger: newLogger() }); - expect(first.status).toBe('undecided'); - expect(first.backfill?.unresolved).toEqual(['g_late']); - expect((await grant(engine, 'g_late'))?.permission_set).toBeNull(); - expect(await ledgerRows(engine)).toHaveLength(0); - - // A package registering the definition after that pass — a later boot's catalog. - engine.registry.registerItem('permission', { name: 'late_reviewer', label: 'Late', objects: {} } as any, 'name' as any, CATALOG_PACKAGE); - const second = await runOneTimeGrantPermissionSetNameBackfill(engine as any, { catalog, logger: newLogger() }); - expect(second.status).toBe('ran'); - expect((await grant(engine, 'g_late'))?.permission_set).toBe('late_reviewer'); - expect(await ledgerRows(engine)).toHaveLength(1); - }); - - it('a name write that does not land is reported at error and leaves the verdict unrecorded', async () => { - const world = await boot('single'); - await seedPrincipals(world, 'single'); - await clearAllNames(world.engine); - const realUpdate = world.engine.update.bind(world.engine); - vi.spyOn(world.engine, 'update').mockImplementation(async (object: string, data: any, options?: any) => { - if (object === 'sys_user_permission_set') throw new Error('disk full'); - return realUpdate(object, data, options); - }); - - const logger = newLogger(); - const outcome = await runOneTimeGrantPermissionSetNameBackfill(world.engine as any, { catalog: world.catalog, logger }); - - expect(outcome.status).toBe('undecided'); - expect(outcome.backfill?.failed.length).toBe(3); - expect(logger.error).toHaveBeenCalledTimes(1); - expect(String(logger.error.mock.calls[0][0])).toMatch(/name write\(s\) did NOT land/); - expect(await ledgerRows(world.engine)).toHaveLength(0); - }); -}); - -describe('[ADR-0131 D4] grant name backfill — a name never crosses organizations', () => { - for (const posture of ['single', 'isolated'] as const) { - it(`${posture}: a grant whose id names another organization's set row keeps no name, and nothing of that organization is logged`, async () => { - const { engine, catalog } = await boot(posture); - await insertUser(engine, 'usr_member', 'member@eq.example', '2025-03-01T00:00:00.000Z'); - await insertSet(engine, { id: 'ps_other', name: OTHER_SET.name, label: 'Auditor' }, OTHER_ORG); - await insertSet(engine, { id: 'ps_own', name: OWN_SET.name, label: 'Reviewer' }, ORG); - const [bucketViewer] = (await engine.find('sys_permission_set', { - where: { name: 'viewer_readonly', organization_id: null }, context: SYS, - })) as any[]; - await insertUnnamedGrants(engine, [ - // The organization's grant on the other organization's set. - { id: 'g_cross', user_id: 'usr_member', permission_set_id: 'ps_other', organization_id: ORG }, - // An organization-less grant (it applies in every organization) on an organization's set. - { id: 'g_global_cross', user_id: 'usr_member', permission_set_id: 'ps_other' }, - { id: 'g_global_own', user_id: 'usr_member', permission_set_id: 'ps_own' }, - // Controls inside the wall: the organization's own set, and the organization-less platform row. - { id: 'g_own', user_id: 'usr_member', permission_set_id: 'ps_own', organization_id: ORG }, - { id: 'g_bucket', user_id: 'usr_member', permission_set_id: bucketViewer.id, organization_id: ORG }, - { id: 'g_global_bucket', user_id: 'usr_member', permission_set_id: bucketViewer.id }, - ]); - - const logger = newLogger(); - const outcome = await runOneTimeGrantPermissionSetNameBackfill(engine as any, { catalog, logger }); - - expect(outcome.backfill?.crossOrganization).toEqual(['g_cross', 'g_global_cross', 'g_global_own']); - for (const id of ['g_cross', 'g_global_cross', 'g_global_own']) { - expect((await grant(engine, id))?.permission_set, id).toBeNull(); - } - expect((await grant(engine, 'g_own'))?.permission_set).toBe(OWN_SET.name); - expect((await grant(engine, 'g_bucket'))?.permission_set).toBe('viewer_readonly'); - expect((await grant(engine, 'g_global_bucket'))?.permission_set).toBe('viewer_readonly'); - - const [, meta] = logger.warn.mock.calls.find(([m]) => String(m).includes('outside the grant'))!; - expect(meta).toMatchObject({ count: 3, grants: { ids: ['g_cross', 'g_global_cross', 'g_global_own'] } }); - const text = loggedText(logger); - expect(text).not.toContain(OTHER_SET.name); - expect(text).not.toContain(OTHER_ORG); - expect(text).not.toContain('ps_other'); - // Decided: nothing a later pass could name. - expect(outcome.status).toBe('ran'); - }); - } -}); - -describe('[ADR-0131 D4] grant name backfill — once, and remembered in sys_migration', () => { - it('a second pass writes nothing, and the ledger row is written once', async () => { - const world = await boot('single'); - await seedPrincipals(world, 'single'); - await clearAllNames(world.engine); - - const first = await runOneTimeGrantPermissionSetNameBackfill(world.engine as any, { - catalog: world.catalog, logger: newLogger(), - }); - expect(first.status).toBe('ran'); - const [row] = await ledgerRows(world.engine); - expect(row).toMatchObject({ id: GRANT_SET_NAME_BACKFILL_MIGRATION_ID, blocking: 0 }); - expect(row.verified_at ?? null).toBeNull(); - expect(JSON.parse(row.details)).toEqual({ unnamed: 3, named: 3, dangling: 0, crossOrganization: 0 }); - - const update = vi.spyOn(world.engine, 'update'); - const insert = vi.spyOn(world.engine, 'insert'); - const find = vi.spyOn(world.engine, 'find'); - const second = await runOneTimeGrantPermissionSetNameBackfill(world.engine as any, { - catalog: world.catalog, logger: newLogger(), - }); - expect(second).toEqual({ status: 'already-run', ledger: 'recorded' }); - expect(update).not.toHaveBeenCalled(); - expect(insert).not.toHaveBeenCalled(); - expect(find).not.toHaveBeenCalled(); - expect(await ledgerRows(world.engine)).toHaveLength(1); - }); - - it('with no ledger on the kernel the pass still names, says it cannot remember, and a second pass renames nothing', async () => { - const world = await boot('single', { ledger: false }); - await seedPrincipals(world, 'single'); - await clearAllNames(world.engine); - - const logger = newLogger(); - const first = await runOneTimeGrantPermissionSetNameBackfill(world.engine as any, { catalog: world.catalog, logger }); - expect(first.status).toBe('ran-unrecorded'); - expect(first.ledger).toBe('unavailable'); - expect(first.backfill?.named.length).toBe(3); - expect(logger.warn.mock.calls.some(([m]) => String(m).includes('cannot record it'))).toBe(true); - - const update = vi.spyOn(world.engine, 'update'); - const second = await runOneTimeGrantPermissionSetNameBackfill(world.engine as any, { - catalog: world.catalog, logger: newLogger(), - }); - expect(second.backfill?.unnamed).toBe(0); - expect(update).not.toHaveBeenCalled(); - }); -}); - -describe('[ADR-0131 D4] grant name backfill — the write path', () => { - it('the backfill writes the name itself: nothing on the write path derives it from the id', async () => { - const world = await boot('isolated'); - await seedPrincipals(world, 'isolated'); - await clearAllNames(world.engine); - - const outcome = await runOneTimeGrantPermissionSetNameBackfill(world.engine as any, { - catalog: world.catalog, logger: newLogger(), - }); - expect(outcome.status).toBe('ran'); - for (const g of await grants(world.engine)) { - const [set] = (await world.engine.find('sys_permission_set', { where: { id: g.permission_set_id }, context: SYS })) as any[]; - expect(g.permission_set, g.id).toBe(set.name); - } - }); -}); - -describe('[ADR-0131 D4] grant name backfill — on a real kernel boot', () => { - it('a permission set a kernel:ready handler registers AFTER SecurityPlugin’s is named on the same boot, and the verdict is recorded', async () => { - const engine = new ObjectQL(); - engine.registerDriver( - new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }), - true, - ); - await engine.init(); - engine.registerApp({ - id: CATALOG_PACKAGE, - name: 'Grant name backfill', - version: '1.0.0', - type: 'plugin', - scope: 'system', - objects: [ - SysUser, SysAccount, SysMember, SysOrganization, - SysPosition, SysUserPosition, SysPermissionSet, SysPositionPermissionSet, SysUserPermissionSet, SysMigration, - ], - } as any); - await engine.syncSchemas(); - engines.push(engine); - vi.spyOn((engine as any).logger, 'warn').mockImplementation(() => undefined); - for (const ps of defaultPermissionSets) { - engine.registry.registerItem('permission', structuredClone(ps) as any, 'name' as any, CATALOG_PACKAGE); - } - // Stored before this boot: the set row, and a grant written before the - // name column existed. - await insertSet(engine, { id: 'ps_late', name: 'late_reviewer', label: 'Late' }); - await insertUser(engine, 'usr_member', 'member@eq.example', '2025-03-01T00:00:00.000Z'); - await insertUnnamedGrants(engine, [{ id: 'g_late', user_id: 'usr_member', permission_set_id: 'ps_late' }]); - expect((await grant(engine, 'g_late'))?.permission_set).toBeNull(); - - const metadata = { - get: async (type: string, name: string) => - type === 'permission' - ? ([...defaultPermissionSets] as Array<{ name?: string }>).find((s) => s?.name === name) ?? null - : engine.getSchema(name) ?? null, - list: async () => [...defaultPermissionSets], - }; - const kernel = new LiteKernel({ logger: { level: 'silent' } }); - // The engine's stand-in: SecurityPlugin depends on the engine plugin by name. - kernel.use({ - name: 'com.objectstack.engine.objectql', - init: async (ctx: any) => { - ctx.registerService('objectql', engine); - ctx.registerService('metadata', metadata); - ctx.registerService('manifest', { register: () => undefined }); - }, - start: async () => undefined, - } as any); - kernel.use(new SecurityPlugin({ fallbackPermissionSet: 'member_default' })); - // The late provider: its `kernel:ready` handler is registered after - // SecurityPlugin's own, so it runs after the platform bootstrap. - kernel.use({ - name: 'com.objectstack.qa.late-catalog', - dependencies: ['com.objectstack.security'], - init: async () => undefined, - start: async (ctx: any) => { - ctx.hook('kernel:ready', async () => { - engine.registry.registerItem( - 'permission', { name: 'late_reviewer', label: 'Late', objects: {} } as any, 'name' as any, CATALOG_PACKAGE, - ); - }); - }, - } as any); - try { - await kernel.bootstrap(); - expect((await grant(engine, 'g_late'))?.permission_set).toBe('late_reviewer'); - expect(await ledgerRows(engine)).toHaveLength(1); - } finally { - await kernel.shutdown(); - } - }); -}); - -describe('[ADR-0131 D4] grant name backfill — boot wiring', () => { - it('SecurityPlugin runs it at kernel:bootstrapped, through the catalog read over the engine registry', async () => { - const world = await boot('single'); - await seedPrincipals(world, 'single'); - await clearAllNames(world.engine); - - // Two `kernel:bootstrapped` handlers, in registration order: this - // backfill, then the catalog reference report, which - // `catalog-reference-report.test.ts` drives. Only the first runs here. - const handlers = world.hooks.get('kernel:bootstrapped') ?? []; - expect(handlers).toHaveLength(2); - const errorsBefore = world.pluginLogger.error.mock.calls.length; - const warningsBefore = world.pluginLogger.warn.mock.calls.length; - await handlers[0](); - - expect((await grants(world.engine)).every((g) => typeof g.permission_set === 'string')).toBe(true); - expect(await ledgerRows(world.engine)).toHaveLength(1); - expect(world.pluginLogger.error.mock.calls.length).toBe(errorsBefore); - expect(world.pluginLogger.warn.mock.calls.length).toBe(warningsBefore); - }); -}); - -/** - * The resolver's envelope per principal — recorded from the tree BEFORE the - * name column existed (the S4a grant-equivalence golden). The backfill brings - * every principal back to it once the resolver reads the name (stage S5a). - * One recorded move since, the same in every principal: `member_default` through - * `everyone`. `init()` declares the deployment's baseline on that anchor - * (`builtin-positions.declaration-phase.test.ts`), and this harness runs - * `init()` and `start()` but fires no `kernel:ready`, where a started kernel - * has always derived it (`builtin-positions.boot.test.ts`'s grant golden). - */ -const GOLDEN: Record = { - single: { - agent: { - onBehalfOf: { principalKind: 'human', userId: 'usr_orgadmin' }, - permissions: ['mcp_agent_data_read'], - positions: [], - principalKind: 'agent', - systemPermissions: ['manage_mailboxes', 'manage_org_users', 'setup.access', 'setup.write'], - }, - member: { - accessible_org_ids: ['org_eq'], - email: 'member@eq.example', - org_user_ids: ['usr_member', 'usr_orgadmin'], - permissions: ['member_default', 'viewer_readonly'], - positions: ['everyone', 'org_member'], - posture: 'MEMBER', - systemPermissions: [], - }, - organizationAdmin: { - accessible_org_ids: ['org_eq'], - email: 'orgadmin@eq.example', - org_user_ids: ['usr_member', 'usr_orgadmin'], - permissions: ['member_default', 'organization_admin_no_bypass'], - positions: ['everyone', 'org_owner'], - posture: 'TENANT_ADMIN', - systemPermissions: ['manage_mailboxes', 'manage_org_users', 'setup.access', 'setup.write'], - }, - platformAdmin: { - accessible_org_ids: [], - email: 'admin@eq.example', - org_user_ids: ['usr_admin', 'usr_member', 'usr_orgadmin'], - permissions: ['admin_full_access', 'member_default'], - positions: ['everyone', 'platform_admin'], - posture: 'PLATFORM_ADMIN', - systemPermissions: [ - 'manage_mailboxes', 'manage_metadata', 'manage_platform_settings', 'manage_sharing', 'manage_users', - 'setup.access', 'setup.write', 'studio.access', 'view_all_audit_log', - ], - }, - }, - isolated: { - agent: { - onBehalfOf: { principalKind: 'human', userId: 'usr_orgadmin' }, - permissions: ['mcp_agent_data_read'], - positions: [], - principalKind: 'agent', - systemPermissions: ['manage_mailboxes', 'manage_org_users', 'setup.access', 'setup.write'], - }, - member: { - accessible_org_ids: ['org_eq'], - email: 'member@eq.example', - org_user_ids: ['usr_member', 'usr_orgadmin'], - permissions: ['member_default', 'viewer_readonly'], - positions: ['everyone', 'org_member'], - posture: 'MEMBER', - systemPermissions: [], - }, - organizationAdmin: { - accessible_org_ids: ['org_eq'], - email: 'orgadmin@eq.example', - org_user_ids: ['usr_member', 'usr_orgadmin'], - permissions: ['member_default', 'organization_admin'], - positions: ['everyone', 'org_owner'], - posture: 'TENANT_ADMIN', - systemPermissions: ['manage_mailboxes', 'manage_org_users', 'setup.access', 'setup.write'], - }, - platformAdmin: { - accessible_org_ids: [], - email: 'admin@eq.example', - org_user_ids: ['usr_admin', 'usr_member', 'usr_orgadmin'], - permissions: ['admin_full_access', 'member_default'], - positions: ['everyone', 'platform_admin'], - posture: 'PLATFORM_ADMIN', - systemPermissions: [ - 'manage_mailboxes', 'manage_metadata', 'manage_platform_settings', 'manage_sharing', 'manage_users', - 'setup.access', 'setup.write', 'studio.access', 'view_all_audit_log', - ], - }, - }, -}; diff --git a/packages/plugins/plugin-security/src/grant-permission-set-name-backfill.ts b/packages/plugins/plugin-security/src/grant-permission-set-name-backfill.ts deleted file mode 100644 index 6391af139f7..00000000000 --- a/packages/plugins/plugin-security/src/grant-permission-set-name-backfill.ts +++ /dev/null @@ -1,575 +0,0 @@ -// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. - -/** - * The one-time backfill of `sys_user_permission_set.permission_set` — the - * grant's permission set BY NAME ([ADR-0131] D4) — on the grants written - * before the column existed. - * - * ## What it does - * - * Every grant written since the column landed names its set: the platform's - * writers write both columns, and the engine hooks in - * `grant-permission-set-name.ts` derive the name from `permission_set_id` for - * every other writer. Grants written before that carry `NULL`. This pass gives - * each of them the name its own id already points at, so every grant obeys the - * column's one rule — system-written, agreeing with the id — before any reader - * is switched onto the name. It changes no reader, no id and no grant's - * meaning, and it deletes nothing (deletions are ADR-0131 C7's). - * - * For each grant with no name: - * - * 1. **The set row its id names, read inside the grant's own wall.** The read - * runs under the grant's organization (`seedCtx`), so it routes through the - * driver's tenant scope — the one governed spelling of the wall, the same - * read the hooks and the authorization resolver's position read make — - * and sees that organization's rows and the organization-less ones. The - * grant rule the resolver applies to every stored grant row is then asked - * of the set row: an organization-less row applies everywhere, a row of an - * organization applies only to a grant of that same organization. So an - * organization-less grant (one that applies in every organization) may - * name only an organization-less set. - * 2. **The name is verified through the security catalog read** (S1's - * `createSecurityCatalogReader`, `@objectstack/core`) before it is written: - * a name the catalog does not resolve is not written. - * 3. **Written as a system update of the name alone**, under the grant's own - * organization. The update hook re-derives the name from the row's stored - * id through its own walled read and refuses a disagreement, so a grant - * whose id moved between this pass's read and its write is refused, not - * mislabelled. The hook stamps a name by itself only on a write that - * carries `permission_set_id`; this write carries the verified name and no - * id, so nothing keyed on the id column (the last-administrator guard's - * standing keys among them) re-judges the grant. ⛔ The wall in step 1 is - * this pass's own: for a SYSTEM write whose stored id the hook cannot - * resolve inside the writer's wall, the hook stands down and lets the - * value land, so it would not stop a name carried across organizations. - * - * ## Report, never guess - * - * A grant this pass cannot name keeps `NULL` and is reported, by count and - * grant row id. Nothing about another organization is logged: - * - * - **an id that names no set row** (`warn`) — it grants nothing today either; - * - **an id whose set row belongs to another organization** (`warn`) — its - * name is never written: a name carries no organization, so writing another - * organization's name would point the grant at whatever this organization - * calls by that name. The authorization resolver reads a grant's set by - * name, so such a grant confers nothing; - * - **a set row whose name the catalog read does not resolve** (`error`) — - * the grant points at a definition the catalog does not hold, so it is left - * unnamed and confers nothing until a later pass names it. - * - * Every grant left unnamed is also listed, by organization, at every boot by - * the catalog reference report (`catalog-reference-report.ts`) — including - * after this pass has recorded its verdict and no longer runs. - * - * ## Once, and remembered - * - * The pass runs until it has reached a verdict once, records that verdict in - * the deployment ledger `sys_migration`, and every later pass reads the record - * and does nothing — the shape of `plugin-auth`'s - * `membership-backfill-ledger.ts`. The row: its own id - * ({@link GRANT_SET_NAME_BACKFILL_MIGRATION_ID}), `verified_at: null` (no - * consumer gates on it, so it certifies nothing) and `blocking: 0` (it holds - * no consumer closed); `details` carries what the pass did. - * - * Recorded — nothing a later pass could change: every unnamed grant was named, - * or names no set row, or names another organization's set row. - * - * Not recorded — a later pass may still decide differently, so it gets its - * turn on the next boot: - * - * - a set row whose name the catalog read did not resolve: the catalog is - * filled by code, environment metadata and installed packages, and a - * definition missing now may be registered later; - * - a read that did not happen (the grant scan, a set row, the catalog), or a - * name write that did not land. - * - * Without a readable ledger the pass still runs — it only fills a `NULL` with - * the name the row's own id already points at, so running it twice writes - * nothing the first run did not — but its verdict cannot be remembered, and - * every boot scans again. This is where it departs from the membership - * backfill, which does not run without its ledger because a second run of - * that pass would decide again what the first one decided. - * - * ## When it runs - * - * At `kernel:bootstrapped`, from `SecurityPlugin.start`: after every - * `kernel:ready` handler has settled — the platform bootstrap that seeds the - * catalog rows, and the environment metadata hydrated before it — so the - * catalog read sees every code and environment definition this boot will - * register, including one a provider registers from a `kernel:ready` handler - * that runs after this plugin's own. A definition that arrives later still (a - * package installed into the running process) cannot turn into a recorded - * "missing": an unresolved name leaves the verdict unrecorded, and the next - * boot judges it again. - * - * ADR anchors: ADR-0131 D4 (references by name), D10 (the id column is dropped - * after a verified rewrite — not here), C7 (deletions — not here). - */ - -import { classifyFilterToken } from '@objectstack/spec/data'; -import { DATA_MIGRATION_FLAG_OBJECT, type DataMigrationFlag } from '@objectstack/spec/system'; -import type { SecurityCatalogReader } from '@objectstack/core'; -import { - GRANT_OBJECT, - GRANT_SET_ID_FIELD, - GRANT_SET_NAME_FIELD, - PERMISSION_SET_CATALOG_OBJECT, -} from './grant-permission-set-name.js'; -import { rowOrganizationId, seedCtx } from './organization-scope.js'; - -/** Ledger row id of the one-time grant name backfill. */ -export const GRANT_SET_NAME_BACKFILL_MIGRATION_ID = 'adr-0131-grant-permission-set-name-backfill'; - -/** Rows per page of the unnamed-grant scan. */ -const SCAN_PAGE_SIZE = 500; - -/** Grant row ids printed per category; the count is always complete. */ -const LOGGED_ID_LIMIT = 50; - -const SYSTEM_CTX = { isSystem: true } as const; - -/** The engine surface the backfill uses — ObjectQL satisfies it as it stands. */ -export interface GrantNameBackfillEngine { - getObject(name: string): unknown; - find(object: string, options: Record): Promise; - findOne(object: string, options: Record): Promise | null>; - insert(object: string, data: Record, options?: Record): Promise; - update(object: string, data: Record, options?: Record): Promise; -} - -/** The kernel logger, as this module uses it. */ -export interface GrantNameBackfillLogger { - info?: (message: string, meta?: Record) => void; - warn: (message: string, meta?: Record) => void; - /** The kernel logger's shape: message, cause, meta. */ - error?: (message: string, cause?: Error, meta?: Record) => void; -} - -/** Why a pass stopped before it reached a verdict. */ -export type GrantNameBackfillStop = - /** No grant object, or no permission-set catalog object, in this composition. */ - | 'objects-absent' - /** The unnamed-grant scan could not be read in full. */ - | 'scan-unreadable' - /** A set row could not be read. */ - | 'set-row-unreadable' - /** The security catalog read did not happen. */ - | 'catalog-unreadable'; - -/** What one pass did. Every list holds grant row ids. */ -export interface GrantNameBackfillResult { - /** Unnamed grants the scan found. */ - unnamed: number; - /** Grants this pass named. */ - named: string[]; - /** Grants whose id names no set row. */ - dangling: string[]; - /** Grants whose id names another organization's set row. */ - crossOrganization: string[]; - /** Grants whose set row's name the catalog read did not resolve. */ - unresolved: string[]; - /** Grants whose name write did not land. */ - failed: string[]; - /** Set when the pass stopped before it judged every unnamed grant. */ - stopped?: GrantNameBackfillStop; -} - -export interface GrantNameBackfillDeps { - /** S1's security catalog read; every name is verified through it before it is written. */ - catalog: SecurityCatalogReader; - logger?: GrantNameBackfillLogger; -} - -/** What the set row a grant's id names comes to, inside the grant's wall. */ -type SetVerdict = - | { kind: 'name'; name: string } - | { kind: 'dangling' } - | { kind: 'cross-organization' } - | { kind: 'unresolved'; name: string }; - -/** Thrown inside the pass to stop it; never escapes the module. */ -class BackfillStop extends Error { - constructor(readonly stop: GrantNameBackfillStop, readonly reason: unknown) { - super(stop); - } -} - -/** The store a stop could not read, as the report names it. */ -const STOPPED_READING: Record = { - 'objects-absent': 'this composition', - 'scan-unreadable': GRANT_OBJECT, - 'set-row-unreadable': PERMISSION_SET_CATALOG_OBJECT, - 'catalog-unreadable': 'the security catalog', -}; - -const errorText = (e: unknown): string => (e instanceof Error ? e.message : String(e)); - -/** Whether a grant row's name column carries a name — `null` and a blank do not. */ -function carriesName(value: unknown): boolean { - return typeof value === 'string' && value.trim() !== ''; -} - -/** - * The id a `permission_set_id` value reads by, or `undefined` for a value that - * names no row: only a non-blank string or a finite number is an id, and a - * placeholder-shaped string (`{…}`) is never one — in `where` it would be - * resolved as a filter token rather than compared. - */ -function idKey(value: unknown): string | undefined { - let key: string | undefined; - if (typeof value === 'string') key = value; - else if (typeof value === 'number' && Number.isFinite(value)) key = String(value); - if (key === undefined || key.trim() === '') return undefined; - if (classifyFilterToken(key) !== null) return undefined; - return key; -} - -/** - * The grant rule the authorization resolver asks of every stored grant row, - * asked here of the set row a grant names: an organization-less row applies - * everywhere; a row of an organization applies only within that organization. - */ -function setRowAppliesToGrant(setOrganizationId: string | null, grantOrganizationId: string | null): boolean { - return setOrganizationId === null || setOrganizationId === grantOrganizationId; -} - -/** Read every unnamed grant in full, ordered by id, before anything is written. */ -async function scanUnnamedGrants(engine: GrantNameBackfillEngine): Promise[]> { - const byId = new Map>(); - // NULL is the pre-column shape; a blank is what a system write may have left - // beside an id that resolved nothing. Neither is a name. - for (const value of [null, '']) { - for (let offset = 0; ; offset += SCAN_PAGE_SIZE) { - let page: unknown; - try { - page = await engine.find(GRANT_OBJECT, { - where: { [GRANT_SET_NAME_FIELD]: value }, - fields: ['id', GRANT_SET_ID_FIELD, GRANT_SET_NAME_FIELD, 'organization_id'], - orderBy: [{ field: 'id', order: 'asc' }], - limit: SCAN_PAGE_SIZE, - offset, - context: SYSTEM_CTX, - }); - } catch (e) { - throw new BackfillStop('scan-unreadable', e); - } - const rows = Array.isArray(page) ? page : []; - for (const row of rows) { - const id = (row as Record | null)?.id; - if (typeof id === 'string' && id !== '' && !carriesName((row as Record)[GRANT_SET_NAME_FIELD])) { - byId.set(id, row as Record); - } - } - if (rows.length < SCAN_PAGE_SIZE) break; - } - } - // Code-unit order — the order the driver's id sort gives, independent of locale. - return [...byId.values()].sort((a, b) => (String(a.id) < String(b.id) ? -1 : String(a.id) > String(b.id) ? 1 : 0)); -} - -/** What the set row `setId` comes to for a grant of `grantOrganizationId`. */ -async function judgeSetRow( - engine: GrantNameBackfillEngine, - catalog: SecurityCatalogReader, - setId: string, - grantOrganizationId: string | null, - resolvedNames: Map, -): Promise { - let row: Record | null; - try { - row = await engine.findOne(PERMISSION_SET_CATALOG_OBJECT, { - where: { id: setId }, - fields: ['id', 'name', 'organization_id'], - context: seedCtx(grantOrganizationId ?? undefined), - }); - } catch (e) { - throw new BackfillStop('set-row-unreadable', e); - } - if (row && !setRowAppliesToGrant(rowOrganizationId(row), grantOrganizationId)) { - return { kind: 'cross-organization' }; - } - if (!row) { - // Nothing inside the wall. Whether the id names a row OUTSIDE it decides - // which report the grant lands in; only the row's existence is read. - let outside: Record | null; - try { - outside = await engine.findOne(PERMISSION_SET_CATALOG_OBJECT, { - where: { id: setId }, - fields: ['id'], - context: SYSTEM_CTX, - }); - } catch (e) { - throw new BackfillStop('set-row-unreadable', e); - } - return outside ? { kind: 'cross-organization' } : { kind: 'dangling' }; - } - - const name = row.name; - if (typeof name !== 'string' || name.trim() === '') return { kind: 'unresolved', name: String(name ?? '') }; - let resolves = resolvedNames.get(name); - if (resolves === undefined) { - try { - const entry = await catalog.resolve('permission', name); - resolves = entry !== undefined && entry.name === name; - } catch (e) { - throw new BackfillStop('catalog-unreadable', e); - } - resolvedNames.set(name, resolves); - } - return resolves ? { kind: 'name', name } : { kind: 'unresolved', name }; -} - -/** At most {@link LOGGED_ID_LIMIT} ids, and how many more there are. */ -function listed(ids: readonly string[]): { ids: string[]; more?: number } { - return ids.length > LOGGED_ID_LIMIT - ? { ids: ids.slice(0, LOGGED_ID_LIMIT), more: ids.length - LOGGED_ID_LIMIT } - : { ids: [...ids] }; -} - -function logError(logger: GrantNameBackfillLogger | undefined, message: string, meta: Record): void { - if (logger?.error) logger.error(message, undefined, meta); - else logger?.warn(message, meta); -} - -/** Report what one pass could not name — once per category, never once per row. */ -function reportPass(result: GrantNameBackfillResult, unresolvedNames: string[], logger?: GrantNameBackfillLogger): void { - if (result.named.length > 0) { - logger?.info?.( - `[security] ${result.named.length} of ${result.unnamed} unnamed ${GRANT_OBJECT} grant(s) now name their ` + - `permission set (ADR-0131 D4): each name was read from the set row its ${GRANT_SET_ID_FIELD} points at, ` + - `inside the grant's own organization, and resolved in the security catalog before it was written`, - { named: result.named.length, unnamed: result.unnamed }, - ); - } - if (result.dangling.length > 0) { - logger?.warn( - `[security] ${result.dangling.length} ${GRANT_OBJECT} grant(s) keep no ${GRANT_SET_NAME_FIELD}: their ` + - `${GRANT_SET_ID_FIELD} names no ${PERMISSION_SET_CATALOG_OBJECT} row, so they grant nothing today and ` + - `there is no name to give them. Nothing was deleted; review these grants and remove or re-point them.`, - { count: result.dangling.length, grants: listed(result.dangling) }, - ); - } - if (result.crossOrganization.length > 0) { - logger?.warn( - `[security] ${result.crossOrganization.length} ${GRANT_OBJECT} grant(s) keep no ${GRANT_SET_NAME_FIELD}: ` + - `their ${GRANT_SET_ID_FIELD} names a ${PERMISSION_SET_CATALOG_OBJECT} row outside the grant's own ` + - `organization (an organization-less grant may name only an organization-less set), and a name is never ` + - `carried across organizations. Nothing was changed; re-point each grant at a set of its own organization.`, - { count: result.crossOrganization.length, grants: listed(result.crossOrganization) }, - ); - } - if (result.unresolved.length > 0) { - logError( - logger, - `[security] ${result.unresolved.length} ${GRANT_OBJECT} grant(s) were NOT given a ${GRANT_SET_NAME_FIELD}: ` + - `the ${PERMISSION_SET_CATALOG_OBJECT} row each one points at carries a name the security catalog does not ` + - `resolve at this boot, so the name cannot be shown to name a definition. These grants confer nothing ` + - `until a later boot names them: the authorization resolver reads a grant's permission set by name, and ` + - `a grant with no name grants nothing. Fix: register the permission set definition (its package or ` + - `environment metadata), or re-point the grants; the backfill runs again on the next boot.`, - { count: result.unresolved.length, names: unresolvedNames, grants: listed(result.unresolved) }, - ); - } - if (result.failed.length > 0) { - logError( - logger, - `[security] ${result.failed.length} ${GRANT_OBJECT} name write(s) did NOT land, so those grants still ` + - `carry no ${GRANT_SET_NAME_FIELD} and confer nothing until a later boot names them, while every other ` + - `line of this backfill reads clean. The backfill runs again on the next boot; if it fails again, check ` + - `that ${GRANT_OBJECT} is writable by the system context.`, - { count: result.failed.length, grants: listed(result.failed) }, - ); - } -} - -/** - * One pass: name every unnamed grant whose name can be verified, report the - * rest. A read that did not happen stops the pass; the stop is returned in - * `stopped` and reported, never thrown. - */ -export async function backfillGrantPermissionSetNames( - engine: GrantNameBackfillEngine, - deps: GrantNameBackfillDeps, -): Promise { - const result: GrantNameBackfillResult = { - unnamed: 0, named: [], dangling: [], crossOrganization: [], unresolved: [], failed: [], - }; - const { catalog, logger } = deps; - if (!engine?.getObject?.(GRANT_OBJECT) || !engine.getObject(PERMISSION_SET_CATALOG_OBJECT)) { - result.stopped = 'objects-absent'; - return result; - } - - const unresolvedNames = new Set(); - try { - const grants = await scanUnnamedGrants(engine); - result.unnamed = grants.length; - const verdicts = new Map(); - const resolvedNames = new Map(); - - for (const grant of grants) { - const grantId = String(grant.id); - const setId = idKey(grant[GRANT_SET_ID_FIELD]); - if (setId === undefined) { - result.dangling.push(grantId); - continue; - } - const grantOrganizationId = rowOrganizationId(grant); - const memoKey = JSON.stringify([grantOrganizationId, setId]); - let verdict = verdicts.get(memoKey); - if (!verdict) { - verdict = await judgeSetRow(engine, catalog, setId, grantOrganizationId, resolvedNames); - verdicts.set(memoKey, verdict); - } - - if (verdict.kind === 'dangling') result.dangling.push(grantId); - else if (verdict.kind === 'cross-organization') result.crossOrganization.push(grantId); - else if (verdict.kind === 'unresolved') { - result.unresolved.push(grantId); - unresolvedNames.add(verdict.name); - } else { - try { - await engine.update( - GRANT_OBJECT, - { id: grantId, [GRANT_SET_NAME_FIELD]: verdict.name }, - { context: seedCtx(grantOrganizationId ?? undefined) }, - ); - result.named.push(grantId); - } catch { - result.failed.push(grantId); - } - } - } - } catch (e) { - if (!(e instanceof BackfillStop)) throw e; - result.stopped = e.stop; - logger?.warn( - `[security] the ${GRANT_OBJECT} name backfill stopped before it judged every unnamed grant: ` + - `${STOPPED_READING[e.stop]} could not be read, and an unread answer is never taken for "no such set". ` + - `No name was written past that point; the backfill runs again on the next boot.`, - { stop: e.stop, error: errorText(e.reason), named: result.named.length }, - ); - } - reportPass(result, [...unresolvedNames].sort(), logger); - return result; -} - -/** A pass's verdict is final when nothing a later pass could decide differently remains. */ -export function isRecordableGrantNameVerdict(result: GrantNameBackfillResult): boolean { - return result.stopped === undefined && result.unresolved.length === 0 && result.failed.length === 0; -} - -/** What the deployment ledger says about this backfill. */ -export type GrantNameLedgerReading = 'recorded' | 'absent' | 'unavailable' | 'unreadable'; - -/** Read the ledger row by its id. Never throws. */ -export async function readGrantNameBackfillLedger(engine: GrantNameBackfillEngine): Promise { - try { - if (!engine?.getObject?.(DATA_MIGRATION_FLAG_OBJECT)) return 'unavailable'; - const row = await engine.findOne(DATA_MIGRATION_FLAG_OBJECT, { - where: { id: GRANT_SET_NAME_BACKFILL_MIGRATION_ID }, - context: SYSTEM_CTX, - }); - return row?.id === GRANT_SET_NAME_BACKFILL_MIGRATION_ID ? 'recorded' : 'absent'; - } catch { - return 'unreadable'; - } -} - -/** The ledger row for a decided pass — pure. */ -export function buildGrantNameBackfillRecord(result: GrantNameBackfillResult, now: string): DataMigrationFlag { - return { - id: GRANT_SET_NAME_BACKFILL_MIGRATION_ID, - last_run_at: now, - applied_at: now, - verified_at: null, - blocking: 0, - details: JSON.stringify({ - unnamed: result.unnamed, - named: result.named.length, - dangling: result.dangling.length, - crossOrganization: result.crossOrganization.length, - }), - }; -} - -/** - * Insert the ledger row. THROWS on failure — the caller decides what a lost - * record means. - */ -async function persistGrantNameBackfillRecord(engine: GrantNameBackfillEngine, flag: DataMigrationFlag): Promise { - const now = flag.last_run_at; - await engine.insert(DATA_MIGRATION_FLAG_OBJECT, { ...flag, created_at: now, updated_at: now }, { context: SYSTEM_CTX }); -} - -export type GrantNameBackfillStatus = - /** The pass decided, and its record landed. */ - | 'ran' - /** The pass decided, but its record did not land (or there is no ledger to land it in). */ - | 'ran-unrecorded' - /** The pass left something a later pass may decide differently — retried on the next boot. */ - | 'undecided' - /** The ledger already records the verdict — nothing was scanned or written. */ - | 'already-run'; - -export interface OneTimeGrantNameBackfillResult { - status: GrantNameBackfillStatus; - ledger: GrantNameLedgerReading; - /** The pass's own summary, present whenever it ran. */ - backfill?: GrantNameBackfillResult; -} - -/** - * Run the grant name backfill unless the deployment ledger already records - * its verdict, and record the verdict when this pass reaches one. A read the - * pass could not make is reported and returned, never thrown; the boot hook - * that calls this still guards against anything unforeseen. - */ -export async function runOneTimeGrantPermissionSetNameBackfill( - engine: GrantNameBackfillEngine, - deps: GrantNameBackfillDeps, -): Promise { - const { logger } = deps; - const ledger = await readGrantNameBackfillLedger(engine); - if (ledger === 'recorded') return { status: 'already-run', ledger }; - - const backfill = await backfillGrantPermissionSetNames(engine, deps); - if (!isRecordableGrantNameVerdict(backfill)) return { status: 'undecided', ledger, backfill }; - - if (ledger !== 'absent') { - logger?.warn( - `[security] the ${GRANT_OBJECT} name backfill reached its verdict but cannot record it: the deployment ` + - `ledger ${DATA_MIGRATION_FLAG_OBJECT} is ${ledger === 'unavailable' ? 'not available on this kernel' : 'unreadable'}, ` + - `so every boot scans the grants again. Nothing is renamed twice. Compose PlatformObjectsPlugin (it ` + - `provisions the ledger) to let the verdict be remembered.`, - { ledger }, - ); - return { status: 'ran-unrecorded', ledger, backfill }; - } - - const flag = buildGrantNameBackfillRecord(backfill, new Date().toISOString()); - try { - await persistGrantNameBackfillRecord(engine, flag); - } catch (e) { - // A concurrent boot that landed the same id first has recorded it. - if ((await readGrantNameBackfillLedger(engine)) === 'recorded') return { status: 'ran', ledger, backfill }; - // At `error`: the pass's writes stand and every line reads clean, while the - // record that makes it one-time is absent. - logError( - logger, - `[security] the ${GRANT_OBJECT} name backfill ran, but recording it in ${DATA_MIGRATION_FLAG_OBJECT} failed ` + - `(${errorText(e)}). The next boot scans every grant again and re-reports what it cannot name. Fix: make ` + - `${DATA_MIGRATION_FLAG_OBJECT} writable on this deployment (it is provisioned by PlatformObjectsPlugin) ` + - `and verify with SELECT * FROM ${DATA_MIGRATION_FLAG_OBJECT} WHERE id = '${GRANT_SET_NAME_BACKFILL_MIGRATION_ID}'.`, - { error: errorText(e) }, - ); - return { status: 'ran-unrecorded', ledger, backfill }; - } - logger?.info?.( - `[security] the ${GRANT_OBJECT} name backfill is recorded in ${DATA_MIGRATION_FLAG_OBJECT} ` + - `(id '${GRANT_SET_NAME_BACKFILL_MIGRATION_ID}') — later boots will not run it again`, - { id: flag.id, details: flag.details }, - ); - return { status: 'ran', ledger, backfill }; -} diff --git a/packages/plugins/plugin-security/src/grant-permission-set-name.test.ts b/packages/plugins/plugin-security/src/grant-permission-set-name.test.ts index dfb7e55e27e..3da2f565814 100644 --- a/packages/plugins/plugin-security/src/grant-permission-set-name.test.ts +++ b/packages/plugins/plugin-security/src/grant-permission-set-name.test.ts @@ -2,10 +2,14 @@ /** * [ADR-0131 D4] A grant names its permission set BY NAME - * (`sys_user_permission_set.permission_set`), and nothing derives that name - * from `permission_set_id` any more (D10's first step): a non-system write - * naming a set the security catalog does not hold is refused, and a grant that - * carries no name is refused as `required`. + * (`sys_user_permission_set.permission_set`), its one reference: the legacy + * `permission_set_id` is retired (D10, C7b), so a write naming it is refused + * as an undeclared field; a non-system write naming a set the security + * catalog does not hold is refused, and a grant that carries no name is + * refused as `required`. One grant per user, set and organization: the + * declared unique index (`unique: 'organization'`, NULL-safe on the + * organization) refuses a second, platform-wide grants included, on every + * write path, so a revoke leaves no grant of that set in force. * * Measured on a REAL `ObjectQL` engine over a real SQL driver with the REAL * `SecurityPlugin` started on it the way a kernel composition does, because @@ -22,7 +26,7 @@ import { describe, it, expect, afterEach, vi } from 'vitest'; import { ObjectQL } from '@objectstack/objectql'; import { SqlDriver } from '@objectstack/driver-sql'; -import { resolveThrownHttpError } from '@objectstack/types'; +import { mapDataError, resolveThrownHttpError } from '@objectstack/types'; import type { PermissionSet } from '@objectstack/spec/security'; import { SecurityPlugin } from './security-plugin.js'; @@ -169,7 +173,7 @@ describe('the data door (a non-system writer)', () => { await h.engine.insert('sys_user_permission_set', { user_id: 'u_1', permission_set: 'qa_alpha' }, { context: ADMIN } as any); const [row] = await grantsOf(h, 'u_1'); expect(row).toMatchObject({ permission_set: 'qa_alpha' }); - expect(row.permission_set_id ?? null).toBeNull(); + expect(row).not.toHaveProperty('permission_set_id'); expect(await h.engine.find('sys_permission_set', { context: SYS })).toEqual([]); }); @@ -183,15 +187,25 @@ describe('the data door (a non-system writer)', () => { expect(await grantsOf(h, 'u_2')).toHaveLength(0); }); - it('NEGATIVE — a grant written with the id alone is refused 400 VALIDATION_FAILED, required at permission_set: nothing derives a name from the id', async () => { + it('NEGATIVE — a grant written with the retired id is refused 400 INVALID_FIELD: the column is no longer declared (C7b)', async () => { const h = await boot(); const err = await refusalOf(() => h.engine.insert( - 'sys_user_permission_set', { user_id: 'u_3', permission_set_id: 'ps_alpha' }, { context: ADMIN } as any, + 'sys_user_permission_set', { user_id: 'u_3', permission_set_id: 'ps_alpha', permission_set: 'qa_alpha' }, { context: ADMIN } as any, + )); + const env = envelopeOf(err); + expect([env.code, env.status]).toEqual(['INVALID_FIELD', 400]); + expect(await grantsOf(h, 'u_3')).toHaveLength(0); + }); + + it('NEGATIVE — a grant naming no set is refused 400 VALIDATION_FAILED, required at permission_set', async () => { + const h = await boot(); + const err = await refusalOf(() => h.engine.insert( + 'sys_user_permission_set', { user_id: 'u_3b' }, { context: ADMIN } as any, )); const env = envelopeOf(err); expect([env.code, env.status]).toEqual(['VALIDATION_FAILED', 400]); expect(env.fields.map((f: any) => [f.field, f.code])).toContainEqual(['permission_set', 'required']); - expect(await grantsOf(h, 'u_3')).toHaveLength(0); + expect(await grantsOf(h, 'u_3b')).toHaveLength(0); }); it('a batch insert with one row naming no catalog set is refused whole; nothing is stored', async () => { @@ -253,7 +267,7 @@ describe('a system writer', () => { it('NEGATIVE — a system grant naming nothing is still refused as required (the engine judges every writer)', async () => { const h = await boot(); const err = await refusalOf(() => h.engine.insert( - 'sys_user_permission_set', { user_id: 'u_s2', permission_set_id: 'ps_beta' }, { context: SYS } as any, + 'sys_user_permission_set', { user_id: 'u_s2' }, { context: SYS } as any, )); const env = envelopeOf(err); expect([env.code, env.status]).toEqual(['VALIDATION_FAILED', 400]); @@ -261,6 +275,98 @@ describe('a system writer', () => { }); }); +// --------------------------------------------------------------------------- +// One grant per user, set and organization (seat answers on #15211: Q5 A, Q-S3a) +// --------------------------------------------------------------------------- + +describe('one grant per user, set and organization — the declared unique \'organization\' index', () => { + const ORG = 'org_dup'; + const grantsOfSet = async (h: Harness, userId: string, set: string) => + (await grantsOf(h, userId)).filter((g) => g.permission_set === set); + /** The engine's envelope, and what the data doors answer it with on the wire. */ + const duplicateShape = (err: unknown) => { + const wire = mapDataError(err, 'sys_user_permission_set'); + return [envelopeOf(err).code, envelopeOf(err).status, wire.body.code, wire.status]; + }; + const DUPLICATE = ['DUPLICATE_RECORD', 409, 'UNIQUE_VIOLATION', 409]; + const knexOf = (h: Harness): any => (h.engine as any).getDriver('sys_user_permission_set').knex; + + it('NEGATIVE — a second organization-scoped grant is refused 409 (UNIQUE_VIOLATION on the wire); the revoke of the one leaves none in force', async () => { + const h = await boot(); + await h.engine.insert('sys_member', { id: 'mem_d1', user_id: 'u_d1', organization_id: ORG, role: 'member' }, { context: SYS } as any); + await h.engine.insert('sys_user_permission_set', { id: 'ups_d1', user_id: 'u_d1', permission_set: 'qa_alpha', organization_id: ORG }, { context: ADMIN } as any); + const err = await refusalOf(() => h.engine.insert( + 'sys_user_permission_set', { id: 'ups_d1b', user_id: 'u_d1', permission_set: 'qa_alpha', organization_id: ORG }, { context: SYS } as any, + )); + expect(duplicateShape(err)).toEqual(DUPLICATE); + expect((await grantsOfSet(h, 'u_d1', 'qa_alpha')).map((g) => g.id)).toEqual(['ups_d1']); + + await h.engine.delete('sys_user_permission_set', { where: { id: 'ups_d1' }, context: ADMIN } as any); + expect(await grantsOfSet(h, 'u_d1', 'qa_alpha')).toEqual([]); + }); + + it('NEGATIVE — a second platform-wide grant (organization NULL) is refused 409 by the index itself, for every writer; the revoke leaves none in force', async () => { + const h = await boot(); + await h.engine.insert('sys_user_permission_set', { id: 'ups_p1', user_id: 'u_p1', permission_set: 'qa_alpha', organization_id: null }, { context: ADMIN } as any); + for (const context of [ADMIN, SYS]) { + const err = await refusalOf(() => h.engine.insert( + 'sys_user_permission_set', { user_id: 'u_p1', permission_set: 'qa_alpha' }, { context } as any, + )); + expect(duplicateShape(err)).toEqual(DUPLICATE); + } + expect((await grantsOfSet(h, 'u_p1', 'qa_alpha')).map((g) => g.id)).toEqual(['ups_p1']); + + await h.engine.delete('sys_user_permission_set', { where: { id: 'ups_p1' }, context: ADMIN } as any); + expect(await grantsOfSet(h, 'u_p1', 'qa_alpha')).toEqual([]); + }); + + it('NEGATIVE — the index refuses a platform-wide duplicate on paths no hook sees: a two-row batch insert stores nothing, and raw SQL beneath the engine is refused', async () => { + const h = await boot(); + const err = await refusalOf(() => h.engine.insert( + 'sys_user_permission_set', + [{ user_id: 'u_b1', permission_set: 'qa_alpha' }, { user_id: 'u_b1', permission_set: 'qa_alpha' }], + { context: SYS } as any, + )); + expect(duplicateShape(err)).toEqual(DUPLICATE); + expect(await grantsOf(h, 'u_b1')).toEqual([]); + + await knexOf(h)('sys_user_permission_set').insert({ id: 'raw_1', user_id: 'u_r1', permission_set: 'qa_alpha', organization_id: null }); + await expect( + knexOf(h)('sys_user_permission_set').insert({ id: 'raw_2', user_id: 'u_r1', permission_set: 'qa_alpha', organization_id: null }), + ).rejects.toThrow(/UNIQUE constraint failed/); + expect((await grantsOf(h, 'u_r1')).map((g) => g.id)).toEqual(['raw_1']); + }); + + it('NEGATIVE — an update that makes a platform-wide duplicate (re-naming the set) is refused 409 and the row is untouched', async () => { + const h = await boot(); + await h.engine.insert('sys_user_permission_set', { id: 'ups_u1', user_id: 'u_u1', permission_set: 'qa_alpha' }, { context: ADMIN } as any); + await h.engine.insert('sys_user_permission_set', { id: 'ups_u2', user_id: 'u_u1', permission_set: 'qa_beta' }, { context: ADMIN } as any); + const err = await refusalOf(() => h.engine.update( + 'sys_user_permission_set', { id: 'ups_u2', permission_set: 'qa_alpha' }, { context: ADMIN } as any, + )); + expect(duplicateShape(err)).toEqual(DUPLICATE); + expect((await grantsOf(h, 'u_u1')).find((g) => g.id === 'ups_u2')).toMatchObject({ permission_set: 'qa_beta' }); + }); + + it('CONTROL — one set to one user in two organizations and platform-wide, two users of one set (a batch and raw SQL alike), and an edit of a held grant all land', async () => { + const h = await boot(); + for (const org of ['org_c1', 'org_c2']) { + await h.engine.insert('sys_member', { id: `mem_${org}`, user_id: 'u_c1', organization_id: org, role: 'member' }, { context: SYS } as any); + await h.engine.insert('sys_user_permission_set', { user_id: 'u_c1', permission_set: 'qa_alpha', organization_id: org }, { context: ADMIN } as any); + } + await h.engine.insert('sys_user_permission_set', { id: 'ups_c1', user_id: 'u_c1', permission_set: 'qa_alpha' }, { context: ADMIN } as any); + await h.engine.insert( + 'sys_user_permission_set', + [{ user_id: 'u_c2', permission_set: 'qa_alpha' }, { user_id: 'u_c3', permission_set: 'qa_alpha' }], + { context: SYS } as any, + ); + await knexOf(h)('sys_user_permission_set').insert({ id: 'raw_c4', user_id: 'u_c4', permission_set: 'qa_alpha', organization_id: null }); + await h.engine.update('sys_user_permission_set', { id: 'ups_c1', permission_set: 'qa_alpha', reason: 'edited' }, { context: ADMIN } as any); + expect(await grantsOfSet(h, 'u_c1', 'qa_alpha')).toHaveLength(3); + for (const user of ['u_c2', 'u_c3', 'u_c4']) expect(await grantsOfSet(h, user, 'qa_alpha')).toHaveLength(1); + }); +}); + describe('registration', () => { it('both hooks bind under one package when the plugin starts, and unbind when it is destroyed', async () => { const h = await boot(); diff --git a/packages/plugins/plugin-security/src/grant-permission-set-name.ts b/packages/plugins/plugin-security/src/grant-permission-set-name.ts index 19be3c037fa..ab1b99a0a1d 100644 --- a/packages/plugins/plugin-security/src/grant-permission-set-name.ts +++ b/packages/plugins/plugin-security/src/grant-permission-set-name.ts @@ -13,12 +13,12 @@ * security catalog's definition of that name. Every grant writer in this * repository writes the name and nothing else (the organization-admin * reconcile, the platform-admin promotion, self-registration, the verify RLS - * persona, Setup's grant picker). D10 orders the id column's removal after - * every grant names its set; this is that first step, so `permission_set_id` - * is optional, no writer fills it, and no write derives the name from it. The - * column and its data go with D10's later stage. Rows written before the name - * column existed are named by the one-time backfill - * (`grant-permission-set-name-backfill.ts`), never here. + * persona, Setup's grant picker). D10 ordered the id column's removal after + * every grant names its set: the declaration of `permission_set_id` is + * retired (C7b), and rows written before the name column existed are named by + * the v18 ceremony's verified rewrite (`os migrate organization-ownership + * --apply`), which reads the physical column before its column-drops step + * drops it — never here. * * ## What is refused — exactly "the catalog holds no set by this name" * @@ -58,6 +58,15 @@ * dialect for "this reference names nothing". No new error code (ADR-0112, * ADR-0114). * + * ## One grant per user, set and organization + * + * Not judged here. The declared unique index (`user_id`, `permission_set`, + * `organization_id`) is `unique: 'organization'`, whose organization key part + * is NULL-safe (ADR-0120 D3), so it refuses a second grant of one set to one + * user in one organization AND a second platform-wide grant (organization + * NULL), for every writer and every write path: a duplicate confers nothing + * more, but a revoke that deletes one row would leave the other granting. + * * ## Hooks, not a middleware * * Registered as engine `beforeInsert` / `beforeUpdate` hooks at priority 30: @@ -77,12 +86,8 @@ import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.j /** The grant object this module judges. */ export const GRANT_OBJECT = 'sys_user_permission_set'; -/** The grant's legacy id reference to a catalog row (dropped by ADR-0131 D10; read only by the backfill and the D10 report). */ -export const GRANT_SET_ID_FIELD = 'permission_set_id'; /** The grant's NAME reference to the permission set (ADR-0131 D4). */ export const GRANT_SET_NAME_FIELD = 'permission_set'; -/** The legacy row table the backfill reads an id's name from. */ -export const PERMISSION_SET_CATALOG_OBJECT = 'sys_permission_set'; /** The security catalog type a `permission_set` value must name an item of. */ const PERMISSION_SET_CATALOG_TYPE = 'permission'; /** The `packageId` both hooks register under, so they bind and unbind as one. */ @@ -156,16 +161,12 @@ function judgedName(value: unknown): string | undefined { /** * [ADR-0131 D4] The permission set a STORED grant names, as every by-name * grant reader asks it: the grant's `permission_set`, or `undefined` for a - * grant that names none — `NULL` or blank, which is a grant written before the - * column existed that the backfill has not named yet, or one it could not name - * (an id with no set row, or a set row of another organization). + * grant that names none — `NULL` or blank, a grant written before the column + * existed that the v18 ceremony's rewrite has not named (its verification + * blocks the ceremony on every such grant before the id column is dropped). * * A grant that names nothing grants nothing through a by-name reader: it is - * never matched to a set, never shown as held, never counted. A reader whose - * job is to RESTRICT — a revocation, a refused promotion, a duplicate it must - * not insert — may still reach such a legacy grant through its id until - * ADR-0131 D10 counts the unnamed grants and drops that column; no reader - * reaches it through the id to confer anything. + * never matched to a set, never shown as held, never counted. */ export function grantSetNameOf(row: unknown): string | undefined { if (!row || typeof row !== 'object') return undefined; diff --git a/packages/plugins/plugin-security/src/grant-readers-unnamed-grant.test.ts b/packages/plugins/plugin-security/src/grant-readers-unnamed-grant.test.ts index 22ec4152182..fd112068131 100644 --- a/packages/plugins/plugin-security/src/grant-readers-unnamed-grant.test.ts +++ b/packages/plugins/plugin-security/src/grant-readers-unnamed-grant.test.ts @@ -4,26 +4,26 @@ * [ADR-0131 D4] The grant that names nothing, as each `plugin-security` grant * reader treats it once the readers key on `sys_user_permission_set.permission_set`. * - * Such a grant is real state: a grant written before the name column existed - * carries `NULL` until the one-time backfill names it, and the backfill runs at - * `kernel:bootstrapped` — after the `kernel:ready` passes (the platform-admin - * bootstrap, the organization-admin backfill) that read grants. On an upgraded - * deployment's first boot every grant is unnamed when those passes run. The - * backfill also leaves a grant unnamed for good when its id names no set row, - * or a set row of another organization. + * Such a grant is real state on a database the v18 ceremony has not run on: a + * grant written before the name column existed carries `NULL` until the + * ceremony's verified rewrite (`os migrate organization-ownership --apply`, + * ADR-0131 D10) names it from the legacy `permission_set_id`, a column no + * runtime reader declares any more (C7b). * * Each reader takes its fail-closed direction: * * - **explain** reports nothing about it — it names no set; - * - **the platform-admin bootstrap** reads an unscoped one on the admin set row - * as an administrator who may already exist, and withholds the promotion — - * without naming it as the holder; - * - **the organization-administrator reconcile** still reaches it through its - * id to revoke it, and never inserts a duplicate beside it. + * - **the platform-admin bootstrap** reads an unscoped one as an administrator + * who may already exist, and withholds the promotion — without naming it as + * the holder; + * - **the organization-administrator reconcile** reads grants by name alone: + * it neither counts such a grant nor revokes it, and writes NOTHING for a + * qualifying pair holding one in that organization (the ceremony's rename + * needs its key); once the rewrite names it, the boot sweep revokes it where + * its pair no longer qualifies. * * Real ObjectQL over the SQL driver with the real `SecurityPlugin`; a grant is - * unnamed by writing `NULL` with the name hooks unbound, the state the backfill - * leaves, then the hooks are bound again. + * unnamed by writing `NULL` beneath the engine. */ import { describe, it, expect, afterEach, vi } from 'vitest'; @@ -120,24 +120,24 @@ async function user(engine: ObjectQL, id: string, createdAt: string): Promise => - String(((await engine.find('sys_permission_set', { where: { name, organization_id: null }, context: SYS })) as any[])[0]?.id); +const knexOf = (engine: ObjectQL): any => ((engine as any).getDriver('sys_user_permission_set') as any).knex; /** - * The state a grant written before the name column carries, or one the - * backfill has not named: `NULL` name and the legacy id of the set row it - * names (organization-less). Grants are written by name today (ADR-0131 D4) - * and the engine refuses one naming nothing, so it is written beneath it. + * The state a grant written before the name column carries until the v18 + * ceremony's rewrite names it: a `NULL` name. Grants are written by name + * today (ADR-0131 D4) and the engine refuses one naming nothing, so it is + * written beneath it. */ async function unname(engine: ObjectQL, where: Record): Promise { - const knex = ((engine as any).getDriver('sys_user_permission_set') as any).knex; - for (const g of await knex('sys_user_permission_set').where(where)) { - const legacyId = g.permission_set_id ?? (g.permission_set ? await setIdOf(engine, g.permission_set) : null); - await knex('sys_user_permission_set').where({ id: g.id }).update({ permission_set: null, permission_set_id: legacyId }); - } + await knexOf(engine)('sys_user_permission_set').where(where).update({ permission_set: null }); const rows = (await engine.find('sys_user_permission_set', { where, context: SYS })) as any[]; expect(rows.length).toBeGreaterThan(0); - expect(rows.every((r) => r.permission_set == null && r.permission_set_id)).toBe(true); + expect(rows.every((r) => r.permission_set == null)).toBe(true); +} + +/** What the ceremony's verified rewrite does to such a grant: it names it. */ +async function rename(engine: ObjectQL, id: string, name: string): Promise { + await knexOf(engine)('sys_user_permission_set').where({ id }).update({ permission_set: name }); } const grantsOf = async (engine: ObjectQL, userId: string): Promise => @@ -243,20 +243,26 @@ describe('[ADR-0131 D4] a grant that names nothing — the organization-administ return engine; } - it('a qualifying pair holding it is not handed a duplicate grant', async () => { + it('[Q-S3b (ii)] a qualifying pair holding it is written nothing — a named grant beside it would hold the key the ceremony\'s rename needs — and it confers nothing meanwhile', async () => { const engine = await orgAdmin(); - expect(await reconcileOrgAdminGrant(engine, 'usr_orgadmin', ORG)).toEqual({ action: 'noop' }); - expect(await grantsOf(engine, 'usr_orgadmin')).toHaveLength(1); + expect(await reconcileOrgAdminGrant(engine, 'usr_orgadmin', ORG)).toEqual({ action: 'skipped', reason: 'unnamed_grant_awaits_ceremony' }); + expect(await backfillOrgAdminGrants(engine)).toMatchObject({ granted: 0 }); + expect((await grantsOf(engine, 'usr_orgadmin')).map((g) => g.permission_set ?? null)).toEqual([null]); }); - it('a demotion still revokes it — reached through its id', async () => { + it('a demotion does not reach it while it is unnamed; once the rewrite names it, the boot sweep revokes it', async () => { const engine = await orgAdmin(); await engine.update('sys_member', { id: 'm_owner', role: 'member' }, { context: orgCtx } as any); await reconcileOrgAdminGrant(engine, 'usr_orgadmin', ORG); + const [left] = await grantsOf(engine, 'usr_orgadmin'); + expect(left).toMatchObject({ permission_set: null }); + + await rename(engine, String(left.id), 'organization_admin_no_bypass'); + await backfillOrgAdminGrants(engine); expect(await grantsOf(engine, 'usr_orgadmin')).toEqual([]); }); - it('the boot backfill still revokes it once its pair has no membership left', async () => { + it('the boot sweep does not reach it while its pair has no membership and it is unnamed; once named, it revokes it', async () => { const engine = await orgAdmin(); await user(engine, 'usr_orphan', '2025-05-01T00:00:00.000Z'); await engine.insert( @@ -268,8 +274,11 @@ describe('[ADR-0131 D4] a grant that names nothing — the organization-administ { context: orgCtx } as any, ); await unname(engine, { id: 'g_orphan' }); - const summary = await backfillOrgAdminGrants(engine); - expect(summary.revoked).toBe(1); + expect((await backfillOrgAdminGrants(engine)).revoked).toBe(0); + expect(await grantsOf(engine, 'usr_orphan')).toHaveLength(1); + + await rename(engine, 'g_orphan', 'organization_admin_no_bypass'); + expect((await backfillOrgAdminGrants(engine)).revoked).toBe(1); expect(await grantsOf(engine, 'usr_orphan')).toEqual([]); }); }); diff --git a/packages/plugins/plugin-security/src/objects/sys-user-permission-set.object.ts b/packages/plugins/plugin-security/src/objects/sys-user-permission-set.object.ts index 1dbc575dd51..d0b541fbc4f 100644 --- a/packages/plugins/plugin-security/src/objects/sys-user-permission-set.object.ts +++ b/packages/plugins/plugin-security/src/objects/sys-user-permission-set.object.ts @@ -14,9 +14,13 @@ import { F, P } from '@objectstack/spec'; * table when building the per-request `ExecutionContext.permissions[]`. * * The same permission set can be granted independently in each org context - * the user belongs to. The declared unique index still names - * `permission_set_id`, the column ADR-0131 D10 drops with its data (C7b); a - * grant written by name carries NULL there, which SQL keys apart. + * the user belongs to, and once in each: the declared unique index is + * (`user_id`, `permission_set`, `organization_id`), `unique: 'organization'`, + * so its organization key part is NULL-safe (ADR-0120 D3) and a second + * platform-wide grant (organization NULL) is refused too — by the index, for + * every writer and write path. [ADR-0131 D10, C7b] The legacy + * `permission_set_id` column is retired: the v18 ceremony's verified rewrite + * names every grant from it, then its column-drops step drops it. * * @namespace sys */ @@ -58,8 +62,8 @@ export const SysUserPermissionSet = ObjectSchema.create({ // neither is hidden, permission-guarded or masked on this object, so the // title carries nothing the declared read path withholds. Both are // required (the set by name, ADR-0131 D4), so the expression needs no null - // guard; a legacy grant the one-time backfill names at boot is the only - // row that ever stored the name NULL. + // guard; a legacy grant the v18 ceremony's rewrite names is the only row + // that ever stored the name NULL. display_title: Field.formula({ label: 'Title', returnType: 'text', @@ -73,29 +77,6 @@ export const SysUserPermissionSet = ObjectSchema.create({ description: 'Foreign key to sys_user.', }), - // [ADR-0131 D10] The grant's former id reference to a catalog row. No - // writer fills it and no reader confers anything through it: a grant names - // its set by `permission_set` below. Optional, and kept only until D10 - // drops the column with its data (C7b), whose verified rewrite reads it on - // grants written before the name column existed. - // - // [C3 stage 8] Plain text, no longer a lookup: the catalog row object it - // pointed at is retired. Same name, same stored ids, same two indexes - // below, and the same `varchar(255)` a lookup is stored in, so the column - // itself does not change. - permission_set_id: Field.text({ - label: 'Permission Set (legacy id)', - required: false, - maxLength: 255, - // [#15204 B2] Off the forms: it grants nothing, so an editable id - // beside `permission_set` only invites a meaningless value. The column - // stays until its data is dropped (ADR-0131 D10). - hidden: true, - description: - '[ADR-0131 D10] Legacy id of a retired permission-set catalog row, dropped with its data by a later stage. ' + - 'Not written and not read to grant anything: the grant names its set by permission_set.', - }), - // [ADR-0131 D4] The permission set this grant holds, BY NAME — the grant's // one reference to its set. The set is the security catalog's definition // of that name (ADR-0131 D3), so no `sys_permission_set` row is needed to @@ -104,7 +85,7 @@ export const SysUserPermissionSet = ObjectSchema.create({ // `grant-permission-set-name.ts`), as `sys_user_position.position` is. // Same width and shape as that sibling (a machine name, 100). Required on // every write; a grant written before this column existed carries NULL - // until the one-time backfill names it, and an update that leaves the + // until the v18 ceremony's rewrite names it, and an update that leaves the // column alone does not have to name it. permission_set: Field.text({ label: 'Permission Set', @@ -281,10 +262,11 @@ export const SysUserPermissionSet = ObjectSchema.create({ ], indexes: [ - { fields: ['user_id', 'permission_set_id', 'organization_id'], unique: 'global' }, + // One grant of a set to a user per organization, platform-wide (NULL) + // included (the module note): a duplicate would keep a revoked grant in force. + { fields: ['user_id', 'permission_set', 'organization_id'], unique: 'organization' }, { fields: ['user_id'] }, { fields: ['organization_id'] }, - { fields: ['permission_set_id'] }, ], enable: { diff --git a/packages/plugins/plugin-security/src/orgless-position-name-fold.test.ts b/packages/plugins/plugin-security/src/orgless-position-name-fold.test.ts index f55aed9fe2e..ac067559b48 100644 --- a/packages/plugins/plugin-security/src/orgless-position-name-fold.test.ts +++ b/packages/plugins/plugin-security/src/orgless-position-name-fold.test.ts @@ -158,7 +158,7 @@ async function boot(): Promise { await insertSet(e, 'ps_0_other_platform_ops', 'platform_ops', ORG_OTHER, ['probe.other_platform_ops']); await insertSet(e, 'ps_1_global_platform_ops', 'platform_ops', null, ['probe.global_platform_ops']); await e.insert('sys_user_permission_set', { - id: 'ups_global_platform_ops', user_id: USER_GLOBAL, permission_set_id: 'ps_1_global_platform_ops', + id: 'ups_global_platform_ops', user_id: USER_GLOBAL, permission_set: 'platform_ops', organization_id: null, }, SYS); // [ADR-0131 D3/D4] The two GLOBAL sets are declared in the catalog, which is diff --git a/packages/plugins/plugin-security/src/position-catalog-refusal.ts b/packages/plugins/plugin-security/src/position-catalog-refusal.ts index 04c6fe82a24..2aa2ad89971 100644 --- a/packages/plugins/plugin-security/src/position-catalog-refusal.ts +++ b/packages/plugins/plugin-security/src/position-catalog-refusal.ts @@ -16,9 +16,9 @@ * nothing in `current_user.positions` that any reader could act on. The holder * signed in to an app that reads nothing, and no layer said why. The measured * spelling is the position's record ID written where its NAME belongs — the - * neighbouring `user_id` is an id, and the analogous - * `sys_user_permission_set.permission_set_id` is genuinely id-typed, so the id - * is what an author reaches for by analogy. + * neighbouring `user_id` is an id, and the analogous grant once carried a + * genuinely id-typed `sys_user_permission_set.permission_set_id` (retired, + * ADR-0131 D10), so the id is what an author reaches for by analogy. * * ## The predicate — exactly "the name resolves to no position" * diff --git a/packages/plugins/plugin-security/src/resolve-authz-grant-set-by-name.test.ts b/packages/plugins/plugin-security/src/resolve-authz-grant-set-by-name.test.ts index a00e81742d6..3617b4ff735 100644 --- a/packages/plugins/plugin-security/src/resolve-authz-grant-set-by-name.test.ts +++ b/packages/plugins/plugin-security/src/resolve-authz-grant-set-by-name.test.ts @@ -7,25 +7,21 @@ * driver's tenant scope and the write hooks included, which the core suite's * recording double answers for neither of. * - * - **Another organization's set.** A grant whose id names another - * organization's set row confers nothing, whether it is left unnamed (what - * the write hook and the backfill leave it) or carries that set's name (what - * the hook could write before it judged the set row's organization). - * - **An unnamed grant** confers nothing through the resolver, and its id - * still restricts where it restricted: the organization-administrator - * reconcile still revokes it through its id. - * - **The upgrade boot.** Grants stored before the name column existed are - * unnamed until the one-time backfill at `kernel:bootstrapped` names them. - * No request reaches the resolver before then: the HTTP socket opens at - * `kernel:listening`, strictly after every `kernel:bootstrapped` handler, - * and by then the grant confers again. A `kernel:ready` handler — boot code, - * never a request — still sees it unnamed. + * - **Another organization's set.** A grant of a set only another + * organization's row carries confers nothing, whether it is left unnamed or + * carries that set's name. + * - **An unnamed grant** confers nothing through the resolver, and nothing + * reaches it through the retired `permission_set_id` any more (ADR-0131 + * D10, C7b): the v18 ceremony's verified rewrite names such a grant, or + * blocks on it, before the column is dropped — the boot-time backfill that + * named it is retired, and so is its pin here (its duplicate is the + * ceremony's acceptance fixture, `organization-ownership-apply.test.ts`). */ import { describe, it, expect, afterEach, vi } from 'vitest'; import { ObjectQL } from '@objectstack/objectql'; import { SqlDriver } from '@objectstack/driver-sql'; -import { LiteKernel, resetPlatformAdminEmailMemo, resolveUserAuthzGrants } from '@objectstack/core'; +import { resetPlatformAdminEmailMemo, resolveUserAuthzGrants } from '@objectstack/core'; import { SysUser, SysAccount, SysMember, SysOrganization } from '@objectstack/platform-objects/identity'; import { SysMigration } from '@objectstack/platform-objects/system'; @@ -137,7 +133,7 @@ async function grant(engine: Engine, id: string): Promise | return (await engine.findOne('sys_user_permission_set', { where: { id }, context: SYS })) as any; } -describe('[ADR-0131 D4] a grant whose id names another organization’s set confers nothing through the resolver', () => { +describe('[ADR-0131 D4] a grant of a set only another organization’s row carries confers nothing through the resolver', () => { async function world(): Promise { const engine = await boot({ walled: true }); await insertUser(engine, 'usr_x'); @@ -147,9 +143,9 @@ describe('[ADR-0131 D4] a grant whose id names another organization’s set conf return engine; } - it('left unnamed (what the write hook and the backfill leave it): nothing, in either organization or none', async () => { + it('left unnamed: nothing, in either organization or none', async () => { const engine = await world(); - await insertGrantUnhooked(engine, { id: 'g_x', user_id: 'usr_x', permission_set_id: 'ps_b_tools', organization_id: null }); + await insertGrantUnhooked(engine, { id: 'g_x', user_id: 'usr_x', organization_id: null }); for (const tenantId of [undefined, ORG_A, ORG_B]) { const out = await resolveUserAuthzGrants(engine, 'usr_x', tenantId ? { tenantId } : {}); expect(out.permissions, String(tenantId)).not.toContain('b_tools'); @@ -160,10 +156,10 @@ describe('[ADR-0131 D4] a grant whose id names another organization’s set conf it('carrying that set’s name: nothing either — no row the grant may name bears it', async () => { const engine = await world(); await insertGrantUnhooked(engine, { - id: 'g_y', user_id: 'usr_x', permission_set_id: 'ps_b_tools', permission_set: 'b_tools', organization_id: null, + id: 'g_y', user_id: 'usr_x', permission_set: 'b_tools', organization_id: null, }); await insertGrantUnhooked(engine, { - id: 'g_z', user_id: 'usr_x', permission_set_id: 'ps_b_tools', permission_set: 'b_tools', organization_id: ORG_A, + id: 'g_z', user_id: 'usr_x', permission_set: 'b_tools', organization_id: ORG_A, }); for (const tenantId of [undefined, ORG_A, ORG_B]) { const out = await resolveUserAuthzGrants(engine, 'usr_x', tenantId ? { tenantId } : {}); @@ -195,8 +191,8 @@ describe('[ADR-0131 D4] a grant whose id names another organization’s set conf }); }); -describe('[ADR-0131 D4] an unnamed grant confers nothing through the resolver; its id still restricts', () => { - it('an unnamed organization-administrator grant confers nothing, and the reconcile still revokes it through its id', async () => { +describe('[ADR-0131 D4/D10] an unnamed grant confers nothing through the resolver; no id reaches it', () => { + it('an unnamed grant beside an organization-administrator membership confers nothing, and the reconcile neither grants through it nor reaches it', async () => { const engine = await boot(); await insertUser(engine, 'usr_admin'); await insertUser(engine, 'usr_owner'); @@ -204,86 +200,21 @@ describe('[ADR-0131 D4] an unnamed grant confers nothing through the resolver; i await seedLegacyPlatformBucket(engine, defaultPermissionSets); await bootstrapPlatformAdmin(engine, defaultPermissionSets); await engine.insert('sys_organization', { id: ORG_A, name: 'A', slug: 'a' }, { context: SYS } as any); - // A plain member holding an organization-administrator grant the backfill - // has not named — what a demoted owner's left-behind grant looks like. + // A plain member holding a grant that names nothing — a legacy row the v18 + // ceremony's rewrite has not named yet. await engine.insert('sys_member', { id: 'm_owner', user_id: 'usr_owner', organization_id: ORG_A, role: 'member' }, { context: { isSystem: true, tenantId: ORG_A } } as any); - const [set] = (await engine.find('sys_permission_set', { - where: { name: 'organization_admin_no_bypass', organization_id: null }, context: SYS, - })) as any[]; - await insertGrantUnhooked(engine, { id: 'g_oa', user_id: 'usr_owner', permission_set_id: set.id, organization_id: ORG_A }); + await insertGrantUnhooked(engine, { id: 'g_oa', user_id: 'usr_owner', organization_id: ORG_A }); const out = await resolveUserAuthzGrants(engine, 'usr_owner', { tenantId: ORG_A }); expect(out.permissions).not.toContain('organization_admin_no_bypass'); expect(out.posture).toBe('MEMBER'); + // The reconcile reads grants by name alone: for a plain member there is + // nothing to revoke, and the unnamed row is the ceremony's, untouched. const demotion = await reconcileOrgAdminGrant(engine, 'usr_owner', ORG_A, { posture: 'single' }); - expect(demotion.action).toBe('revoked'); - expect(await grant(engine, 'g_oa')).toBeNull(); - }); -}); - -describe('[ADR-0131 D4] the upgrade boot — no request reaches the resolver before the backfill names the grants', () => { - it('a grant stored before the name column: unnamed at kernel:ready, named and conferring at kernel:listening', async () => { - const engine = await newEngine(); - for (const ps of defaultPermissionSets) { - engine.registry.registerItem('permission', structuredClone(ps) as any, 'name' as any, CATALOG_PACKAGE); - } - engine.registry.registerItem( - 'permission', - { name: 'upgrade_reviewer', label: 'Reviewer', objects: {}, systemPermissions: ['cap_review'] } as any, - 'name' as any, - CATALOG_PACKAGE, - ); - // Stored before this boot: the set row, and a grant with no name. - await engine.insert('sys_permission_set', - { id: 'ps_up', name: 'upgrade_reviewer', label: 'Reviewer', system_permissions: '["cap_review"]', active: true }, - { context: SYS } as any); - await insertUser(engine, 'usr_member'); - await insertGrantUnhooked(engine, { id: 'g_up', user_id: 'usr_member', permission_set_id: 'ps_up' }); - expect((await grant(engine, 'g_up'))?.permission_set).toBeNull(); - - const seen: Record = {}; - const metadata = { - get: async (type: string, name: string) => - type === 'permission' - ? ([...defaultPermissionSets] as Array<{ name?: string }>).find((s) => s?.name === name) ?? null - : engine.getSchema(name) ?? null, - list: async () => [...defaultPermissionSets], - }; - const kernel = new LiteKernel({ logger: { level: 'silent' } }); - kernel.use({ - name: 'com.objectstack.engine.objectql', - init: async (ctx: any) => { - ctx.registerService('objectql', engine); - ctx.registerService('metadata', metadata); - ctx.registerService('manifest', { register: () => undefined }); - }, - start: async () => undefined, - } as any); - kernel.use(new SecurityPlugin({ fallbackPermissionSet: 'member_default' })); - // Registered after SecurityPlugin, so each of its handlers runs after - // SecurityPlugin's own handler for the same hook. - kernel.use({ - name: 'com.objectstack.qa.resolver-probe', - dependencies: ['com.objectstack.security'], - init: async () => undefined, - start: async (ctx: any) => { - const probe = (phase: string) => async () => { - seen[phase] = (await resolveUserAuthzGrants(engine, 'usr_member', { bypassGrantsCache: true })).systemPermissions; - }; - ctx.hook('kernel:ready', probe('ready')); - ctx.hook('kernel:listening', probe('listening')); - }, - } as any); - try { - await kernel.bootstrap(); - expect(seen.ready).not.toContain('cap_review'); - expect(seen.listening).toContain('cap_review'); - expect((await grant(engine, 'g_up'))?.permission_set).toBe('upgrade_reviewer'); - } finally { - await kernel.shutdown(); - } + expect(demotion.action).not.toBe('revoked'); + expect(await grant(engine, 'g_oa')).toMatchObject({ id: 'g_oa', permission_set: null }); }); }); diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index a3c88b7d419..4a198e6cdb8 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -40,7 +40,6 @@ import { registerGrantPermissionSetNameHooks, unregisterGrantPermissionSetNameHooks, } from './grant-permission-set-name.js'; -import { runOneTimeGrantPermissionSetNameBackfill } from './grant-permission-set-name-backfill.js'; import { reportCatalogReferences } from './catalog-reference-report.js'; import { explainAccessForAsker, @@ -4465,41 +4464,13 @@ export class SecurityPlugin implements Plugin { void runBootstrap(); } - // [ADR-0131 D4] Name the permission set on every grant written before - // `sys_user_permission_set.permission_set` existed — once per deployment, - // recorded in `sys_migration`. At `kernel:bootstrapped`, not `kernel:ready`: - // every `kernel:ready` handler has settled by then (the bootstrap above - // among them), so the security catalog read each - // name is verified through sees every definition this boot registers. A - // name it still does not resolve leaves the verdict unrecorded, so the next - // boot tries again. See `grant-permission-set-name-backfill.ts`. - const runGrantNameBackfill = async (): Promise => { - try { - await runOneTimeGrantPermissionSetNameBackfill(ql as any, { - catalog: createSecurityCatalogReader({ registry: (ql as any).registry, metadata: this.metadata }), - logger: ctx.logger, - }); - } catch (e) { - ctx.logger.warn( - '[security] the sys_user_permission_set name backfill did not run — grants written before the ' + - 'permission_set column existed keep no name until a later boot runs it', - { error: (e as Error)?.message }, - ); - } - }; - if (typeof (ctx as any).hook === 'function') { - (ctx as any).hook('kernel:bootstrapped', runGrantNameBackfill); - } else { - void runGrantNameBackfill(); - } - // [ADR-0131 D3/D4] The catalog reference report: per organization, every // assignment and grant whose name the security catalog does not resolve, // and every grant that names nothing. A reference that resolves nowhere - // confers nothing; this is what makes that loud. Registered AFTER the - // grant-name backfill above, so it runs after it (handlers run in - // registration order) and does not report a name it resolved this boot. - // It writes nothing. Only at `kernel:bootstrapped`: run any earlier, it + // confers nothing; this is what makes that loud. A grant written before + // the name column existed is named by the v18 ceremony's verified + // rewrite (`os migrate organization-ownership --apply`, ADR-0131 D10), + // never at boot. It writes nothing. Only at `kernel:bootstrapped`: run any earlier, it // would judge a catalog that is still being filled. See `catalog-reference-report.ts`. if (typeof (ctx as any).hook === 'function') { (ctx as any).hook('kernel:bootstrapped', async (): Promise => { diff --git a/packages/plugins/plugin-security/src/translations/en.objects.generated.ts b/packages/plugins/plugin-security/src/translations/en.objects.generated.ts index 834fe5e27cb..564c8050911 100644 --- a/packages/plugins/plugin-security/src/translations/en.objects.generated.ts +++ b/packages/plugins/plugin-security/src/translations/en.objects.generated.ts @@ -32,10 +32,6 @@ export const enObjects: NonNullable = { label: "User", help: "Foreign key to sys_user." }, - permission_set_id: { - label: "Permission Set (legacy id)", - help: "[ADR-0131 D10] Legacy id of a retired permission-set catalog row, dropped with its data by a later stage. Not written and not read to grant anything: the grant names its set by permission_set." - }, permission_set: { label: "Permission Set", help: "[ADR-0131 D4] Machine name of the permission set this grant holds, as the security catalog names it. A name the catalog does not hold is refused. NULL only on a grant written before the column existed." diff --git a/packages/plugins/plugin-security/src/translations/es-ES.objects.generated.ts b/packages/plugins/plugin-security/src/translations/es-ES.objects.generated.ts index daadd6f3961..e99fd4e3551 100644 --- a/packages/plugins/plugin-security/src/translations/es-ES.objects.generated.ts +++ b/packages/plugins/plugin-security/src/translations/es-ES.objects.generated.ts @@ -32,10 +32,6 @@ export const esESObjects: NonNullable = { label: "Usuario", help: "Clave foránea a sys_user." }, - permission_set_id: { - label: "Conjunto de permisos (id heredado)", - help: "[ADR-0131 D10] Referencia heredada por id a una fila de sys_permission_set, que se eliminará con sus datos en una etapa posterior. No se escribe ni se lee para otorgar nada: la asignación nombra su conjunto mediante permission_set." - }, permission_set: { label: "Conjunto de permisos", help: "[ADR-0131 D4] Nombre de máquina del conjunto de permisos que otorga esta asignación, tal como lo nombra el catálogo de seguridad. Un nombre que el catálogo no contiene se rechaza. NULL solo en una asignación escrita antes de que existiera la columna." diff --git a/packages/plugins/plugin-security/src/translations/ja-JP.objects.generated.ts b/packages/plugins/plugin-security/src/translations/ja-JP.objects.generated.ts index 1ed611dbadd..30adca29967 100644 --- a/packages/plugins/plugin-security/src/translations/ja-JP.objects.generated.ts +++ b/packages/plugins/plugin-security/src/translations/ja-JP.objects.generated.ts @@ -32,10 +32,6 @@ export const jaJPObjects: NonNullable = { label: "ユーザー", help: "sys_user への外部キー。" }, - permission_set_id: { - label: "権限セット（旧 ID）", - help: "[ADR-0131 D10] sys_permission_set 行への旧 ID 参照。後続の段階でデータとともに削除されます。書き込まれず、何かを付与するために読まれることもありません。付与は permission_set で権限セットを名前で指定します。" - }, permission_set: { label: "権限セット", help: "[ADR-0131 D4] この付与が保持する権限セットのマシン名（セキュリティカタログでの名前）。カタログにない名前は拒否されます。この列が存在する前に書き込まれた付与でのみ NULL です。" diff --git a/packages/plugins/plugin-security/src/translations/zh-CN.objects.generated.ts b/packages/plugins/plugin-security/src/translations/zh-CN.objects.generated.ts index a8e4c79921b..b21476351f6 100644 --- a/packages/plugins/plugin-security/src/translations/zh-CN.objects.generated.ts +++ b/packages/plugins/plugin-security/src/translations/zh-CN.objects.generated.ts @@ -32,10 +32,6 @@ export const zhCNObjects: NonNullable = { label: "用户", help: "指向 sys_user 的外键。" }, - permission_set_id: { - label: "权限集（旧版 ID）", - help: "[ADR-0131 D10] 指向 sys_permission_set 行的旧版 ID 引用，将在后续阶段连同数据一并删除。不再写入，也不再据此授予任何权限：授权通过 permission_set 按名称指明其权限集。" - }, permission_set: { label: "权限集", help: "[ADR-0131 D4] 此授权所持有的权限集的机器名，以安全目录中的名称为准。安全目录中不存在的名称将被拒绝。仅在此列存在之前写入的授权中为 NULL。" diff --git a/packages/qa/dogfood/test/grant-name-pickers.dogfood.test.ts b/packages/qa/dogfood/test/grant-name-pickers.dogfood.test.ts index 6ed8f35b952..f1d348e3908 100644 --- a/packages/qa/dogfood/test/grant-name-pickers.dogfood.test.ts +++ b/packages/qa/dogfood/test/grant-name-pickers.dogfood.test.ts @@ -10,8 +10,9 @@ * are gone; "New" opens the grant form, and the form's name field carries a * widget hint the console resolves to a catalog picker (`field:position-ref` * lists `GET /meta/position`, `field:permission-ref` lists - * `GET /meta/permission`). The legacy `permission_set_id` lookup grants - * nothing and is hidden from the forms (maintainer ruling, 2026-10-12). + * `GET /meta/permission`). The legacy `permission_set_id` granted nothing, + * was hidden from the forms (maintainer ruling, 2026-10-12), and is now + * retired from the declaration (ADR-0131 D10, C7b): it is not served at all. * * Read from `GET /meta/object/` on a booted showcase: what the console * actually receives, not the source declaration, so the pin fails if the spec @@ -51,9 +52,9 @@ describe('[#15204 B2] grant forms pick names from the catalog', () => { expect(served.sys_user_permission_set?.permission_set?.widget).toBe('permission-ref'); }); - it('permission_set_id is served hidden, and the name field it sits beside is not', () => { - expect(served.sys_user_permission_set?.permission_set_id?.hidden).toBe(true); - // Control: the read is not answering `hidden` for every field. + it('the retired permission_set_id is not served, and the name field that replaced it is served visible', () => { + expect(served.sys_user_permission_set).not.toHaveProperty('permission_set_id'); + // Control: the read serves the object's fields at all. expect(served.sys_user_permission_set?.permission_set?.hidden).not.toBe(true); }); }); diff --git a/packages/spec/src/migrations/entries/semantic/18.ups-permission-set-id-column-retired.ts b/packages/spec/src/migrations/entries/semantic/18.ups-permission-set-id-column-retired.ts new file mode 100644 index 00000000000..5b25121aa3d --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.ups-permission-set-id-column-retired.ts @@ -0,0 +1,50 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +// #15211 (ADR-0131 D4/D10, C7b) — a platform-object COLUMN retirement, not a +// spec-key retirement: no authorable spec key moves, so nothing lands in +// RETIRED_KEYS_BY_MAJOR and no D2 conversion exists to pair with (the +// ups-delegated-from-column-retired shape). +export const entry: SemanticMigration = { + id: 'ups-permission-set-id-column-retired', + // No backticks in `surface` — build-upgrade-guide.ts renders it inside a + // code span AND a table cell. + surface: + 'sys_user_permission_set.permission_set_id — the legacy id reference of a permission-set grant ' + + 'left the platform grant table declared by plugin-security (packages/plugins/plugin-security/' + + 'src/objects/sys-user-permission-set.object.ts), with its (permission_set_id) index. The unique ' + + 'index (user_id, permission_set_id, organization_id) is replaced by one on (user_id, ' + + 'permission_set, organization_id) declared unique per organization, whose NULL-safe organization ' + + 'key part also refuses a second platform-wide grant (organization NULL) of one set to one user, ' + + 'with 409 UNIQUE_VIOLATION', + replacement: + '`permission_set`, the machine name of the permission set the grant holds (ADR-0131 D4). Delete ' + + 'the key from any authored `sys_user_permission_set` seed row or data-door write that still ' + + 'carries it, and name the set instead. Existing databases: run the v18 ceremony, ' + + '`os migrate organization-ownership --apply`, whose verified rewrite names every grant from the ' + + 'column and whose column-drops step then drops the two retired indexes and then the column; ' + + 'restart the server afterwards', + reason: + 'ADR-0131 D4 makes every reference to a catalog item a name, and D3 retired the permission-set ' + + 'catalog rows the id pointed at. Since then no writer fills the column and no reader confers ' + + 'anything through it: a grant holds its set by permission_set. D10 orders the column\'s removal ' + + 'after every grant names its set, which is the v18 ceremony\'s verified id-to-name rewrite: it ' + + 'reads the physical column by raw SQL, names each grant, verifies every name against the ' + + 'security catalog, and only then drops the column through the schema-drift machinery (schema ' + + 'sync is additive, so an undeclared column stays until it is dropped). The boot-time name ' + + 'backfill that did the same job is retired with it. The old unique index keyed the id, which ' + + 'grants written by name leave NULL, so it never refused a duplicate grant; a duplicate confers ' + + 'nothing more, but a revoke that deletes one row leaves the other granting. The new index keys ' + + 'the name, NULL-safe on the organization (ADR-0120 D3), so it is the one guard every writer and ' + + 'write path meets. The organization-administrator sweep writes no grant beside a legacy one the ' + + 'rewrite has not named. The ceremony deletes no grant: a legacy grant whose name its user already ' + + 'holds in its organization is not named and blocks the rewrite, naming both ids, and named ' + + 'duplicates block the column\'s drop, until the operator has removed one of each pair.', + acceptanceCriteria: + 'No authored stack seed or client write names `permission_set_id` on `sys_user_permission_set`: ' + + 'the engine refuses the undeclared field with 400 INVALID_FIELD. A second grant of one set to ' + + 'one user in one organization is refused by the unique index with 409 UNIQUE_VIOLATION, ' + + 'platform-wide grants (organization NULL) included, on every write path. On a legacy database, after `os migrate organization-ownership --apply` the column ' + + 'is gone and the boot no longer reports it orphaned.', +}; diff --git a/scripts/adr-anchors/packages__plugins__plugin-security__src__grant-permission-set-name-backfill.ts.json b/scripts/adr-anchors/packages__plugins__plugin-security__src__grant-permission-set-name-backfill.ts.json deleted file mode 100644 index 73b3033aad0..00000000000 --- a/scripts/adr-anchors/packages__plugins__plugin-security__src__grant-permission-set-name-backfill.ts.json +++ /dev/null @@ -1,7 +0,0 @@ -{ - "file": "packages/plugins/plugin-security/src/grant-permission-set-name-backfill.ts", - "adrs": [ - "ADR-0131" - ], - "invariant": "A grant stored before its by-name column existed is given a name only when the name is the set row its own id names inside the grant's organization (that organization or the organization-less bucket) and the security catalog read resolves it (D4); an id naming no set row, another organization's set row, or a name the catalog does not resolve stays unnamed and is reported, never guessed. The pass is idempotent, records its verdict once in sys_migration, and deletes nothing: deletions are C7's and the id column is dropped only by D10's later step." -} diff --git a/scripts/adr-anchors/packages__plugins__plugin-security__src__grant-permission-set-name.ts.json b/scripts/adr-anchors/packages__plugins__plugin-security__src__grant-permission-set-name.ts.json index 72075fe95c0..823136b2ff5 100644 --- a/scripts/adr-anchors/packages__plugins__plugin-security__src__grant-permission-set-name.ts.json +++ b/scripts/adr-anchors/packages__plugins__plugin-security__src__grant-permission-set-name.ts.json @@ -3,5 +3,5 @@ "adrs": [ "ADR-0131" ], - "invariant": "A grant names its permission set BY NAME beside the id (D4), and the two never disagree: on every sys_user_permission_set write that carries permission_set_id, for every caller, the name is derived from the id and a supplied name that names another set is refused. The id column may be dropped (D10) only once every grant's name is written and verified this way." + "invariant": "A grant names its permission set BY NAME alone (D4): the legacy permission_set_id is retired (D10), so nothing derives a name from an id, and a non-system write naming a set the security catalog does not hold is refused 400 reference_not_found. These hooks judge no duplicate: one grant per user, set and organization is the declared unique 'organization' index's, whose NULL-safe organization key part (ADR-0120 D3) refuses a second platform-wide grant too, for every writer and write path." } diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index a298069e707..9c69fcc72d3 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -1,6 +1,11 @@ { "$comment": "GENERATED — the RETAINED ledger of check-engine-double-contract.mjs (#9680). Regenerate with `node scripts/check-engine-double-contract.mjs --write`; never hand-edit. Each row is one (file, verb) whose engine double routes through the producer's dispatch predicate. This is the OPPOSITE polarity to engine-double-contract.baseline.json: that ledger records DEBT and may only shrink, this one records COVERAGE and may only grow. A row that disappears is a pinned double that left the population — the blind spot #9680 measured, where deleting a double's delete() member took 319 pinned to 318 with the gate green. Read a removal in this file's diff as a coverage loss and check it was intended.", "entries": [ + { + "file": "packages/cli/src/utils/organization-ownership-apply.test.ts", + "verb": "delete", + "pinned": 1 + }, { "file": "packages/cloud-connection/src/marketplace-install-local-sample-data-not-loaded.test.ts", "verb": "delete",