From f30370c93bf5ff74704b02a809ba761d8dbf6a7a Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 29 Jul 2026 10:49:25 +0000 Subject: [PATCH 1/2] fix(runtime): carry code/fields across the sandbox boundary so form actions can anchor validation errors (#3918 follow-up) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by dogfooding the merged #3918 chain against a running app. Submitting a record that fails validation through a form ACTION came back as HTTP 200 { success: true, data: { success: false, error: "ValidationError: issued_on is required" } } — no status to branch on, no code, no fields[]. The chain's dispatcher fixes could not help: the field list was already gone before any dispatcher exit ran. It was lost at the QuickJS boundary, twice: 1. host → VM: `vm.newError({name, message})` dropped every other property, so a body reaching a record ValidationError through `ctx.api.object(x).update(…)` saw bare prose. 2. VM → host: the wrapper's reject handler flattened the error to the string `: ` before the host ever saw it. Both hops now carry an explicit ALLOWLIST — `code` and `fields` — alongside the message, and SandboxError exposes them as `.code` / `.fields`. The allowlist is a security boundary, not a style choice: host errors routinely hang driver state, connection details or whole record payloads off themselves, and anything crossing INTO the VM is readable by untrusted sandboxed code. `/actions` then surfaces them so a form can highlight the offending input. Its wire contract is deliberately UNCHANGED: status stays 200 and `success: false` remains the failure signal — that route has always reported business failure in the payload and every caller branches on `data.success`, so `code`/`fields` are purely additive and omitted when absent. Message channels are byte-identical: `.message` keeps the debug wrapper for server logs, `.innerMessage` stays the toast text. Also adds dispatcher-validation-error.real.test.ts, pinning both dispatcher exits against the REAL objectql ValidationError rather than a fixture — including its deliberate absence of `.status`, the assumption the whole #3918 fix rests on. Covered by 19 new cases across sandbox, /actions and the dispatcher exits; 7 of them fail on the pre-fix source. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LeZ7DmSKjiKmQzYh8L5CJS --- .../sandbox-structured-error-passthrough.md | 61 +++++++ .../dispatcher-validation-error.real.test.ts | 137 +++++++++++++++ .../actions-validation-envelope.test.ts | 128 ++++++++++++++ packages/runtime/src/domains/actions.ts | 25 ++- .../src/sandbox/error-passthrough.test.ts | 162 ++++++++++++++++++ .../runtime/src/sandbox/quickjs-runner.ts | 135 +++++++++++++-- 6 files changed, 634 insertions(+), 14 deletions(-) create mode 100644 .changeset/sandbox-structured-error-passthrough.md create mode 100644 packages/runtime/src/dispatcher-validation-error.real.test.ts create mode 100644 packages/runtime/src/domains/actions-validation-envelope.test.ts create mode 100644 packages/runtime/src/sandbox/error-passthrough.test.ts diff --git a/.changeset/sandbox-structured-error-passthrough.md b/.changeset/sandbox-structured-error-passthrough.md new file mode 100644 index 0000000000..90825481ff --- /dev/null +++ b/.changeset/sandbox-structured-error-passthrough.md @@ -0,0 +1,61 @@ +--- +"@objectstack/runtime": patch +--- + +fix(runtime): carry `code` / `fields[]` across the sandbox boundary so form actions can anchor validation errors (#3918 follow-up) + +Found by dogfooding the merged #3918 chain against a running app. Submitting a +record that fails validation through a form **action** came back as: + +``` +HTTP 200 +{ "success": true, "data": { "success": false, + "error": "ValidationError: issued_on is required" } } +``` + +No status a client could branch on, no code, no `fields[]`. The chain's +dispatcher fixes could not help: the field list was already gone before any +dispatcher exit ran. It was lost at the QuickJS boundary, twice — + +1. **host → VM.** `vm.newError({ name, message })` dropped every other property, + so a body reaching a record `ValidationError` through + `ctx.api.object(x).update(...)` saw bare prose. +2. **VM → host.** The wrapper's reject handler flattened the error to the string + `: ` before the host ever saw it. + +Both hops now carry an explicit **allowlist** — `code` and `fields` — alongside +the message, and `SandboxError` exposes them as `.code` / `.fields`. The +allowlist is a security boundary, not a style choice: host errors routinely hang +driver state, connection details or whole record payloads off themselves, and +anything crossing INTO the VM is readable by untrusted sandboxed code. Copying +the error's own enumerable keys would leak all of it. + +`/actions` then surfaces them, so a form can highlight the offending input: + +``` +HTTP 200 +{ "success": true, "data": { "success": false, + "error": "ValidationError: issued_on is required", + "code": "VALIDATION_FAILED", + "fields": [ { "field": "issued_on", "code": "required", … } ] } } +``` + +**The `/actions` wire contract is deliberately unchanged.** The status stays +200 and `success: false` remains the failure signal: that route has always +reported business failure in the payload (an action that "fails" is a normal +outcome, not a transport error) and every caller branches on `data.success`. +Making it a 4xx would be a break in exchange for a strictly additive fix, so the +fix is additive — `code` and `fields` are simply omitted when absent, and a +caller that ignores them sees exactly what it saw before. + +Message channels are byte-identical: `SandboxError.message` keeps the +` '' threw:` debug wrapper for server logs and `.innerMessage` stays +the plain business text a toast shows. The structured payload rides alongside +them, never instead of them. + +Also adds `dispatcher-validation-error.real.test.ts`, which pins both dispatcher +exits against the **real** objectql `ValidationError` rather than a hand-built +fixture — including its deliberate absence of `.status`, the assumption the +whole #3918 fix rests on. The existing fixture-based tests restate that contract; +these check it, so a future change to the class fails a test instead of quietly +regressing production. diff --git a/packages/runtime/src/dispatcher-validation-error.real.test.ts b/packages/runtime/src/dispatcher-validation-error.real.test.ts new file mode 100644 index 0000000000..7b20df1da0 --- /dev/null +++ b/packages/runtime/src/dispatcher-validation-error.real.test.ts @@ -0,0 +1,137 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #3918 follow-up — pin both dispatcher exits against the REAL `ValidationError`. + * + * `dispatcher-validation-error.test.ts` drives the exits with a hand-built + * fixture: an `Error` with `name = 'ValidationError'`, `code = + * 'VALIDATION_FAILED'` and a `fields[]`. That is deliberate — it proves the + * duck-typing predicate accepts the SHAPE, including for hand-thrown errors + * that never touched objectql. + * + * What it cannot prove is that the shape it asserts is still the shape objectql + * actually throws. The whole fix rests on three facts about that class — + * `.code`, `.fields[]`, and the deliberate ABSENCE of `.status` — and a fixture + * restates those facts rather than checking them. Give `ValidationError` a + * `.status` one day, or rename `.fields`, and every fixture-based test here + * keeps passing while production quietly regresses. + * + * So these construct the genuine article and send it through both exits. They + * are the tests that fail if the contract moves. + */ + +import { describe, it, expect } from 'vitest'; +import { ValidationError } from '@objectstack/objectql'; + +import { HttpDispatcher } from './http-dispatcher.js'; +import { createDispatcherPlugin } from './dispatcher-plugin.js'; + +const FIELDS = [ + { field: 'email', code: 'invalid_email' as const, message: 'email must be a valid email address' }, + { field: 'name', code: 'required' as const, message: 'name is required' }, +]; + +describe('#3918 — the real objectql ValidationError still has the shape the fix relies on', () => { + const err = new ValidationError(FIELDS); + + it('carries `code` and `fields[]`', () => { + expect(err.code).toBe('VALIDATION_FAILED'); + expect(err.fields).toEqual(FIELDS); + expect(err.name).toBe('ValidationError'); + }); + + it('carries NO `status` / `statusCode` — which is why the boundary must supply 400', () => { + // If this ever fails, the dispatcher's `.status`-first precedence will + // start winning over the 400 default and these exits change behaviour + // silently. That is the assumption the whole fix is built on. + expect((err as any).status).toBeUndefined(); + expect((err as any).statusCode).toBeUndefined(); + }); + + it('carries no `issues` — the property the old `details` builder read', () => { + expect((err as any).issues).toBeUndefined(); + }); +}); + +// --------------------------------------------------------------------------- + +/** Exit 1 — the RETURNED path, through a route whose fallback is 500. */ +async function publishDrafts(thrown: unknown) { + const protocol = { publishPackageDrafts: async () => { throw thrown; } }; + const objectql = { registry: {} }; + const resolve = (name: string) => + name === 'protocol' ? protocol : name === 'objectql' ? objectql : undefined; + const kernel: any = { getService: resolve, getServiceAsync: async (n: string) => resolve(n) }; + const result: any = await new HttpDispatcher(kernel).dispatch( + 'POST', '/packages/demo/publish-drafts', {}, {}, {} as any, + ); + return result.response; +} + +/** Exit 2 — the THROWN path, through the plugin's real route handler. */ +async function analyticsQuery(thrown: unknown) { + const handlers: Record any> = {}; + const rec = (verb: string) => (path: string, h: any) => { handlers[`${verb} ${path}`] = h; }; + const server = { + get: rec('GET'), post: rec('POST'), put: rec('PUT'), + delete: rec('DELETE'), patch: rec('PATCH'), + }; + const analytics = { + query: async () => { throw thrown; }, + getMeta: async () => ({ cubes: [] }), + generateSql: async () => ({ sql: null }), + }; + const kernel = { + getService: (n: string) => (n === 'analytics' ? analytics : undefined), + getServiceAsync: async (n: string) => (n === 'analytics' ? analytics : undefined), + }; + const plugin = createDispatcherPlugin({ prefix: '/api/v1', securityHeaders: false }); + await plugin.start?.({ + getKernel: () => kernel, + getService: (n: string) => (n === 'http.server' ? server : undefined), + environmentId: undefined, + logger: { info() {}, warn() {}, error() {}, debug() {} }, + hook: () => {}, on: () => {}, + } as any); + + const res: any = { + statusCode: undefined, body: undefined, + status(c: number) { res.statusCode = c; return res; }, + header() { return res; }, + json(b: any) { res.body = b; return res; }, + }; + await handlers['POST /api/v1/analytics/query']({ body: { cube: 'x', query: {} }, query: {} }, res); + return res; +} + +describe('#3918 — both exits serve the real ValidationError as 400 + fields[]', () => { + it('errorFromThrown (returned path)', async () => { + const res = await publishDrafts(new ValidationError(FIELDS)); + + expect(res.status).toBe(400); + expect(res.body.error.details).toEqual({ code: 'VALIDATION_FAILED', fields: FIELDS }); + // The class builds its message from the human field messages — that is + // what a toast shows, so it must survive intact rather than being + // replaced by the 5xx sanitiser. + expect(res.body.error.message).toContain('email must be a valid email address'); + }); + + it('errorResponseBase (thrown path)', async () => { + const res = await analyticsQuery(new ValidationError(FIELDS)); + + expect(res.statusCode).toBe(400); + expect(res.body.error.details).toEqual({ code: 'VALIDATION_FAILED', fields: FIELDS }); + expect(res.body.error.message).toContain('name is required'); + // Not a server fault: the errorReporter side-channel stays clear. + expect(res.__obsRecordedError).toBeUndefined(); + }); + + it('an empty-fields ValidationError still answers 400, not 500', async () => { + // The class defaults its message to 'Validation failed' for an empty + // list; the status must not depend on the list being non-empty. + const res = await publishDrafts(new ValidationError([])); + + expect(res.status).toBe(400); + expect(res.body.error.details.fields).toEqual([]); + }); +}); diff --git a/packages/runtime/src/domains/actions-validation-envelope.test.ts b/packages/runtime/src/domains/actions-validation-envelope.test.ts new file mode 100644 index 0000000000..aca5c4f8e2 --- /dev/null +++ b/packages/runtime/src/domains/actions-validation-envelope.test.ts @@ -0,0 +1,128 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #3918 follow-up — the LAST hop: what `/actions` puts on the wire. + * + * The sandbox now carries `code` / `fields` back out of the VM on a + * `SandboxError` (see `sandbox/error-passthrough.test.ts`). This pins the other + * half: the actions route surfacing them, which is what a form actually reads. + * + * Before, this envelope was the message string and nothing else, so a form + * action could raise a toast but never highlight the offending input — the + * symptom in the original report. + * + * **The status stays 200.** `/actions` has always reported business failure in + * the payload (`{ success: false }`), not as a transport error, and every + * caller branches on `data.success`. Making this a 4xx would be a wire-contract + * break in exchange for a strictly additive fix, so the fix is additive: the + * same envelope, now carrying what the client needs. That choice is asserted + * below so it cannot drift silently. + */ + +import { describe, it, expect, vi } from 'vitest'; + +import { HttpDispatcher } from '../http-dispatcher.js'; +import { SandboxError } from '../sandbox/quickjs-runner.js'; + +const FIELDS = [ + { field: 'issued_on', code: 'required', message: 'issued_on is required' }, +]; + +const scriptAction = { + name: 'submit_signoff', + objectName: 'crm_invoice', + type: 'script', + body: { language: 'js', source: 'return 1;', capabilities: ['api.write'] }, +}; + +/** A dispatcher whose action handler rejects with `thrown`. */ +function makeDispatcher(thrown: unknown) { + const objectDef = { name: 'crm_invoice', actions: [scriptAction] }; + const ql: any = { + executeAction: vi.fn(async () => { throw thrown; }), + getSchema: (n: string) => (n === objectDef.name ? objectDef : undefined), + registry: { getObject: (n: string) => (n === objectDef.name ? objectDef : undefined), getItem: () => undefined }, + find: vi.fn(async () => [{ id: 'inv_1', status: 'draft' }]), + insert: vi.fn(), update: vi.fn(), delete: vi.fn(), + }; + const metadata: any = { + load: vi.fn(async () => null), + listObjects: vi.fn(async () => [objectDef]), + getObject: vi.fn(async () => objectDef), + }; + const kernel: any = { + context: { + getService: (n: string) => + n === 'objectql' || n === 'data' ? ql : n === 'metadata' ? metadata : null, + }, + }; + return new HttpDispatcher(kernel); +} + +async function invoke(thrown: unknown) { + const res: any = await makeDispatcher(thrown).handleActions( + '/crm_invoice/submit_signoff/inv_1', + 'POST', + {}, + { request: {}, environmentId: 'platform', executionContext: { userId: 'u1', systemPermissions: [] } } as any, + ); + return res.response; +} + +/** What the sandbox now throws for a record validation failure. */ +const validationSandboxError = () => + new SandboxError( + "action 'submit_signoff' threw: ValidationError: issued_on is required", + 'ValidationError: issued_on is required', + { code: 'VALIDATION_FAILED', fields: FIELDS }, + ); + +describe('#3918 follow-up — /actions surfaces code + fields on a validation failure', () => { + it('carries `fields[]` so a form can anchor the error to the input', async () => { + const response = await invoke(validationSandboxError()); + + expect(response.body.data).toMatchObject({ + success: false, + code: 'VALIDATION_FAILED', + fields: FIELDS, + }); + }); + + it('keeps the human message as the toast text', async () => { + const response = await invoke(validationSandboxError()); + + expect(response.body.data.error).toBe('ValidationError: issued_on is required'); + }); + + it('KEEPS HTTP 200 and `success: false` — the wire contract is unchanged', async () => { + // Deliberate: see the file header. If this flips to 4xx it must be a + // conscious, documented break, not a side effect. + const response = await invoke(validationSandboxError()); + + expect(response.status).toBe(200); + expect(response.body.success).toBe(true); // dispatcher envelope + expect(response.body.data.success).toBe(false); // business outcome + }); + + it('omits `code` / `fields` entirely for an ordinary failure', async () => { + // Callers branch on presence; emitting empty values would claim a + // field-anchored failure that never happened. + const response = await invoke( + new SandboxError("action 'x' threw: boom", 'boom'), + ); + + expect(response.body.data).toEqual({ success: false, error: 'boom' }); + expect('code' in response.body.data).toBe(false); + expect('fields' in response.body.data).toBe(false); + }); + + it('carries a code without fields when that is all the error had', async () => { + const response = await invoke( + new SandboxError("action 'x' threw: pick another", 'pick another', { code: 'DUPLICATE' }), + ); + + expect(response.body.data).toEqual({ + success: false, error: 'pick another', code: 'DUPLICATE', + }); + }); +}); diff --git a/packages/runtime/src/domains/actions.ts b/packages/runtime/src/domains/actions.ts index 944da0ec71..3d39d07487 100644 --- a/packages/runtime/src/domains/actions.ts +++ b/packages/runtime/src/domains/actions.ts @@ -272,6 +272,29 @@ export async function handleActionsRequest(deps: DomainHandlerDeps, path: string const inner: unknown = err?.innerMessage; const clientMsg = (typeof inner === 'string' && inner) ? inner : full; if (clientMsg !== full) console.error(`[action ${objectName}/${actionName}] ${full}`); - return { handled: true, response: deps.success({ success: false, error: clientMsg }) }; + // [#3918 follow-up] Carry the structured payload when the failure was a + // record validation error. Until the sandbox learned to pass `code` / + // `fields` back OUT of the VM, this envelope was the message string and + // nothing else — so a form action could only ever raise a toast, never + // highlight the field the user actually got wrong, which is the symptom + // the original report was about. + // + // The HTTP status stays 200 and `success: false` remains the failure + // signal. `/actions` has always reported business failure in the payload + // (an action that "fails" is a normal outcome, not a transport error) and + // every existing caller branches on `data.success`; turning this into a + // 4xx would be a wire-contract break in exchange for a strictly additive + // fix. + const code: unknown = err?.code; + const fields: unknown = err?.fields; + return { + handled: true, + response: deps.success({ + success: false, + error: clientMsg, + ...(typeof code === 'string' && code ? { code } : {}), + ...(Array.isArray(fields) ? { fields } : {}), + }), + }; } } diff --git a/packages/runtime/src/sandbox/error-passthrough.test.ts b/packages/runtime/src/sandbox/error-passthrough.test.ts new file mode 100644 index 0000000000..43720d40c8 --- /dev/null +++ b/packages/runtime/src/sandbox/error-passthrough.test.ts @@ -0,0 +1,162 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #3918 follow-up — structured host errors crossing the sandbox boundary. + * + * Found by dogfooding the merged #3918 chain against a running app: submitting + * a record that fails validation through a form ACTION came back as + * + * HTTP 200 { success: true, data: { success: false, + * error: "ValidationError: issued_on is required" } } + * + * — no status a client could branch on, no code, and no `fields[]`. The chain's + * dispatcher fixes could not help, because the field list was already gone + * before any dispatcher exit ran. It was lost at the VM boundary, twice: + * + * 1. host → VM: `vm.newError({ name, message })` dropped every other property. + * 2. VM → host: the wrapper's reject handler flattened the error to the + * string `: `. + * + * Both hops now carry an explicit ALLOWLIST (`code`, `fields`) alongside the + * message. The allowlist is the security-relevant part and is asserted here: + * host errors routinely hang driver state, connection details or whole record + * payloads off themselves, and anything that crosses INTO the VM is readable by + * untrusted sandboxed code. + */ + +import { describe, it, expect } from 'vitest'; +import { ValidationError } from '@objectstack/objectql'; + +import { QuickJSScriptRunner, SandboxError } from './quickjs-runner.js'; +import type { ScriptContext, ScriptRunOptions } from './script-runner.js'; + +const runner = new QuickJSScriptRunner({ hookTimeoutMs: 10_000 }); +const actionOpts: ScriptRunOptions = { origin: { kind: 'action', name: 'submit' } }; + +const FIELDS = [ + { field: 'issued_on', code: 'required' as const, message: 'issued_on is required' }, +]; + +/** A ctx.api whose single write rejects with `thrown`. */ +function apiThrowing(thrown: unknown) { + return { object: () => ({ update: async () => { throw thrown; } }) } as any; +} + +/** Run an action body against that api. */ +function run(source: string, thrown: unknown) { + return runner.run( + { language: 'js', source, capabilities: ['api.write'] }, + { input: {}, api: apiThrowing(thrown) } as ScriptContext, + actionOpts, + ); +} + +describe('#3918 follow-up — host error → VM', () => { + it('a body can read `code` and `fields` off a rejected write', async () => { + const r = await run( + `try { await ctx.api.object('invoice').update({ id: 1 }); } + catch (e) { return { code: e.code, fields: e.fields, message: e.message, name: e.name }; } + return { never: true };`, + new ValidationError(FIELDS), + ); + + expect(r.value).toEqual({ + code: 'VALIDATION_FAILED', + fields: FIELDS, + message: 'issued_on is required', + name: 'ValidationError', + }); + }); + + it('does NOT carry anything outside the allowlist into the VM', async () => { + // The security assertion. `internal` stands in for the driver handles, + // DSNs and record payloads real host errors carry; sandboxed code must + // not be able to read them off a rejection. + const err = Object.assign(new ValidationError(FIELDS), { + internal: { dsn: 'postgres://user:pw@host/db' }, + record: { ssn: '000-00-0000' }, + }); + const r = await run( + `try { await ctx.api.object('invoice').update({ id: 1 }); } + catch (e) { return { internal: e.internal ?? null, record: e.record ?? null, keys: Object.keys(e) }; } + return { never: true };`, + err, + ); + + const v = r.value as any; + expect(v.internal).toBeNull(); + expect(v.record).toBeNull(); + // Nothing beyond the message channels and the two allowlisted keys. + // (`vm.newError` makes `name`/`message` own enumerable props in QuickJS.) + expect(v.keys.sort()).toEqual(['code', 'fields', 'message', 'name']); + }); + + it('leaves an ordinary error as bare name/message', async () => { + const r = await run( + `try { await ctx.api.object('invoice').update({ id: 1 }); } + catch (e) { return { name: e.name, message: e.message, code: e.code ?? null, fields: e.fields ?? null }; } + return { never: true };`, + new Error('backend unavailable'), + ); + + expect(r.value).toEqual({ + name: 'Error', message: 'backend unavailable', code: null, fields: null, + }); + }); +}); + +describe('#3918 follow-up — VM error → host', () => { + /** Let the rejection escape the body, the way a real action body does. */ + const UNCAUGHT = `await ctx.api.object('invoice').update({ id: 1 }); return { never: true };`; + + it('carries `code` and `fields` onto the thrown SandboxError', async () => { + const err = await run(UNCAUGHT, new ValidationError(FIELDS)).catch((e) => e); + + expect(err).toBeInstanceOf(SandboxError); + expect(err.code).toBe('VALIDATION_FAILED'); + expect(err.fields).toEqual(FIELDS); + }); + + it('leaves the message channels byte-identical', async () => { + // `.message` (server log, with the debug wrapper) and `.innerMessage` + // (what a toast shows) are what every existing consumer reads. The + // structured payload rides ALONGSIDE them, never instead of them. + const err = await run(UNCAUGHT, new ValidationError(FIELDS)).catch((e) => e); + + expect(err.message).toBe("action 'submit' threw: ValidationError: issued_on is required"); + expect(err.innerMessage).toBe('ValidationError: issued_on is required'); + }); + + it('leaves an ordinary failure with no code/fields at all', async () => { + const err = await run(UNCAUGHT, new Error('backend unavailable')).catch((e) => e); + + expect(err).toBeInstanceOf(SandboxError); + expect(err.code).toBeUndefined(); + expect(err.fields).toBeUndefined(); + expect(err.innerMessage).toBe('backend unavailable'); + }); + + it('carries the payload for an error the BODY re-throws', async () => { + // A body that catches, inspects and re-throws must not lose it either. + const err = await run( + `try { await ctx.api.object('invoice').update({ id: 1 }); } + catch (e) { throw e; }`, + new ValidationError(FIELDS), + ).catch((e) => e); + + expect(err.code).toBe('VALIDATION_FAILED'); + expect(err.fields).toEqual(FIELDS); + }); + + it('survives a body that throws its own error with a code', async () => { + // Author-thrown structured errors get the same treatment — nothing here + // is objectql-specific. + const err = await run( + `var e = new Error('pick another'); e.code = 'DUPLICATE'; throw e;`, + new Error('unused'), + ).catch((e) => e); + + expect(err.code).toBe('DUPLICATE'); + expect(err.fields).toBeUndefined(); + }); +}); diff --git a/packages/runtime/src/sandbox/quickjs-runner.ts b/packages/runtime/src/sandbox/quickjs-runner.ts index 4aab2e580b..c3cde088b9 100644 --- a/packages/runtime/src/sandbox/quickjs-runner.ts +++ b/packages/runtime/src/sandbox/quickjs-runner.ts @@ -271,16 +271,31 @@ export class QuickJSScriptRunner implements ScriptRunner { // 1. yield to the host event loop (lets host promises settle) // 2. drain QuickJS pending jobs (advances the .then chain) // 3. read __result/__error from the VM + // `__error` stays EXACTLY the flattened `: ` string it has + // always been — it is what the SandboxError message/innerMessage are built + // from, and every existing consumer reads it. `__errorInfo` is a strictly + // additive side-channel carrying the structured bits + // (SANDBOX_ERROR_PASSTHROUGH) that the flattening discards, so a record + // `ValidationError` keeps its `fields[]` on the way back out to the host. + const REJECT_HANDLER = + `function(e){ + globalThis.__error = (e && e.message) ? (e.name + ': ' + e.message) : String(e); + try { + globalThis.__errorInfo = (e && (e.code || e.fields)) + ? JSON.stringify({ code: e.code, fields: e.fields }) + : undefined; + } catch (_) { globalThis.__errorInfo = undefined; } + }`; const wrapped = args.origin.kind === 'hook' - ? `globalThis.__result = undefined; globalThis.__error = undefined; + ? `globalThis.__result = undefined; globalThis.__error = undefined; globalThis.__errorInfo = undefined; (async (ctx) => { ${args.source} })(globalThis.__ctx).then( function(v){ globalThis.__result = JSON.stringify(v === undefined ? null : v); }, - function(e){ globalThis.__error = (e && e.message) ? (e.name + ': ' + e.message) : String(e); } + ${REJECT_HANDLER} );` - : `globalThis.__result = undefined; globalThis.__error = undefined; + : `globalThis.__result = undefined; globalThis.__error = undefined; globalThis.__errorInfo = undefined; (async (input, ctx) => { ${args.source} })(globalThis.__input, globalThis.__ctx).then( function(v){ globalThis.__result = JSON.stringify(v === undefined ? null : v); }, - function(e){ globalThis.__error = (e && e.message) ? (e.name + ': ' + e.message) : String(e); } + ${REJECT_HANDLER} );`; sliceStart = Date.now(); @@ -367,6 +382,7 @@ export class QuickJSScriptRunner implements ScriptRunner { throw new SandboxError( `${args.origin.kind} '${args.origin.name}' threw: ${errStr}`, userFacingMessage(String(errStr)), + readErrorInfo(vm), ); } @@ -516,10 +532,7 @@ export class QuickJSScriptRunner implements ScriptRunner { deferred.resolve(vm.undefined); } catch (err) { if (!vm.alive) return; - const errH = - err instanceof Error - ? vm.newError({ name: err.name || 'Error', message: err.message }) - : vm.newError({ name: 'Error', message: String(err) }); + const errH = hostErrorToVm(vm, err); deferred.reject(errH); errH.dispose(); } @@ -715,10 +728,9 @@ function installApiMethod( h.dispose(); } catch (err) { if (!vm.alive) return; - const errH = - err instanceof Error - ? vm.newError({ name: err.name || 'Error', message: err.message }) - : vm.newError({ name: 'Error', message: String(err) }); + // The load-bearing one: this is the `ctx.api.object(x).()` rejection, + // i.e. where a record `ValidationError` enters the VM. + const errH = hostErrorToVm(vm, err); deferred.reject(errH); errH.dispose(); } @@ -764,6 +776,55 @@ function safeJsonStringify(v: unknown): string { return json ?? 'null'; } +/** + * Structured properties a HOST error may carry ACROSS the sandbox boundary, + * in addition to `name`/`message`. + * + * This is an explicit ALLOWLIST, and that is a security decision rather than a + * style one: everything placed on the handle below becomes readable by + * untrusted sandboxed code, and host errors routinely hang driver state, + * connection details, or whole record payloads off themselves. Copying the + * error's own enumerable keys would leak all of it. Only these two are safe and + * useful — they are already destined for the HTTP client. + * + * Why they need to cross at all: a record `ValidationError` reaching a body via + * `ctx.api.object(x).update(...)` used to arrive as bare `name`/`message`, so + * its `fields[]` was gone before any dispatcher exit could map it (#3918 + * follow-up) — a form action could only ever show prose, never highlight the + * offending input. + */ +const SANDBOX_ERROR_PASSTHROUGH = ['code', 'fields'] as const; + +/** + * Marshal a HOST error into the VM as a rejectable QuickJS error handle, + * carrying {@link SANDBOX_ERROR_PASSTHROUGH} when present. + * + * The caller owns the returned handle and must dispose it. + */ +function hostErrorToVm(vm: QuickJSContext, err: unknown): QuickJSHandle { + const e = err as { name?: string; message?: string; code?: unknown; fields?: unknown }; + const errH = err instanceof Error + ? vm.newError({ name: e.name || 'Error', message: e.message ?? '' }) + : vm.newError({ name: 'Error', message: String(err) }); + // Best-effort: a malformed `fields` must never turn an ordinary rejection + // into a marshalling failure, which would replace the body's real error. + try { + if (typeof e?.code === 'string' && e.code) { + const h = vm.newString(e.code); + vm.setProp(errH, 'code', h); + h.dispose(); + } + if (Array.isArray(e?.fields)) { + const h = jsonToHandle(vm, e.fields); + vm.setProp(errH, 'fields', h); + h.dispose(); + } + } catch { + /* keep the bare name/message error */ + } + return errH; +} + /** Marshal a host JSON-serializable value into a QuickJS handle. */ function jsonToHandle(vm: QuickJSContext, v: unknown): QuickJSHandle { const json = safeJsonStringify(v); @@ -861,11 +922,59 @@ export class SandboxError extends Error { * which have no user-meaningful inner message. */ readonly innerMessage?: string; - constructor(message: string, innerMessage?: string) { + /** + * The semantic code of the error that crossed OUT of the VM, when it carried + * one — most usefully `'VALIDATION_FAILED'`. + */ + readonly code?: string; + /** + * Per-field validation envelopes belonging to that error. Present only when a + * record `ValidationError` reached the body (typically via + * `ctx.api.object(x).update(...)`), so a caller can highlight the offending + * input instead of showing the message alone. + */ + readonly fields?: unknown[]; + constructor(message: string, innerMessage?: string, info?: SandboxErrorInfo) { super(message); this.name = 'SandboxError'; this.innerMessage = innerMessage; + if (info?.code) this.code = info.code; + if (info?.fields) this.fields = info.fields; + } +} + +/** The structured payload carried out of the VM alongside the flattened message. */ +export interface SandboxErrorInfo { + code?: string; + fields?: unknown[]; +} + +/** + * Read the `__errorInfo` side-channel the wrapper's reject handler writes. + * Returns `undefined` when the body's error carried nothing structured — which + * is the common case, and keeps `SandboxError` unchanged for it. + */ +function readErrorInfo(vm: QuickJSContext): SandboxErrorInfo | undefined { + let raw: unknown; + try { + const h = vm.getProp(vm.global, '__errorInfo'); + raw = vm.dump(h); + h.dispose(); + } catch { + return undefined; + } + if (typeof raw !== 'string' || !raw) return undefined; + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + return undefined; } + const p = parsed as { code?: unknown; fields?: unknown }; + const info: SandboxErrorInfo = {}; + if (typeof p?.code === 'string' && p.code) info.code = p.code; + if (Array.isArray(p?.fields)) info.fields = p.fields; + return info.code || info.fields ? info : undefined; } /** From b7ee914f6dacf168e562f332cd2c20561d271c63 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 29 Jul 2026 10:51:17 +0000 Subject: [PATCH 2/2] docs(automation): document structured errors from ctx.api in hook bodies The docs-drift check on #3937 flagged `automation/hook-bodies.mdx`. Nothing there was made stale by this branch, but the page is the reference for what a sandboxed body gets from `ctx.api`, and this branch gives it something new: a rejected call now carries `code` and `fields` alongside `name`/`message`. Adds a short "Errors from ctx.api" section under nested cross-object writes, covering the two properties, the worked `VALIDATION_FAILED` case, the fact that the passthrough is a deliberate ALLOWLIST (body code can read anything on a rejection, so host errors must not arrive with driver state or record payloads attached), and that an escaping or re-thrown error keeps the payload on the way back out to the action's HTTP response. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LeZ7DmSKjiKmQzYh8L5CJS --- content/docs/automation/hook-bodies.mdx | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/content/docs/automation/hook-bodies.mdx b/content/docs/automation/hook-bodies.mdx index d33b3307a8..3902b7dc20 100644 --- a/content/docs/automation/hook-bodies.mdx +++ b/content/docs/automation/hook-bodies.mdx @@ -151,6 +151,31 @@ Per-invocation budgets default to **250ms** (hooks) / **5000ms** (actions) of ** A body may write *other* objects — e.g. `await ctx.api.object('parent').update({ ... })` from a child's `afterInsert`/`afterUpdate` (requires `api.write`). The target's own hooks fire too: the nested write runs in a **fresh sandbox VM** while the calling body is suspended, and this composes to any depth. This is the natural "when a child changes, roll the total up to the parent" automation — it does **not** need a denormalized, hand-maintained mirror field. Because each body's budget is **CPU time** (ADR-0102), the caller is **not** charged for the nested write's own run — so the stock 250ms default comfortably covers deep rollup chains, and you rarely need to raise `timeoutMs` (the spec still permits up to 30_000ms for a genuinely CPU-heavy body). +### Errors from `ctx.api` + +A rejected `ctx.api` call gives your body the host error's `name` and `message`, plus two structured properties when the host supplied them: + +| Property | Meaning | +|:---|:---| +| `e.code` | The semantic code, e.g. `'VALIDATION_FAILED'` | +| `e.fields` | Per-field validation envelopes — `{ field, code, message }[]` | + +```js +try { + await ctx.api.object('invoice').update({ id: input.id, status: 'sent' }); +} catch (e) { + if (e.code === 'VALIDATION_FAILED') { + // e.fields → [{ field: 'issued_on', code: 'required', message: 'issued_on is required' }] + throw e; // re-throwing keeps the payload; see below + } + throw e; +} +``` + +Nothing else crosses into the VM. That is a deliberate **allowlist**, not an oversight: host errors routinely carry driver state, connection details or whole record payloads, and anything reachable on a rejection is readable by body code. + +An error your body lets escape — or re-throws — keeps `code` and `fields` on the way back out too, so an action's HTTP response can carry them (`data.code` / `data.fields`) and a form can highlight the offending input rather than only raising a toast. Errors you construct yourself are treated the same way: set `e.code` before throwing and it reaches the caller. + ## L3 — Compiled modules (intentionally disabled) An earlier design allowed the CLI to emit a sibling `objectstack-runtime..mjs` that `objectos` would `import()` at runtime. We removed that path because: