diff --git a/.changeset/packages-registry-serializable-manifest.md b/.changeset/packages-registry-serializable-manifest.md new file mode 100644 index 0000000000..109b7176a5 --- /dev/null +++ b/.changeset/packages-registry-serializable-manifest.md @@ -0,0 +1,51 @@ +--- +"@objectstack/objectql": patch +"@objectstack/runtime": patch +"@objectstack/rest": patch +--- + +fix(objectql,runtime,rest): store a serializable manifest in the package registry so `/packages` stops answering 500 (#14309) + +On a stock showcase boot, signed in as the seeded admin, every read door that +serialises a package answered `500 INTERNAL_ERROR`: + +``` +GET /api/v1/packages -> 500 +GET /api/v1/packages/com.example.showcase -> 500 +GET /api/v1/meta/package/com.example.showcase -> 500 +GET /api/v1/meta/package/com.objectstack.setup -> 200 +``` + +with `Converting circular structure to JSON · _ObjectQL -> actionActivation -> +store -> engine`. Studio asks for the list three times on every open. + +**Cause.** `SchemaRegistry.installPackage(manifest)` kept the caller's object +verbatim as `pkg.manifest`. For a code-defined stack that object is the live +`defineStack()` one, and its `plugins: [new ConnectorRestPlugin(), …]` entries +hold the engine once they initialise — a cycle since the engine grew +`actionActivation -> store -> engine`. Measured on that boot: of the 26 +installed packages exactly ONE manifest key was unserializable (`plugins`, on +`com.example.showcase`), and only after plugin init — during boot the same +manifest serialised cleanly, which is why a package with no plugin instances +(`com.objectstack.setup`) kept answering 200. + +**Fix, at the producer.** `installPackage` now stores a serializable projection: +the registry item is a record, not the runtime. The projection drops by shape +rather than by key name — functions, class instances, `Map`/`Set` and reference +cycles are dropped; primitives, plain objects, arrays and `Date` survive — so a +future live member cannot re-open the same hole. The kernel keeps the live +object (`ObjectQL.manifests`), and the one reader of `manifest.plugins[]` reads +its own parameter, never the record, so nothing downstream loses a member it was +using. The caller's manifest is copied, never stripped in place. + +**Defence at the read doors.** `GET /packages` and `GET /packages/:id` project a +registry entry onto its declared record fields instead of spreading it whole, so +an undeclared member appearing on the *item* degrades to a field the response +never mentions instead of failing the whole list for every caller. Applied at +both twins — `packages/runtime/src/domains/packages.ts` (the handler that +actually answered the 500; the 404 wording identifies it) and the +`packages/rest` routes. The database half of the REST merge is deliberately not +projected: its shape belongs to `PackageService`. + +No response field is added or renamed. Responses that already served fine are +byte-identical; what disappears is a member that could never be serialised. diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 211a566390..2e5234dcca 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -135,7 +135,7 @@ The largest single consumer — **20 of the 109 sites**. | 34 | `revoke()` deletes directly, **before** the non-manual-source guard | Get: the evaluator can revoke its own grants. Lose: the `CONFLICT` guard that warns a rule-materialised grant will be silently re-granted on the next reconcile | `plugin-sharing/src/sharing-service.ts:1286` (guard at `:1311`) | | 35 | `listShares()` skips the management gate | Get: full enumeration of who can see a record | `plugin-sharing/src/sharing-service.ts:1338` | | 36 | `sys_record_share` reads are **not** self-scoped | Get: tenant-wide share listing without `manage_sharing` | `sharing-plugin.ts:1077` | -| 37 | Share-link policy `enabled` check bypassed; system callers re-enter under a system context | Get: link **creation** while the policy is off — resolution is **not** bypassed since #14033 (`publicSharing.enabled` is a standing policy held at every redemption): a link minted this way does not resolve until the block is enabled | `plugin-sharing/src/share-link-service.ts:449`, `:503`, `:507`, `:580`, `:610` | +| 37 | Share-link policy `enabled` check bypassed; system callers re-enter under a system context | Get: link creation/resolution while the policy is off | `plugin-sharing/src/share-link-service.ts:449`, `:503`, `:507`, `:580`, `:610` | | 38 | Sharing-rule provenance stamp skipped | Lose: the row is not marked as an admin customization — seeder / `defineRule` / boot reconcilers are "the package door" | `sharing-rule-provenance.ts:47` | | 39 | Sharing-rule service write + delete paths return early | Lose: the manage-rules gate on the service surface, and the platform-global-rule delete guard | `sharing-rule-service.ts:157`, `:382` | @@ -160,10 +160,10 @@ The largest single consumer — **20 of the 109 sites**. | 49 | Action `requiredPermissions` bypassed | runtime | Get: engine self-invocation runs any action | `action-execution.ts:399` | | 50 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability | `domains/meta.ts:471`, `:874`, `rest-server.ts:4716`, `:6079`, `:6327`, `:6758`, `:6951` | | 51 | The shared metadata-write verdict itself returns `allowed` | metadata-core | Get: the one function all of row 50's doors consult answers yes before any capability is examined | `meta-write-capability.ts:134` | -| 52 | Anonymous-deny seam satisfied on the domain dispatchers and the package/federation routes | runtime, rest | Get: passes with no `userId` | `domains/actions.ts:411`, `domains/ai.ts:60`, `domains/automation.ts:989`, `domains/meta.ts:232`, `domains/security.ts:78`, `domains/packages.ts:276`, `external-datasource-routes.ts:302`, `package-routes.ts:97` | +| 52 | Anonymous-deny seam satisfied on the domain dispatchers and the package/federation routes | runtime, rest | Get: passes with no `userId` | `domains/actions.ts:411`, `domains/ai.ts:60`, `domains/automation.ts:989`, `domains/meta.ts:232`, `domains/security.ts:78`, `domains/packages.ts:326`, `external-datasource-routes.ts:302`, `package-routes.ts:97` | | 53 | MCP principal check satisfied | runtime | Get: MCP surface reachable with no user | `domains/mcp.ts:61` | | 54 | Package REST route capability gate bypassed | rest | Get: package read/write over REST without `manage_metadata` / `studio.access` / `setup.access` | `package-routes.ts:102` | -| 55 | Package domain capability gates bypassed | runtime | Get: package management and package-inventory reads without the capability | `domains/packages.ts:95`, `:128` | +| 55 | Package domain capability gates bypassed | runtime | Get: package management and package-inventory reads without the capability | `domains/packages.ts:145`, `:178` | | 56 | Activation write / authoring refusals do not fire | runtime | Get: activation artifacts writable and authorable without the activation-authoring capability | `activation-gate.ts:139`, `:190` | | 57 | Automation run-state read, flow-authoring write and unrelated-screen read all pass | runtime | Get: run state, flow writes and screen reads with no grant | `domains/automation.ts:254`, `:545`, `:635` | | 58 | Audience-binding suggestion recording skipped | plugin-security | Lose: install-time suggestions are not recorded for system callers | `suggested-audience-bindings.ts:703` | diff --git a/packages/objectql/src/registry-package-manifest-serializable.test.ts b/packages/objectql/src/registry-package-manifest-serializable.test.ts new file mode 100644 index 0000000000..b52c8991e4 --- /dev/null +++ b/packages/objectql/src/registry-package-manifest-serializable.test.ts @@ -0,0 +1,194 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `installPackage` stores a RECORD, never the caller's live object. + * + * ## What was wrong + * + * `installPackage(manifest)` kept the argument verbatim as `pkg.manifest`. For a + * code-defined stack that argument is the live `defineStack()` object, and its + * `plugins: [new ConnectorRestPlugin(), …]` entries hold the engine once they + * initialise. Since the engine grew `actionActivation -> store -> engine` that + * reference closes a CYCLE, so `JSON.stringify` of the registry item threw and + * every read door that serialises a package answered `500 INTERNAL_ERROR` on a + * stock showcase boot — `GET /packages`, `GET /packages/:id`, + * `GET /meta/package/:id`, while `GET /meta/package/` + * stayed 200. + * + * ## Why the assertions are shaped this way + * + * ⚠️ "the manifest still round-trips" passes on the old code for every package + * that has no plugins, which is 25 of the 26 a showcase boot installs. So the + * cases below pin the MECHANISM: a live instance reaching the record, a plain + * reference cycle, and a member that is not data at all — each asserted on + * `JSON.stringify(registry.getPackage(id))`, the exact expression the doors run. + * + * ⚠️ Timing is part of the defect and is pinned too. Measured on the failing + * boot: the same showcase manifest serialised CLEANLY during boot and only + * became cyclic after plugin init, so a check that ran at install time would + * have called the record healthy. The `becomes cyclic only after init` case + * below reproduces that ordering — the projection must not depend on when it is + * asked. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { SchemaRegistry } from './registry'; + +/** + * A plugin instance in the shape that broke: a class instance the host + * constructs in `objectstack.config.ts` and hands to `defineStack({ plugins })`, + * which takes the engine when it initialises. + */ +class FakeConnectorPlugin { + name = 'connector-rest'; + engine: unknown; + init(engine: unknown) { + this.engine = engine; + } +} + +/** The engine's own `actionActivation -> store -> engine` cycle, reproduced. */ +function makeCyclicEngine(): Record { + const engine: Record = { name: '_ObjectQL' }; + const store: Record = { name: 'ObjectStoreActionActivationStore', engine }; + engine.actionActivation = { name: 'ActionActivationProjection', store }; + return engine; +} + +function baseManifest(overrides: Record = {}): any { + return { + id: 'com.example.showcase', + name: 'Showcase', + namespace: 'showcase', + version: '1.2.3', + type: 'app', + scope: 'user', + description: 'Kitchen-sink example', + dependencies: ['com.objectstack.plugin-auth'], + objects: [{ name: 'invoice', fields: { total: { type: 'currency' } } }], + apps: [{ name: 'showcase', label: 'Showcase' }], + ...overrides, + }; +} + +describe('SchemaRegistry.installPackage — the record is serializable', () => { + let registry: SchemaRegistry; + + beforeEach(() => { + registry = new SchemaRegistry({ multiTenant: false, collisionPolicy: 'error' }); + registry.logLevel = 'silent'; + }); + + it('survives a plugin instance that closes a cycle through the engine', () => { + const plugin = new FakeConnectorPlugin(); + plugin.init(makeCyclicEngine()); + registry.installPackage(baseManifest({ plugins: [plugin] })); + + // The expression every read door runs. + expect(() => JSON.stringify(registry.getPackage('com.example.showcase'))).not.toThrow(); + // The instance itself is gone from the record — this is a projection, not a + // replacement value that still points at the engine. + expect(JSON.stringify(registry.getPackage('com.example.showcase'))).not.toContain('_ObjectQL'); + }); + + it('survives a manifest that becomes cyclic only AFTER install', () => { + // The measured ordering: at install the plugin holds no engine and the + // manifest serialises fine; the cycle appears when the plugin initialises. + // A projection taken at install must still hold, because it copied out of + // the live object rather than aliasing it. + const plugin = new FakeConnectorPlugin(); + const manifest = baseManifest({ plugins: [plugin] }); + expect(() => JSON.stringify(manifest)).not.toThrow(); + + registry.installPackage(manifest); + plugin.init(makeCyclicEngine()); + + expect(() => JSON.stringify(registry.getPackage('com.example.showcase'))).not.toThrow(); + }); + + it('survives a reference cycle among PLAIN data in the manifest', () => { + const cyclic: Record = { name: 'self' }; + cyclic.self = cyclic; + registry.installPackage(baseManifest({ data: { node: cyclic } })); + + const record = registry.getPackage('com.example.showcase')!; + expect(() => JSON.stringify(record)).not.toThrow(); + // The back-edge is dropped; everything ahead of it survives. + expect((record.manifest as any).data.node.name).toBe('self'); + expect((record.manifest as any).data.node.self).toBeUndefined(); + }); + + it('keeps the declarative half of the manifest byte-for-byte', () => { + const manifest = baseManifest({ plugins: [new FakeConnectorPlugin()] }); + registry.installPackage(manifest); + const stored = registry.getPackage('com.example.showcase')!.manifest as any; + + for (const key of [ + 'id', 'name', 'namespace', 'version', 'type', 'scope', 'description', + 'dependencies', 'objects', 'apps', + ]) { + expect(stored[key]).toEqual((manifest as any)[key]); + } + }); + + it('drops members that are not data, and keeps a Date as data', () => { + const publishedAt = new Date('2026-09-02T00:00:00.000Z'); + registry.installPackage(baseManifest({ + onEnable: () => undefined, + registryHandle: new Map([['a', 1]]), + publishedAt, + })); + const stored = registry.getPackage('com.example.showcase')!.manifest as any; + + expect(stored.onEnable).toBeUndefined(); + expect(stored.registryHandle).toBeUndefined(); + expect(new Date(stored.publishedAt).toISOString()).toBe(publishedAt.toISOString()); + }); + + it('does not mutate the caller’s manifest — the kernel keeps the live object', () => { + // `ObjectQL.registerApp` reads `manifest.plugins[]` from ITS OWN parameter + // to register nested plugins, and hands the same object to `installPackage`. + // Projecting must therefore copy, never strip in place. + const plugin = new FakeConnectorPlugin(); + const manifest = baseManifest({ plugins: [plugin] }); + registry.installPackage(manifest); + + expect(manifest.plugins).toHaveLength(1); + expect(manifest.plugins[0]).toBe(plugin); + expect(registry.getPackage('com.example.showcase')!.manifest).not.toBe(manifest); + }); + + it('projects on REINSTALL too (rebuild / HMR overwrite)', () => { + registry.installPackage(baseManifest()); + const plugin = new FakeConnectorPlugin(); + plugin.init(makeCyclicEngine()); + registry.installPackage(baseManifest({ plugins: [plugin] })); + + expect(() => JSON.stringify(registry.getPackage('com.example.showcase'))).not.toThrow(); + }); + + it('keeps the whole LIST serializable when one package carries the cycle', () => { + // The list door's failure mode: one unserializable item took out every + // caller's whole listing, not just the offending package. + const plugin = new FakeConnectorPlugin(); + plugin.init(makeCyclicEngine()); + registry.installPackage(baseManifest({ plugins: [plugin] })); + registry.installPackage({ + id: 'com.objectstack.setup', name: 'Setup', namespace: 'setup', version: '9.3.0', + } as any); + + expect(() => JSON.stringify(registry.getAllPackages())).not.toThrow(); + expect(registry.getAllPackages()).toHaveLength(2); + }); + + it('still records the namespace and the lifecycle state it always did', () => { + // The projection must not cost the record its non-manifest half. + registry.installPackage(baseManifest({ plugins: [new FakeConnectorPlugin()] })); + const record = registry.getPackage('com.example.showcase')!; + + expect(record.status).toBe('installed'); + expect(record.enabled).toBe(true); + expect(typeof record.installedAt).toBe('string'); + expect(registry.getNamespaceOwners('showcase')).toEqual(['com.example.showcase']); + }); +}); diff --git a/packages/objectql/src/registry.ts b/packages/objectql/src/registry.ts index 5a2abe9e74..3ef1f3591c 100644 --- a/packages/objectql/src/registry.ts +++ b/packages/objectql/src/registry.ts @@ -1121,6 +1121,125 @@ function isShareableNamespace(ns: string): boolean { return RESERVED_NAMESPACES.has(ns) || ns === 'sys'; } +/** + * Sentinel for "this value is not data and does not belong in the record". + * A distinct symbol rather than `undefined`, so a manifest that genuinely + * carries `undefined` is not confused with a value that was dropped. + */ +const NOT_RECORD_DATA = Symbol('objectstack.registry.notRecordData'); + +/** Whether `value` is a bare `{}`-shaped object (or a null-prototype bag). */ +function isPlainDataObject(value: object): boolean { + const proto = Object.getPrototypeOf(value); + return proto === Object.prototype || proto === null; +} + +/** + * One step of {@link toRecordManifest} — project a value, or report that it is + * not data. `seen` carries the ancestors of the current value, so a CYCLE is + * detected at the second visit and the back-reference is dropped rather than + * followed. Siblings are re-projected independently (the ancestor is deleted on + * the way out), so a manifest that legitimately mentions the same sub-object + * twice keeps both copies. + */ +function projectRecordValue(value: unknown, seen: Set): unknown { + if (value === null) return null; + const kind = typeof value; + if (kind === 'string' || kind === 'number' || kind === 'boolean') return value; + // `undefined`, `function`, `symbol`, `bigint` — none of which survive + // `JSON.stringify` as themselves (`bigint` is the one that THROWS). + if (kind !== 'object') return NOT_RECORD_DATA; + + const obj = value as object; + // A `Date` is exotic but has a declared JSON form, so it is data. Copied + // rather than shared: the record must not alias a mutable runtime value. + if (obj instanceof Date) return new Date(obj.getTime()); + if (seen.has(obj)) return NOT_RECORD_DATA; + + if (Array.isArray(obj)) { + seen.add(obj); + const out: unknown[] = []; + for (const entry of obj) { + const projected = projectRecordValue(entry, seen); + // Non-data entries are OMITTED, not held as `null` holes: nothing reads a + // manifest collection by index (every one of them is name-keyed), and a + // `plugins: [null, null, …]` on the wire says less than `plugins: []`. + if (projected !== NOT_RECORD_DATA) out.push(projected); + } + seen.delete(obj); + return out; + } + + if (!isPlainDataObject(obj)) return NOT_RECORD_DATA; + + seen.add(obj); + const out: Record = {}; + for (const [key, entry] of Object.entries(obj)) { + const projected = projectRecordValue(entry, seen); + if (projected !== NOT_RECORD_DATA) out[key] = projected; + } + seen.delete(obj); + return out; +} + +/** + * Project an authored manifest into the SERIALIZABLE RECORD the registry keeps. + * + * {@link SchemaRegistry.installPackage} used to store the caller's object + * verbatim. For a code-defined stack (`objectstack.config.ts`) that object is + * the LIVE runtime one — `plugins: [new ConnectorRestPlugin(), …]` — and a + * plugin instance holds the engine once it initialises. Since the engine grew + * `actionActivation -> store -> engine` that reference closes a CYCLE, so + * `JSON.stringify` of the registry item THREW and every read door that + * serialises a package answered `500 INTERNAL_ERROR` on a stock showcase boot: + * `GET /packages`, `GET /packages/:id`, `GET /meta/package/:id`. Before that + * cycle existed the same doors serialised the whole engine graph into the + * payload instead, which is the same defect with a quieter symptom. + * + * Measured on that boot: of the 26 installed packages exactly ONE manifest key + * was unserializable — `plugins`, on `com.example.showcase` — and only AFTER + * plugin init; the same manifest serialised cleanly during boot. So an + * install-time `JSON.stringify` probe would have called the record healthy. The + * projection therefore drops by SHAPE, at install, and never depends on when it + * is asked. + * + * The registry item is a RECORD, not the runtime. The kernel keeps the live + * object (`ObjectQL.manifests`), and the one reader of `manifest.plugins[]` — + * `ObjectQL.registerApp`'s nested-plugin seam — reads its OWN parameter, never + * the record, so nothing downstream loses a member it was using. + * + * The rule is STRUCTURAL rather than a key denylist, because the fault is not + * "the key is called `plugins`" — it is "a live object reached the record". + * JSON data survives; everything else is dropped: + * + * - primitives, plain objects, arrays and `Date` are data; + * - functions / symbols / bigints are dropped (a function was already + * invisible to `JSON.stringify`; dropping it only makes the record honest, + * and a bigint would have thrown); + * - class instances, `Map`, `Set` and every other exotic object are dropped — + * a declarative manifest has none, and a code-defined stack's are host + * wiring; + * - a reference cycle among plain data is dropped at the back-edge, so a + * self-referencing manifest degrades to a missing field instead of throwing. + * + * ⛔ This is the PRODUCER's repair, not a consumer-side tolerance: no reader is + * taught to survive an unserializable record, because the record is never + * unserializable to begin with (AGENTS.md Prime Directive #12). + */ +function toRecordManifest(manifest: ObjectStackManifest): ObjectStackManifest { + // The ROOT is projected key-by-key unconditionally: a host whose + // `defineStack()` returns a class instance must still yield a record, and + // `projectRecordValue` would drop the whole thing. + if (manifest === null || typeof manifest !== 'object') return manifest; + const seen = new Set([manifest as object]); + const out: Record = {}; + for (const [key, value] of Object.entries(manifest as Record)) { + const projected = projectRecordValue(value, seen); + if (projected !== NOT_RECORD_DATA) out[key] = projected; + } + return out as ObjectStackManifest; +} + /** * Raised when a package is installed whose `manifest.namespace` is already owned * by a **different** installed package in this installation (ADR-0048 Phase 1). @@ -3811,7 +3930,11 @@ export class SchemaRegistry { const now = new Date().toISOString(); const disabled = this.initialDisabledPackageIds.has(manifest.id); const pkg: InstalledPackage = { - manifest, + // The RECORD's manifest, not the caller's live object — see + // {@link toRecordManifest}. Every other read below (`manifest.id`, + // `manifest.namespace`) deliberately keeps reading the ARGUMENT: the + // projection is what the registry hands out, never what it decides with. + manifest: toRecordManifest(manifest), status: disabled ? 'disabled' : 'installed', enabled: !disabled, installedAt: now, diff --git a/packages/rest/src/package-registry-item-projection.test.ts b/packages/rest/src/package-registry-item-projection.test.ts new file mode 100644 index 0000000000..0cc07b22d4 --- /dev/null +++ b/packages/rest/src/package-registry-item-projection.test.ts @@ -0,0 +1,200 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The REST package read doors project a REGISTRY-sourced entry onto its + * declared fields instead of spreading it whole. + * + * ## Where this sits + * + * The card behind it is the `500 Converting circular structure to JSON` a stock + * showcase boot answered on `GET /api/v1/packages` — `SchemaRegistry.install- + * Package` stored the caller's live `defineStack()` object, whose `plugins: [...]` + * held initialised plugin instances and through them the engine. The REPAIR is + * at that producer (`@objectstack/objectql`), so by the time an entry reaches + * this door there is nothing unserializable left in it. + * + * ⚠️ Measured, and worth writing down because the card attributed the 500 here: + * in the showcase composition these two same-pattern routes are NOT the ones + * that answered — the dispatcher twin (`packages/runtime/src/domains/packages.ts`) + * did. The 404 wording separates them: `Package "x" was not found.` here, + * `Package 'x' not found` there, and the live probe returned the latter. So what + * this file pins is this door's own posture, not the reproduction of the boot. + * + * ## What it pins + * + * `{ ...item, source: 'registry' }` gave ONE undeclared member on ONE package + * the power to fail the whole list for every caller. The projection turns that + * into a field the response never mentions. Both halves matter and both are + * asserted: the undeclared member is DROPPED, and every declared field — + * including the `_diagnostics` the protocol's own decoration grafts on — is + * still SERVED. A projection that quietly ate `_diagnostics` would pass a + * "no longer 500" assertion just as well. + * + * ⛔ The DATABASE half is deliberately not projected and is asserted unchanged: + * its shape belongs to `PackageService`, not to this door. + */ + +import { describe, it, expect } from 'vitest'; +import type { RouteHandler } from '@objectstack/spec/contracts'; +import { registerPackageRoutes } from './package-routes.js'; + +const PKGS = '/api/v1/packages'; + +interface Captured { status: number; body: any } + +/** The engine's own `actionActivation -> store -> engine` cycle, reproduced. */ +function cyclicEngine(): Record { + const engine: Record = { name: '_ObjectQL' }; + const store: Record = { name: 'ObjectStoreActionActivationStore', engine }; + engine.actionActivation = { name: 'ActionActivationProjection', store }; + return engine; +} + +/** + * A registry entry in the shape `protocol.getMetaItems({ type: 'package' })` + * yields: the installed-package record, plus the `_diagnostics` that + * `decorateMetadataItem` grafts on, plus — the case under test — an undeclared + * member holding a live object. + */ +function registryEntry(extra: Record = {}) { + return { + manifest: { id: 'com.example.showcase', name: 'Showcase', version: '0.3.16' }, + status: 'installed', + enabled: true, + installedAt: '2026-09-02T00:00:00.000Z', + updatedAt: '2026-09-02T00:00:00.000Z', + settings: { theme: 'dark' }, + _diagnostics: [{ code: 'SOMETHING', message: 'noted' }], + ...extra, + }; +} + +function mount(svc: any, protocolItems: unknown[]) { + const routes = new Map(); + const server = { + get: (p: string, h: RouteHandler) => { routes.set(`GET:${p}`, h); }, + post: (p: string, h: RouteHandler) => { routes.set(`POST:${p}`, h); }, + put: () => {}, + delete: (p: string, h: RouteHandler) => { routes.set(`DELETE:${p}`, h); }, + patch: () => {}, + use: () => {}, + listen: async () => {}, + close: async () => {}, + } as any; + // The authorization gate (#7033 / #7023) is not this file's subject. + registerPackageRoutes(server, () => svc as any, '/api/v1', { + resolveExecutionContext: async () => ({ + userId: 'u_pkg', systemPermissions: ['manage_metadata', 'studio.access', 'setup.access'], + }), + protocol: { getMetaItems: async () => ({ items: protocolItems }) }, + } as any); + return routes; +} + +async function drive( + routes: Map, + method: string, + path: string, + req: Record = {}, +): Promise { + const handler = routes.get(`${method}:${path}`); + if (!handler) throw new Error(`no handler for ${method} ${path}`); + const captured: Captured = { status: 200, body: undefined }; + const res: any = { + json(data: any) { captured.body = data; }, + send() {}, + status(code: number) { captured.status = code; return res; }, + header() { return res; }, + }; + await handler( + { params: {}, query: {}, body: undefined, headers: {}, method, path, ...req } as any, + res, + ); + return captured; +} + +/** A durable half that finds nothing, so only the registry half is exercised. */ +const EMPTY_DB = { list: async () => [], get: async () => null }; + +describe('GET /packages — registry entries are projected, not spread', () => { + it('serves the entry when it carries an undeclared LIVE member, instead of 500', async () => { + const routes = mount(EMPTY_DB, [registryEntry({ liveEngineHandle: cyclicEngine() })]); + const { status, body } = await drive(routes, 'GET', PKGS); + + expect(status).toBe(200); + // The step the transport takes next, and the one that threw on the boot. + expect(() => JSON.stringify(body)).not.toThrow(); + expect(body.data.packages).toHaveLength(1); + expect('liveEngineHandle' in body.data.packages[0]).toBe(false); + }); + + it('still serves every declared field, and the protocol’s `_diagnostics`', async () => { + const routes = mount(EMPTY_DB, [registryEntry({ liveEngineHandle: cyclicEngine() })]); + const { body } = await drive(routes, 'GET', PKGS); + const served = body.data.packages[0]; + + expect(served.manifest).toEqual({ id: 'com.example.showcase', name: 'Showcase', version: '0.3.16' }); + expect(served.status).toBe('installed'); + expect(served.enabled).toBe(true); + expect(served.installedAt).toBe('2026-09-02T00:00:00.000Z'); + expect(served.updatedAt).toBe('2026-09-02T00:00:00.000Z'); + expect(served.settings).toEqual({ theme: 'dark' }); + expect(served._diagnostics).toEqual([{ code: 'SOMETHING', message: 'noted' }]); + // The provenance marker the door adds is unchanged. + expect(served.source).toBe('registry'); + }); + + it('omits a declared field that is absent rather than serialising `undefined`', async () => { + const routes = mount(EMPTY_DB, [{ + manifest: { id: 'com.objectstack.setup' }, status: 'installed', enabled: true, + }]); + const { body } = await drive(routes, 'GET', PKGS); + + expect(Object.keys(body.data.packages[0]).sort()) + .toEqual(['enabled', 'manifest', 'source', 'status']); + }); + + it('leaves the DATABASE half’s shape alone — this door does not own it', async () => { + const dbRow = { + id: 'com.acme.published', + manifest: { id: 'com.acme.published', version: '2.0.0' }, + publishedBy: 'u_release', + artifactSize: 4096, + }; + const routes = mount({ list: async () => [dbRow], get: async () => null }, []); + const { body } = await drive(routes, 'GET', PKGS); + + // Fields that are NOT part of the installed-package record still travel. + expect(body.data.packages[0].publishedBy).toBe('u_release'); + expect(body.data.packages[0].artifactSize).toBe(4096); + expect(body.data.packages[0].source).toBe('database'); + }); +}); + +describe('GET /packages/:id — the registry fallback is projected too', () => { + it('serves the entry with the undeclared member dropped', async () => { + const routes = mount(EMPTY_DB, [registryEntry({ liveEngineHandle: cyclicEngine() })]); + const { status, body } = await drive( + routes, 'GET', `${PKGS}/:id`, { params: { id: 'com.example.showcase' } }, + ); + + expect(status).toBe(200); + expect(() => JSON.stringify(body)).not.toThrow(); + expect('liveEngineHandle' in body.data.package).toBe(false); + expect(body.data.package.manifest.id).toBe('com.example.showcase'); + expect(body.data.package._diagnostics).toEqual([{ code: 'SOMETHING', message: 'noted' }]); + expect(body.data.package.source).toBe('registry'); + }); + + it('a genuine MISS is still a 404 with this door’s own wording', async () => { + // The other direction: projection must not turn "absent" into "present". + const routes = mount(EMPTY_DB, [registryEntry()]); + const { status, body } = await drive( + routes, 'GET', `${PKGS}/:id`, { params: { id: 'no.such.package' } }, + ); + + expect(status).toBe(404); + expect(body.error.code).toBe('RESOURCE_NOT_FOUND'); + expect(body.error.message).toBe('Package "no.such.package" was not found.'); + }); +}); diff --git a/packages/rest/src/package-routes.ts b/packages/rest/src/package-routes.ts index 079ebd21bb..38988de559 100644 --- a/packages/rest/src/package-routes.ts +++ b/packages/rest/src/package-routes.ts @@ -203,6 +203,73 @@ async function refusePackageRequest( * case in `package-door-5xx-message-sanitization.test.ts` so it goes red the * day either lands. */ +/** + * The fields a REGISTRY-sourced package entry is declared to carry on this + * door: the installed-package record shape (`InstalledPackageSchema`, + * `@objectstack/spec/kernel` — that schema is the authority; this list mirrors + * it deliberately, so publishing a newly declared field here is a decision + * rather than a side effect), plus `_diagnostics`, which is not part of the + * record at all — `decorateMetadataItem` in `@objectstack/metadata-protocol` + * grafts it onto every item leaving `getMetaItems`, and this door serves that + * output. It is listed because it is MEASURED to be the only thing the + * decoration adds for `type: 'package'`, not because the record declares it. + */ +const REGISTRY_PACKAGE_RESPONSE_FIELDS = [ + 'manifest', + 'status', + 'enabled', + 'installedAt', + 'updatedAt', + 'installedVersion', + 'previousVersion', + 'statusChangedAt', + 'errorMessage', + 'settings', + 'upgradeHistory', + 'registeredNamespaces', + '_diagnostics', + // [#14375] The ADR-0070 D2 writability verdict. Like `_diagnostics` this is + // NOT a record field: `getMetaItems({ type: 'package' })` stamps it on every + // registry item, and this door's contract is to CARRY it (the durable row is + // spread over the registry item, so a durable copy without the key must leave + // the verdict standing) — pinned in `package-list-writable-carry.test.ts`. + // Listed here because an allowlist that omitted it would drop the field + // SILENTLY, answering 200 with the verdict simply gone. + 'writable', +] as const; + +/** + * Project a registry-sourced entry onto its declared fields before it is spread + * into a response — defence in depth behind the `500 Converting circular + * structure to JSON` repair, not the repair itself. + * + * The repair is at the PRODUCER: `SchemaRegistry.installPackage` + * (`@objectstack/objectql`) stores a serializable projection of the manifest + * instead of the caller's live `defineStack()` object, whose `plugins: [...]` + * held initialised plugin instances and through them the engine — a cycle since + * the engine grew `actionActivation -> store -> engine`. So this door has + * nothing unserializable left to hand out. + * + * What the projection adds is the failure MODE for the next undeclared member: + * `{ ...item }` let ONE bad member on ONE package fail the whole list for every + * caller, and an explicit field list degrades the same member to a field this + * response never mentions. Only the REGISTRY half is projected — the database + * half below is `PackageService`'s own durable JSON, whose shape this door does + * not own and must not narrow. + * + * Undefined fields are omitted, so the bytes are unchanged for every entry that + * already served fine. + */ +function toRegistryPackageResponse(item: unknown): Record { + if (item === null || typeof item !== 'object') return {}; + const src = item as Record; + const out: Record = {}; + for (const field of REGISTRY_PACKAGE_RESPONSE_FIELDS) { + if (src[field] !== undefined) out[field] = src[field]; + } + return out; +} + function sendThrownError(res: any, error: unknown): void { const thrown = resolveThrownHttpError(error); // The dispatcher twin's expression, byte for byte — one rule, two doors. @@ -730,7 +797,7 @@ export function registerPackageRoutes( const id = item.manifest?.id || item.id; if (id) { packagesMap.set(id, { - ...item, + ...toRegistryPackageResponse(item), source: 'registry', }); } @@ -882,7 +949,7 @@ export function registerPackageRoutes( (item.manifest?.id || item.id) === packageId ); if (match) { - sendOk(res, { package: { ...match, source: 'registry' } }); + sendOk(res, { package: { ...toRegistryPackageResponse(match), source: 'registry' } }); return; } } diff --git a/packages/runtime/src/domains/packages-serializable-response.test.ts b/packages/runtime/src/domains/packages-serializable-response.test.ts new file mode 100644 index 0000000000..b5cdca471b --- /dev/null +++ b/packages/runtime/src/domains/packages-serializable-response.test.ts @@ -0,0 +1,206 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `/packages` read doors must never hand back a response that cannot be + * serialised. + * + * ## What was wrong + * + * On a stock showcase boot, signed in as the seeded admin: + * + * GET /api/v1/packages -> 500 INTERNAL_ERROR + * GET /api/v1/packages/com.example.showcase -> 500 INTERNAL_ERROR + * GET /api/v1/meta/package/com.example.showcase -> 500 + * GET /api/v1/meta/package/com.objectstack.setup -> 200 + * + * with `Converting circular structure to JSON · _ObjectQL -> actionActivation -> + * store -> engine`. `SchemaRegistry.installPackage` stored the caller's live + * `defineStack()` object, whose `plugins: [...]` hold initialised plugin + * instances and through them the engine. Studio asks for the list three times + * per open. + * + * ## Which door this file drives, and why that matters + * + * ⚠️ Measured on the failing boot rather than assumed: `GET /api/v1/packages` + * and `GET /api/v1/packages/:id` are answered by THIS handler + * (`handlePackagesRequest`), not by the same-pattern routes in + * `packages/rest/src/package-routes.ts`. The 404 wording decides it — + * `Package 'x' not found` (this file) against `Package "x" was not found.` + * (the REST twin) — and a live probe returned the former. + * + * ## Two different claims, pinned separately + * + * 1. THE FIX, at the producer: with a real `SchemaRegistry` holding a + * showcase-shaped manifest, both doors answer 200 and their bodies + * round-trip. Remove `toRecordManifest` from `installPackage` and these go + * red with the exact `Converting circular structure to JSON` this card is + * about. + * 2. THE DEFENCE, at this door: an undeclared member appearing on the registry + * ITEM — not the manifest — degrades to a field the response never mentions, + * instead of failing the whole read. That is what `{ ...pkg }` could not do: + * one bad member on one package took out the list for every caller. + */ + +import { describe, it, expect } from 'vitest'; +import { SchemaRegistry } from '@objectstack/objectql'; +import { HttpDispatcher } from '../http-dispatcher.js'; + +/** Authenticated caller holding the ADR-0106 D4 read capability. */ +const reader = (): any => ({ + request: {}, + environmentId: 'platform', + executionContext: { userId: 'u_admin', isSystem: false, systemPermissions: ['studio.access'] }, +}); + +/** The engine's own `actionActivation -> store -> engine` cycle, reproduced. */ +function cyclicEngine(): Record { + const engine: Record = { name: '_ObjectQL' }; + const store: Record = { name: 'ObjectStoreActionActivationStore', engine }; + engine.actionActivation = { name: 'ActionActivationProjection', store }; + return engine; +} + +/** A host-constructed connector plugin that takes the engine on init. */ +class FakeConnectorPlugin { + name = 'connector-rest'; + engine: unknown; + init(engine: unknown) { this.engine = engine; } +} + +/** + * A registry in the showcase's shape: one app package whose manifest carries + * live, initialised plugin instances, plus one plugin-less platform package + * (the `com.objectstack.setup` that kept answering 200 throughout). + */ +function showcaseShapedRegistry(): SchemaRegistry { + const registry = new SchemaRegistry({ multiTenant: false, collisionPolicy: 'error' }); + (registry as any).logLevel = 'silent'; + + const plugin = new FakeConnectorPlugin(); + registry.installPackage({ + id: 'com.example.showcase', + name: 'Showcase', + namespace: 'showcase', + version: '0.3.16', + type: 'app', + scope: 'user', + description: 'Kitchen-sink showcase workspace', + objects: [{ name: 'invoice', fields: { total: { type: 'currency' } } }], + apps: [{ name: 'showcase', label: 'Showcase' }], + plugins: [plugin], + } as any); + // Init AFTER install — the measured ordering: the manifest serialised + // cleanly during boot and only became cyclic once the plugins came up. + plugin.init(cyclicEngine()); + + registry.installPackage({ + id: 'com.objectstack.setup', + name: 'Setup', + namespace: 'setup', + version: '9.3.0', + type: 'plugin', + scope: 'system', + } as any); + + return registry; +} + +function dispatcherOver(registry: SchemaRegistry): HttpDispatcher { + const kernel: any = { + context: { getService: (n: string) => (n === 'objectql' ? { registry } : null) }, + }; + return new HttpDispatcher(kernel); +} + +describe('/packages read doors — the response always serialises', () => { + it('GET /packages answers 200 over a showcase-shaped registry and round-trips', async () => { + const registry = showcaseShapedRegistry(); + const r = await dispatcherOver(registry).handlePackages('/', 'GET', undefined, {}, reader()); + + expect(r.response?.status).toBe(200); + // The exact step the HTTP layer takes next, and the one that threw. + expect(() => JSON.stringify(r.response?.body)).not.toThrow(); + + const packages = r.response?.body?.data?.packages; + expect(packages).toHaveLength(2); + expect(r.response?.body?.data?.total).toBe(2); + expect(packages.map((p: any) => p.manifest.id).sort()) + .toEqual(['com.example.showcase', 'com.objectstack.setup']); + }); + + it('GET /packages/:id answers 200 for the package that carried the cycle', async () => { + const registry = showcaseShapedRegistry(); + const r = await dispatcherOver(registry) + .handlePackages('/com.example.showcase', 'GET', undefined, {}, reader()); + + expect(r.response?.status).toBe(200); + expect(() => JSON.stringify(r.response?.body)).not.toThrow(); + expect(r.response?.body?.data?.manifest?.id).toBe('com.example.showcase'); + }); + + it('serves the declared record fields, and no engine reference among them', () => { + // Guards the OTHER direction of the projection: dropping the runtime + // half must not cost the record the lifecycle half the doors publish. + const registry = showcaseShapedRegistry(); + const record: any = registry.getPackage('com.example.showcase'); + expect(record.status).toBe('installed'); + expect(record.enabled).toBe(true); + expect(JSON.stringify(record)).not.toContain('_ObjectQL'); + }); + + it('an undeclared LIVE member on the registry item degrades to a missing field', async () => { + // The door's own defence, independent of the producer's repair: this + // member is on the ITEM, not inside the manifest, so no projection at + // install time can reach it. + const registry = showcaseShapedRegistry(); + (registry.getPackage('com.example.showcase') as any).liveEngineHandle = cyclicEngine(); + + const list = await dispatcherOver(registry).handlePackages('/', 'GET', undefined, {}, reader()); + expect(list.response?.status).toBe(200); + expect(() => JSON.stringify(list.response?.body)).not.toThrow(); + expect(list.response?.body?.data?.packages + .some((p: any) => 'liveEngineHandle' in p)).toBe(false); + + const detail = await dispatcherOver(registry) + .handlePackages('/com.example.showcase', 'GET', undefined, {}, reader()); + expect(detail.response?.status).toBe(200); + expect(() => JSON.stringify(detail.response?.body)).not.toThrow(); + expect('liveEngineHandle' in detail.response?.body?.data).toBe(false); + // …and the declared half is untouched by the drop. + expect(detail.response?.body?.data?.manifest?.id).toBe('com.example.showcase'); + expect(detail.response?.body?.data?.status).toBe('installed'); + }); + + it('stamps the #14375 `writable` verdict AFTER the projection, on both doors', async () => { + // ORDER PIN. `toPackageResponse` is an allowlist of the declared record + // fields and `writable` is not one of them — it is this door's own + // computed answer. Stamped BEFORE the projection it would be deleted, + // and deleted silently: a 200 with the field simply absent. Asserting + // the KEY's presence rather than its value keeps this about the order, + // which is the thing a refactor can quietly get wrong; the verdict's + // own semantics are pinned in `packages-writable-verdict.test.ts`. + const registry = showcaseShapedRegistry(); + + const list = await dispatcherOver(registry).handlePackages('/', 'GET', undefined, {}, reader()); + expect(list.response?.status).toBe(200); + for (const row of list.response?.body?.data?.packages) { + expect('writable' in row).toBe(true); + } + + const detail = await dispatcherOver(registry) + .handlePackages('/com.example.showcase', 'GET', undefined, {}, reader()); + expect(detail.response?.status).toBe(200); + expect('writable' in detail.response?.body?.data).toBe(true); + // …and the projection still did its job in the same pass. + expect(() => JSON.stringify(detail.response?.body)).not.toThrow(); + }); + + it('a genuine MISS is still a 404 with the wording this door owns', async () => { + // Both directions: the projection must not turn "absent" into "present + // but empty", and the wording is what identifies which door answered. + const r = await dispatcherOver(showcaseShapedRegistry()) + .handlePackages('/no.such.package', 'GET', undefined, {}, reader()); + expect(r.response?.status).toBe(404); + expect(r.response?.body?.error?.message).toBe("Package 'no.such.package' not found"); + }); +}); diff --git a/packages/runtime/src/domains/packages.ts b/packages/runtime/src/domains/packages.ts index dbae972119..cf0772d166 100644 --- a/packages/runtime/src/domains/packages.ts +++ b/packages/runtime/src/domains/packages.ts @@ -90,6 +90,56 @@ export function createPackagesDomain(deps: DomainHandlerDeps): DomainRoute { * primitives the deployment supports, and (b) nothing is written or deleted * before the refusal — "delete first, refuse second" is the worst shape here. */ +/** + * The fields an installed-package RECORD is declared to carry + * (`InstalledPackageSchema`, `@objectstack/spec/kernel` — that schema is the + * authority; this list mirrors it and is deliberately not derived from it, so + * adding a field to the schema is a decision to publish it here, not an + * automatic one). + */ +const INSTALLED_PACKAGE_RESPONSE_FIELDS = [ + 'manifest', + 'status', + 'enabled', + 'installedAt', + 'updatedAt', + 'installedVersion', + 'previousVersion', + 'statusChangedAt', + 'errorMessage', + 'settings', + 'upgradeHistory', + 'registeredNamespaces', +] as const; + +/** + * Project a registry item onto the declared record fields before it goes on the + * wire — the second half of the `500 Converting circular structure to JSON` + * repair, and defence in depth rather than the fix. + * + * The fix is at the PRODUCER: `SchemaRegistry.installPackage` now stores a + * serializable projection of the manifest, so this door has nothing + * unserializable to hand out. What this adds is the failure MODE for the next + * time: an undeclared member appearing on the registry item — a live handle, a + * back-reference — degrades to a field this response never mentions, instead of + * failing the whole read with a 500. `{ ...pkg }` had the opposite property: + * ONE bad member on ONE package took out the entire list for every caller, + * which is exactly how a stock showcase boot answered 500 on `GET /packages` + * while Studio asked for it three times per open. + * + * Undefined fields are omitted rather than serialised as explicit `undefined`, + * so the response bytes are unchanged for every record that was already fine. + */ +function toPackageResponse(pkg: unknown): unknown { + if (pkg === null || typeof pkg !== 'object') return pkg; + const src = pkg as Record; + const out: Record = {}; + for (const field of INSTALLED_PACKAGE_RESPONSE_FIELDS) { + if (src[field] !== undefined) out[field] = src[field]; + } + return out; +} + function requireManageMetadata(deps: DomainHandlerDeps, context: HttpProtocolContext): HttpDispatcherResult | null { const ec: any = context?.executionContext; if (!ec?.isSystem && !new Set(ec?.systemPermissions ?? []).has('manage_metadata')) { @@ -307,7 +357,19 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin // [#14375] Every row carries the server's own writability verdict // (see `withWritableVerdict`) — copies, so the registry records the // filters above selected stay untouched. - const rows = packages.map((p: any) => withWritableVerdict(qlService, p)); + // + // [#14309] ORDER IS LOAD-BEARING: project FIRST, stamp the verdict + // onto the projection SECOND. `toPackageResponse` is an allowlist of + // the DECLARED record fields, and `writable` is not one of them — it + // is this door's own computed answer. Stamped first, the projection + // would delete it, and the deletion would be silent: a 200 with the + // field simply absent. `withWritableVerdict` reads only + // `manifest.id`, which the projection keeps, so it is indifferent to + // running second. Pinned in `packages-serializable-response.test.ts` + // so a future reorder is a red test rather than a missing field. + const rows = packages.map( + (p: any) => withWritableVerdict(qlService, toPackageResponse(p) as any), + ); return { handled: true, response: deps.success({ packages: rows, total: rows.length }) }; } @@ -883,8 +945,12 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin const id = decodeURIComponent(parts[0]); const pkg = registry.getPackage(id); if (!pkg) return { handled: true, response: deps.error(`Package '${id}' not found`, 404) }; - // [#14375] Same verdict, same predicate as the list door. - return { handled: true, response: deps.success(withWritableVerdict(qlService, pkg)) }; + // [#14375] Same verdict, same predicate as the list door — and + // [#14309] the same project-then-stamp order, for the same reason. + return { + handled: true, + response: deps.success(withWritableVerdict(qlService, toPackageResponse(pkg) as any)), + }; } // PATCH /packages/:id → edit the manifest (name / description /