diff --git a/.changeset/20492-uninstall-refuse-before-mutate.md b/.changeset/20492-uninstall-refuse-before-mutate.md new file mode 100644 index 0000000000..9f3f7efa56 --- /dev/null +++ b/.changeset/20492-uninstall-refuse-before-mutate.md @@ -0,0 +1,16 @@ +--- +'@objectstack/runtime': patch +'@objectstack/spec': patch +--- + +fix(runtime): `DELETE /packages/:id` refuses an uninstall that names no organization before it touches the running registry (#20492) + +Clause-②: no + +A caller holding `manage_metadata` with no active organization — a member removed from an organization whose session still names it, or a caller who never selected one — sent `DELETE /api/v1/packages/:id` and was answered `400 TENANT_SCOPE_REQUIRED`. The dispatcher had already run the registry uninstall by then, so the package and every object it registers had left the running process for everyone it serves, while its stored rows still said it was installed. The state lasted until a restart re-seeded the registry. + +The door now asks the persisted delete's organization-scope question first, from the same organization value it hands `deletePackage`, and only when a persisted delete will run. The same refusal (`400 TENANT_SCOPE_REQUIRED`) now arrives before anything changes: the package stays served, listed and registered, and its stored rows are untouched. The refusal's message names what an HTTP caller can do, which is to select an organization they are a member of and retry. + +- **Unchanged:** a caller acting in an organization uninstalls exactly as before. A read-only package is still refused `422 WRITABLE_PACKAGE_REQUIRED` first. A host with no persisted delete (no `deletePackage` on its `protocol` service) still uninstalls from the registry alone, because there is no refusal to mirror there. The protocol keeps its own refusal as a second line. + +- **`@objectstack/spec`:** `PROVENANCE_WAIVERS` (the error-code ledger) gains one entry: `@objectstack/runtime` stamps `TENANT_SCOPE_REQUIRED`, which stays registered under `@objectstack/metadata-protocol`. The door mirrors `deletePackage`'s refusal and does not emit a second vocabulary. The registered code union and `ErrorCode` are unchanged. diff --git a/packages/runtime/src/domains/packages-capability-gate.test.ts b/packages/runtime/src/domains/packages-capability-gate.test.ts index 4ebd76d11d..85349231d3 100644 --- a/packages/runtime/src/domains/packages-capability-gate.test.ts +++ b/packages/runtime/src/domains/packages-capability-gate.test.ts @@ -41,10 +41,11 @@ const ctx = (executionContext?: any): any => ({ const anon = () => ctx(); /** Identity resolved but sessionless (no `userId`) — also anonymous. */ const anonResolved = () => ctx({ isSystem: false, positions: [], permissions: [], systemPermissions: [] }); -/** Authenticated, holding exactly `caps`. */ -const authed = (caps: string[] = []) => ctx({ userId: 'u_portal', isSystem: false, systemPermissions: caps }); +/** Authenticated, holding exactly `caps` — in `tenantId` when one is given. */ +const authed = (caps: string[] = [], tenantId?: string) => + ctx({ userId: 'u_portal', isSystem: false, systemPermissions: caps, ...(tenantId ? { tenantId } : {}) }); /** Engine self-invocation — never settable from the wire. */ -const system = () => ctx({ isSystem: true }); +const system = (tenantId?: string) => ctx({ isSystem: true, ...(tenantId ? { tenantId } : {}) }); // ── fake kernel ────────────────────────────────────────────────────────────── function make(overrides: { protocol?: any; metadata?: any; registry?: any } = {}) { @@ -140,7 +141,12 @@ describe('/packages — anonymous-deny floor (#7033/#7023)', () => { // 2. Write gate — `manage_metadata` on every state-changing route // ══════════════════════════════════════════════════════════════════════════════ -type WriteCase = { name: string; path: string; method: string; body?: any; query?: any; target: (p: any, r: any) => any }; +type WriteCase = { + name: string; path: string; method: string; body?: any; query?: any; + /** The organization the ALLOW-path callers act in, for a route that refuses a caller with none. */ + tenantId?: string; + target: (p: any, r: any) => any; +}; const WRITE_ROUTES: WriteCase[] = [ // `overwrite` so the allow-path clears the 409 duplicate guard (the shared // registry double answers `getPackage` truthy for any id); the write gate @@ -158,7 +164,11 @@ const WRITE_ROUTES: WriteCase[] = [ { name: 'POST /:id/adopt-orphans', path: '/pkg-a/adopt-orphans', method: 'POST', target: (p) => p.reassignOrphanedMetadata }, { name: 'POST /:id/duplicate', path: '/pkg-a/duplicate', method: 'POST', body: { targetPackageId: 'pkg-b' }, target: (p) => p.duplicatePackage }, { name: 'PATCH /:id (manifest)', path: '/pkg-a', method: 'PATCH', body: { name: 'renamed' }, target: (p) => p.updatePackage }, - { name: 'DELETE /:id', path: '/pkg-a', method: 'DELETE', target: (p) => p.deletePackage }, + // [#20492] The uninstall's allow-path acts in an organization: the door + // refuses one that names none before anything else runs, as the persisted + // delete itself does, so an org-less caller never reaches the target. + // Pinned in `packages-uninstall-refuse-before-mutate.test.ts`. + { name: 'DELETE /:id', path: '/pkg-a', method: 'DELETE', tenantId: 'org_a', target: (p) => p.deletePackage }, ]; describe('/packages — write gate: every state-changing route demands `manage_metadata`', () => { @@ -182,7 +192,7 @@ describe('/packages — write gate: every state-changing route demands `manage_m it(`lets a manage_metadata caller through on ${wc.name}`, async () => { const protocol = fullProtocol(); const { dispatcher, registry } = make({ protocol }); - const r = await dispatcher.handlePackages(wc.path, wc.method, wc.body ?? {}, wc.query ?? {}, authed(['manage_metadata'])); + const r = await dispatcher.handlePackages(wc.path, wc.method, wc.body ?? {}, wc.query ?? {}, authed(['manage_metadata'], wc.tenantId)); expect(r.response?.status).not.toBe(403); expect(r.response?.status).not.toBe(401); expect(wc.target(protocol, registry)).toHaveBeenCalled(); @@ -191,7 +201,7 @@ describe('/packages — write gate: every state-changing route demands `manage_m it(`lets an isSystem caller through on ${wc.name}`, async () => { const protocol = fullProtocol(); const { dispatcher, registry } = make({ protocol }); - const r = await dispatcher.handlePackages(wc.path, wc.method, wc.body ?? {}, wc.query ?? {}, system()); + const r = await dispatcher.handlePackages(wc.path, wc.method, wc.body ?? {}, wc.query ?? {}, system(wc.tenantId)); expect(r.response?.status).not.toBe(403); expect(wc.target(protocol, registry)).toHaveBeenCalled(); }); diff --git a/packages/runtime/src/domains/packages-read-delete-response-conformance.test.ts b/packages/runtime/src/domains/packages-read-delete-response-conformance.test.ts index 18cd35a0fa..745e33686d 100644 --- a/packages/runtime/src/domains/packages-read-delete-response-conformance.test.ts +++ b/packages/runtime/src/domains/packages-read-delete-response-conformance.test.ts @@ -110,8 +110,17 @@ const CODE_PKG = { name: 'Code Defined', objects: [{ name: 'code_lead', fields: { title: { type: 'text' } } }], }; +/** + * [#20492] The admin's session carries an organization, backed by a + * membership: `DELETE /packages/:id` refuses an uninstall that names none + * before anything else runs (as the persisted delete itself does), and the + * rows under test here are the ones a completed uninstall serves. + */ +const ORG = 'org_acme'; + /** The permission store the shared authz resolver reads, in its shipped shapes. */ const TABLES: Record = { + sys_member: [{ user_id: 'u_admin', organization_id: ORG, role: 'member' }], sys_user: [{ id: 'u_admin', email: 'u_admin@example.com' }], sys_user_permission_set: [{ user_id: 'u_admin', permission_set_id: 'ps_pkg' }], sys_permission_set: [ @@ -161,7 +170,7 @@ function dispatcher(manifests: any[], protocol?: unknown): HttpDispatcher { return typeof q?.limit === 'number' ? rows.slice(0, q.limit) : rows; }, }; - const auth = { api: { getSession: async () => ({ user: { id: 'u_admin' } }) } }; + const auth = { api: { getSession: async () => ({ user: { id: 'u_admin' }, session: { activeOrganizationId: ORG } }) } }; const services: Record = { objectql: ql, auth, ...(protocol ? { protocol } : {}) }; return new HttpDispatcher({ getState: () => 'running', diff --git a/packages/runtime/src/domains/packages-readonly-gate.test.ts b/packages/runtime/src/domains/packages-readonly-gate.test.ts index 1cdde8128d..4faeaa1a59 100644 --- a/packages/runtime/src/domains/packages-readonly-gate.test.ts +++ b/packages/runtime/src/domains/packages-readonly-gate.test.ts @@ -129,11 +129,16 @@ function make() { return { dispatcher: new HttpDispatcher(kernel), registry, protocol }; } -/** Authorized under #7033 — holds the write capability on every call below. */ +/** + * Authorized under #7033 — holds the write capability on every call below. + * [#20492] Acting in the organization that owns the writable base: an + * uninstall that names no organization is refused before the registry is + * touched, so an org-less admin would never reach the allowed delete below. + */ const admin = (): any => ({ request: {}, environmentId: 'pkg-readonly-gate-test', - executionContext: { userId: 'u_admin', isSystem: false, systemPermissions: ['manage_metadata'] }, + executionContext: { userId: 'u_admin', isSystem: false, systemPermissions: ['manage_metadata'], tenantId: 'org_acme' }, }); /** Engine self-invocation. Read-only is about the package, not the caller. */ const system = (): any => ({ diff --git a/packages/runtime/src/domains/packages-uninstall-envelope.test.ts b/packages/runtime/src/domains/packages-uninstall-envelope.test.ts index 7a318ef54f..6d58be55a2 100644 --- a/packages/runtime/src/domains/packages-uninstall-envelope.test.ts +++ b/packages/runtime/src/domains/packages-uninstall-envelope.test.ts @@ -66,10 +66,13 @@ import { describe, it, expect, vi } from 'vitest'; import { HttpDispatcher } from '../http-dispatcher.js'; +// [#20492] The caller acts in an organization: an uninstall that names none is +// refused before the registry is touched (as the persisted delete itself +// does), so it would never reach the persisted outcomes this file pins. const authed = (caps: string[] = ['manage_metadata']): any => ({ request: {}, environmentId: 'platform', - executionContext: { userId: 'u_admin', isSystem: false, systemPermissions: caps }, + executionContext: { userId: 'u_admin', isSystem: false, systemPermissions: caps, tenantId: 'org_acme' }, }); function make(deletePackageResult: any, opts: { registryRemoved?: boolean } = {}) { diff --git a/packages/runtime/src/domains/packages-uninstall-refuse-before-mutate.test.ts b/packages/runtime/src/domains/packages-uninstall-refuse-before-mutate.test.ts new file mode 100644 index 0000000000..89108401b4 --- /dev/null +++ b/packages/runtime/src/domains/packages-uninstall-refuse-before-mutate.test.ts @@ -0,0 +1,297 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#20492] `DELETE /packages/:id` refuses an uninstall that names no + * organization BEFORE it touches the running registry. + * + * ## The defect + * + * The door ran `registry.uninstallPackage(id)` and only then reached the + * persisted delete, whose `deletePackage` refuses an uninstall that names no + * organization (`400 TENANT_SCOPE_REQUIRED`). So a caller holding + * `manage_metadata` with no active organization was answered that refusal while + * the package, and every object it registers, had already left the running + * process for everyone it serves — `GET /packages/:id` 200 before, 404 after — + * and its stored rows still said it was installed, until a restart re-seeded + * the registry. A refused request had changed the process. + * + * ## The rig + * + * Identity is the REAL resolution on every request, as in + * `packages-vetted-org-source.test.ts`: `dispatch()` runs + * `resolveRequestScope` → `resolveExecutionContext` → `resolveAuthzContext` + * under an `isolated` posture, and every caller holds the ONE shared permission + * set, so only the organization separates the arms. The registry is a REAL + * `SchemaRegistry` holding the package and one object it owns, so "the package + * left the process" is read off the registry itself and through the door's own + * `GET`. The `protocol` double keeps the package's stored rows, records every + * `deletePackage` request, and refuses an org-less one the way + * `@objectstack/metadata-protocol`'s `deletePackage` does (its request type: + * "Omitted together with `allTenants` ⇒ refused"). + * + * Two refused populations, both measured reaching the old half-applied state: + * a member removed from the organization whose session still names it (the + * resolver drops the claim), and a caller who never selected an organization. + * The control is a current member, who uninstalls exactly as before. + */ + +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { describe, it, expect, vi, beforeAll, afterAll, beforeEach, afterEach } from 'vitest'; +import { SchemaRegistry } from '@objectstack/objectql'; +import { HttpDispatcher } from '../http-dispatcher.js'; + +const PKG = 'com.acme.crm'; +const ALPHA = 'org_alpha'; +const BETA = 'org_beta'; +const SESSION_HEADER = 'x-test-session'; + +// ── Identity: sessions and the permission store, in the shipped shapes ──────── + +/** Equality plus `$in`, and a loud refusal of every other shape. */ +function matchesWhere(row: any, where: any): boolean { + return Object.entries(where ?? {}).every(([field, cond]) => { + if (field.startsWith('$')) throw new Error(`fixture where-matcher: unsupported combinator '${field}'`); + if (cond !== null && typeof cond === 'object') { + const ops = Object.keys(cond as object); + if (ops.length !== 1 || ops[0] !== '$in' || !Array.isArray((cond as any).$in)) { + throw new Error(`fixture where-matcher: unsupported operator shape on '${field}'`); + } + return (cond as any).$in.includes(row[field]); + } + return row[field] === cond; + }); +} + +/** + * `u_member` is a current `org_alpha` member. `u_exmember`'s only current + * membership is `org_beta`, so a session naming `org_alpha` is a claim the + * resolver drops. `u_orgless` belongs to no organization at all. + */ +const TABLES: Record = { + sys_api_key: [], + sys_member: [ + { user_id: 'u_member', organization_id: ALPHA, role: 'member' }, + { user_id: 'u_exmember', organization_id: BETA, role: 'member' }, + ], + sys_user: [ + { id: 'u_member', email: 'u_member@example.com' }, + { id: 'u_exmember', email: 'u_exmember@example.com' }, + { id: 'u_orgless', email: 'u_orgless@example.com' }, + ], + sys_user_permission_set: [ + { user_id: 'u_member', permission_set_id: 'ps_shared' }, + { user_id: 'u_exmember', permission_set_id: 'ps_shared' }, + { user_id: 'u_orgless', permission_set_id: 'ps_shared' }, + ], + sys_permission_set: [ + { id: 'ps_shared', name: 'shared_access', system_permissions: ['manage_metadata', 'studio.access'] }, + ], +}; + +type Who = 'member' | 'exmember' | 'orgless'; + +const SESSIONS: Record = { + member: { id: 'ses_member', token: 'tok_member', userId: 'u_member', activeOrganizationId: ALPHA }, + // The claim outlived the membership that backed it. + exmember: { id: 'ses_exmember', token: 'tok_exmember', userId: 'u_exmember', activeOrganizationId: ALPHA }, + // Signed in, and never selected an organization. + orgless: { id: 'ses_orgless', token: 'tok_orgless', userId: 'u_orgless' }, +}; + +function authService() { + return { + api: { + getSession: async ({ headers }: any) => { + const who = (typeof headers?.get === 'function' ? headers.get(SESSION_HEADER) : headers?.[SESSION_HEADER]) as Who | undefined; + const row = who ? SESSIONS[who] : undefined; + if (!row) return undefined; + return { user: { id: row.userId, email: `${row.userId}@example.com` }, session: { ...row } }; + }, + }, + }; +} + +// ── The package: a real registry, and the stored rows behind it ─────────────── + +interface Rig { + registry: SchemaRegistry; + /** The object the package owns, by the name the registry answers it under. */ + objectName: string; + /** The package's stored rows — what `deletePackage` removes. */ + rows: string[]; + /** Every `deletePackage` request the door made. */ + deleteRequests: any[]; + call(method: string, who: Who, path: string): Promise<{ status: number; code?: string; body: any }>; +} + +function rig(opts: { persistedHalf?: boolean } = {}): Rig { + const registry = new SchemaRegistry({ multiTenant: false, collisionPolicy: 'error' }); + (registry as any).logLevel = 'silent'; + registry.installPackage({ id: PKG, namespace: 'crm', name: 'CRM', version: '1.0.0', type: 'app', scope: 'project' } as any); + registry.registerObject({ name: 'crm_lead', fields: { title: { type: 'text' } } } as any, PKG, 'crm', 'own'); + const objectName = registry.getAllObjects().map((o: any) => o.name).find((n: string) => n.endsWith('crm_lead'))!; + + const rows = ['object:crm_lead', 'view:crm_lead_list']; + const deleteRequests: any[] = []; + const protocol = { + deletePackage: async (req: any) => { + deleteRequests.push({ ...req }); + if (!req?.organizationId && req?.allTenants !== true) { + throw Object.assign(new Error('Refusing to uninstall with no organization scope.'), { + code: 'TENANT_SCOPE_REQUIRED', status: 400, + }); + } + const deleted = rows.splice(0); + return { success: true, deletedCount: deleted.length, failedCount: 0, deleted: [], failed: [], cleanups: [] }; + }, + }; + + const services: Record = { + objectql: { + registry, + find: async (object: string, q: any = {}) => { + const found = (TABLES[object] ?? []).filter((row: any) => matchesWhere(row, q?.where)); + return typeof q?.limit === 'number' ? found.slice(0, q.limit) : found; + }, + }, + auth: authService(), + tenancy: { posture: 'isolated' }, + ...(opts.persistedHalf === false ? {} : { protocol }), + }; + const get = (n: string) => services[n] ?? null; + const dispatcher = new HttpDispatcher({ context: { getService: get }, getService: get, getServiceAsync: async (n: string) => get(n) } as any); + + return { + registry, + objectName, + rows, + deleteRequests, + call: async (method, who, path) => { + const res = await dispatcher.dispatch(method, path, undefined, {}, { + request: { headers: { [SESSION_HEADER]: who } }, + } as any); + const body = JSON.parse(JSON.stringify(res.response?.body ?? null)); + return { status: res.response?.status ?? 0, code: body?.error?.code, body }; + }, + }; +} + +/** What a reader of the process sees about the package right now, through the door and in the registry. */ +async function observed(r: Rig) { + const detail = await r.call('GET', 'member', `/packages/${PKG}`); + const list = await r.call('GET', 'member', '/packages'); + return { + detailStatus: detail.status, + listed: (list.body?.data?.packages ?? []).map((p: any) => p?.manifest?.id).includes(PKG), + inRegistry: r.registry.getPackage(PKG) !== undefined, + objectRegistered: r.registry.getObject(r.objectName) !== undefined, + storedRows: [...r.rows], + }; +} + +const UNTOUCHED = { + detailStatus: 200, + listed: true, + inRegistry: true, + objectRegistered: true, + storedRows: ['object:crm_lead', 'view:crm_lead_list'], +}; + +// `setPackageDisabled` writes a REAL state file under the ObjectStack home on +// every uninstall that removed a registry row (the control arm below), so the +// home is a temp dir for the whole file. +const envSnapshot = { OS_HOME: process.env.OS_HOME }; +let home: string; +beforeAll(() => { + home = mkdtempSync(join(tmpdir(), 'os-20492-')); + process.env.OS_HOME = home; +}); +afterAll(() => { + if (envSnapshot.OS_HOME === undefined) delete process.env.OS_HOME; + else process.env.OS_HOME = envSnapshot.OS_HOME; + rmSync(home, { recursive: true, force: true }); +}); + +let warnSpy: ReturnType; +beforeEach(() => { warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); }); +afterEach(() => { warnSpy.mockRestore(); }); + +// ── The subject: a refused uninstall changes nothing ────────────────────────── + +const REFUSED: Array<[Who, string]> = [ + ['exmember', 'a member removed from the organization, whose session still names it'], + ['orgless', 'a caller who never selected an organization'], +]; + +describe('[#20492] DELETE /packages/:id with no organization is refused before the registry is touched', () => { + for (const [who, label] of REFUSED) { + it(`${label}: answered 400 TENANT_SCOPE_REQUIRED by the door, and deletePackage is never asked`, async () => { + const r = rig(); + const answer = await r.call('DELETE', who, `/packages/${PKG}`); + expect({ status: answer.status, code: answer.code, httpStatus: answer.body?.error?.httpStatus }) + .toEqual({ status: 400, code: 'TENANT_SCOPE_REQUIRED', httpStatus: 400 }); + expect(r.deleteRequests).toEqual([]); + }); + + it(`${label}: afterwards the package is still served, listed and registered with its object, and its stored rows are untouched`, async () => { + const r = rig(); + expect(await observed(r), 'precondition: the package is installed').toEqual(UNTOUCHED); + await r.call('DELETE', who, `/packages/${PKG}`); + expect(await observed(r)).toEqual(UNTOUCHED); + }); + } + + it('the rig really drops the removed member\'s claim — that arm is not a session that never presented one', async () => { + const r = rig(); + await r.call('DELETE', 'exmember', `/packages/${PKG}`); + const dropped = warnSpy.mock.calls.map((args: unknown[]) => String(args[0])) + .filter((line: string) => line.includes('Session organization claim dropped')); + expect(dropped.length).toBeGreaterThan(0); + expect(dropped[0]).toContain(`organization=${ALPHA}`); + }); +}); + +// ── The control: a member with an organization uninstalls as before ─────────── + +describe('[#20492] control: a current member uninstalls exactly as before', () => { + it('200; the package and its object leave the registry, GET answers 404, and deletePackage removes the rows in that organization', async () => { + const r = rig(); + const answer = await r.call('DELETE', 'member', `/packages/${PKG}`); + expect(answer.status).toBe(200); + expect(answer.body?.data).toMatchObject({ packageId: PKG, success: true, registryRemoved: true }); + expect(r.deleteRequests).toEqual([{ packageId: PKG, organizationId: ALPHA }]); + expect(await observed(r)).toEqual({ + detailStatus: 404, + listed: false, + inRegistry: false, + objectRegistered: false, + storedRows: [], + }); + }); +}); + +// ── The mirror's reach: exactly the refusal the persisted half would give ───── + +describe('[#20492] the door mirrors the persisted refusal — no wider, no narrower', () => { + it('a host with no persisted half (no deletePackage) has no refusal to mirror: the org-less uninstall proceeds as before', async () => { + const r = rig({ persistedHalf: false }); + const answer = await r.call('DELETE', 'orgless', `/packages/${PKG}`); + expect(answer.status).toBe(200); + expect(r.registry.getPackage(PKG)).toBeUndefined(); + }); + + it('no isSystem bypass — the protocol refuses an org-less uninstall whoever asks, so the door does too, before the registry', async () => { + const r = rig(); + const registry = r.registry; + const get = (n: string) => (n === 'objectql' ? { registry } : n === 'protocol' ? { deletePackage: vi.fn() } : null); + const d = new HttpDispatcher({ context: { getService: get } } as any); + const res = await d.handlePackages(`/${PKG}`, 'DELETE', undefined, {}, { + request: {}, executionContext: { isSystem: true }, + } as any); + expect({ status: res.response?.status, code: (res.response?.body as any)?.error?.code }) + .toEqual({ status: 400, code: 'TENANT_SCOPE_REQUIRED' }); + expect(registry.getPackage(PKG)).toBeDefined(); + }); +}); diff --git a/packages/runtime/src/domains/packages-vetted-org-source.test.ts b/packages/runtime/src/domains/packages-vetted-org-source.test.ts index 3b57525cd9..19e0331d82 100644 --- a/packages/runtime/src/domains/packages-vetted-org-source.test.ts +++ b/packages/runtime/src/domains/packages-vetted-org-source.test.ts @@ -387,7 +387,10 @@ describe('[#20477] controls: the rig can tell the organizations apart', () => { describe('[#20477] a session claim the resolver DROPPED reaches no organization on any /packages door', () => { for (const [transport, entry] of TRANSPORTS) { - for (const door of DOORS) { + // [#20492] The uninstall door is pinned on its own, below: it refuses a + // caller with no organization BEFORE the protocol is asked at all, so + // there is no protocol call for this generic pin to read. + for (const door of DOORS.filter((d) => d.verb !== 'deletePackage')) { it(`${transport} · ${door.name}: the protocol is handed no organization, and the left organization's rows are neither read nor written`, async () => { const answer = await entry()(door.method, 'exmember', door.path, door.body); expect(state.calls.map((c) => c.verb)).toContain(door.verb); @@ -397,9 +400,15 @@ describe('[#20477] a session claim the resolver DROPPED reaches no organization }); } - it(`${transport} · DELETE /packages/:id: the org-less uninstall gets the protocol's ruled refusal, and nothing is deleted`, async () => { + // [#20492] The refusal is the door's own now, taken before the registry + // is touched: the protocol is never handed the org-less request. The + // registry half of "nothing changed" is pinned in + // `packages-uninstall-refuse-before-mutate.test.ts` (this rig's + // registry cannot uninstall anything). + it(`${transport} · DELETE /packages/:id: the org-less uninstall is refused by the door before the protocol is asked, and nothing is deleted`, async () => { const answer = await entry()('DELETE', 'exmember', `/packages/${PKG}`); expect({ status: answer.status, code: answer.code }).toEqual({ status: 400, code: 'TENANT_SCOPE_REQUIRED' }); + expect(state.calls).toEqual([]); expect(state.store).toEqual(seedStore()); }); } diff --git a/packages/runtime/src/domains/packages.ts b/packages/runtime/src/domains/packages.ts index 17d9ae0411..86b509c9da 100644 --- a/packages/runtime/src/domains/packages.ts +++ b/packages/runtime/src/domains/packages.ts @@ -411,6 +411,65 @@ function requireWritablePackage( }; } +/** + * [#20492] `DELETE /packages/:id` — the ORGANIZATION-SCOPE refusal of the + * persisted delete, asked by the door before anything is mutated. + * + * ## The measurement + * + * A caller holding `manage_metadata` with no active organization sent + * `DELETE /api/v1/packages/:id` through `dispatch()` and was answered + * `400 TENANT_SCOPE_REQUIRED` — yet the package had ALREADY left the running + * registry (`GET /packages/:id` 200 before, 404 after; the listing empty), and + * its stored rows were kept. The door ran `registry.uninstallPackage(id)` first + * and reached `deletePackage`'s refusal second. A refused request had changed + * the process for everyone it serves, until a restart re-seeded the registry. + * + * ## The rule it mirrors + * + * `deletePackage` (`@objectstack/metadata-protocol`) refuses an uninstall + * whose request names neither `organizationId` nor `allTenants: true` — its + * declared request type says so ("Omitted together with `allTenants` ⇒ + * refused"). This door never sends `allTenants`, so the request it builds is + * refused exactly when the caller's vetted organization is absent. That + * absence is the whole condition: nothing about the package or its rows enters + * it, so the door can decide it up front, from the SAME value it hands the + * protocol. + * + * ## Shape + * + * Same code and status as the protocol's refusal — `TENANT_SCOPE_REQUIRED`, + * `400` — so no caller sees a second vocabulary for one condition. The sentence + * is the door's own, for the reason {@link requireWritablePackage}'s is: the + * protocol's remedy ("pass organizationId … or allTenants: true") names request + * keys an HTTP caller cannot send. What this caller can do is select an + * organization; and what the door can now truthfully add is that nothing + * changed. + * + * ⛔ No `isSystem` bypass: the protocol refuses an org-less uninstall whoever + * asks, so a mirror that exempted anyone would disagree with it. Returns a + * refusal result to short-circuit on, or `null` to proceed. Callers MUST run + * it before `uninstallPackage`, and only when `deletePackage` will run. + */ +function requireUninstallOrganizationScope( + deps: DomainHandlerDeps, + id: string, + organizationId: string | undefined, +): HttpDispatcherResult | null { + if (organizationId) return null; + return { + handled: true, + response: deps.error( + `Refusing to uninstall '${id}' with no organization scope: this request carries no active ` + + `organization, and an uninstall that names none would delete every organization's rows for ` + + `this package. Nothing was changed — select an organization you are a member of as your ` + + `active organization, then retry.`, + 400, + { code: 'TENANT_SCOPE_REQUIRED', packageId: id }, + ), + }; +} + /** * [#14451] `POST /packages/:id/duplicate` — the SOURCE must be a BASE. * @@ -1954,6 +2013,27 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin // listing (and with it every object the package registers) until // the next restart. const readOnly = requireWritablePackage(deps, qlService, id, 'delete'); if (readOnly) return readOnly; + // [#20492] The persisted delete's organization-scope refusal, taken + // HERE, before `uninstallPackage` — the same "refuse before you + // mutate" ordering as the gate above. `deletePackage` refuses an + // uninstall that names no organization, and it used to be asked + // only AFTER the registry had already dropped the package: the + // caller got `400 TENANT_SCOPE_REQUIRED` while the package and every + // object it registers had left the running process for everyone it + // serves, with the stored rows still saying it was installed. + // + // ONE organization read, and it is the value handed to + // `deletePackage` below, so this check and the protocol's cannot + // disagree. Asked only when the persisted half will run: with no + // `deletePackage` there is no refusal to mirror, and the in-memory + // uninstall proceeds exactly as before. ⛔ Not a compensating + // re-install after the fact — nothing is touched before this. + // The protocol keeps its own refusal as the second line. + const protocol = await resolveProtocol(deps, _context); + const organizationId = await deps.resolveActiveOrganizationId(_context); + if (protocol && typeof protocol.deletePackage === 'function') { + const unscoped = requireUninstallOrganizationScope(deps, id, organizationId); if (unscoped) return unscoped; + } const registryRemoved = registry.uninstallPackage(id); // ⭐ [#18877 ruling item 3] A package that no longer exists has no @@ -2009,10 +2089,12 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin // which is optional anyway), the slot takes whatever a host registers under // the name, and registrants carrying no `deletePackage` are real in-tree. // A capability question, asked as a capability probe — not a cast. - const protocol = await resolveProtocol(deps, _context); + // + // [#20492] `protocol` and `organizationId` are the ones resolved above, + // before the registry was touched: the organization this request + // carries is the one the scope check already read. if (protocol && typeof protocol.deletePackage === 'function') { try { - const organizationId = await deps.resolveActiveOrganizationId(_context); const keepData = query?.keepData === 'true' || query?.keepData === '1'; persisted = await protocol.deletePackage({ packageId: id, diff --git a/packages/spec/src/api/error-code-ledger.zod.ts b/packages/spec/src/api/error-code-ledger.zod.ts index 4b551580a0..cc11bc0262 100644 --- a/packages/spec/src/api/error-code-ledger.zod.ts +++ b/packages/spec/src/api/error-code-ledger.zod.ts @@ -1762,4 +1762,16 @@ export const PROVENANCE_WAIVERS: readonly ProvenanceWaiver[] = [ 'constructor there is one stamp site, and rows for packages that stamp nothing ' + 'would be the dead weight this file\'s gate refuses.', }, + { + package: '@objectstack/runtime', + code: 'TENANT_SCOPE_REQUIRED', + registeredUnder: '@objectstack/metadata-protocol', + reason: 'The door mirrors the producer\'s refusal; it is not a second emitter. ' + + '`DELETE /packages/:id` (domains/packages.ts, `requireUninstallOrganizationScope`) asks ' + + '`deletePackage`\'s organization-scope question BEFORE `registry.uninstallPackage`, and ' + + 'answers with the code `deletePackage` refuses with (#7780), so a refused uninstall ' + + 'changes nothing (#20492). The door never sends `allTenants`, so its condition is exactly ' + + 'the producer\'s "no organization"; the protocol keeps its own refusal as the second line ' + + 'and stays the registered emitter.', + }, ];