Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .changeset/20492-uninstall-refuse-before-mutate.md
Original file line number Diff line number Diff line change
@@ -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.
24 changes: 17 additions & 7 deletions packages/runtime/src/domains/packages-capability-gate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 } = {}) {
Expand Down Expand Up @@ -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
Expand All @@ -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`', () => {
Expand All @@ -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();
Expand All @@ -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();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, any[]> = {
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: [
Expand Down Expand Up @@ -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<string, unknown> = { objectql: ql, auth, ...(protocol ? { protocol } : {}) };
return new HttpDispatcher({
getState: () => 'running',
Expand Down
9 changes: 7 additions & 2 deletions packages/runtime/src/domains/packages-readonly-gate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 } = {}) {
Expand Down
Loading
Loading