From 252723a35f7cf4b2f83a051d6d5f716bcfdd4ad6 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 31 Jul 2026 14:47:10 +0000 Subject: [PATCH] test: run action and hook bodies through the real QuickJS sandbox MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The action side had no sandbox harness. `test/global-actions.test.ts` ran each `body.source` through `new Function`, which proves what a body computes and is structurally blind to everything the runtime imposes on it: no module scope, a JSON-only boundary, a capability gate, and a `ctx.api` built by the engine rather than by the author. So the one constraint this repo relies on by name — the shared factory in `src/objects/_line-item-price-fill.ts` is safe only while its handler body never reads a factory parameter — lived in a comment, with nothing anywhere failing if it were violated. `test/helpers/action-sandbox.ts` runs nothing of its own: it hands bodies to the same `QuickJSScriptRunner` + `actionBodyRunnerFactory` / `hookBodyRunnerFactory` the runtime wires at boot, lowered by the same `extractHookBody` pass the CLI build uses. The only stand-in is the engine underneath — an ObjectQL-shaped recorder, deliberately without `.object()` so the runtime builds the repo facade itself and a body meets the real `update(data, options)` signature. Its two hand-written rules are pinned against a real ObjectQL on the in-memory driver, the way `hook-query-predicate.test.ts` pins the read predicate. `test/action-sandbox.test.ts` then executes all seven script action bodies, shows the capability gate denying an undeclared capability, shows a body has no module scope, and turns the price-fill constraint into a guard with a negative control — plus the generalisation the build only mentions in passing: all 24 callables still lower to metadata-only bodies, and a hook that stops doing so is silently bundled instead, not caught. `global-actions.test.ts` keeps its assertions and switches to the real executor. One finding it surfaced is recorded, not fixed: `mass_update_stage` calls `update(id, doc)`, which the engine facade rejects on every invocation (#508 already documents the action as having no live invocation path). Refs #575. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Ku5mr51i1UNcbq9VNbaNQb --- .../real-quickjs-action-sandbox-harness.md | 19 + src/objects/_line-item-price-fill.ts | 8 + test/action-sandbox.test.ts | 434 ++++++++++++++++++ test/global-actions.test.ts | 77 +--- test/helpers/action-sandbox.ts | 321 +++++++++++++ 5 files changed, 795 insertions(+), 64 deletions(-) create mode 100644 .changeset/real-quickjs-action-sandbox-harness.md create mode 100644 test/action-sandbox.test.ts create mode 100644 test/helpers/action-sandbox.ts diff --git a/.changeset/real-quickjs-action-sandbox-harness.md b/.changeset/real-quickjs-action-sandbox-harness.md new file mode 100644 index 00000000..648a0593 --- /dev/null +++ b/.changeset/real-quickjs-action-sandbox-harness.md @@ -0,0 +1,19 @@ +--- +'hotcrm': patch +--- + +Run action and hook bodies through the REAL QuickJS sandbox in tests. The +action side had no sandbox harness at all — `test/global-actions.test.ts` ran +each `body.source` through `new Function`, which proves what a body computes +and is structurally blind to everything the runtime imposes on it: no module +scope, a JSON-only boundary, a capability gate, and a `ctx.api` the engine +builds rather than the author. `test/helpers/action-sandbox.ts` now hands the +same bodies to the runtime's own `QuickJSScriptRunner` + +`actionBodyRunnerFactory` / `hookBodyRunnerFactory`, over an ObjectQL-shaped +recording engine whose update contract is pinned against a real kernel on the +in-memory driver. `test/action-sandbox.test.ts` executes all seven script +action bodies, asserts the capability gate denies an undeclared capability, and +turns `src/objects/_line-item-price-fill.ts`'s comment-only constraint — the +shared factory is safe only while the handler body never reads a factory +parameter — into an executable guard, with a negative control that fails when +it is violated. Refs #575. diff --git a/src/objects/_line-item-price-fill.ts b/src/objects/_line-item-price-fill.ts index b8809bb2..cd423c3f 100644 --- a/src/objects/_line-item-price-fill.ts +++ b/src/objects/_line-item-price-fill.ts @@ -27,6 +27,14 @@ import type { HookApi } from './_hook-api'; * plain metadata on the returned Hook, and the handler body below closes over * NOTHING but its own `ctx`. Never move a value the body reads into this * factory's parameters; it would resolve here and be undefined in the sandbox. + * + * That rule is enforced, not just documented: `test/action-sandbox.test.ts` + * lowers this handler with the CLI build's own extraction pass (which rejects a + * body referencing anything out of scope) and runs the result under the real + * QuickJS runner, with a negative control that shows the guard still bites. + * Worth knowing why a test has to: when extraction fails the build does not, + * it silently keeps the handler in its bundled form and the hook stops being + * deployable as pure metadata. */ export function createLineItemPriceFill(objectName: string, hookName: string): Hook { return { diff --git a/test/action-sandbox.test.ts b/test/action-sandbox.test.ts new file mode 100644 index 00000000..5ef8123f --- /dev/null +++ b/test/action-sandbox.test.ts @@ -0,0 +1,434 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { InMemoryDriver } from '@objectstack/driver-memory'; +import stack from '../objectstack.config'; +import { allHooks } from '../src/hooks'; +import { createLineItemPriceFill } from '../src/objects/_line-item-price-fill'; +import oppLineItemHooks from '../src/objects/opportunity_line_item.hook'; +import quoteLineItemHooks from '../src/objects/quote_line_item.hook'; +import { hookNamed } from './helpers/hook-harness'; +import { + extractSandboxBody, + makeSandboxEngine, + runActionBody, + runHookBody, + type Rec, +} from './helpers/action-sandbox'; + +/** + * The action / hook SANDBOX guards (issue #575 A1). + * + * Everything else that executes app code in this suite executes it as ordinary + * JavaScript: `hooks-runtime*.test.ts` calls `hook.handler(ctx)` with the + * closure intact, and `global-actions.test.ts` runs the action bodies for what + * they compute. That answers "is the logic right" and is structurally unable to + * answer "does this survive the runtime", because the runtime does not run a + * closure — it evaluates a lowered `body.source` string inside QuickJS, with no + * module scope, a JSON-only boundary, a capability gate, and a `ctx.api` built + * by the engine rather than by the author. + * + * This file is the missing half. It runs the real `QuickJSScriptRunner` through + * the real body-runner factories (see `test/helpers/action-sandbox.ts`) and + * pins the constraints that only exist there — starting with the one that has + * so far lived only in a comment: the shared price-fill factory in + * `src/objects/_line-item-price-fill.ts` is safe ONLY while its handler body + * never reads a factory parameter. + */ + +type AnyRec = Record; + +const stackActions: AnyRec[] = (stack as any).actions ?? []; +const action = (name: string): AnyRec => { + const found = stackActions.find((a) => a.name === name); + if (!found) throw new Error(`no ${name} action registered`); + return found; +}; + +/** Every action the runtime will execute as a sandboxed body. */ +const SCRIPT_ACTIONS = stackActions.filter((a) => a.body?.language === 'js'); + +// ─────────────────────────── the stub engine's contract is the kernel's ── + +/** + * `makeSandboxEngine` stands in for ObjectQL, and a stand-in that accepts calls + * the real engine rejects is worse than no test at all — it is the exact defect + * `hook-harness.ts` documents in its `where`/`filter` note. So the two rules + * the stub implements by hand are pinned here against a REAL ObjectQL on the + * in-memory driver, the same way `hook-query-predicate.test.ts` pins the read + * predicate. + */ +describe('the sandbox engine stub obeys the kernel’s update contract', () => { + let ql: Awaited>; + + const makeEngine = () => + ObjectQL.create({ + datasources: { default: new InMemoryDriver({ persistence: false }) }, + objects: { + crm_opportunity: { + name: 'crm_opportunity', + fields: { id: { type: 'text' }, name: { type: 'text' }, stage: { type: 'text' } }, + }, + } as never, + }); + + beforeAll(async () => { + ql = await makeEngine(); + }); + afterAll(async () => { + await ql?.close(); + }); + + /** A body doing exactly one write, so the engine call under test is unambiguous. */ + const writerAction = (source: string): AnyRec => ({ + name: 'sandbox_probe', + objectName: 'crm_opportunity', + type: 'script', + body: { language: 'js', source, capabilities: ['api.write'], timeoutMs: 2000 }, + }); + + it('rejects update(id, doc) — the id never reaches the engine', async () => { + // The real kernel first. `ctx.api` in production is an ObjectRepository (or + // the runtime's identical repo facade), whose update takes `(data, options)` + // — so passing an id string as `data` leaves the engine with nothing to + // resolve, and it says so rather than updating every row. + const api = ql.createContext({ isSystem: true }); + const row = await api.object('crm_opportunity').insert({ name: 'A', stage: 'prospecting' }); + await expect( + (api.object('crm_opportunity') as AnyRec).update(row.id, { stage: 'closed_won' }), + ).rejects.toThrow(/Update requires an ID or options\.multi=true/); + expect((await api.object('crm_opportunity').findOne({ where: { id: row.id } }))?.stage).toBe('prospecting'); + + // Now the stub, reached the way a body reaches it: through the runtime's + // own repo facade inside QuickJS. + const engine = makeSandboxEngine({ crm_opportunity: [{ id: 'opp_1', stage: 'prospecting' }] }); + await expect( + runActionBody(writerAction(`await ctx.api.object('crm_opportunity').update('opp_1', { stage: 'closed_won' });`), { engine }), + ).rejects.toThrow(/Update requires an ID or options\.multi=true/); + expect(engine.rows('crm_opportunity')[0]!.stage).toBe('prospecting'); + }); + + it('applies update(data, { where }) — the spelling that works', async () => { + const api = ql.createContext({ isSystem: true }); + const row = await api.object('crm_opportunity').insert({ name: 'B', stage: 'prospecting' }); + await api.object('crm_opportunity').update({ id: row.id, stage: 'negotiation' }, { where: { id: row.id } }); + expect((await api.object('crm_opportunity').findOne({ where: { id: row.id } }))?.stage).toBe('negotiation'); + + const engine = makeSandboxEngine({ crm_opportunity: [{ id: 'opp_1', stage: 'prospecting' }] }); + await runActionBody( + writerAction(`await ctx.api.object('crm_opportunity').update({ id: 'opp_1', stage: 'negotiation' }, { where: { id: 'opp_1' } });`), + { engine }, + ); + expect(engine.rows('crm_opportunity')[0]!.stage).toBe('negotiation'); + }); +}); + +// ─────────────────────────────────── the sandbox boundary itself ── + +describe('the sandbox boundary is real', () => { + it('denies a capability the body did not declare', async () => { + // `log_call` writes; stripping `api.write` must stop it at the call, not + // let it through with a shrug. This is the check that makes every + // `capabilities: [...]` list in `src/actions/` load-bearing metadata + // rather than documentation. + const stripped = { ...action('log_call'), body: { ...action('log_call').body, capabilities: [] } }; + await expect( + runActionBody(stripped, { objectName: 'crm_case', record: { id: 'case_1' }, input: { subject: 'x' } }), + ).rejects.toThrow(/capability 'api\.write' not granted/); + }); + + it('gives the body no module scope at all', async () => { + // Not a hypothetical: this is what a lowered body IS. The runner evaluates + // the source string in a fresh VM, so any identifier that was a closure at + // authoring time is simply absent. + await expect( + runActionBody({ + name: 'scope_probe', + type: 'script', + body: { language: 'js', source: 'return { v: someModuleConst };', capabilities: [], timeoutMs: 2000 }, + }), + ).rejects.toThrow(/ReferenceError/); + }); + + it('hands the dispatcher’s params through as `input`, and no `ctx.object`', async () => { + // Both dispatch paths in the runtime build `params: { ...params, recordId, + // objectName }` and set no `object` key on the action context — so a body + // that identifies its object via `ctx.object` alone reads undefined. The + // activity actions read `input.objectName` because of this. + const { result } = await runActionBody({ + name: 'ctx_probe', + type: 'script', + body: { + language: 'js', + source: 'return { fromInput: input.objectName ?? null, fromCtx: ctx.object ?? null, recordId: ctx.recordId ?? null };', + capabilities: [], + timeoutMs: 2000, + }, + }, { objectName: 'crm_case', record: { id: 'case_1' } }); + expect(result).toEqual({ fromInput: 'crm_case', fromCtx: null, recordId: 'case_1' }); + }); +}); + +// ────────────────────────────── every shipped action body, executed ── + +describe('every script action body executes under QuickJS', () => { + /** + * One invocation per action, shaped the way its own doc comment says it is + * dispatched. The bar is deliberately the runtime's: a body that references a + * missing identifier, uses a construct the sandbox forbids, or calls the + * engine with an argument shape it rejects fails here — none of which + * `os validate`, `build`, or a metadata assertion can see. + */ + const INVOCATIONS: Record[1]; seed?: Record }> = { + clone_opportunity: { + opts: { + objectName: 'crm_opportunity', + record: { id: 'opp_1', name: 'Acme Expansion', crm_account: 'acc_1', amount: 50000, primary_contact: 'con_1' }, + }, + }, + create_campaign: { + opts: { objectName: 'crm_lead', record: { id: 'lead_1' }, input: { crm_campaign: 'cmp_1' } }, + }, + log_call: { + opts: { objectName: 'crm_case', record: { id: 'case_1', display_title: 'CASE-1' }, input: { subject: 'Intro call' } }, + }, + log_meeting: { + opts: { objectName: 'crm_case', record: { id: 'case_1', display_title: 'CASE-1' }, input: { subject: 'Kickoff' } }, + }, + mark_primary: { + opts: { objectName: 'crm_contact', record: { id: 'con_1' } }, + seed: { crm_contact: [{ id: 'con_1', is_primary: false }] }, + }, + send_email: { + opts: { + objectName: 'crm_contact', + record: { id: 'con_1', email: 'ada@example.com', full_name: 'Ada Lovelace' }, + input: { subject: 'Hello', body: 'Body text' }, + }, + }, + mass_update_stage: { + opts: { objectName: 'crm_opportunity', record: { id: 'opp_1' }, input: { stage: 'negotiation' } }, + seed: { crm_opportunity: [{ id: 'opp_1', stage: 'prospecting' }] }, + }, + }; + + /** Asserted separately below, because it does not currently reach the engine. */ + const BROKEN = 'mass_update_stage'; + + it('covers every action the runtime will sandbox', () => { + expect(Object.keys(INVOCATIONS).sort()).toEqual(SCRIPT_ACTIONS.map((a) => a.name).sort()); + }); + + it.each(SCRIPT_ACTIONS.map((a) => a.name).filter((n) => n !== BROKEN))('%s', async (name) => { + const { opts, seed } = INVOCATIONS[name]!; + const engine = makeSandboxEngine(seed ?? {}); + const { result } = await runActionBody(action(name), { ...opts, engine }); + expect(result, `${name} returned nothing`).toBeTruthy(); + }); + + /** + * The defect this executor was built to be able to find, recorded rather than + * fixed: `mass_update_stage` calls `update(id, { stage })`, but `ctx.api` is + * the engine repo facade, whose update takes `(data, options)` — so the id + * lands in the `data` slot and the engine refuses the write (the same + * rejection pinned against the real kernel at the top of this file). Every + * invocation of this action fails, and nothing before this file could say so: + * the metadata is valid, the body parses, and the action has no live + * invocation path to fail on (#508, `src/actions/opportunity.actions.ts`). + * + * Fixing the body is a change to a shipped action and belongs to whoever + * revives it — at which point this assertion is the thing that tells them the + * behaviour moved. + */ + it(`${BROKEN} is rejected by the engine — it passes an id where the facade wants a document`, async () => { + const { opts, seed } = INVOCATIONS[BROKEN]!; + const engine = makeSandboxEngine(seed ?? {}); + await expect(runActionBody(action(BROKEN), { ...opts, engine })).rejects.toThrow( + /Update requires an ID or options\.multi=true/, + ); + expect(engine.rows('crm_opportunity')[0]!.stage, 'the stage move silently landed').toBe('prospecting'); + }); + + it('clone_opportunity copies the required fields onto a fresh prospecting deal', async () => { + const engine = makeSandboxEngine(); + const { result } = await runActionBody(action('clone_opportunity'), { ...INVOCATIONS.clone_opportunity!.opts, engine }); + const [clone] = engine.inserted('crm_opportunity'); + expect(clone!.name).toBe('Copy of Acme Expansion'); + expect(clone!.crm_account).toBe('acc_1'); + expect(clone!.amount).toBe(50000); + expect(clone!.stage).toBe('prospecting'); + expect(clone!.owner).toBe('usr_1'); + // A 90-day horizon computed INSIDE the VM — `Date` is one of the few host + // globals a body may rely on, and this proves it is really there. + expect(clone!.close_date).toMatch(/^\d{4}-\d{2}-\d{2}$/); + expect(new Date(clone!.close_date).getTime()).toBeGreaterThan(Date.now()); + // The id comes back from the engine, so it is only ever on the stored row — + // never on the document the body sent. + expect(result.id).toBe(engine.rows('crm_opportunity')[0]!.id); + }); + + it('mark_primary calls update with the engine facade’s (data, options) shape', async () => { + const engine = makeSandboxEngine({ crm_contact: [{ id: 'con_1', is_primary: false }] }); + await runActionBody(action('mark_primary'), { objectName: 'crm_contact', record: { id: 'con_1' }, engine }); + const [call] = engine.callsFor('crm_contact', 'update'); + expect(call!.args[0]).toEqual({ id: 'con_1', is_primary: true }); + expect(call!.args[1]).toEqual({ where: { id: 'con_1' } }); + expect(engine.rows('crm_contact')[0]!.is_primary).toBe(true); + }); + + it('create_campaign skips a lead already enrolled on the campaign', async () => { + const engine = makeSandboxEngine({ + crm_campaign_member: [{ id: 'cm_1', crm_campaign: 'cmp_1', crm_lead: 'lead_1' }], + }); + const { result } = await runActionBody(action('create_campaign'), { + objectName: 'crm_lead', + record: { id: 'lead_1' }, + input: { crm_campaign: 'cmp_1' }, + engine, + }); + expect(result).toMatchObject({ campaignId: 'cmp_1', count: 0, skipped: 1 }); + expect(engine.callsFor('crm_campaign_member', 'insert')).toHaveLength(0); + }); + + it('send_email writes the email and its timeline pointer', async () => { + const engine = makeSandboxEngine(); + const { result } = await runActionBody(action('send_email'), { ...INVOCATIONS.send_email!.opts, engine }); + const [email] = engine.rows('sys_email'); + const [activity] = engine.rows('sys_activity'); + expect(email!.to_addresses).toBe('ada@example.com'); + expect(email!.status).toBe('queued'); + expect(activity!.source_object).toBe('sys_email'); + // The pointer is the id the FIRST insert returned — so this also proves the + // body's two awaits really sequenced across the VM boundary. + expect(activity!.source_id).toBe(email!.id); + expect(activity!.record_label).toBe('Ada Lovelace'); + expect(result).toEqual({ emailId: email!.id, activityId: activity!.id }); + }); +}); + +// ──────────────────── the constraint the shared price-fill factory rests on ── + +describe('the shared line-item price fill stays shippable body-only', () => { + const HOOKS = [ + { label: 'opportunity', hook: hookNamed(oppLineItemHooks, 'opportunity_line_item_price_fill') }, + { label: 'quote', hook: hookNamed(quoteLineItemHooks, 'quote_line_item_price_fill') }, + ]; + + /** + * The factory's doc comment says the sharing is safe because it happens at + * AUTHORING time — `objectName` / `hookName` end up as plain metadata and the + * handler body closes over nothing but its own `ctx`. Until now that was an + * assertion in a comment. `extractSandboxBody` is the CLI build's own + * lowering pass, and it throws when the handler references an identifier that + * will not exist at runtime, so calling it IS the check. + * + * Note what the build does with that throw: it catches it and silently keeps + * the handler in its bundled form. Nothing goes red, the app keeps working + * locally, and the only thing lost is the metadata-only body — the form that + * deploys. That is why this has to be asserted here instead of relying on the + * build to complain. + */ + it.each(HOOKS)('$label lowers to a body with no free identifiers', ({ hook }) => { + expect(() => extractSandboxBody(hook.handler, `hook '${hook.name}'`)).not.toThrow(); + }); + + it('lowers to the same source and the same inferred capabilities on both objects', () => { + const [opp, quote] = HOOKS.map(({ hook }) => extractSandboxBody(hook.handler, `hook '${hook.name}'`)); + expect(quote!.source).toBe(opp!.source); + // Inferred from the `findOne` the body performs — and `api.write` is + // correctly absent: the hook mutates `ctx.input`, it does not write. + expect(opp!.capabilities).toEqual(['api.read']); + }); + + it.each(HOOKS)('$label stamps list_price and defaults unit_price on insert, in the VM', async ({ hook }) => { + const engine = makeSandboxEngine({ crm_product: [{ id: 'p1', list_price: 250 }] }); + const { input } = await runHookBody(hook, { + event: 'beforeInsert', + input: { crm_product: 'p1' }, + engine, + }); + expect(input.list_price).toBe(250); + expect(input.unit_price).toBe(250); + }); + + it.each(HOOKS)('$label re-syncs list_price but never clobbers a negotiated unit_price', async ({ hook }) => { + const engine = makeSandboxEngine({ crm_product: [{ id: 'p1', list_price: 250 }] }); + const { input } = await runHookBody(hook, { + event: 'beforeUpdate', + input: { crm_product: 'p1', unit_price: 199 }, + engine, + }); + expect(input.list_price).toBe(250); + expect(input.unit_price).toBe(199); + }); + + it.each(HOOKS)('$label is a no-op when no product is part of the write', async ({ hook }) => { + const engine = makeSandboxEngine({ crm_product: [{ id: 'p1', list_price: 250 }] }); + const { input } = await runHookBody(hook, { event: 'beforeInsert', input: { quantity: 2 }, engine }); + expect(input.list_price).toBeUndefined(); + expect(engine.calls).toHaveLength(0); + }); + + /** + * The negative control. Without it the guard above is just "the current code + * passes"; with it we know the check can still fail. + */ + it('rejects a factory whose handler reads a factory parameter', () => { + const leaky = createLeakyPriceFill('crm_quote_line_item', 'leaky_price_fill'); + expect(() => extractSandboxBody(leaky.handler, `hook '${leaky.name}'`)).toThrow(/objectName/); + // And the real factory is the same shape apart from that one read, so the + // guard is discriminating rather than allergic to factories in general. + const real = createLineItemPriceFill('crm_quote_line_item', 'quote_line_item_price_fill'); + expect(() => extractSandboxBody(real.handler, `hook '${real.name}'`)).not.toThrow(); + }); + + it('is a ReferenceError in the VM if such a body ever ships', async () => { + // Extraction is the first line of defence, not the only outcome that + // matters: it declines to report free identifiers when it cannot parse the + // handler, in which case a body like this one ships and the failure moves + // to runtime — on a write, in production. This is what that looks like. + await expect( + runHookBody(HOOKS[0]!.hook, { + event: 'beforeInsert', + input: { crm_product: 'p1' }, + source: 'ctx.input.stamped_object = objectName;', + }), + ).rejects.toThrow(/ReferenceError/); + }); +}); + +describe('every registered hook still lowers to a metadata-only body', () => { + /** + * The generalisation of the guard above. `objectstack build` reports this — + * "Skipping legacy runtime bundle (all 24 callables are body-only)" — but it + * reports the opposite just as quietly: a hook that stops lowering is caught, + * bundled into `objectstack-runtime.{hash}.mjs`, and the build still passes. + * The artifact silently grows a second delivery mechanism and the hook stops + * being deployable as pure metadata. + * + * A hook that genuinely needs a closure is allowed to exist — but it should + * arrive as a deliberate change to this list, not as a build line nobody read. + */ + it.each(allHooks.map((h) => h.name))('%s', (name) => { + const hook = allHooks.find((h) => h.name === name)!; + expect(() => extractSandboxBody(hook.handler, `hook '${name}'`)).not.toThrow(); + }); +}); + +/** + * A price fill written the way the factory's doc comment forbids: the handler + * reads `objectName`, which is a closure here and nothing at all once the body + * is lowered. Kept next to the test that rejects it so the forbidden shape is + * legible rather than described. + */ +function createLeakyPriceFill(objectName: string, hookName: string): AnyRec { + return { + name: hookName, + object: objectName, + events: ['beforeInsert'], + handler: async (ctx: AnyRec) => { + ctx.input.stamped_object = objectName; + }, + }; +} diff --git a/test/global-actions.test.ts b/test/global-actions.test.ts index 9846bad3..af442ebb 100644 --- a/test/global-actions.test.ts +++ b/test/global-actions.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect } from 'vitest'; import stack from '../objectstack.config'; +import { runActionBody, type ActionRunOpts } from './helpers/action-sandbox'; /** * Behavioral guards for the activity-logging actions in @@ -15,11 +16,13 @@ import stack from '../objectstack.config'; * with each other. A source-text assertion would have pinned the fix's * spelling; running the body pins its behavior. * - * The harness is deliberately tiny — the bodies' only host surface is - * `ctx.api.object(...).insert(...)`, so a recording stub covers it. This is - * NOT a sandbox-accurate harness (the real runtime executes the source under - * QuickJS); it is a plain `new Function` over the same source string, which is - * enough to prove what the body computes. + * The execution is the real one (#575 A1): `test/helpers/action-sandbox.ts` + * runs these bodies under QuickJS through the same body-runner factory the + * runtime binds at boot, so the ctx they see is the dispatcher's — notably + * WITHOUT `ctx.object`, which is why the label lookup below has to reach + * `input.objectName`. The previous `new Function` stand-in could not have + * shown that. `test/action-sandbox.test.ts` covers the sandbox's own + * constraints; this file stays about what these two actions compute. */ type AnyRec = Record; @@ -37,66 +40,12 @@ const action = (name: string): AnyRec => { /** The two twins this file is about, always exercised as a pair. */ const TWINS = ['log_call', 'log_meeting'] as const; -type RunOpts = { - /** Modal params the user submitted. */ - input?: AnyRec; - /** The record the action was invoked on, as the dispatcher loaded it. */ - record?: AnyRec; - /** The object the action was dispatched against. */ - objectName?: string; -}; - -type RunResult = { - result: any; - /** Every `ctx.api.object(o).insert(doc)` the body performed, in order. */ - inserted: { object: string; doc: AnyRec }[]; -}; - -/** - * Execute an action's metadata body against a recording `ctx`. - * - * Mirrors the runtime's action sandbox context (`buildActionSandboxContext`): - * the body sees `input` (the submitted params, into which the dispatcher - * merges `recordId` / `objectName`) and `ctx` (`recordId`, `record`, `object`, - * `user`, `api`). - */ -async function runActionBody(a: AnyRec, opts: RunOpts = {}): Promise { - const source: string = a.body?.source ?? ''; - if (!source) throw new Error(`action ${a.name} has no body source to run`); - - const inserted: RunResult['inserted'] = []; - const recordId = opts.record?.id ?? null; - const ctx = { - recordId, - record: opts.record, - object: opts.objectName, - user: { id: 'usr_1', name: 'Ada Lovelace' }, - api: { - object: (object: string) => ({ - insert: async (doc: AnyRec) => { - inserted.push({ object, doc }); - return { id: `${object}_1` }; - }, - }), - }, - }; - const input = { ...(opts.input ?? {}), recordId, objectName: opts.objectName }; - - // eslint-disable-next-line @typescript-eslint/no-implied-eval - const run = new Function('input', 'ctx', `return (async () => {${source}})();`) as ( - i: AnyRec, - c: AnyRec, - ) => Promise; - const result = await run(input, ctx); - return { result, inserted }; -} - /** The single `sys_activity` row an activity action is expected to write. */ -async function loggedActivity(actionName: string, opts: RunOpts = {}): Promise { - const { inserted } = await runActionBody(action(actionName), opts); - const activities = inserted.filter((i) => i.object === 'sys_activity'); +async function loggedActivity(actionName: string, opts: ActionRunOpts = {}): Promise { + const { engine } = await runActionBody(action(actionName), opts); + const activities = engine.inserted('sys_activity'); expect(activities, `${actionName} wrote ${activities.length} sys_activity rows, expected 1`).toHaveLength(1); - return activities[0]!.doc; + return activities[0]!; } /** Objects that declare a `nameField`, i.e. that have a resolvable display name. */ @@ -219,7 +168,7 @@ describe('log_call and log_meeting stay twins (#514 item 15)', () => { }); it('emits the same activity row apart from the summary prefix and metadata', async () => { - const opts: RunOpts = { + const opts: ActionRunOpts = { objectName: 'crm_case', record: { id: 'case_1', display_title: 'CASE-00042 - Printer offline' }, input: { subject: 'Quarterly sync', duration: 30, notes: 'Agreed next steps' }, diff --git a/test/helpers/action-sandbox.ts b/test/helpers/action-sandbox.ts new file mode 100644 index 00000000..a3b49a05 --- /dev/null +++ b/test/helpers/action-sandbox.ts @@ -0,0 +1,321 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +import { QuickJSScriptRunner, actionBodyRunnerFactory, hookBodyRunnerFactory } from '@objectstack/runtime'; +import { extractHookBody } from '@objectstack/cli/dist/utils/extract-hook-body.js'; + +/** + * REAL QuickJS harness for action and hook bodies (issue #575 A1). + * + * `test/helpers/hook-harness.ts` runs hook handlers as plain JS functions, and + * `global-actions.test.ts` used to run action bodies through `new Function`. + * Both prove what a body COMPUTES; neither can see the constraints that only + * exist because the runtime evaluates these bodies inside QuickJS: + * + * - a body is shipped **body-only** — it runs with no module scope, so a + * reference to an import, a top-level const, or (see + * `src/objects/_line-item-price-fill.ts`) a shared factory's parameter is + * not a closure at runtime, it is a `ReferenceError`; + * - `ctx.api` is not the object the handler was written against — it is the + * engine repo facade the runtime builds, whose `update`/`delete` take + * `(data, options)` / `(options)`, not an id; + * - a capability the body does not declare is denied at call time; + * - everything crossing the VM boundary is JSON — functions, `undefined` + * and `Date` instances do not survive. + * + * So this harness runs nothing of its own: it hands the body to the SAME + * `QuickJSScriptRunner` + `actionBodyRunnerFactory` / `hookBodyRunnerFactory` + * the runtime wires at boot, shaped by the same `extractHookBody` pass the CLI + * build uses to lower a `handler` function into `body.source`. The only stand-in + * is the engine underneath — {@link makeSandboxEngine}, an ObjectQL-shaped + * recorder whose contract is pinned against the real kernel in + * `test/action-sandbox.test.ts`. + * + * `extractHookBody` is reached by a deep import because the CLI publishes no + * export map for it. Both packages are pinned to an exact version in + * `package.json`, and if a platform upgrade moves the file the import fails at + * load — loudly, on every test in this harness — rather than degrading to a + * lookalike check. + */ + +export type Rec = Record; + +/** One engine operation a body performed, in order. */ +export interface EngineCall { + op: 'find' | 'count' | 'insert' | 'update' | 'delete' | 'upsert'; + object: string; + args: unknown[]; +} + +export interface SandboxEngine { + /** The ObjectQL-shaped stub handed to the body-runner factory as `ql`. */ + engine: Rec; + store: Record; + calls: EngineCall[]; + /** Rows of one object (creating the collection if absent). */ + rows(object: string): Rec[]; + /** Calls recorded against one object, optionally filtered by op. */ + callsFor(object: string, op?: EngineCall['op']): EngineCall[]; + /** Documents inserted into one object, in order. */ + inserted(object: string): Rec[]; +} + +/** Does `row` satisfy every key of `where`? Equality only — see `find` below. */ +function matches(row: Rec, where: Rec = {}): boolean { + return Object.entries(where).every(([field, cond]) => row[field] === cond); +} + +/** Project `fields` off a row when the caller asked for a subset. */ +function project(row: Rec, fields?: string[]): Rec { + if (!Array.isArray(fields) || fields.length === 0) return { ...row }; + const out: Rec = {}; + for (const f of fields) out[f] = row[f]; + if ('id' in row) out.id = row.id; + return out; +} + +/** + * An in-memory engine with ObjectQL's method shape — `(objectName, …)` on every + * operation, exactly what `@objectstack/runtime`'s `buildEngineRepoFacade` + * expects to wrap. + * + * Deliberately NOT a `ctx.api`-shaped object: `buildSandboxApi` hands `ctx.api` + * straight through when the engine exposes `.object(...)`, and the whole point + * here is that the runtime builds the repo surface itself. A body therefore + * meets the real `update(data, options)` / `delete(options)` signatures, not a + * friendlier invention of this file. + * + * Two behaviours are copied from the kernel rather than guessed, and + * `test/action-sandbox.test.ts` re-verifies both against a real ObjectQL on the + * in-memory driver so they cannot rot: + * + * 1. `update` resolves the target id from `data.id`, falling back to + * `options.where.id`, and rejects with `Update requires an ID or + * options.multi=true` when neither is present. + * 2. The read predicate is `where` and only `where`. An unknown key (the + * `filter` misspelling `_hook-api.ts` documents) is not an error and not a + * synonym — it is ignored, which is how a `findOne` silently degrades to + * "the object's first row". + */ +export function makeSandboxEngine(seed: Record = {}): SandboxEngine { + const store: Record = seed; + const calls: EngineCall[] = []; + let seq = 0; + const rows = (object: string): Rec[] => (store[object] ??= []); + const record = (op: EngineCall['op'], object: string, args: unknown[]) => { + calls.push({ op, object, args }); + }; + const resolveId = (data: unknown, options: Rec | undefined): string | undefined => { + const fromData = (data as Rec | undefined)?.id; + if (typeof fromData === 'string') return fromData; + const fromWhere = options?.where?.id; + return typeof fromWhere === 'string' ? fromWhere : undefined; + }; + + const engine: Rec = { + async find(object: string, options: Rec = {}) { + record('find', object, [options]); + const hits = rows(object).filter((r) => matches(r, options.where ?? {})); + const limited = typeof options.top === 'number' ? hits.slice(0, options.top) : hits; + return limited.map((r) => project(r, options.fields)); + }, + async count(object: string, options: Rec = {}) { + record('count', object, [options]); + return rows(object).filter((r) => matches(r, options.where ?? {})).length; + }, + async insert(object: string, data: Rec) { + record('insert', object, [data]); + const row = { id: data?.id ?? `${object}_${++seq}`, ...data }; + rows(object).push(row); + return row; + }, + async update(object: string, data: Rec, options: Rec = {}) { + record('update', object, [data, options]); + const id = resolveId(data, options); + if (!id && options.multi !== true) { + throw new Error('Update requires an ID or options.multi=true'); + } + const row = rows(object).find((r) => r.id === id); + if (row) Object.assign(row, data, { id: row.id }); + return row ?? null; + }, + async upsert(object: string, data: Rec, options: Rec = {}) { + record('upsert', object, [data, options]); + const id = resolveId(data, options); + const row = id ? rows(object).find((r) => r.id === id) : undefined; + if (row) { + Object.assign(row, data, { id: row.id }); + return row; + } + const created = { id: id ?? `${object}_${++seq}`, ...data }; + rows(object).push(created); + return created; + }, + async delete(object: string, options: Rec = {}) { + record('delete', object, [options]); + const id = resolveId(undefined, options); + if (!id && options.multi !== true) { + throw new Error('Delete requires an ID or options.multi=true'); + } + const list = rows(object); + const i = list.findIndex((r) => r.id === id); + if (i >= 0) list.splice(i, 1); + return { deleted: i >= 0 ? 1 : 0 }; + }, + }; + + return { + engine, + store, + calls, + rows, + callsFor: (object, op) => + calls.filter((c) => c.object === object && (op === undefined || c.op === op)), + inserted: (object) => + calls.filter((c) => c.object === object && c.op === 'insert').map((c) => c.args[0] as Rec), + }; +} + +/** + * One runner for the whole suite. `QuickJSScriptRunner.execute` builds a fresh + * WASM module and context per invocation, so runs cannot leak state into each + * other; sharing the instance only avoids re-reading its options. + */ +const runner = new QuickJSScriptRunner(); + +/** How the dispatcher invoked the action. */ +export interface ActionRunOpts { + /** Modal / toolbar params the user submitted. */ + input?: Rec; + /** The record the dispatcher pre-loaded for a record-scoped action. */ + record?: Rec; + /** The object the action was dispatched against. */ + objectName?: string; + /** Explicit record id; defaults to `record.id`. */ + recordId?: string; + /** The acting user, as the dispatcher resolves it from the session. */ + user?: Rec; + /** Reuse an engine across runs (to assert on accumulated state). */ + engine?: SandboxEngine; +} + +export interface ActionRunResult { + /** The body's return value, after its round trip through JSON. */ + result: any; + engine: SandboxEngine; +} + +const DEFAULT_USER = { id: 'usr_1', name: 'Ada Lovelace', email: 'ada@hotcrm.local' }; + +/** + * Execute an action's metadata body under the real sandbox. + * + * The `actionCtx` built here is the one BOTH dispatch paths in + * `@objectstack/runtime` build — REST (`handleActions`) and MCP + * (`invokeBusinessAction`) — which is worth being literal about, because two of + * its properties are load-bearing and neither is obvious: + * + * - `params` carries `recordId` and `objectName` merged on top of the + * submitted params, which is how a body reaches them as `input.*`; + * - there is NO `object` key on the context, so `ctx.object` is `undefined` + * in the sandbox even though `ScriptContext` declares it. A body that reads + * only `ctx.object` to identify its object gets nothing at runtime. + */ +export async function runActionBody(action: Rec, opts: ActionRunOpts = {}): Promise { + const engine = opts.engine ?? makeSandboxEngine(); + const bind = actionBodyRunnerFactory(runner, { ql: engine.engine, appId: 'hotcrm' }); + const handler = bind(action as never); + if (!handler) { + throw new Error(`action ${action.name}: no runnable body (body missing or rejected by HookBodySchema)`); + } + + const recordId = opts.recordId ?? (typeof opts.record?.id === 'string' ? opts.record.id : undefined); + // The dispatcher stamps the id onto the record it hands the body even when the + // record itself could not be loaded. + const record: Rec = { ...(opts.record ?? {}) }; + if (record.id == null && recordId) record.id = recordId; + + const result = await handler({ + record, + user: opts.user ?? DEFAULT_USER, + session: { user: opts.user ?? DEFAULT_USER }, + params: { ...(opts.input ?? {}), recordId, objectName: opts.objectName }, + }); + return { result, engine }; +} + +/** + * Lower a `handler` function to the metadata body the CLI build would ship. + * + * This is `@objectstack/cli`'s own `extractHookBody` — the pass that peels the + * function down to its statements, rejects the forbidden tokens (`import`, + * `require`, `fetch`, `process`, `globalThis`, `eval`, `new Function`), infers + * the capabilities from the `ctx.api` / `ctx.log` / `ctx.crypto` calls it can + * see, and THROWS when the handler references an identifier that will not exist + * at runtime. The build catches that throw and falls back to bundling the + * closure, so nothing goes red there — which is precisely why a test has to + * call it directly to find out whether a hook is still shippable body-only. + */ +export function extractSandboxBody(handler: unknown, label: string): { + source: string; + capabilities: string[]; +} { + const { source, capabilities } = extractHookBody(handler as never, label); + return { source, capabilities }; +} + +export interface HookRunOpts { + event: string; + /** The write in flight. Mutations the body makes are written back onto it. */ + input?: Rec; + previous?: Rec; + user?: Rec; + engine?: SandboxEngine; + /** Override the extracted body (to run a deliberately broken source). */ + source?: string; + capabilities?: string[]; +} + +export interface HookRunResult { + /** `ctx.input` after the runtime wrote the sandbox's mutations back. */ + input: Rec; + engine: SandboxEngine; +} + +/** + * Execute a hook's SHIPPED body — the lowered `body.source`, not the handler + * function — under the real sandbox. + * + * Running `hook.handler(ctx)` directly (what every other runtime test here + * does) keeps the closure alive, so it proves the logic and is blind to the + * body-only constraint. This runs what the artifact actually carries. + */ +export async function runHookBody(hook: Rec, opts: HookRunOpts): Promise { + const engine = opts.engine ?? makeSandboxEngine(); + const extracted = opts.source !== undefined + ? { source: opts.source, capabilities: opts.capabilities ?? [] } + : extractSandboxBody(hook.handler, `hook '${hook.name}'`); + + const bind = hookBodyRunnerFactory(runner, { ql: engine.engine, appId: 'hotcrm' }); + const handler = bind({ + name: hook.name, + object: hook.object, + events: hook.events, + body: { + language: 'js', + source: extracted.source, + capabilities: opts.capabilities ?? extracted.capabilities, + timeoutMs: 5000, + }, + } as never); + if (!handler) throw new Error(`hook ${hook.name}: body rejected by HookBodySchema`); + + const input: Rec = opts.input ?? {}; + await handler({ + event: opts.event, + input, + previous: opts.previous, + user: opts.user, + object: hook.object, + }); + return { input, engine }; +}