diff --git a/.changeset/21490-install-local-uninstall-cleanups.md b/.changeset/21490-install-local-uninstall-cleanups.md new file mode 100644 index 00000000000..d6af060c807 --- /dev/null +++ b/.changeset/21490-install-local-uninstall-cleanups.md @@ -0,0 +1,18 @@ +--- +'@objectstack/cloud-connection': patch +'@objectstack/metadata-protocol': minor +--- + +fix(cloud-connection): an install-local uninstall runs the protocol's registered uninstall cleanups, so the package's permission sets and their grants go with it + +Clause-②: yes + +`DELETE /api/v1/marketplace/install-local/:manifestId` removed the package's ledger entry and nothing else. After a restart the package's objects were gone, but its `managed_by: package` rows in `sys_permission_set`, and every grant of them, survived the uninstall. That broke ADR-0090's "No ghost grants" promise on this door. + +The door now runs the uninstall cleanups that domain plugins register with the protocol (`registerUninstallCleanup`) once the ledger entry is gone. It uses the same registry and the same runner as the protocol's own uninstall, so `plugin-security`'s `security.package-permissions` cleanup removes the package's sets with their position and user bindings, and any cleanup registered later fires here too. The cleanups run with the package's manifest id and no organization, because an install-local package is installed for the whole runtime. + +The response carries each outcome as `data.cleanups`, the way the protocol's uninstall reports them. A failed cleanup is reported there and named in the operator log with its remedy (install the package again, then uninstall it again). When the protocol cannot run the cleanups, the response says so as one failed `protocol.runUninstallCleanups` outcome. An uninstall that does not happen (a refused caller, an id this door never installed, a ledger write that fails) revokes nothing. + +`@objectstack/metadata-protocol`: `ObjectStackProtocolImplementation` gains `runUninstallCleanups({ packageId, organizationId?, actor? })`, the one runner of the uninstall-cleanup registry. It runs every registered cleanup for the package and answers one `UninstallCleanupOutcome` per cleanup. It never throws: a failed cleanup is an outcome, and a thrown fault's driver text goes to the operator log, not into the outcome. `deletePackage` now calls it as its last step in place of its own loop, and its `cleanups` are unchanged. The only visible difference there is the log tag of a failed cleanup's warning, now `[protocol.runUninstallCleanups]` instead of `[protocol.deletePackage]`. + +`@objectstack/cloud-connection` now declares its dependency on `@objectstack/metadata-protocol`, which it already received through `@objectstack/runtime`, for the cleanup outcome types. diff --git a/packages/cli/test/package-install-local-uninstall-cleanups.integration.test.ts b/packages/cli/test/package-install-local-uninstall-cleanups.integration.test.ts new file mode 100644 index 00000000000..72bd1cc16ef --- /dev/null +++ b/packages/cli/test/package-install-local-uninstall-cleanups.integration.test.ts @@ -0,0 +1,452 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #21490 — an install-local uninstall (`DELETE /api/v1/marketplace/install-local/:id`) + * runs the protocol's registered uninstall cleanups, so the package's permission + * sets and their bindings die with it (ADR-0090: "No ghost grants"). + * + * ## The defect, measured at this door on `main` 9ff74285f1 + * + * A package installed through install-local declares a permission set, and the + * set is projected into `sys_permission_set` with `managed_by: package` (#21322 + * made that happen on the hot install, as a restart already did). The DELETE + * answered 200 and removed the ledger entry, and nothing else: the package's + * object answered 404 after a restart, but its `sys_permission_set` row — and + * every grant of it — survived, on both orders of events: + * + * - hot install → DELETE → restart; + * - install → restart → DELETE → restart. + * + * The protocol's own uninstall (`deletePackage`) runs every cleanup a domain + * plugin registered through `registerUninstallCleanup` — plugin-security's + * `security.package-permissions` is the one that removes package-owned sets with + * their position and user bindings. This door never ran that registry. + * + * ## What each `it` reads + * + * One fixture, two orders of events, each probed through the data route a user + * and an admin use: the set by name, the user grant of it by set id, and — after + * the restart — the package's object. The grant is made through the data door + * before the uninstall, so "no binding" is read off a row that existed, not off + * an empty table. + * + * A third order of events measures the re-seed window — DELETE, then a hot + * install of ANOTHER package, then restart — and records, as `it.fails`, the + * defect it found there; the block above that `describe` says what it is. + * + * ## Spawn shape + * + * Shared with `package-install-local-boot-steps.integration.test.ts` (#21322): + * the tsx source entry, one process group per `os start`, every workspace + * package — `@objectstack/cloud-connection` and `@objectstack/metadata-protocol` + * included — resolved through its `exports` to `dist/`, so an ablation of either + * package's source reaches this file only after that package is rebuilt. Every + * boot runs in `beforeAll`; the `it`s only read what the phases recorded. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { spawn, type ChildProcess } from 'node:child_process'; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { + CLI, + childEnv, + E2E_SECRET_KEY, + portContentionError, + portDriftError, + probeThroughChild, + randomPort, + TSX, +} from './helpers/serve-process.js'; + +/** The banner's tail — every row above it has printed. */ +const READY = /Press Ctrl\+C to stop/; +const BOOT_TIMEOUT_MS = 180_000; + +const APP_ID = 'com.example.tasksapp'; +const TASK = 'tasks_app_task'; +const PERMISSION_SET = 'tasks_app_task_user'; +const UNINSTALL = `/api/v1/marketplace/install-local/${APP_ID}`; +/** The development dev-admin seed — see the #21321 sibling for why it is the operator on every boot. */ +const EMAIL = 'admin@objectos.ai'; +const PASSWORD = 'admin123'; + +/** `dist/objectstack.json` as `os build` writes it for this app (schema defaults trimmed). */ +const ARTIFACT = { + manifest: { id: APP_ID, namespace: 'tasks_app', version: '0.1.0', type: 'app', name: 'Tasks App' }, + objects: [{ + name: TASK, + label: 'Task', + sharingModel: 'public_read_write', + fields: { name: { type: 'text', label: 'Name' } }, + }], + permissions: [{ + name: PERMISSION_SET, + label: 'Tasks App Task User', + objects: { [TASK]: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: false } }, + }], +}; + +/** + * A second, unrelated package for the re-seed order of events: its hot install + * announces `metadata:reloaded`, which re-runs plugin-security's + * declared-permission seeding over every package the running kernel still holds. + */ +const OTHER_APP_ID = 'com.example.notesapp'; +const OTHER_ARTIFACT = { + manifest: { id: OTHER_APP_ID, namespace: 'notes_app', version: '0.1.0', type: 'app', name: 'Notes App' }, + objects: [{ + name: 'notes_app_note', + label: 'Note', + sharingModel: 'public_read_write', + fields: { name: { type: 'text', label: 'Name' } }, + }], +}; + +const groups: ChildProcess[] = []; +const dirs: string[] = []; + +interface LiveStart { + child: ChildProcess; + base: string; + output: () => string; +} + +function bootStart(cwd: string, home: string, port: string): Promise { + return new Promise((resolveBoot, rejectBoot) => { + const child = spawn(TSX, [CLI, 'start', '-p', port, '--home', home, '--auth-secret', E2E_SECRET_KEY, '--no-ui'], { + cwd, + // `childEnv`, never a bare `...process.env` — see its header. + env: childEnv({ NO_COLOR: '1', OS_CLOUD_URL: 'off', OS_LOG_LEVEL: 'warn', OS_SECRET_KEY: E2E_SECRET_KEY }), + stdio: ['ignore', 'pipe', 'pipe'], + // Own process group: `os start` supervises a `serve` grandchild. + detached: true, + }); + groups.push(child); + let out = ''; + let settled = false; + const settle = (err: Error | null) => { + if (settled) return; + settled = true; + clearTimeout(timer); + if (err) rejectBoot(err); + else resolveBoot({ child, base: `http://localhost:${port}`, output: () => out }); + }; + const timer = setTimeout( + () => settle(new Error(`os start never printed ${READY}\n--- output ---\n${out.slice(-4000)}`)), + BOOT_TIMEOUT_MS, + ); + const onData = (d: unknown) => { + out += String(d); + // The child is the authority on the port it bound. + if (READY.test(out)) settle(portDriftError(out, 'os start', port)); + }; + child.stdout?.on('data', onData); + child.stderr?.on('data', onData); + child.on('exit', (code) => + settle(portContentionError(out, 'os start', port) + ?? new Error(`os start exited ${String(code)} before ${READY}\n--- output ---\n${out.slice(-4000)}`)), + ); + }); +} + +async function stopGroup(child: ChildProcess): Promise { + if (child.pid === undefined || child.exitCode !== null || child.signalCode !== null) return; + await new Promise((done) => { + const give = setTimeout(() => { + try { process.kill(-child.pid!, 'SIGKILL'); } catch { /* group already gone */ } + done(); + }, 15_000); + child.once('exit', () => { clearTimeout(give); done(); }); + try { process.kill(-child.pid!, 'SIGTERM'); } catch { clearTimeout(give); done(); } + }); +} + +interface Answer { status: number; body: any } + +/** One exchange against the running `os start`, attributed to the child if the transport fails. ⛔ No assertion inside it. */ +function http(live: LiveStart, method: string, path: string, token: string, body?: unknown): Promise { + return probeThroughChild( + { + child: live.child, + transcript: () => `\n--- child output ---\n${live.output().slice(-4000)}`, + label: 'package-install-local-uninstall-cleanups', + what: `${method} ${path}`, + }, + async () => { + const r = await fetch(`${live.base}${path}`, { + method, + headers: { + origin: live.base, + ...(body !== undefined ? { 'content-type': 'application/json' } : {}), + ...(token ? { authorization: `Bearer ${token}` } : {}), + }, + ...(body !== undefined ? { body: JSON.stringify(body) } : {}), + }); + const text = await r.text(); + let parsed: any = text; + try { parsed = JSON.parse(text); } catch { /* keep the text */ } + return { status: r.status, body: parsed }; + }, + ); +} + +interface Session { token: string; userId: string } + +async function authenticate(live: LiveStart): Promise { + const res = await http(live, 'POST', '/api/v1/auth/sign-in/email', '', { email: EMAIL, password: PASSWORD }); + const token = res.body?.token; + const userId = res.body?.user?.id; + if (res.status !== 200 || typeof token !== 'string' || typeof userId !== 'string') { + throw new Error(`auth answered ${res.status}: ${JSON.stringify(res.body)}\n--- output ---\n${live.output().slice(-3000)}`); + } + return { token, userId }; +} + +/** + * `os package install ./dist/objectstack.json` against the running runtime. + * ⛔ Asynchronous on purpose — see the #21321 sibling: a `spawnSync` stops this + * process draining the server's pipes for the whole install. + */ +function packageInstall(appDir: string, live: LiveStart): Promise<{ exit: number | null; output: string }> { + return new Promise((done) => { + const child = spawn(TSX, [CLI, 'package', 'install', './dist/objectstack.json', '--runtime', live.base, '--email', EMAIL, '--password', PASSWORD], { + cwd: appDir, + env: childEnv({ NO_COLOR: '1' }), + stdio: ['ignore', 'pipe', 'pipe'], + }); + let output = ''; + child.stdout?.on('data', (d) => { output += String(d); }); + child.stderr?.on('data', (d) => { output += String(d); }); + const timer = setTimeout(() => child.kill('SIGKILL'), 120_000); + child.on('close', (code) => { clearTimeout(timer); done({ exit: code, output }); }); + }); +} + +/** The rows of a `GET /api/v1/data/:object` list answer, whichever envelope it came in. */ +function rowsOf(answer: Answer): any[] { + const b = answer.body?.data ?? answer.body; + if (Array.isArray(b)) return b; + if (Array.isArray(b?.records)) return b.records; + if (Array.isArray(b?.items)) return b.items; + return []; +} + +const recordOf = (a: Answer) => a.body?.data ?? a.body?.record ?? a.body; + +/** What the data route answers about the package's grants at one moment. */ +interface Grants { + /** `GET /data/sys_permission_set?name=…` — the projection the admin surface grants from. */ + sets: Answer; + /** `GET /data/sys_user_permission_set?permission_set_id=…` — the user grant made before the uninstall. */ + bindings: Answer; +} + +async function readGrants(live: LiveStart, token: string, setId: string): Promise { + return { + sets: await http(live, 'GET', `/api/v1/data/sys_permission_set?name=${PERMISSION_SET}`, token), + bindings: await http(live, 'GET', `/api/v1/data/sys_user_permission_set?permission_set_id=${encodeURIComponent(setId)}`, token), + }; +} + +/** One order of events, recorded phase by phase; the `it`s read it. */ +interface Run { + install?: { exit: number | null; output: string }; + /** The set and its grant, as they stood right before the DELETE. */ + before?: Grants; + /** `POST /data/sys_user_permission_set` — the grant whose survival is the defect's second half. */ + granted?: Answer; + setId?: string; + uninstall?: Answer; + /** Same process, right after the DELETE answered. */ + after?: Grants; + /** A restart on the same home. */ + restarted?: Grants; + /** The package's object after the restart — the uninstall's own effect, as the control. */ + object?: Answer; + /** Re-seed order only: the hot install of {@link OTHER_APP_ID} after the DELETE. */ + otherInstall?: { exit: number | null; output: string }; + /** Re-seed order only: same process, right after that second install. */ + afterOtherInstall?: Grants; +} + +/** + * Read the set, grant it to the operator, DELETE the package, read again. + * The grant is made here, on whichever boot runs the uninstall, so it is + * the same act on both orders of events. + */ +async function grantThenUninstall(live: LiveStart, session: Session, run: Run): Promise { + const sets = await http(live, 'GET', `/api/v1/data/sys_permission_set?name=${PERMISSION_SET}`, session.token); + const setId = rowsOf(sets)[0]?.id; + if (typeof setId !== 'string') { + throw new Error(`precondition: no ${PERMISSION_SET} row to grant — ${sets.status} ${JSON.stringify(sets.body)}\n--- output ---\n${live.output().slice(-3000)}`); + } + run.setId = setId; + run.granted = await http(live, 'POST', '/api/v1/data/sys_user_permission_set', session.token, { + user_id: session.userId, + permission_set_id: setId, + }); + run.before = await readGrants(live, session.token, setId); + run.uninstall = await http(live, 'DELETE', UNINSTALL, session.token); + run.after = await readGrants(live, session.token, setId); +} + +async function readAfterRestart(live: LiveStart, run: Run): Promise { + const { token } = await authenticate(live); + run.restarted = await readGrants(live, token, run.setId!); + run.object = await http(live, 'GET', `/api/v1/data/${TASK}`, token); +} + +const runs: Record<'hot' | 'restarted' | 'reseed', Run> = { hot: {}, restarted: {}, reseed: {} }; + +beforeAll(async () => { + const root = mkdtempSync(join(tmpdir(), 'install-local-uninstall-')); + dirs.push(root); + const appDir = join(root, 'app'); + mkdirSync(join(appDir, 'dist'), { recursive: true }); + writeFileSync(join(appDir, 'dist', 'objectstack.json'), JSON.stringify(ARTIFACT, null, 2), 'utf8'); + const otherAppDir = join(root, 'other-app'); + mkdirSync(join(otherAppDir, 'dist'), { recursive: true }); + writeFileSync(join(otherAppDir, 'dist', 'objectstack.json'), JSON.stringify(OTHER_ARTIFACT, null, 2), 'utf8'); + const port = randomPort(); + + // ── order 1: hot install → DELETE → restart ──────────────────────────── + const hotDir = join(root, 'hot'); + mkdirSync(hotDir, { recursive: true }); + const hotHome = join(hotDir, 'home'); + const a = await bootStart(hotDir, hotHome, port); + const aSession = await authenticate(a); + runs.hot.install = await packageInstall(appDir, a); + await grantThenUninstall(a, aSession, runs.hot); + await stopGroup(a.child); + const b = await bootStart(hotDir, hotHome, port); + await readAfterRestart(b, runs.hot); + await stopGroup(b.child); + + // ── order 2: install → restart → DELETE → restart ────────────────────── + // The DELETE meets a package the ledger REHYDRATED, not one this process + // hot-installed — the path that existed before #21322's change. + const coldDir = join(root, 'cold'); + mkdirSync(coldDir, { recursive: true }); + const coldHome = join(coldDir, 'home'); + const c = await bootStart(coldDir, coldHome, port); + await authenticate(c); + runs.restarted.install = await packageInstall(appDir, c); + await stopGroup(c.child); + const d = await bootStart(coldDir, coldHome, port); + await grantThenUninstall(d, await authenticate(d), runs.restarted); + await stopGroup(d.child); + const e = await bootStart(coldDir, coldHome, port); + await readAfterRestart(e, runs.restarted); + await stopGroup(e.child); + + // ── order 3: hot install → DELETE → hot install of ANOTHER package → restart ── + // The DELETE does not withdraw the package from the running kernel, so it is + // still registered when the second install announces `metadata:reloaded`. + const reseedDir = join(root, 'reseed'); + mkdirSync(reseedDir, { recursive: true }); + const reseedHome = join(reseedDir, 'home'); + const f = await bootStart(reseedDir, reseedHome, port); + const fSession = await authenticate(f); + runs.reseed.install = await packageInstall(appDir, f); + await grantThenUninstall(f, fSession, runs.reseed); + runs.reseed.otherInstall = await packageInstall(otherAppDir, f); + runs.reseed.afterOtherInstall = await readGrants(f, fSession.token, runs.reseed.setId!); + await stopGroup(f.child); + const g = await bootStart(reseedDir, reseedHome, port); + await readAfterRestart(g, runs.reseed); + await stopGroup(g.child); +}, 8 * BOOT_TIMEOUT_MS); + +afterAll(async () => { + for (const child of groups) await stopGroup(child); + for (const dir of dirs) rmSync(dir, { recursive: true, force: true }); +}, 60_000); + +describe('#21490: an install-local uninstall runs the registered uninstall cleanups', () => { + for (const name of ['hot', 'restarted'] as const) { + describe(`${name === 'hot' ? 'hot install' : 'install → restart'} → DELETE → restart`, () => { + it('precondition: the install landed, and the set and its grant exist before the DELETE', () => { + const run = runs[name]; + expect(run.install?.exit, run.install?.output).toBe(0); + expect(run.granted?.status, JSON.stringify(run.granted?.body)).toBe(201); + expect(rowsOf(run.before!.sets).map((r) => [r?.name, r?.managed_by, r?.package_id])) + .toEqual([[PERMISSION_SET, 'package', APP_ID]]); + expect(rowsOf(run.before!.bindings).map((r) => r?.id)).toEqual([recordOf(run.granted!)?.id]); + }); + + it('the DELETE answers 200 and reports the security cleanup it ran', () => { + const run = runs[name]; + expect(run.uninstall?.status, JSON.stringify(run.uninstall?.body)).toBe(200); + const cleanups: any[] = run.uninstall?.body?.data?.cleanups ?? []; + const security = cleanups.find((o) => o?.name === 'security.package-permissions'); + expect(security, JSON.stringify(run.uninstall?.body)).toMatchObject({ success: true }); + }); + + it('right after the DELETE: no package-managed set, no grant of it', () => { + const run = runs[name]; + expect(run.after!.sets.status).toBe(200); + expect(rowsOf(run.after!.sets), JSON.stringify(run.after!.sets.body)).toEqual([]); + expect(run.after!.bindings.status).toBe(200); + expect(rowsOf(run.after!.bindings), JSON.stringify(run.after!.bindings.body)).toEqual([]); + }); + + it('after a restart: still no set and no grant, and the package object is gone', () => { + const run = runs[name]; + expect(run.object?.status, JSON.stringify(run.object?.body)).toBe(404); + expect(run.restarted!.sets.status).toBe(200); + expect(rowsOf(run.restarted!.sets), JSON.stringify(run.restarted!.sets.body)).toEqual([]); + expect(run.restarted!.bindings.status).toBe(200); + expect(rowsOf(run.restarted!.bindings), JSON.stringify(run.restarted!.bindings.body)).toEqual([]); + }); + }); + } + + // ── The re-seed window: MEASURED RED, reported for filing, not fixed here ── + // + // This DELETE leaves the package registered in the running kernel until the + // next restart (the response's own note says so), and plugin-security's + // `metadata:reloaded` subscriber re-runs the declared-permission seeding over + // every package the kernel holds. So another package's hot install before + // that restart re-projects the uninstalled package's set as a fresh + // `managed_by: package` row, and the restart leaves it orphaned: the package + // is gone, its set is not. The grant does NOT come back — the cleanup deleted + // the binding and the seeding writes none — and that half is pinned plainly. + // + // The two set readings are `it.fails`: each turns red the day its half is + // fixed, which is the cue to promote it to a plain assertion. + describe('hot install → DELETE → hot install of another package → restart (the re-seed window)', () => { + it('precondition: both installs landed, and the DELETE revoked the set and its grant', () => { + const run = runs.reseed; + expect(run.install?.exit, run.install?.output).toBe(0); + expect(run.otherInstall?.exit, run.otherInstall?.output).toBe(0); + expect(run.granted?.status, JSON.stringify(run.granted?.body)).toBe(201); + expect(rowsOf(run.before!.sets).map((r) => [r?.name, r?.managed_by, r?.package_id])) + .toEqual([[PERMISSION_SET, 'package', APP_ID]]); + expect(run.uninstall?.status, JSON.stringify(run.uninstall?.body)).toBe(200); + expect(rowsOf(run.after!.sets), JSON.stringify(run.after!.sets.body)).toEqual([]); + expect(rowsOf(run.after!.bindings), JSON.stringify(run.after!.bindings.body)).toEqual([]); + }); + + it('the grant stays revoked through the other install and the restart, and the package object is gone', () => { + const run = runs.reseed; + for (const answer of [run.afterOtherInstall!.sets, run.afterOtherInstall!.bindings, run.restarted!.sets, run.restarted!.bindings]) { + expect(answer.status, JSON.stringify(answer.body)).toBe(200); + } + expect(rowsOf(run.afterOtherInstall!.bindings), JSON.stringify(run.afterOtherInstall!.bindings.body)).toEqual([]); + expect(rowsOf(run.restarted!.bindings), JSON.stringify(run.restarted!.bindings.body)).toEqual([]); + expect(run.object?.status, JSON.stringify(run.object?.body)).toBe(404); + }); + + it.fails('KNOWN-BROKEN: the other package\'s hot install re-projects the uninstalled package\'s set (promote to a plain assertion once fixed)', () => { + const run = runs.reseed; + expect(rowsOf(run.afterOtherInstall!.sets), JSON.stringify(run.afterOtherInstall!.sets.body)).toEqual([]); + }); + + it.fails('KNOWN-BROKEN: that re-projected set survives the restart as an orphan row (promote to a plain assertion once fixed)', () => { + const run = runs.reseed; + expect(rowsOf(run.restarted!.sets), JSON.stringify(run.restarted!.sets.body)).toEqual([]); + }); + }); +}); diff --git a/packages/cloud-connection/package.json b/packages/cloud-connection/package.json index 214ae204eec..475bab1c952 100644 --- a/packages/cloud-connection/package.json +++ b/packages/cloud-connection/package.json @@ -25,6 +25,7 @@ }, "dependencies": { "@objectstack/core": "workspace:*", + "@objectstack/metadata-protocol": "workspace:*", "@objectstack/runtime": "workspace:*", "@objectstack/spec": "workspace:*", "@objectstack/types": "workspace:*" diff --git a/packages/cloud-connection/src/cloud-connection-route-ledger.ts b/packages/cloud-connection/src/cloud-connection-route-ledger.ts index 0adf6e197a6..db53775821d 100644 --- a/packages/cloud-connection/src/cloud-connection-route-ledger.ts +++ b/packages/cloud-connection/src/cloud-connection-route-ledger.ts @@ -223,7 +223,9 @@ export const CLOUD_CONNECTION_ROUTE_LEDGER: readonly CloudConnectionRouteLedgerE mountedIn: 'marketplace-install-local-plugin.ts', disposition: 'server-only', note: - 'removes the cached manifest from this runtime\'s disk; the kernel must restart to fully unload, since ' + 'removes the cached manifest from this runtime\'s disk and runs the uninstall cleanups registered with the ' + + 'protocol (the package\'s permission sets and their grants are revoked, each outcome reported as `cleanups`); ' + + 'the kernel must restart to fully unload, since ' + '`engine.registerApp` is additive only. A filesystem-mutating, restart-coupled operation local to one runtime ' + 'is deliberately not SDK surface — the CLI and the Console Setup view drive it. Requires `manage_metadata`, ' + 'never merely a signed-in session.', diff --git a/packages/cloud-connection/src/marketplace-install-local-plugin.ts b/packages/cloud-connection/src/marketplace-install-local-plugin.ts index 87b3ae89c68..f9929f2f700 100644 --- a/packages/cloud-connection/src/marketplace-install-local-plugin.ts +++ b/packages/cloud-connection/src/marketplace-install-local-plugin.ts @@ -36,9 +36,12 @@ * outright (#8976). * * DELETE /api/v1/marketplace/install-local/:manifestId - * → removes the cached manifest. Kernel must be restarted to fully - * unload — `engine.registerApp` is additive only. We document - * this in the response message. + * → removes the cached manifest, then runs the uninstall cleanups + * domain plugins registered with the protocol (#21490) — the + * package's permission sets and their grants go with it — and + * reports each outcome as `cleanups`. Kernel must be restarted to + * fully unload — `engine.registerApp` is additive only. We + * document this in the response message. * * Persistence layout: * /.objectstack/installed-packages/.json @@ -87,9 +90,36 @@ import { import { ConnectionCredentialStore } from './connection-credential-store.js'; import { MARKETPLACE_INSTALLED_UI_BUNDLE } from './marketplace-ui.js'; import type { IHttpServer, IMetadataService, IObjectQLEngine } from '@objectstack/spec/contracts'; +import type { DeletePackageRequest, UninstallCleanupOutcome } from '@objectstack/metadata-protocol'; const ROUTE_BASE = '/api/v1/marketplace/install-local'; +/** + * [#21490] The one `protocol` verb this plugin calls: the runner of the + * uninstall cleanups domain plugins register through + * `registerUninstallCleanup` — the same registry, through the same runner, + * that the protocol's own uninstall (`deletePackage`) runs. + * + * The `protocol` slot is uncontracted (`ServiceSlotContracts` leaves it + * unmapped), so this is a per-consumer narrowing, the shape + * `PackagesDomainProtocol` gives `deletePackage` in + * `packages/runtime/src/domains/packages.ts`: the verb's name is this file's, + * its request and outcome types are the producer's own declared ones — never + * a restatement here. + * + * Optional, and asked as a capability question at the call site: a protocol + * from a build without the runner holds cleanups this door cannot run, and + * the uninstall says so instead of answering as if they had run. + */ +type UninstallCleanupRunner = { + runUninstallCleanups?( + request: Pick, + ): Promise; +}; + +/** The outcome name this door reports when the runner itself could not run. */ +const UNINSTALL_CLEANUP_RUNNER = 'protocol.runUninstallCleanups'; + /** * [#8976] The capability every MUTATING install-local route demands. * @@ -1092,16 +1122,100 @@ export class MarketplaceInstallLocalPlugin implements Plugin { } catch (err: any) { return c.json({ success: false, error: { code: 'MARKETPLACE_STORAGE_FAILED', message: err?.message ?? String(err) } }, 500); } - ctx.logger?.info?.(`[MarketplaceInstallLocal] uninstalled ${manifestId} (cached manifest removed; restart runtime to unload from running kernel)`); + // [#21490] Only now — the ledger entry is gone, so the package will not + // come back at the next restart — revoke what its metadata granted. + // Never before: an uninstall whose ledger write failed above leaves the + // package installed, and it must keep its grants. + const cleanups = await this.runUninstallCleanups(ctx, manifestId, admission.userId); + ctx.logger?.info?.(`[MarketplaceInstallLocal] uninstalled ${manifestId} (cached manifest removed; ${cleanups.length} uninstall cleanup(s) ran; restart runtime to unload from running kernel)`); return c.json({ success: true, data: { manifestId, - note: 'Cached manifest removed. The app remains loaded in the running kernel until the next restart (the kernel API does not support unregistering apps in-place).', + cleanups, + note: 'Cached manifest removed, and the uninstall cleanups this runtime\'s plugins registered ran — each one\'s outcome is in `cleanups`. The app remains loaded in the running kernel until the next restart (the kernel API does not support unregistering apps in-place).', }, }, 200); }; + /** + * [#21490] Run the protocol's registered uninstall cleanups for a package + * this door just removed from its ledger — ADR-0086 D3's data-plane + * revocation, which ADR-0090 states as "(removing its sets by `packageId`, + * ADR-0086 D3) revokes it everywhere at once. No ghost grants." + * + * Before this, the door removed the ledger entry and nothing else: the + * package's object answered 404 after a restart, while its `managed_by: + * package` `sys_permission_set` row — and every grant of it — survived. + * `plugin-security` registers exactly that revocation + * (`security.package-permissions`); only the protocol's own uninstall ran it. + * + * ⛔ No second revocation path lives here. The cleanups are whatever domain + * plugins registered with the protocol, run by the protocol's own runner — + * this door knows no table, and a cleanup registered tomorrow fires here + * with no edit to this file. + * + * The request: + * - `packageId` is the MANIFEST id, never the ledger's `packageId`. The + * registry, and so every row a domain plugin stamped for the package, + * knows the package by `manifest.id`; the ledger's `packageId` is the + * marketplace catalog's id on a cloud install and can differ from it. + * - no `organizationId`: an install-local package is installed for the + * whole runtime (the ledger is per runtime, and the rehydrate registers + * it for every tenant), so its revocation is too. + * - `actor` is the operator the admission resolved. + * + * What comes back is reported, never swallowed — on the response, as the + * protocol's own uninstall reports it (`cleanups`). Never throws: the + * uninstall has already happened, so a cleanup that could not run is an + * outcome, not a failed request. Three answers: + * - no `protocol` service — no cleanup registry exists, so none is + * registered and none is owed: `[]`; + * - a protocol without the runner, or a runner that throws — one failed + * outcome named {@link UNINSTALL_CLEANUP_RUNNER}, so the caller can + * tell "nothing to revoke" from "the revocation never ran"; + * - otherwise the runner's outcomes, verbatim. + */ + private runUninstallCleanups = async ( + ctx: PluginContext, + manifestId: string, + actor: string, + ): Promise => { + let protocol: UninstallCleanupRunner | undefined; + try { protocol = ctx.getService('protocol'); } catch { /* no protocol service */ } + if (!protocol) return []; + + const notRun = (error: string): UninstallCleanupOutcome[] => [ + { name: UNINSTALL_CLEANUP_RUNNER, success: false, removed: 0, error }, + ]; + let outcomes: UninstallCleanupOutcome[]; + if (typeof protocol.runUninstallCleanups !== 'function') { + outcomes = notRun( + 'this runtime\'s protocol cannot run uninstall cleanups — upgrade @objectstack/metadata-protocol ' + + 'alongside @objectstack/cloud-connection', + ); + } else { + try { + outcomes = await protocol.runUninstallCleanups({ packageId: manifestId, actor }); + } catch (err: any) { + ctx.logger?.warn?.(`[MarketplaceInstallLocal] the uninstall cleanups of ${manifestId} could not be run: ${err?.message ?? err}`); + outcomes = notRun('the uninstall cleanups could not be run'); + } + } + + const failed = outcomes.filter((o) => o.success !== true); + if (failed.length > 0) { + ctx.logger?.warn?.( + `[MarketplaceInstallLocal] uninstalled ${manifestId}, but ${failed.length} uninstall cleanup(s) did not ` + + `complete (${failed.map((o) => o.name).join(', ')}) — what they revoke, such as the package's ` + + 'permission sets and their grants, SURVIVES the uninstall. Each outcome is on the response ' + + '(`cleanups`). Remedy: once the cause is fixed, install the package again and uninstall it again — ' + + 'the cleanups re-select by package id, so a second pass removes whatever the first could not.', + ); + } + return outcomes; + }; + /** * [ADR-0120 D5e] Decide whether this install must stop for the * `isolated`-posture confirmation, and what attestation the ledger entry diff --git a/packages/cloud-connection/src/marketplace-install-local-uninstall-cleanups.test.ts b/packages/cloud-connection/src/marketplace-install-local-uninstall-cleanups.test.ts new file mode 100644 index 00000000000..f418b2098dc --- /dev/null +++ b/packages/cloud-connection/src/marketplace-install-local-uninstall-cleanups.test.ts @@ -0,0 +1,268 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #21490 — `DELETE /api/v1/marketplace/install-local/:manifestId` runs the + * protocol's registered uninstall cleanups, through the protocol's own runner, + * once the ledger entry is gone, and reports each outcome as `cleanups`. + * + * The defect: the door removed the ledger entry and nothing else, so the + * package's `managed_by: package` permission sets — and every grant of them — + * outlived the uninstall (ADR-0090: "No ghost grants"). `plugin-security` + * registers that revocation with the protocol (`security.package-permissions`); + * only the protocol's own uninstall ran the registry. + * + * What this file pins about the plugin's half (the runner's half is pinned in + * `@objectstack/metadata-protocol`, and end to end — the rows themselves, + * before and after a restart — in the CLI suite + * `package-install-local-uninstall-cleanups.integration.test.ts`): + * + * - the runner is called once, with the MANIFEST id, the operator as actor + * and no organization, AFTER the ledger entry is gone, and its outcomes are + * answered verbatim; + * - a failed cleanup is reported on the response, never swallowed, and the + * operator log names it and the remedy; + * - nothing is revoked when the uninstall did not happen: a refused caller, + * an id this door never installed, a ledger write that failed; + * - the three composition answers: no protocol (nothing registered, `[]`), + * a protocol without the runner, a runner that throws (one failed outcome + * each, so "nothing to revoke" and "the revocation never ran" differ). + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { existsSync, mkdtempSync, rmSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +// The first load of the runtime's dist paid at module top, never inside a +// clocked `it` (`scripts/check-test-source-alias.mjs`): the plugin reaches the +// same module through a dynamic `import()` for its handler binder on rehydrate. +import '@objectstack/runtime'; +import { MarketplaceInstallLocalPlugin } from './marketplace-install-local-plugin.js'; +import { INSTALLER_USER_ID, installerAuthService, withInstallerGrants } from './install-local-principal.fixtures.js'; +import { LocalManifestSource } from './local-manifest-source.js'; + +const APP_ID = 'com.example.tasksapp'; +/** A cloud install's ledger `packageId` — the catalog's id, NOT the manifest id the registry knows. */ +const CATALOG_ID = 'pkg_01tasksapp'; + +type Handler = (c: any) => Promise; + +function makeRawApp() { + const routes = new Map(); + return { + routes, + get: (p: string, h: Handler) => routes.set(`GET ${p}`, h), + post: (p: string, h: Handler) => routes.set(`POST ${p}`, h), + delete: (p: string, h: Handler) => routes.set(`DELETE ${p}`, h), + }; +} + +function makeDeleteC(manifestId: string) { + const json = vi.fn((payload: any, status?: number) => ({ payload, status: status ?? 200 })); + return { + req: { + url: `http://localhost:3000/api/v1/marketplace/install-local/${manifestId}`, + raw: new Request('http://localhost:3000/x'), + json: async () => ({}), + param: (name: string) => (name === 'manifestId' ? manifestId : undefined), + }, + json, + }; +} + +let dir: string; +beforeEach(() => { dir = mkdtempSync(join(tmpdir(), 'mil-uninstall-')); }); +afterEach(() => { rmSync(dir, { recursive: true, force: true }); vi.restoreAllMocks(); }); + +/** A ledger entry as a CLOUD install writes it: catalog `packageId`, manifest `manifestId`. */ +function seedLedger(): void { + new LocalManifestSource(dir).write({ + packageId: CATALOG_ID, + versionId: 'v1', + manifestId: APP_ID, + version: '0.1.0', + manifest: { id: APP_ID, name: 'Tasks App', objects: [] }, + installedAt: '2026-01-01T00:00:00.000Z', + installedBy: INSTALLER_USER_ID, + withSampleData: false, + }); +} + +const ledgerFile = () => join(dir, `${APP_ID}.json`); + +type ProtocolMode = + | { kind: 'runner'; outcomes?: unknown[] } + | { kind: 'throws' } + | { kind: 'no-runner' } + | { kind: 'absent' }; + +/** + * Boot the plugin to `kernel:ready` and hand back its DELETE route, with a + * `protocol` service whose runner records every call — and, at the moment of + * each, whether the ledger file was still on disk. + */ +async function bootPlugin(protocolMode: ProtocolMode = { kind: 'runner' }, opts: { auth?: unknown } = {}) { + const calls: Array<{ request: unknown; ledgerOnDisk: boolean }> = []; + const hooks = new Map(); + const logger = { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }; + const rawApp = makeRawApp(); + const services: Record = { + manifest: { register: vi.fn() }, + auth: opts.auth ?? installerAuthService(), + objectql: withInstallerGrants({ syncSchemas: async () => undefined }), + }; + if (protocolMode.kind === 'runner') { + services.protocol = { + runUninstallCleanups: vi.fn(async (request: unknown) => { + calls.push({ request, ledgerOnDisk: existsSync(ledgerFile()) }); + return protocolMode.outcomes ?? [{ name: 'security.package-permissions', success: true, removed: 3 }]; + }), + }; + } else if (protocolMode.kind === 'throws') { + services.protocol = { + runUninstallCleanups: vi.fn(async (request: unknown) => { + calls.push({ request, ledgerOnDisk: existsSync(ledgerFile()) }); + throw new Error('runner exploded'); + }), + }; + } else if (protocolMode.kind === 'no-runner') { + services.protocol = { deletePackage: vi.fn() }; + } + const ctx: any = { + hook: (e: string, h: any) => hooks.set(e, h), + getService: (name: string) => { + if (name === 'http-server') return { getRawApp: () => rawApp }; + const svc = services[name]; + if (svc === undefined) throw new Error(`no ${name}`); + return svc; + }, + logger, + }; + const plugin = new MarketplaceInstallLocalPlugin({ controlPlaneUrl: 'off', storageDir: dir }); + await plugin.start(ctx); + await hooks.get('kernel:ready')?.(); + const uninstall = async (manifestId = APP_ID) => + rawApp.routes.get('DELETE /api/v1/marketplace/install-local/:manifestId')!(makeDeleteC(manifestId)); + const warnings = () => logger.warn.mock.calls.map((c: any[]) => String(c[0])); + return { uninstall, calls, warnings }; +} + +describe('#21490: an install-local uninstall runs the registered uninstall cleanups', () => { + it('calls the runner once — manifest id, the operator, no organization — after the ledger entry is gone, and answers its outcomes', async () => { + seedLedger(); + const outcomes = [ + { name: 'security.package-permissions', success: true, removed: 3 }, + { name: 'another.cleanup', success: true, removed: 0 }, + ]; + const { uninstall, calls } = await bootPlugin({ kind: 'runner', outcomes }); + + const res = await uninstall(); + + expect(res.status, JSON.stringify(res.payload)).toBe(200); + expect(res.payload.success).toBe(true); + expect(calls).toEqual([{ request: { packageId: APP_ID, actor: INSTALLER_USER_ID }, ledgerOnDisk: false }]); + expect(res.payload.data.cleanups).toEqual(outcomes); + expect(existsSync(ledgerFile())).toBe(false); + }); + + it('a failed cleanup is reported on the response, and the log names it with the remedy', async () => { + seedLedger(); + const failed = { name: 'security.package-permissions', success: false, removed: 0, error: 'cleanup failed' }; + const { uninstall, warnings } = await bootPlugin({ kind: 'runner', outcomes: [failed] }); + + const res = await uninstall(); + + // The uninstall itself happened — the ledger entry is gone — so the + // request succeeded; what did not complete is reported, not swallowed. + expect(res.status).toBe(200); + expect(res.payload.success).toBe(true); + expect(res.payload.data.cleanups).toEqual([failed]); + const said = warnings().filter((w) => w.includes('did not complete')); + expect(said).toHaveLength(1); + expect(said[0]).toContain(APP_ID); + expect(said[0]).toContain('security.package-permissions'); + expect(said[0]).toContain('install the package again and uninstall it again'); + }); + + it('a refused caller revokes nothing', async () => { + seedLedger(); + const { uninstall, calls } = await bootPlugin({ kind: 'runner' }, { + auth: { api: { getSession: async () => null } }, + }); + + const res = await uninstall(); + + expect(res.status).toBe(401); + expect(calls).toEqual([]); + expect(existsSync(ledgerFile())).toBe(true); + }); + + it('an id this door never installed revokes nothing — another package\'s grants are not this door\'s to touch', async () => { + const { uninstall, calls } = await bootPlugin({ kind: 'runner' }); + + const res = await uninstall('com.example.someoneelse'); + + expect(res.status).toBe(404); + expect(res.payload.error.code).toBe('RESOURCE_NOT_FOUND'); + expect(calls).toEqual([]); + }); + + it('a ledger write that fails revokes nothing — the package is still installed and keeps its grants', async () => { + seedLedger(); + vi.spyOn(LocalManifestSource.prototype, 'remove').mockImplementation(() => { + throw new Error('EACCES: permission denied'); + }); + const { uninstall, calls } = await bootPlugin({ kind: 'runner' }); + + const res = await uninstall(); + + expect(res.status).toBe(500); + expect(res.payload.error.code).toBe('MARKETPLACE_STORAGE_FAILED'); + expect(calls).toEqual([]); + }); + + it('no protocol service — no cleanup registry, nothing registered: `cleanups` is empty', async () => { + seedLedger(); + const { uninstall, warnings } = await bootPlugin({ kind: 'absent' }); + + const res = await uninstall(); + + expect(res.status).toBe(200); + expect(res.payload.data.cleanups).toEqual([]); + expect(warnings().filter((w) => w.includes('did not complete'))).toEqual([]); + }); + + it('a protocol without the runner — one failed outcome naming the upgrade, never an empty list', async () => { + seedLedger(); + const { uninstall, warnings } = await bootPlugin({ kind: 'no-runner' }); + + const res = await uninstall(); + + expect(res.status).toBe(200); + expect(res.payload.data.cleanups).toHaveLength(1); + expect(res.payload.data.cleanups[0]).toMatchObject({ + name: 'protocol.runUninstallCleanups', + success: false, + removed: 0, + }); + expect(res.payload.data.cleanups[0].error).toContain('@objectstack/metadata-protocol'); + expect(warnings().filter((w) => w.includes('did not complete') && w.includes(APP_ID))).toHaveLength(1); + }); + + it('a runner that throws — one failed outcome, the cause in the log, and the uninstall stands', async () => { + seedLedger(); + const { uninstall, calls, warnings } = await bootPlugin({ kind: 'throws' }); + + const res = await uninstall(); + + expect(res.status).toBe(200); + expect(res.payload.success).toBe(true); + expect(calls).toHaveLength(1); + expect(res.payload.data.cleanups).toEqual([ + { name: 'protocol.runUninstallCleanups', success: false, removed: 0, error: 'the uninstall cleanups could not be run' }, + ]); + // The thrown text goes to the operator log only, never onto the wire. + expect(JSON.stringify(res.payload)).not.toContain('runner exploded'); + expect(warnings().filter((w) => w.includes('runner exploded') && w.includes(APP_ID))).toHaveLength(1); + expect(existsSync(ledgerFile())).toBe(false); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 68412f33aba..3ac2573fbb6 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -5245,6 +5245,73 @@ export class ObjectStackProtocolImplementation implements this.uninstallCleanups.set(name, cleanup); } + /** + * Run every registered uninstall cleanup for one package and report each + * outcome (ADR-0086 D3, #2747) — THE one runner of the registry above. + * + * [#21490] Extracted from {@link deletePackage}, which calls it as its last + * step, so that a door uninstalling a package the protocol never stored — + * install-local's `DELETE /api/v1/marketplace/install-local/:manifestId`, + * whose package has no `sys_packages` row and no `sys_metadata` rows — + * runs the SAME cleanups after its own removal instead of none. Measured on + * that door before this existed: the package's object answered 404 after a + * restart while its `managed_by: package` `sys_permission_set` row and its + * user grant survived. `deletePackage` itself does not fit that door: it + * refuses without a tenant scope, answers `success: false` when it deletes + * no `sys_metadata` row, and withdraws the package from the running + * registry, which that door never did. + * + * Best-effort per cleanup and never throws: a cleanup's failure is an + * outcome (`success: false`), its text quoted only when the cleanup + * declared a refusal. Ghost grants are a security condition, so every + * caller surfaces a failed outcome — on its response, as `deletePackage` + * does — and never swallows it. + */ + async runUninstallCleanups( + request: Pick, + ): Promise { + const cleanups: UninstallCleanupOutcome[] = []; + for (const [name, cleanup] of this.uninstallCleanups) { + try { + const r = await cleanup({ + packageId: request.packageId, + ...(request.organizationId ? { organizationId: request.organizationId } : {}), + ...(request.actor ? { actor: request.actor } : {}), + }); + cleanups.push({ + name, + success: r?.success !== false, + removed: typeof r?.removed === 'number' ? r.removed : 0, + ...(r?.error ? { error: r.error } : {}), + }); + } catch (e: any) { + // [#8136] A cleanup is arbitrary plugin code that goes straight + // at the engine (plugin-security deletes `sys_permission_set` + // rows and their bindings), so a driver failure lands here + // verbatim — and this outcome rides on the RESPONSE by design, + // inside `details`, where no boundary's message withhold can + // reach it. Quoted only when the cleanup declared a refusal. + // [#12536] The mark travels beside the withheld sentence, not + // instead of it: `error` stays whatever #8136's rule licenses, + // and a cleanup that refused in the author's own words is still + // reported as a refusal rather than as one that merely + // "failed". + const cleanupUserMessage = declaredUserMessage(e); + cleanups.push({ + name, + success: false, + removed: 0, + error: clientFacingFailureText(e, 'cleanup failed'), + ...(cleanupUserMessage !== undefined ? { userMessage: cleanupUserMessage } : {}), + }); + console.warn( + `[protocol.runUninstallCleanups] uninstall cleanup '${name}' failed for '${request.packageId}': ${e?.message}`, + ); + } + } + return cleanups; + } + /** * Register the awaited mutation projector for a metadata type (ADR-0094). * Called by the domain plugin that owns the derived read-model (e.g. @@ -21703,45 +21770,9 @@ export class ObjectStackProtocolImplementation implements // sys_permission_set rows and their bindings. Best-effort per cleanup; // outcomes ride on the response so a failed revocation (ghost grants — // a security condition) is visible to the caller, never silent. - const cleanups: UninstallCleanupOutcome[] = []; - for (const [name, cleanup] of this.uninstallCleanups) { - try { - const r = await cleanup({ - packageId: request.packageId, - ...(request.organizationId ? { organizationId: request.organizationId } : {}), - ...(request.actor ? { actor: request.actor } : {}), - }); - cleanups.push({ - name, - success: r?.success !== false, - removed: typeof r?.removed === 'number' ? r.removed : 0, - ...(r?.error ? { error: r.error } : {}), - }); - } catch (e: any) { - // [#8136] A cleanup is arbitrary plugin code that goes straight - // at the engine (plugin-security deletes `sys_permission_set` - // rows and their bindings), so a driver failure lands here - // verbatim — and this outcome rides on the RESPONSE by design, - // inside `details`, where no boundary's message withhold can - // reach it. Quoted only when the cleanup declared a refusal. - // [#12536] The mark travels beside the withheld sentence, not - // instead of it: `error` stays whatever #8136's rule licenses, - // and a cleanup that refused in the author's own words is still - // reported as a refusal rather than as one that merely - // "failed". - const cleanupUserMessage = declaredUserMessage(e); - cleanups.push({ - name, - success: false, - removed: 0, - error: clientFacingFailureText(e, 'cleanup failed'), - ...(cleanupUserMessage !== undefined ? { userMessage: cleanupUserMessage } : {}), - }); - console.warn( - `[protocol.deletePackage] uninstall cleanup '${name}' failed for '${request.packageId}': ${e?.message}`, - ); - } - } + // [#21490] Through the registry's one runner, which install-local's + // uninstall door calls too. + const cleanups = await this.runUninstallCleanups(request); return { success: failed.length === 0 && deleted.length > 0, diff --git a/packages/metadata-protocol/src/protocol.uninstall-cleanups-runner.test.ts b/packages/metadata-protocol/src/protocol.uninstall-cleanups-runner.test.ts new file mode 100644 index 00000000000..046361058c6 --- /dev/null +++ b/packages/metadata-protocol/src/protocol.uninstall-cleanups-runner.test.ts @@ -0,0 +1,106 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #21490 — `runUninstallCleanups` is the ONE runner of the uninstall-cleanup + * registry (`registerUninstallCleanup`, ADR-0086 D3): `deletePackage` calls it + * as its last step, and install-local's uninstall door + * (`DELETE /api/v1/marketplace/install-local/:manifestId`, in + * `@objectstack/cloud-connection`) calls it after its own ledger removal, so + * the same cleanups fire on both doors and neither keeps a second revocation + * path. + * + * What this file pins: + * - every registered cleanup runs once, with the package id and exactly the + * organization / actor the request carried, and each outcome is reported; + * - a cleanup's failure is an outcome, never a throw: a returned + * `success: false` keeps its own error, a thrown undeclared fault is + * reported with the withheld fallback sentence, never its driver text; + * - the control: `deletePackage` reports exactly what the runner returned, + * having called it once with its own request. + * + * End to end — the rows themselves, before and after a restart — lives in the + * CLI suite `package-install-local-uninstall-cleanups.integration.test.ts`. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { ObjectStackProtocolImplementation } from './index.js'; + +const PACKAGE_ID = 'com.example.tasksapp'; + +function makeProtocol() { + const engine = { + registry: { + assertPackageUninstallable: () => undefined, + uninstallPackage: () => true, + }, + find: async () => [], + }; + const services = new Map([ + ['package', { delete: async () => ({ success: true }) }], + ]); + return new ObjectStackProtocolImplementation(engine as never, () => services as never); +} + +describe('#21490: runUninstallCleanups — the uninstall-cleanup registry\'s one runner', () => { + it('runs every registered cleanup once, with the package id, organization and actor it was given, and reports each outcome', async () => { + const protocol = makeProtocol(); + const first = vi.fn(async () => ({ success: true, removed: 3 })); + const second = vi.fn(async () => ({ success: true, removed: 0 })); + protocol.registerUninstallCleanup('security.package-permissions', first); + protocol.registerUninstallCleanup('another.cleanup', second); + + const outcomes = await protocol.runUninstallCleanups({ packageId: PACKAGE_ID, organizationId: 'org_1', actor: 'usr_1' }); + + expect(first.mock.calls).toEqual([[{ packageId: PACKAGE_ID, organizationId: 'org_1', actor: 'usr_1' }]]); + expect(second.mock.calls).toEqual([[{ packageId: PACKAGE_ID, organizationId: 'org_1', actor: 'usr_1' }]]); + expect(outcomes).toEqual([ + { name: 'security.package-permissions', success: true, removed: 3 }, + { name: 'another.cleanup', success: true, removed: 0 }, + ]); + }); + + it('passes no organization and no actor when the request carries none — an installation-wide uninstall', async () => { + const protocol = makeProtocol(); + const cleanup = vi.fn(async () => ({ success: true, removed: 1 })); + protocol.registerUninstallCleanup('security.package-permissions', cleanup); + + await protocol.runUninstallCleanups({ packageId: PACKAGE_ID }); + + expect(cleanup.mock.calls).toEqual([[{ packageId: PACKAGE_ID }]]); + }); + + it('a failed cleanup is an outcome, never a throw — and the rest still run', async () => { + const protocol = makeProtocol(); + const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + protocol.registerUninstallCleanup('returns.failure', async () => ({ success: false, removed: 0, error: 'refused by the store' })); + protocol.registerUninstallCleanup('throws.fault', async () => { throw new Error('SQLITE_ERROR: no such table: sys_permission_set'); }); + const last = vi.fn(async () => ({ success: true, removed: 2 })); + protocol.registerUninstallCleanup('still.runs', last); + + const outcomes = await protocol.runUninstallCleanups({ packageId: PACKAGE_ID }); + + expect(outcomes).toEqual([ + { name: 'returns.failure', success: false, removed: 0, error: 'refused by the store' }, + { name: 'throws.fault', success: false, removed: 0, error: 'cleanup failed' }, + { name: 'still.runs', success: true, removed: 2 }, + ]); + expect(last).toHaveBeenCalledTimes(1); + // The driver text goes to the operator log, never into the outcome. + expect(JSON.stringify(outcomes)).not.toContain('SQLITE_ERROR'); + expect(warn.mock.calls.map((c) => String(c[0])).filter((m) => m.includes('throws.fault') && m.includes(PACKAGE_ID))).toHaveLength(1); + warn.mockRestore(); + }); + + it('control: deletePackage calls the runner once with its own request and reports exactly its outcomes', async () => { + const protocol = makeProtocol(); + protocol.registerUninstallCleanup('security.package-permissions', async () => ({ success: true, removed: 4 })); + const runner = vi.spyOn(protocol, 'runUninstallCleanups'); + const request = { packageId: PACKAGE_ID, allTenants: true as const, actor: 'usr_1' }; + + const res = await protocol.deletePackage(request); + + expect(runner.mock.calls).toEqual([[request]]); + expect(res.cleanups).toEqual(await runner.mock.results[0]!.value); + expect(res.cleanups).toEqual([{ name: 'security.package-permissions', success: true, removed: 4 }]); + }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index b69758e4e0b..249978f37c3 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -758,6 +758,9 @@ importers: '@objectstack/core': specifier: workspace:* version: link:../core + '@objectstack/metadata-protocol': + specifier: workspace:* + version: link:../metadata-protocol '@objectstack/runtime': specifier: workspace:* version: link:../runtime