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
26 changes: 26 additions & 0 deletions .changeset/21689-hook-no-body-save-door.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
---
'@objectstack/metadata-protocol': minor
---

The runtime save door refuses every hook that carries no `body`, including one with neither a `body` nor a `handler`: a hook stored there ships with no code package, so its `body` is the only code it can run

Clause-②: no (narrowing)

<!-- adr-0087: not-required (no-migration-prescription) A validity narrowing at one runtime write door over existing keys: no key of `HookSchema` or of any other metadata schema is removed, renamed or re-shaped, so there is no tombstone and nothing mechanical for `objectstack migrate meta` to rewrite. What `body` a refused hook should carry is authoring intent no conversion entry can decide. New saves are refused with the remedy; a row stored before this change keeps its bytes, and no stored row is re-saved (measured: `migrateStoredMetadata({ apply: true })` on a stored hook row with neither field reports it canonical, failed 0, and the bytes are unchanged). The census of writers through this door, taken first at `1db5322ba2`: Studio at the objectui pin (`ab18797215`, unchanged since the census of the handler-only refusal, which read its hook skeleton carrying a `body`); objectstack `examples/**` and `packages/qa/**` seed no `sys_metadata` hook rows (zero `type: 'hook'` items); `os meta register` forwards the author's own file and emits no hook of its own; the artifact, boot and install-local doors never call `saveMetaItem` for a hook (every call site in the tree saves a fixed type other than `hook`, or forwards an author's request: the REST and dispatcher `/meta` saves, `migrateStoredMetadata`, `duplicatePackage`). The only first-party bodies of this shape were test probes (seven suites), which now carry a `body`. Package duplication of a package holding such a row now reports that row as failed with this refusal (measured: copied 1, failed 1, the source rows' bytes unchanged). Hosted tenants and the cloud AI author were not measured. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id covers this rule and this diff adds none (not registered / already-registered); and the change narrows what a runtime write door accepts, not a runtime interface or a type surface alone (not runtime-interface-only / type-surface-only). -->

**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 every `hook` that carries no `body`. It already refused a hook whose `handler` names a function and that carries no `body`; that refusal is now one case of this rule, with the same envelope and the same message. The new case is a hook with neither field (or with an empty `handler`). Before this change the door answered 200 for it, the hook was served by name, the runtime skipped it at every re-sync (`skipping hook with unresolved handler`, in the server log only), and it 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 prescribes a `body`.

**Before and after** (with `{ name: 'stamp_status', object: 'crm_note', events: ['beforeInsert'] }`):

- Before: 200 `Saved hook 'stamp_status'`, the row stored and served by name, and the hook never run.
- After: 400 `VALIDATION_ERROR`, naming `stamp_status`, and nothing stored.

**What still saves.** A hook with a `body`, with or without a `handler` beside it: the binder runs the body and never consults the name. A malformed `body` still gets the type schema's located `422 INVALID_METADATA`.

**What is unchanged.** `HookSchema` still accepts a hook with no `body`, because a build artifact carries a `handler` hook: `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 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 skips them at every re-sync 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.
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,10 @@ describe('code-only metadata types are refused on every kernel (#5086)', () => {
const result = await protocol.saveMetaItem({
type: 'hook',
name: 'rc3_probe_hook',
item: { name: 'rc3_probe_hook', object: 'task', events: ['beforeUpdate'] },
// [#21689] A body-carrying hook: the save door refuses a hook
// with no `body` (it could never run), and this case measures
// the code-only gate, not that refusal.
item: { name: 'rc3_probe_hook', object: 'task', events: ['beforeUpdate'], body: { language: 'js', source: 'return;' } },
});
expect(result.success).toBe(true);
expect(metaRows(rows).length).toBe(1);
Expand Down Expand Up @@ -511,7 +514,9 @@ describe('code-only metadata types are refused on every kernel (#5086)', () => {
},
{
type: 'hook', // allowRuntimeCreate only
item: { name: 'rc3_receipt_view', object: 'task', events: ['beforeUpdate'] },
// [#21689] With a `body`: the door refuses a hook without one,
// and this matrix measures the receipt, not that refusal.
item: { name: 'rc3_receipt_view', object: 'task', events: ['beforeUpdate'], body: { language: 'js', source: 'return;' } },
},
{
type: 'webhook', // no static registry entry (plugin-registered)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -570,3 +570,50 @@ describe('[#21658] a hook naming a function in `handler` with no `body` is refus
expect(rows.size).toBe(0);
});
});

// ═══════════════════════════════════════════════════════════════════════════
// 8. #21689 — one predicate: the `hook` door refuses every hook with no `body`
// ═══════════════════════════════════════════════════════════════════════════
//
// Section 7's refusal, generalised. A hook this door stores ships with no code
// package, so a `body` is the only code it can run: a hook with neither a
// `body` nor a `handler` is never bound either (the binder skips it at every
// re-sync). The door judges by one predicate — no `body` object — and section
// 7's `handler` form is one case of it, with the same envelope. An empty
// `handler` names no function, so it reads as no `handler`.

describe('[#21689] a hook with no `body` and no function in `handler` is refused at the metadata door', () => {
const bare = (extra: Record<string, unknown> = {}) => ({
name: 'stamp_status',
object: 'hks_note',
events: ['beforeInsert'],
...extra,
});

it.each([
['publish', 'neither field', undefined, {}],
['draft', 'neither field', 'draft', {}],
['publish', 'an empty `handler`', undefined, { handler: '' }],
] as const)('%s mode, %s — VALIDATION_ERROR / 400, naming the hook, prescribing a `body`, nothing stored', async (_label, _shape, mode, extra) => {
const { protocol, rows } = makeProtocol();
let err: any;
try {
await protocol.saveMetaItem({
type: 'hook',
name: 'stamp_status',
item: bare(extra),
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('Give it a `body`');
expect(rows.size).toBe(0);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,10 @@ const SAMPLE: Array<{
type: 'hook',
klass: 'declared',
creatable: true,
item: { name: 'probe_hook', object: 'task', events: ['beforeInsert'] },
// [#21689] With a `body`: the mint door refuses a hook without one (it
// could never run), which would misread the advertisement this suite
// measures, as a 422 from schema resolution would.
item: { name: 'probe_hook', object: 'task', events: ['beforeInsert'], body: { language: 'js', source: 'return;' } },
},
{
// The `false` direction of class 1, and it must be present: a listing
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,9 @@ const OVERLAYLESS_PROBES: Record<string, Record<string, unknown>> = {
sharingModel: 'private',
fields: { name: { type: 'text', label: 'Name' } },
},
hook: { name: 'rc5_acct', object: 'task', events: ['beforeUpdate'] },
// [#21689] With a `body`: the door refuses a hook without one before the
// receipt is built, as it refuses a body the schema rejects.
hook: { name: 'rc5_acct', object: 'task', events: ['beforeUpdate'], body: { language: 'js', source: 'return;' } },
seed: { object: 'task', records: [] },
action: { name: 'rc5_acct', label: 'Convert', type: 'script', objectName: 'task', target: 'convertHandler' },
flow: {
Expand Down
95 changes: 55 additions & 40 deletions packages/metadata-protocol/src/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -781,47 +781,58 @@ function resolveOverlaySchema(type: string, _item: unknown): z.ZodTypeAny | 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".
* [#21658, #21689] The save door's refusal of a `hook` that carries no `body`:
* such a hook can never run once this door has stored it. One predicate, two
* shapes it meets, one envelope: a hook whose `handler` names a function, and a
* hook with neither field.
*
* Why it can never run. 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 a
* `body`, which is stored with the hook, is the only code it can run.
*
* - A `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`), so it has nothing to resolve
* against, and the binder refuses the hook at registration
* (`INVALID_REFERENCE` / 400, logged at `error`).
* - A hook with neither field has nothing to bind at all, and the binder
* skips it at every re-sync (`skipping hook with unresolved handler`,
* logged at `warn`).
*
* Either way the door would already have answered success, and the hook would
* be served by name and never run. Refusing it here says so to the author,
* before anything is stored.
*
* The predicate is the binder's own body-first test, the judgement
* install-local's `collectHooksWithoutBody` makes on its own door (#21585): 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), and every hook without a `body` object is refused, whatever its
* `handler` holds. 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).
* rule only.
*
* 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.
* every body (`savedItemNameRefusal`). The message names the hook (and the
* function, when its `handler` names one), 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,
Expand All @@ -832,12 +843,15 @@ function runtimeHookWithoutBodyRefusal(
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 prescription = '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 ';
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.",
typeof hook.handler === 'string' && hook.handler !== ''
? `Invalid hook: '${saveName}' names the function '${hook.handler}' in its \`handler\` and carries no \`body\`, `
+ `so it can never run. ${prescription}it holds no functions, and a \`handler\` name resolves only inside `
+ "the hook's own package."
: `Invalid hook: '${saveName}' carries no \`body\`, so it has nothing to run. ${prescription}`
+ 'its `body` is the only code it can run.',
) as Error & { code: 'VALIDATION_ERROR'; status: 400 };
err.code = 'VALIDATION_ERROR';
err.status = 400;
Expand Down Expand Up @@ -18947,12 +18961,13 @@ 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}.
// [#21658, #21689] A hook that carries no `body` can never run once
// stored here, whether its `handler` names a function or it has
// neither field: a stored hook ships with no code package, so its
// `body` is the only code it can run. 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,8 @@ describe('#8421 — the traffic that must keep working', () => {
{
type: 'hook',
why: 'declared, runtime-create only',
item: { name: 'probe_item', object: 'task', events: ['beforeUpdate'] },
// [#21689] With a `body`: the door refuses a hook without one.
item: { name: 'probe_item', object: 'task', events: ['beforeUpdate'], body: { language: 'js', source: 'return;' } },
},
{
// `theme` held this slot until commit 35ad101bc retired the themes surface
Expand Down
7 changes: 6 additions & 1 deletion packages/objectql/src/metadata-validation-sweep.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,12 +143,17 @@ const FIXTURES: Record<string, Fixture> = {
invalidatedField: 'type',
},
hook: {
// [#21689] Both carry a `body`: the save door refuses a hook with no
// `body` (it could never run), so a body-less `valid` would measure
// that refusal, and `invalid` stays `valid` minus the one field the
// schema must name.
valid: {
name: 'sweep_hook',
object: 'sweep_account',
events: ['beforeInsert'],
body: { language: 'js', source: 'return;' },
},
invalid: { name: 'sweep_hook', object: 'sweep_account' },
invalid: { name: 'sweep_hook', object: 'sweep_account', body: { language: 'js', source: 'return;' } },
invalidatedField: 'events',
},
validation: {
Expand Down
Loading
Loading