diff --git a/.changeset/21658-hook-handler-without-body-save-door.md b/.changeset/21658-hook-handler-without-body-save-door.md new file mode 100644 index 00000000000..356d033ad2e --- /dev/null +++ b/.changeset/21658-hook-handler-without-body-save-door.md @@ -0,0 +1,26 @@ +--- +'@objectstack/metadata-protocol': minor +--- + +The runtime save door refuses a hook whose `handler` names a function and that carries no `body`: a hook stored there ships with no code package, so that name can never bind + +Clause-②: no (narrowing) + + + +**BREAKING** accept-set narrowing at the runtime save door, shipped as `minor` under the repo's launch-window convention for breaking changes, the grade the same door's earlier refusals shipped with. + +**One rule.** `saveMetaItem`, which `PUT /api/v1/meta/hook/:name` and the dispatcher's metadata save both call, now refuses a `hook` whose `handler` is a function name and that carries no `body`. A hook stored through this door ships with no code package, so it holds no functions, and a `handler` name resolves only inside the hook's own package. Before this change the door answered 200, the runtime then refused the hook at bind (`INVALID_REFERENCE` / 400, in the server log only), and the hook never ran. The refusal is `VALIDATION_ERROR` / 400, in draft and in publish mode, before anything is stored or bound. It names the hook and the function, and prescribes a `body`. + +**Before and after** (with `{ name: 'stamp_status', object: 'crm_note', events: ['beforeInsert'], handler: 'x_stamp' }`): + +- Before: 200 `Saved hook 'stamp_status'`, the row stored, the hook refused at bind and never run, and nothing on the response said so. +- After: 400 `VALIDATION_ERROR`, naming `stamp_status` and `x_stamp`, and nothing stored. + +**What still saves.** A hook with a `body`. A hook carrying both a `body` and a `handler`: the binder runs the body and never consults the name, and the install-local door accepts the same shape. A malformed `body` still gets the type schema's located `422 INVALID_METADATA`. + +**What is unchanged.** `HookSchema` still accepts the string `handler`, because a build artifact carries it: `objectstack build` lowers an inline function to the hook's name and ships the function in the artifact's runtime module. A hook in an artifact or a `defineStack` config binds to its own package's functions on its own door, which never reaches this one. `os validate` and `os build` are unchanged. + +**Rows stored before this change.** They keep their bytes, nothing re-saves them, and the runtime refuses them at bind as before. A new save of one, a re-save included, is refused until it carries a `body`. Package duplication reports such a row as failed with this refusal; `migrate meta --stored` leaves it as it is. Delete stays open. + +**The fix.** Give the hook a `body`: sandboxed JS (`{ language: 'js', source }`) or an expression (`{ language: 'expression', source }`). A hook that must run a package's own function belongs in that package's code, where its `handler` resolves. diff --git a/packages/metadata-protocol/src/protocol.invalid-metadata-422-face-inventory.test.ts b/packages/metadata-protocol/src/protocol.invalid-metadata-422-face-inventory.test.ts index 909498c5dfd..3fe60aae416 100644 --- a/packages/metadata-protocol/src/protocol.invalid-metadata-422-face-inventory.test.ts +++ b/packages/metadata-protocol/src/protocol.invalid-metadata-422-face-inventory.test.ts @@ -492,3 +492,81 @@ describe('[#21565] a hook body bound to a stored-metadata table is refused at th expect([...rows.values()].map((r) => [r.type, r.name])).toEqual([['hook', 'stamp_status']]); }); }); + +// ═══════════════════════════════════════════════════════════════════════════ +// 7. #21658 — the `hook` door refuses a `handler` name with no `body` +// ═══════════════════════════════════════════════════════════════════════════ +// +// The same door as section 6, and the contrast to it: this refusal is NOT the +// type schema's. `HookSchema` keeps accepting the string `handler`, because a +// build artifact legitimately carries that form, and the artifact door never +// reaches `saveMetaItem`. What this door stores ships with no code package, +// and a `handler` name resolves only inside the hook's own package, so a hook +// naming a function and carrying no `body` can never bind once stored. The +// door refuses it with `VALIDATION_ERROR` / 400 — the envelope of the name +// check every body passes — before anything is stored, in publish and in draft +// mode. Rides this file's pinned engine double, as section 6 does. + +describe('[#21658] a hook naming a function in `handler` with no `body` is refused at the metadata door', () => { + const handlerOnly = () => ({ + name: 'stamp_status', + object: 'hks_note', + events: ['beforeInsert'], + handler: 'x_stamp', + }); + const body = { language: 'js', source: "ctx.input.status = 'seen';" }; + + it.each([ + ['publish', undefined], + ['draft', 'draft'], + ] as const)('%s mode — VALIDATION_ERROR / 400, naming the hook and its handler, nothing stored', async (_label, mode) => { + const { protocol, rows } = makeProtocol(); + let err: any; + try { + await protocol.saveMetaItem({ + type: 'hook', + name: 'stamp_status', + item: handlerOnly(), + writeFace: 'meta-envelope', + actor: 'usr_admin', + ...(mode ? { mode } : {}), + }); + } catch (e) { + err = e; + } + + expect(err).toBeInstanceOf(Error); + expect({ code: err.code, status: err.status }).toEqual({ code: 'VALIDATION_ERROR', status: 400 }); + expect(err.message).toContain("'stamp_status'"); + expect(err.message).toContain("'x_stamp'"); + expect(err.message).toContain('Give it a `body`'); + expect(rows.size).toBe(0); + }); + + it('CONTROL — the same hook with a `body` saves', async () => { + const { protocol, rows } = makeProtocol(); + const { handler: _dropped, ...withoutHandler } = handlerOnly(); + const result = await saveHookAsAdministrator(protocol, { ...withoutHandler, body }); + + expect(result instanceof Error ? `${result.message} ${JSON.stringify((result as any).issues ?? [])}` : 'stored').toBe('stored'); + expect([...rows.values()].map((r) => [r.type, r.name])).toEqual([['hook', 'stamp_status']]); + }); + + it('a `body` beside the `handler` saves: the binder runs the body and never consults the name', async () => { + const { protocol, rows } = makeProtocol(); + const result = await saveHookAsAdministrator(protocol, { ...handlerOnly(), body }); + + expect(result instanceof Error ? `${result.message} ${JSON.stringify((result as any).issues ?? [])}` : 'stored').toBe('stored'); + expect([...rows.values()].map((r) => [r.type, r.name])).toEqual([['hook', 'stamp_status']]); + }); + + it('a malformed `body` beside the `handler` gets the schema\'s located 422, not "give it a body"', async () => { + const { protocol, rows } = makeProtocol(); + const err = await saveHookAsAdministrator(protocol, { ...handlerOnly(), body: 'return;' }); + + expect(err).toBeInstanceOf(Error); + expect({ code: err.code, status: err.status }).toEqual({ code: 'INVALID_METADATA', status: 422 }); + expect((err.issues as Array<{ path?: string }>).map((i) => i.path)).toContain('body'); + expect(rows.size).toBe(0); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index f853b383e1b..3496aca9fec 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -779,6 +779,70 @@ function resolveOverlaySchema(type: string, _item: unknown): z.ZodTypeAny | null return getMetadataTypeSchema(singular) ?? null; } +/** + * [#21658] The save door's refusal of a `hook` whose `handler` names a + * function and that carries no `body`: such a hook can never run once this + * door has stored it. + * + * Why it can never bind. A hook's `handler` name resolves inside the hook's + * own package only (the maintainer's ruling on #21604, letter B; the binder's + * `resolveHandler` in `@objectstack/objectql`'s `hook-binder.ts`). A hook this + * door stores ships with no code package: the runtime binds every stored hook + * under the synthetic owner `metadata-service` (`ObjectQLPlugin`'s authored + * hook re-sync), with no `functions` map, and no package of that name + * registers functions. So the name has nothing to resolve against, and the + * binder refuses the hook at registration (`INVALID_REFERENCE` / 400, logged + * at `error`) after this door has already answered success. Refusing it here + * says so to the author, before anything is stored. + * + * The predicate is the binder's own body-first test: a `body` object is bound + * through the body runner and the `handler` is never consulted, so a hook + * carrying BOTH a `body` and a `handler` saves (its body runs), as it installs + * on the install-local door. Asked after the type schema has accepted the + * body, so `body` here is either absent or a declared hook body, and a + * malformed `body` gets the schema's own located `422` instead of this + * refusal's "give it a body". + * + * ⛔ Not a `HookSchema` rule: a build artifact legitimately carries the string + * form (`objectstack build` lowers an inline function to the hook's name and + * ships the function in the artifact's runtime module), and the artifact and + * boot doors never reach `saveMetaItem`. This is the runtime-authoring door's + * rule only, the same shape install-local refuses on its own door (#21585). + * + * Every writer through this door is judged: the REST and dispatcher saves, in + * draft and in publish mode, and the two server-stated re-savers + * (`migrateStoredMetadata`, `duplicatePackage`), which record this refusal as + * the row's failure. A row stored before this rule keeps its bytes. + * + * `VALIDATION_ERROR` / 400, the envelope of the name check the door runs on + * every body (`savedItemNameRefusal`). The message names the hook and its + * `handler`, prescribes the `body` first, and only then explains: a 4xx + * message crosses the REST boundary bounded at 500 characters with its TAIL + * truncated, and the whole sentence stays under that bound for any hook and + * function name shorter than about 65 characters each. Runtime words carry no + * tracker number. + */ +function runtimeHookWithoutBodyRefusal( + singularType: string, + item: unknown, + saveName: string, +): (Error & { code: 'VALIDATION_ERROR'; status: 400 }) | undefined { + if (singularType !== 'hook') return undefined; + if (!item || typeof item !== 'object' || Array.isArray(item)) return undefined; + const hook = item as { handler?: unknown; body?: unknown }; + if (hook.body && typeof hook.body === 'object') return undefined; + if (typeof hook.handler !== 'string' || hook.handler === '') return undefined; + const err = new Error( + `Invalid hook: '${saveName}' names the function '${hook.handler}' in its \`handler\` and carries no \`body\`, ` + + 'so it can never run. Give it a `body` (sandboxed JS, `{ language: \'js\', source }`, or an expression), ' + + 'which is stored with the hook. A hook saved through the metadata API ships with no code package, so it ' + + "holds no functions, and a `handler` name resolves only inside the hook's own package.", + ) as Error & { code: 'VALIDATION_ERROR'; status: 400 }; + err.code = 'VALIDATION_ERROR'; + err.status = 400; + return err; +} + /** * One entry of the `422 INVALID_METADATA` envelope's `issues[]` — the shape * Studio's designer keys on to highlight the offending form control. @@ -18815,6 +18879,17 @@ export class ObjectStackProtocolImplementation implements } } + // [#21658] A hook whose `handler` names a function and that carries + // no `body` can never run once stored here: a stored hook ships with + // no code package, and a `handler` name resolves only inside the + // hook's own package. Refused in draft and in publish mode, after the + // schema (so `body` is absent or a declared body) and before the + // authoring gate and every write. See {@link runtimeHookWithoutBodyRefusal}. + { + const hookRefusal = runtimeHookWithoutBodyRefusal(singularType, request.item, request.name); + if (hookRefusal) throw hookRefusal; + } + // The #4463 runtime authoring gate — the shared author-time rule // registry, on the write path. `active` saves only (D1): this is the // publish verb, and it is the same table `os build` gates on. Placed diff --git a/packages/runtime/src/hook-handler-package-scope.pin.test.ts b/packages/runtime/src/hook-handler-package-scope.pin.test.ts index 72dc20328c7..338f607f0f6 100644 --- a/packages/runtime/src/hook-handler-package-scope.pin.test.ts +++ b/packages/runtime/src/hook-handler-package-scope.pin.test.ts @@ -10,20 +10,27 @@ * measured through the public door before the change: Y's insert came back * stamped by X's function. * ② the metadata door — a hook authored at runtime through - * `PUT /api/v1/meta/hook/:name` that names `x_stamp` is refused the same way - * when the door binds it. A runtime-authored hook ships with no code package - * and holds no functions. + * `PUT /api/v1/meta/hook/:name` that names `x_stamp` and carries no `body` + * is refused AT THE DOOR (#21658): `VALIDATION_ERROR` / 400, naming the + * hook and prescribing a `body`, with nothing stored and nothing bound. A + * runtime-authored hook ships with no code package and holds no functions, + * so the name could only ever be refused again at bind (the binder's + * `metadata-service` refusal, pinned in objectql's + * `hook-binder-package-scope.test.ts`). * * Controls, the two shapes a package's own function takes: app X's hook naming * X's own `functions` entry binds and runs, and app Z's hook naming a function * its `--artifact` runtime module exports (loaded through `loadArtifactBundle`, - * the artifact door's loader) binds and runs. A body hook authored through the - * metadata door binds and runs, which is also the witness that the door's - * re-sync has happened. + * the artifact door's loader) binds and runs — a built artifact's `handler` + * hook through its own door, which the save-door refusal leaves unchanged. A + * body hook authored through the metadata door saves, binds and runs, which is + * also the witness that the door's re-sync has happened; so does one carrying + * both a `body` and a `handler`, whose body is what runs. * * Every observation is a neutral marker appended to a free-text `status` - * column. The refusal's code and status are read off the engine's own logger, - * where the binder records each coded registration refusal at `error`. + * column. The binder's code and status are read off the engine's own logger, + * where it records each coded registration refusal at `error`; the door's are + * read off the HTTP answer. * * Composition: the in-process kernel `@objectstack/verify`'s `bootStack` * mirrors (engine, sqlite-wasm default datasource, HTTP server, the apps, @@ -112,8 +119,6 @@ let app: any; let adminToken: string; let artifactDir: string; let prevNodeEnv: string | undefined; -/** The metadata door's answer to saving the handler-named hook (printed, not asserted). */ -let recordedCrossHookSaveStatus: number | undefined; const req = (path: string, init?: RequestInit) => app.request(`${ORIGIN}${API}${path}`, init); const asAdmin = (method: string, path: string, body?: unknown) => @@ -187,7 +192,6 @@ beforeAll(async () => { }, BOOT_TIMEOUT); afterAll(async () => { - console.info(`[hook handler package scope pin] metadata door save of a handler-named hook answered: ${recordedCrossHookSaveStatus}`); try { await httpServer?.close?.(); } catch { /* best-effort */ } try { await kernel?.shutdown?.(); } catch { /* best-effort */ } try { rmSync(artifactDir, { recursive: true, force: true }); } catch { /* best-effort */ } @@ -211,6 +215,8 @@ describe('a hook handler name resolves inside its own package only — composed }); }); + // A built artifact's `handler` hook through its own door: unchanged by the + // metadata door's refusal in ② (#21658), which no artifact or boot door reaches. it('control: app X\'s hook naming X\'s own `functions` entry binds and runs', async () => { expect(await insertAndReadStatus(X_NOTE, 'x-own-probe')).toContain('x-fn'); expect(refusalsOf('scope_x_own')).toEqual([]); @@ -221,14 +227,28 @@ describe('a hook handler name resolves inside its own package only — composed expect(refusalsOf('scope_z_own')).toEqual([]); }); - it('② the metadata door: a runtime-authored hook naming app X\'s function is refused when the door binds it', async () => { + it('② the metadata door refuses a runtime-authored hook that names a function and carries no `body`: VALIDATION_ERROR / 400, nothing stored', async () => { const crossHook = await asAdmin('PUT', '/meta/hook/scope_authored_cross', { name: 'scope_authored_cross', object: Y_NOTE, events: ['beforeInsert'], handler: 'x_stamp', }); - recordedCrossHookSaveStatus = crossHook.status; + // The `/meta` save door's error body: `{ error: , code }`. + const refusal: any = await crossHook.json(); + expect({ status: crossHook.status, code: refusal?.code }, JSON.stringify(refusal)) + .toEqual({ status: 400, code: 'VALIDATION_ERROR' }); + // The named subject and the prescription: the hook, the function its + // `handler` names, and the `body` that would run. + expect(refusal.error).toContain("'scope_authored_cross'"); + expect(refusal.error).toContain("'x_stamp'"); + expect(refusal.error).toContain('Give it a `body`'); + // Nothing stored: the by-name read finds no row. + const stored = await asAdmin('GET', '/meta/hook/scope_authored_cross'); + expect(stored.status, await stored.text()).toBe(404); + }); + + it('②b a body hook authored through the metadata door saves, binds and runs; so does one carrying both a `body` and a `handler`', async () => { const bodyHook = await asAdmin('PUT', '/meta/hook/scope_authored_body', { name: 'scope_authored_body', object: Y_NOTE, @@ -236,25 +256,32 @@ describe('a hook handler name resolves inside its own package only — composed body: js("ctx.input.status = (typeof ctx.input.status === 'string' ? ctx.input.status : '') + '|authored-body';"), }); expect(bodyHook.status, await bodyHook.text()).toBeLessThan(300); + // A `body` beside a `handler` saves: the binder runs the body and never consults the name. + const bothHook = await asAdmin('PUT', '/meta/hook/scope_authored_both', { + name: 'scope_authored_both', + object: Y_NOTE, + events: ['beforeInsert'], + handler: 'x_stamp', + body: js("ctx.input.status = (typeof ctx.input.status === 'string' ? ctx.input.status : '') + '|authored-both';"), + }); + expect(bothHook.status, await bothHook.text()).toBeLessThan(300); - // The door's re-sync has bound the authored body hook once it fires… + // The door's re-sync has bound both authored body hooks once they fire… let lastStatus = ''; let probe = 0; const bound = await waitFor(async () => { lastStatus = await insertAndReadStatus(Y_NOTE, `authored-probe-${probe++}`); - return lastStatus.includes('authored-body'); + return lastStatus.includes('authored-body') && lastStatus.includes('authored-both'); }); - expect(bound, 'the runtime-authored body hook never bound').toBe(true); - - // …and by then the handler-named one, had it bound, would have run on the same insert. + expect(bound, `the runtime-authored body hooks never both bound (last status: ${lastStatus})`).toBe(true); + // …and nothing named `x_stamp` ran on the same insert. expect(lastStatus, "a runtime-authored hook ran app X's function").not.toContain('x-fn'); - const refusals = refusalsOf('scope_authored_cross'); - expect(refusals.length, 'no coded refusal was recorded for the runtime-authored hook').toBeGreaterThan(0); - expect(refusals[0][2]).toMatchObject({ - code: 'INVALID_REFERENCE', - status: 400, - handler: 'x_stamp', - packageId: 'metadata-service', - }); + expect(refusalsOf('scope_authored_both')).toEqual([]); }, 30_000); + + it('② nothing bound: once the re-sync has run (②b), the refused hook never reached the binder', async () => { + // There was no row for the re-sync to bind, so the binder recorded no + // refusal of it either: the door refused it before anything was stored. + expect(refusalsOf('scope_authored_cross')).toEqual([]); + }); });