diff --git a/.changeset/23542-walled-binding-remedy.md b/.changeset/23542-walled-binding-remedy.md new file mode 100644 index 00000000000..290b8d6c1a7 --- /dev/null +++ b/.changeset/23542-walled-binding-remedy.md @@ -0,0 +1,32 @@ +--- +'@objectstack/cli': patch +--- + +fix(cli): under a wall, `os migrate organization-ownership --apply` names the act that clears a position binding the rewrite gate refuses, instead of pointing at a step that reports done (#23542) + +Clause-②: no + +**What changed.** Under the `group` or `isolated` posture, no ceremony step carries `sys_position_permission_set` rows into a position definition. `authored-conversion` reports and finishes before it reads a binding (ADR-0131 D3). The rewrite gate still refused those rows, rightly, but its remedy sent the operator to `authored-conversion`, which had already reported `done` with no blocker. The gate still refuses, and nothing is deleted or guessed. Only the remedy changes, and it now depends on where the position's definition lives. + +The blocker's first sentence is unchanged: "Binding rows give position NAME sets its catalog definition does not name (SETS); deleting them would lose a grant." The sentence after it changes, under a wall only: + +- **A position no catalog declares.** + - FROM: "The authored-conversion step carries them; resolve its blocker, then re-run." + - TO: "Under the POSTURE posture no ceremony step carries binding rows (ADR-0131 D3). Declare the position NAME with permissionSets naming SETS, through one door only: the deployment's code (compiled into the artifact this ceremony boots), or the metadata door as the platform administrator (PUT /api/v1/meta/position/NAME). A cold boot refuses a position name a package declares and the environment catalog also holds. Then re-run." +- **A declared position the metadata door reports editable, whose definition lacks some of the bound sets.** + - FROM: the same pointer. + - TO: "Under the POSTURE posture no ceremony step carries binding rows (ADR-0131 D3). Add MISSING to the permissionSets of the position NAME through the metadata door as the platform administrator (PUT /api/v1/meta/position/NAME), then re-run." +- **A declared position the metadata door reports not editable.** A definition a package ships is sealed (ADR-0131 D6), so no door on the database adds the sets. The text states the fact and names no act. + - FROM: the same pointer. + - TO, when the definition's provenance names the package: "Under the POSTURE posture no ceremony step carries binding rows (ADR-0131 D3). Package PACKAGE ships the definition of NAME, which is sealed (ADR-0131 D6: the metadata door reports it not editable, lock LOCK), so no door on this database adds those sets." + - TO, otherwise: "Under the POSTURE posture no ceremony step carries binding rows (ADR-0131 D3). The metadata door reports the definition of NAME not editable (lock LOCK), so no door on this database adds those sets." + - When the door's verdict cannot be read, the text says so and names no act. + +Whether a definition is editable is the metadata door's own answer, the `editable` flag of its item read, the same verdict the ceremony already asks before its one in-place write. + +**Unchanged.** + +- Under `single`, the pointer at `authored-conversion` stays, because that step carries the bindings there. +- A built-in position that does not resolve because the boot did not compose the security plugin keeps that composition's remedy, under every posture. +- The `conflicting` blocker, for a name that organizations bind differently, keeps its text. It states the fact and names no act, because no door makes the rows agree. +- `authored-conversion`'s walled note is unchanged. The rewrite gate's blocker is where the operator reads about binding rows. 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 892eb08dd7c..578ff41b425 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 @@ -13,18 +13,28 @@ * The catalog: the apply command's boot (`bootSchemaStack` with its four * composition flags) over a compiled artifact, with an auth secret set so the * security plugin is composed, read through core's security catalog reader as - * the command reads it. The database: the 17.x catalog rows every environment - * database holds — the seeded `everyone` position, the seeded `member_default` - * set and the seeded binding between them — plus the control: a seeded binding - * that names a position no catalog declares. + * the command reads it. The artifact declares two positions in code: + * `zz_in_code` naming `member_default`, and `zz_shipped` naming no set. The + * database: the 17.x catalog rows every environment database holds — the + * seeded `everyone` position, the seeded `member_default` set and the seeded + * binding between them — plus the control: a seeded binding that names a + * position no catalog declares. * * ## What each case measures * * The catalog holds the six built-ins and `everyone`'s baseline. Under * `single`, `authored-conversion` blocks on the control and on nothing else; * under a wall it reports, and the rewrite gate blocks on the control and on - * nothing else. With the declarations made from `start()` and `kernel:ready`, - * which this boot never runs, both block on `everyone` as well. + * nothing else, naming the act the wall allows: declare the position with its + * sets, through the deployment's code or the metadata door. With the + * declarations made from `start()` and `kernel:ready`, which this boot never + * runs, both block on `everyone` as well. + * + * [#23542] Under a wall, the metadata door's own lock verdict — read as the + * command reads it — tells the gate which definitions no door edits: a binding + * the artifact's declaration already names clears the gate (the code door), and + * one that adds a set to `zz_shipped`, a definition a package ships, gets the + * fact and no act (ADR-0131 D6). */ import { afterAll, beforeAll, describe, expect, it } from 'vitest'; @@ -52,6 +62,7 @@ import '@objectstack/platform-objects/plugin'; import '@objectstack/plugin-security'; const CASE_TIMEOUT_MS = 180_000; +const PACKAGE_ID = 'com.example.ownership-catalog'; const ENV_KEYS = ['OS_ARTIFACT_PATH', 'OS_DATABASE_URL', 'DATABASE_URL', 'OS_DATABASE_DRIVER', 'OS_HOME', 'NODE_ENV', 'OS_AUTH_SECRET', 'AUTH_SECRET', 'BETTER_AUTH_SECRET', 'OS_ENVIRONMENT_ID', 'OS_MULTI_ORG_ENABLED'] as const; const savedEnv: Record = {}; @@ -70,7 +81,11 @@ beforeAll(async () => { process.env.OS_HOME = join(dir, 'home'); process.env.OS_ARTIFACT_PATH = join(dir, 'dist', 'objectstack.json'); writeFileSync(process.env.OS_ARTIFACT_PATH, JSON.stringify({ - manifest: { id: 'com.example.ownership-catalog', name: 'ownership-catalog', version: '1.0.0', type: 'app' }, + manifest: { id: PACKAGE_ID, name: 'ownership-catalog', version: '1.0.0', type: 'app' }, + positions: [ + { name: 'zz_in_code', label: 'In code', permissionSets: ['member_default'] }, + { name: 'zz_shipped', label: 'Shipped' }, + ], })); // The apply command's boot, flag for flag. stack = await bootSchemaStack({ @@ -134,7 +149,16 @@ async function ceremony(posture: string, db: Database.Database): Promise [] }, - lock: { verdict: async () => ({ editable: true }) }, + // The save door's own verdict, as the command reads it: the item read's `editable`. + lock: { + verdict: async (type, name, packageId) => { + const protocol = (stack.kernel as { getService(name: string): unknown }).getService('protocol') as { + getMetaItem(request: { type: string; name: string; packageId?: string }): Promise<{ editable?: unknown; lock?: unknown } | null>; + }; + const read = await protocol.getMetaItem({ type, name, ...(packageId ? { packageId } : {}) }); + return { editable: read?.editable !== false, ...(typeof read?.lock === 'string' ? { lock: read.lock } : {}) }; + }, + }, door: { save: async (type, name) => { throw new Error(`the door was asked to save ${type} ${name}`); } }, templates: { find: async () => undefined }, columnDrops: { drop: async (columns) => columns.map((c) => ({ table: c.table, column: c.column, outcome: 'not-proposed' as const })) }, @@ -178,7 +202,7 @@ describe('the ownership ceremony resolves bindings against what a started kernel expect(rewrite).toMatchObject({ status: 'done', summary: { grants: 2, verified: 2 } }); }, CASE_TIMEOUT_MS); - it('walled: the rewrite gate blocks on the undeclared position, and not on everyone', async () => { + it('walled: the rewrite gate blocks on the undeclared position, and not on everyone, naming the act the wall allows', async () => { const report = await ceremony('isolated', v17Database('walled')); expect(step(report, 'authored-conversion').status).toBe('done'); const rewrite = step(report, 'rewrite'); @@ -186,5 +210,29 @@ describe('the ownership ceremony resolves bindings against what a started kernel expect(rewrite.blockers).toHaveLength(1); expect(rewrite.blockers[0]).toContain('position zz_undeclared'); expect(rewrite.blockers.join('\n')).not.toContain('everyone'); + // Not the step that reported done: the declaration, through either of its two doors. + expect(rewrite.blockers[0]).not.toMatch(/resolve its blocker/); + expect(rewrite.blockers[0]).toContain('deployment\'s code'); + expect(rewrite.blockers[0]).toContain('PUT /api/v1/meta/position/zz_undeclared'); + }, CASE_TIMEOUT_MS); + + it('walled: a binding the code declaration names clears the gate; one a sealed package definition lacks gets the fact, and no act', async () => { + const db = v17Database('walled-declared'); + db.exec(` + INSERT INTO sys_position VALUES + ('pos_in_code', NULL, 'zz_in_code', 'In code', 'platform', 0, 1), + ('pos_shipped', NULL, 'zz_shipped', 'Shipped', 'platform', 0, 1); + INSERT INTO sys_position_permission_set VALUES + ('bind_in_code', NULL, 'pos_in_code', 'ps_member_default'), + ('bind_shipped', NULL, 'pos_shipped', 'ps_member_default'); + `); + const rewrite = step(await ceremony('isolated', db), 'rewrite'); + expect(rewrite.status).toBe('blocked'); + expect(rewrite.blockers.join('\n')).not.toContain('zz_in_code'); + const sealed = rewrite.blockers.filter((b) => b.includes('position zz_shipped ')); + expect(sealed).toHaveLength(1); + expect(sealed[0]).toContain(PACKAGE_ID); + expect(sealed[0]).toContain('ADR-0131 D6'); + expect(sealed[0]).not.toMatch(/\b(Declare|Add|remove|resolve)\b|PUT |re-run/); }, CASE_TIMEOUT_MS); }); diff --git a/packages/cli/src/utils/organization-ownership-apply.test.ts b/packages/cli/src/utils/organization-ownership-apply.test.ts index 6946bd50dd2..7b3db21b643 100644 --- a/packages/cli/src/utils/organization-ownership-apply.test.ts +++ b/packages/cli/src/utils/organization-ownership-apply.test.ts @@ -2099,7 +2099,7 @@ describe('ADR-0131 D10 --apply — a name only the uncomposed security plugin de expect(deletes()).toEqual([]); }); - it('under a wall, the rewrite gate: the everyone binding gets the composition; an undeclared position keeps its remedy', async () => { + it('under a wall, the rewrite gate: the everyone binding gets the composition; an undeclared position gets the walled remedy', async () => { db.exec(v17Bindings); const report = await apply(catalog(), ledger(), { posture: 'isolated', securityPlugin: NO_SECRET }); @@ -2108,7 +2108,8 @@ describe('ADR-0131 D10 --apply — a name only the uncomposed security plugin de expect(rewrite.status).toBe('blocked'); const undeclared = blockerOf(rewrite, 'position zz_undeclared '); expect(undeclared).not.toContain(COMPOSITION); - expect(undeclared).toMatch(/resolve its blocker/); + expect(undeclared).toContain('PUT /api/v1/meta/position/zz_undeclared'); + expect(undeclared).not.toMatch(/resolve its blocker/); const everyone = blockerOf(rewrite, 'position everyone '); expect(everyone).toContain(COMPOSITION); expect(everyone).not.toMatch(/\bremove\b/); @@ -2135,7 +2136,140 @@ describe('ADR-0131 D10 --apply — a name only the uncomposed security plugin de `); const rewrite = step(await apply(catalog(), ledger(), { posture: 'isolated', securityPlugin }), 'rewrite'); expect(blockerOf(rewrite, 'the permission set admin_full_access,')).toMatch(/or remove the grants/); - expect(blockerOf(rewrite, 'position everyone ')).toMatch(/resolve its blocker/); + // Not the pointer. This fake catalog declares no `everyone` (a composed boot declares it, sealed), so which + // walled remedy follows is not this control's question. + expect(blockerOf(rewrite, 'position everyone ')).not.toMatch(/resolve its blocker/); expect(rewrite.blockers.join('\n')).not.toContain(COMPOSITION); }); }); + +describe('ADR-0131 D10 --apply — under a wall, the rewrite gate names the act the wall allows for a converted binding', () => { + type Step = OrganizationOwnershipApplyReport['steps'][number]; + const step = (report: OrganizationOwnershipApplyReport, name: Step['step']): Step => report.steps.find((s) => s.step === name)!; + /** The one blocker of `s` that names `subject`. */ + const blockerOf = (s: Step, subject: string): string => { + const found = s.blockers.filter((b) => b.includes(subject)); + expect(found, subject).toHaveLength(1); + return found[0]!; + }; + /** The remedy half of a binding blocker: what follows the fact. */ + const remedyOf = (blocker: string): string => blocker.slice(blocker.indexOf('deleting them would lose a grant.')); + /** Every DELETE the run issued, beside the settings rung that moves before the rewrite. */ + const deletes = (): string[] => writes.filter((w) => /^DELETE/.test(w) && !/^DELETE FROM "sys_setting"/.test(w)); + /** An act, a door or a re-run: what a blocker with no sanctioned act never names. */ + const ACT = /\b(Declare|Add|remove|resolve)\b|PUT |re-run/; + const POINTER = 'The authored-conversion step carries them; resolve its blocker, then re-run.'; + /** The metadata door's lock verdict, answering `answer` and recording what it was asked. */ + const lockAnswering = (answer: Awaited> | Error): MetadataLockPort & { asked: string[] } => { + const asked: string[] = []; + return { + asked, + async verdict(type, name, packageId) { + asked.push(`${type}|${name}|${packageId}`); + if (answer instanceof Error) throw answer; + return answer; + }, + }; + }; + /** A seeded position no catalog declares, its row binding member_default. */ + const UNDECLARED = ` + INSERT INTO sys_position VALUES ('pos_gone', NULL, 'zz_undeclared', 'Undeclared', 'platform', 0, 1); + INSERT INTO sys_position_permission_set VALUES ('bind_gone', NULL, 'pos_gone', 'ps_null'); + `; + /** A seeded position whose rows bind member_default and zz_extra; the catalog's definition names member_default only. */ + const DECLARED = ` + INSERT INTO sys_permission_set VALUES ('ps_extra', NULL, 'zz_extra', 'Extra', 'platform', 1, '{}', '[]'); + INSERT INTO sys_position VALUES ('pos_env', NULL, 'zz_env', 'Env', 'platform', 0, 1); + INSERT INTO sys_position_permission_set VALUES ('bind_env_1', NULL, 'pos_env', 'ps_null'), ('bind_env_2', NULL, 'pos_env', 'ps_extra'); + `; + /** The catalog, declaring zz_env as `definition` from the next boot on. */ + const declaring = async (definition: Record): Promise> => { + const c = catalog(); + await c.save('position', 'zz_env', { name: 'zz_env', label: 'Env', permissionSets: ['member_default'], ...definition }); + return c; + }; + + it('⭐ undeclared: declare it with the bound sets through ONE of its two doors — never the step that reports done; nothing is deleted', async () => { + db.exec(UNDECLARED); + const lock = lockAnswering({ editable: true }); + const report = await apply(catalog(), ledger(), { posture: 'isolated', lock }); + + expect(step(report, 'authored-conversion')).toMatchObject({ status: 'done', blockers: [] }); + const rewrite = step(report, 'rewrite'); + expect(rewrite.status).toBe('blocked'); + const remedy = remedyOf(blockerOf(rewrite, 'position zz_undeclared ')); + expect(remedy).not.toContain(POINTER); + expect(remedy).toContain('member_default'); + // Both doors, and one only: the deployment's code, or the metadata door as the platform administrator. + expect(remedy).toContain('deployment\'s code'); + expect(remedy).toContain('PUT /api/v1/meta/position/zz_undeclared'); + expect(remedy).toMatch(/one door only/); + // The organization-authored position the base fixture binds is undeclared too, and gets the same act. + expect(remedyOf(blockerOf(rewrite, 'position sales_lead '))).toContain('PUT /api/v1/meta/position/sales_lead'); + // An undeclared name has no definition for the door to judge. + expect(lock.asked).toEqual([]); + // Still refused: nothing is deleted or guessed. + expect(step(report, 'mirror-deletion').status).toBe('not-reached'); + expect(deletes()).toEqual([]); + }); + + it('⭐ declared and editable: add the missing sets through the metadata door alone, as the door itself answers', async () => { + db.exec(DECLARED); + const lock = lockAnswering({ editable: true }); + const report = await apply(await declaring({}), ledger(), { posture: 'group', lock }); + + const remedy = remedyOf(blockerOf(step(report, 'rewrite'), 'position zz_env ')); + expect(remedy).not.toContain(POINTER); + expect(remedy).toContain('zz_extra'); + expect(remedy).not.toContain('member_default'); + expect(remedy).toContain('PUT /api/v1/meta/position/zz_env'); + // The environment holds the name: the code door would refuse the cold boot. + expect(remedy).not.toContain('deployment\'s code'); + expect(lock.asked).toEqual(['position|zz_env|null']); + expect(deletes()).toEqual([]); + }); + + it.each([ + ['a package ships it (ADR-0131 D6)', { _packageId: 'com.example.vendor', _provenance: 'package' }, { editable: false, lock: 'no-overlay' }, ['com.example.vendor', 'ADR-0131 D6']], + ['an item lock', {}, { editable: false, lock: 'full', lockReason: 'frozen by the operator' }, ['full', 'frozen by the operator']], + ['the door\'s verdict is unreadable', {}, new Error('no metadata protocol is composed on this stack'), ['no metadata protocol is composed on this stack']], + ] as const)('⭐ declared and not editable — %s: the fact, and no act', async (_label, definition, answer, facts) => { + db.exec(DECLARED); + const lock = lockAnswering(answer as Awaited> | Error); + const report = await apply(await declaring(definition), ledger(), { posture: 'isolated', lock }); + + const blocker = blockerOf(step(report, 'rewrite'), 'position zz_env '); + for (const fact of facts) expect(blocker).toContain(fact); + expect(blocker).not.toMatch(ACT); + expect(lock.asked).toEqual(['position|zz_env|null']); + expect(deletes()).toEqual([]); + }); + + it('conflicting, under a wall: the fact, and no act', async () => { + db.exec(` + INSERT INTO sys_position VALUES ('pos_c1', 'org_default', 'zz_conflict', 'C', 'platform', 0, 1), + ('pos_c2', 'org_b', 'zz_conflict', 'C', 'platform', 0, 1); + INSERT INTO sys_position_permission_set VALUES ('bind_c1', 'org_default', 'pos_c1', 'ps_null'); + `); + const report = await apply(catalog(), ledger(), { posture: 'isolated' }); + + const blocker = blockerOf(step(report, 'rewrite'), 'Position zz_conflict:'); + expect(blocker).not.toMatch(ACT); + expect(deletes()).toEqual([]); + }); + + it('control — single: the gate keeps the pointer at authored-conversion, and asks the door nothing', async () => { + const c = catalog(); + const l = ledger(); + await expect(apply(c, l, { executor: executor({ failOnceOn: /^DELETE FROM "sys_capability"/ }) })).rejects.toThrow(/killed mid-step/); + db.exec(UNDECLARED); + const lock = lockAnswering({ editable: true }); + + const resumed = await apply(c, l, { lock }); + + const deletion = step(resumed, 'mirror-deletion'); + expect(deletion.status).toBe('blocked'); + expect(remedyOf(blockerOf(deletion, 'position zz_undeclared '))).toBe(`deleting them would lose a grant. ${POINTER}`); + expect(lock.asked).toEqual([]); + }); +}); diff --git a/packages/cli/src/utils/organization-ownership-apply.ts b/packages/cli/src/utils/organization-ownership-apply.ts index d3dd0eda148..0fd9290a42f 100644 --- a/packages/cli/src/utils/organization-ownership-apply.ts +++ b/packages/cli/src/utils/organization-ownership-apply.ts @@ -1970,6 +1970,68 @@ async function unnamedForComposition(run: Run, ids: readonly string[]): Promise< return out; } +/** + * [#23542] The remedy for a `converted` binding — rows that bind position + * `name` to sets its catalog definition does not name; `sets` is the + * conversion's answer, the definition's sets followed by the ones the rows add + * — when the gate refuses it under a wall. There no step carries binding rows: + * `authored-conversion` reports and returns before it reads one (ADR-0131 D3, + * the module doc), so pointing at that step points at nothing. + * + * The one act that clears the gate is a definition naming every bound set, + * which the operator makes, through the door the position's definition lives + * behind: + * + * - undeclared: declare it with `sets`, in the deployment's code or through + * the metadata door as the platform administrator — one of the two only, as + * a cold boot refuses a name a package declares and the environment catalog + * also holds; + * - declared, and the metadata door reports it editable: add the missing sets + * to its `permissionSets` through that door; + * - declared, and not editable (ADR-0131 D6: a definition a package ships is + * sealed; or an ADR-0010 lock), or its state unreadable: the fact — the + * package that ships it, when the definition's provenance names one — and + * no act, because no door adds the sets. + * + * Whether it is editable is the metadata door's own answer + * ({@link MetadataLockPort}), never inferred from the body; the body only + * names the shipping package. The gate still refuses; nothing here writes. + */ +async function walledBindingRemedy(run: Run, name: string, sets: readonly string[]): Promise { + const why = `Under the ${run.opts.posture} posture no ceremony step carries binding rows (ADR-0131 D3).`; + const door = `the metadata door as the platform administrator (PUT /api/v1/meta/position/${name})`; + const entry = await resolveName(run, 'position', name); + if (!entry) { + return ( + `${why} Declare the position ${name} with permissionSets naming ${sets.join(', ')}, ` + + `through one door only: the deployment's code (compiled into the artifact this ceremony boots), or ${door}. A ` + + 'cold boot refuses a position name a package declares and the environment catalog also holds. Then re-run.' + ); + } + let verdict: Awaited>; + try { + verdict = await run.opts.lock.verdict('position', name, null); + } catch (error) { + return ( + `${why} Whether the metadata door can edit the definition of ${name} could not be read ` + + `(${error instanceof Error ? error.message : String(error)}), so no act is named here.` + ); + } + if (!verdict.editable) { + const lock = `lock ${verdict.lock ?? 'unknown'}${verdict.lockReason ? `: ${verdict.lockReason}` : ''}`; + const { isCodeArtifactBody } = await import('@objectstack/metadata-core'); + const shipped = isCodeArtifactBody(entry.definition) ? String(entry.definition._packageId) : undefined; + return shipped + ? `${why} Package ${shipped} ships the definition of ${name}, which is sealed (ADR-0131 D6: the metadata door ` + + `reports it not editable, ${lock}), so no door on this database adds those sets.` + : `${why} The metadata door reports the definition of ${name} not editable (${lock}), so no door on this ` + + 'database adds those sets.'; + } + const declared = Array.isArray(entry.definition.permissionSets) ? entry.definition.permissionSets.map(String) : []; + const missing = sets.filter((set) => !declared.includes(set)); + return `${why} Add ${missing.join(', ')} to the permissionSets of the position ${name} through ${door}, then re-run.`; +} + /** Read-only: does every reference resolve by name? The gate of fate 2. */ async function verifyReferences(run: Run): Promise { const out: Verification = { blockers: [], notes: [], grants: 0, verified: 0 }; @@ -2052,7 +2114,10 @@ async function verifyReferences(run: Run): Promise { position.fate === 'conflicting' ? `Position ${position.name}: organizations bind different permission sets to it; the bindings cannot be carried into one definition.` : `Binding rows give position ${position.name} sets its catalog definition does not name (${position.permissionSets.join(', ')}); ` + - `deleting them would lose a grant. ${(await uncomposedSecurityRemedy(run, 'position', position.name)) ?? 'The authored-conversion step carries them; resolve its blocker, then re-run.'}`, + `deleting them would lose a grant. ${(await uncomposedSecurityRemedy(run, 'position', position.name)) ?? + (run.opts.posture === 'single' + ? 'The authored-conversion step carries them; resolve its blocker, then re-run.' + : await walledBindingRemedy(run, position.name, position.permissionSets))}`, ); } }