From 7bd01e068d2d4c89c8bf0a1ae80366687dda3fd2 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 20:18:15 +0000 Subject: [PATCH 1/2] =?UTF-8?q?fix(runtime):=20a=20sandboxed=20body's=20th?= =?UTF-8?q?rown=20plain=20object=20is=20answered=20with=20its=20own=20mess?= =?UTF-8?q?age,=20never=20'undefined:=20=E2=80=A6'=20or=20'[object=20Objec?= =?UTF-8?q?t]'?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude-Session: https://claude.ai/code/session_01Q9PZ5Pzqcus4woVvTZwWNU Co-authored-by: Claude --- .../runtime/src/sandbox/quickjs-runner.ts | 29 +++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/packages/runtime/src/sandbox/quickjs-runner.ts b/packages/runtime/src/sandbox/quickjs-runner.ts index b9f665347f4..bfc8615afe0 100644 --- a/packages/runtime/src/sandbox/quickjs-runner.ts +++ b/packages/runtime/src/sandbox/quickjs-runner.ts @@ -38,7 +38,7 @@ import { type QuickJSDeferredPromise, type QuickJSHandle, } from 'quickjs-emscripten'; -import { resolveSandboxTimeoutMs, sandboxBusinessMessage, isEngineDuplicateRecordEnvelope } from '@objectstack/types'; +import { resolveSandboxTimeoutMs, sandboxBusinessMessage, isEngineDuplicateRecordEnvelope, INTERNAL_ERROR_MESSAGE } from '@objectstack/types'; import type { HookBody, ScriptBody, ExpressionBody, HookBodyCapability } from '@objectstack/spec/data'; import type { ScriptContext, @@ -311,9 +311,34 @@ export class QuickJSScriptRunner implements ScriptRunner { // [#22588] `__errorRefusal` is the third channel, and as additive as the // second: a NESTED refusal's sentence, relayed only when the body // re-threw that error unchanged (see SANDBOX_REFUSAL_PROP). + // + // [#23718] A thrown value that is NOT an `Error` — the plain + // `{ code, message }` object metadata app bodies throw — has no class + // name, so `: ` flattened it to `undefined: `, + // and with no `message` `String(e)` gave `[object Object]`; both became + // the `innerMessage` every door serves as the caller's sentence. Such a + // value is flattened under the default name `Error` (the one + // `userFacingMessage` strips), so its `message` is the sentence byte for + // byte; a `name` on it is a field, never a frame. With no usable + // `message` and no `toString` of its own, the sentence is + // `INTERNAL_ERROR_MESSAGE`, what every door answers for a fault with no + // business message. `code` and the rest still ride `__errorInfo`. An + // `Error`, a primitive, and a value whose reads throw keep the + // flattening below unchanged. const REJECT_HANDLER = `function(e){ - globalThis.__error = (e && e.message) ? (e.name + ': ' + e.message) : String(e); + var flat; + try { + if (e !== null && typeof e === 'object' && !(e instanceof Error)) { + var m = e.message; + if (typeof m === 'string' && m.trim() !== '') flat = 'Error: ' + m; + else { + var s = String(e); + flat = (s !== '' && s !== Object.prototype.toString.call(e)) ? s : ${JSON.stringify(`Error: ${INTERNAL_ERROR_MESSAGE}`)}; + } + } + } catch (_) { flat = undefined; } + globalThis.__error = flat !== undefined ? flat : ((e && e.message) ? (e.name + ': ' + e.message) : String(e)); try { globalThis.__errorInfo = (e && (e.code || e.fields || e.status || e.userMessage || e['${SANDBOX_FAULT_PROP}'])) ? JSON.stringify({ code: e.code, fields: e.fields, status: e.status, userMessage: e.userMessage, sandboxFault: e['${SANDBOX_FAULT_PROP}'] === true }) From a4145db7b2ed7001a8be3349e9b663822058f8ac Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 20:21:05 +0000 Subject: [PATCH 2/2] test(runtime): pin a sandboxed body's thrown plain object at the runner and at the doors; changeset Claude-Session: https://claude.ai/code/session_01Q9PZ5Pzqcus4woVvTZwWNU Co-authored-by: Claude --- .../23718-sandbox-plain-object-throw.md | 27 ++ ...plain-object-throw-client-sentence.test.ts | 251 ++++++++++++++++++ .../plain-object-throw-sentence.test.ts | 113 ++++++++ 3 files changed, 391 insertions(+) create mode 100644 .changeset/23718-sandbox-plain-object-throw.md create mode 100644 packages/runtime/src/sandbox-plain-object-throw-client-sentence.test.ts create mode 100644 packages/runtime/src/sandbox/plain-object-throw-sentence.test.ts diff --git a/.changeset/23718-sandbox-plain-object-throw.md b/.changeset/23718-sandbox-plain-object-throw.md new file mode 100644 index 00000000000..89acfc4324c --- /dev/null +++ b/.changeset/23718-sandbox-plain-object-throw.md @@ -0,0 +1,27 @@ +--- +'@objectstack/runtime': patch +--- + +fix(runtime): a sandboxed body that throws a plain `{ code, message }` object is answered with its own `message` as the client sentence, never `undefined: MESSAGE` or `[object Object]`; its code is kept (#23718) + +**Client sentence change, status and code unchanged.** It applies to `POST /api/v1/actions/:object/:action` on a `type: 'script'` action whose sandboxed body throws a value that is not an `Error`, and to a sandboxed hook body that does the same (its refusal at the action door behind a write, and in the `/data` error classification). The QuickJS runner flattened every thrown value as `NAME: MESSAGE`, else `String(value)`. A plain object has no class name and no `toString` of its own, so the doors served `undefined: MESSAGE`, or `[object Object]` when it had no `message`. + +- `throw { code: 'EMAIL_TAKEN', message: 'That address is already on file' }` + - FROM `{"success":false,"error":{"code":"VALIDATION_ERROR","message":"undefined: That address is already on file","httpStatus":400,"declaredCode":"EMAIL_TAKEN"}}` + - TO `{"success":false,"error":{"code":"VALIDATION_ERROR","message":"That address is already on file","httpStatus":400,"declaredCode":"EMAIL_TAKEN"}}` +- `throw { code: 'DUPLICATE_RECORD' }` + - FROM `{"success":false,"error":{"code":"DUPLICATE_RECORD","message":"[object Object]","httpStatus":400}}` + - TO `{"success":false,"error":{"code":"DUPLICATE_RECORD","message":"Internal server error","httpStatus":400}}` + +A value with no usable `message` and no `toString` of its own is answered with `Internal server error`, the sentence every door already answers for a fault with no business message. To give the caller words, throw a `message` (or an `Error`). + +Two outcomes move with it, both for a thrown plain object only: + +- A `name` on a plain object is a field, not a class: `throw { name: 'TypeError', message: 'forged' }` was read as a crash (`500 INTERNAL_ERROR`); it is now a refusal, `400` with `forged`. +- A plain object whose `String()` is empty (`throw []`) recorded no rejection, so the request waited for the sandbox's wall-clock ceiling and answered `500`; it now answers at once with the no-message sentence. + +Control, unchanged: a thrown `Error` (or subclass) keeps `NAME: MESSAGE` in the server log and the client sentence it got before. A thrown string is its own sentence, and an object with a `toString` of its own keeps it. + +In-process: the `SandboxError` a caller catches for such a throw carries the value's `message` as `innerMessage`, and its log-only `message` reads `KIND 'NAME' threw: Error: MESSAGE`. + +Clause-②: no diff --git a/packages/runtime/src/sandbox-plain-object-throw-client-sentence.test.ts b/packages/runtime/src/sandbox-plain-object-throw-client-sentence.test.ts new file mode 100644 index 00000000000..270225430f0 --- /dev/null +++ b/packages/runtime/src/sandbox-plain-object-throw-client-sentence.test.ts @@ -0,0 +1,251 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#23718] A sandboxed body that throws a plain `{ code, message }` object is + * answered at the doors with that `message` as the client sentence, and its + * code kept, with no change at any door. + * + * ## What was wrong + * + * The wrapper's reject handler (`sandbox/quickjs-runner.ts`) flattened every + * rejection as `: `, else `String(e)`, and that string became + * the `innerMessage` every door serves as the caller's sentence. A plain object + * has no class name and no `toString` of its own, so + * `POST /api/v1/actions/:object/:action` answered + * `400 … "undefined: That address is already on file"` for + * `throw { code: 'EMAIL_TAKEN', message: 'That address is already on file' }` + * and `400 DUPLICATE_RECORD "[object Object]"` for + * `throw { code: 'DUPLICATE_RECORD' }`. A sandboxed HOOK body throwing the same + * values broke the same way, at the action door behind a write and in the + * `/data` classification. + * + * ## The rig + * + * PR #23717's: better-sqlite3 `:memory:` under the real `ObjectQL`, action + * bodies bound through `actionBodyRunnerFactory` over a real + * `QuickJSScriptRunner`, hook bodies through `hookBodyRunnerFactory`, and the + * dispatcher reached through `dispatch()` over a real Fetch `Request`, the + * identity step the one stubbed seam. The `/data` classification is read at + * function level (`mapDataError`), as PR #23717 read it. + * + * The sentence IS the contract here (the author's own words, relayed verbatim), + * so it is asserted exactly; for a value with no message it is asserted to be + * the shared `INTERNAL_ERROR_MESSAGE` constant, never a literal. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { ObjectQL, bindHooksToEngine } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { INTERNAL_ERROR_MESSAGE, mapDataError } from '@objectstack/types'; +import { HttpDispatcher } from './http-dispatcher.js'; +import { actionBodyRunnerFactory, hookBodyRunnerFactory } from './sandbox/body-runner.js'; +import { QuickJSScriptRunner } from './sandbox/quickjs-runner.js'; + +const PREFIX = '/api/v1'; +/** `registry.registerObject` requires a package id (see the sibling real-stack tests). */ +const TEST_PACKAGE_ID = 'objectstack-test'; +const SENTENCE = 'That address is already on file'; + +/** A `type: 'script'` action whose body is `source`. */ +const scriptAction = (name: string, source: string) => ({ + name, + label: name, + type: 'script', + body: { language: 'js', source, capabilities: ['api.read', 'api.write'] }, +}); + +/** A write to `tdoc` whose `name` tells the sandboxed `guard` hook what to throw. */ +const insertTdoc = (name: string) => + `await ctx.api.object('tdoc').insert({ id: 't_' + Math.random().toString(36).slice(2), name: '${name}' });`; + +const ACTIONS = [ + scriptAction('throw_message', `throw { code: 'EMAIL_TAKEN', message: '${SENTENCE}' };`), + scriptAction('throw_no_message', `throw { code: 'DUPLICATE_RECORD' };`), + // CONTROL — a thrown `Error` answers as before. + scriptAction('throw_error', `throw new Error('x');`), + // Two VM hops: this body's write fires `guard`'s sandboxed hook, which throws the plain object. + scriptAction('write_hooked_message', `${insertTdoc('message')} return { written: true };`), +]; + +/** Host-code handlers whose engine write fires `guard`'s sandboxed hook. */ +const HOOKED_BY_HANDLER = ['message', 'no_message', 'error'] as const; + +const PDOC = { + name: 'pdoc', + label: 'Plain-throw doc', + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true, label: 'ID' }, + name: { name: 'name', type: 'text' as const, label: 'Name' }, + }, + actions: [ + ...ACTIONS, + ...HOOKED_BY_HANDLER.map((k) => ({ name: `hooked_${k}`, label: `Hooked ${k}`, type: 'script', target: `hooked_${k}` })), + ], +}; + +const TDOC = { + name: 'tdoc', + label: 'Hook-guarded doc', + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true, label: 'ID' }, + name: { name: 'name', type: 'text' as const, label: 'Name' }, + }, +}; + +/** A sandboxed `beforeInsert` hook body that throws what the record's `name` asks for. */ +const GUARD_HOOK = { + name: 'guard', + object: 'tdoc', + events: ['beforeInsert'], + body: { + language: 'js', + source: ` + if (ctx.input.name === 'message') throw { code: 'EMAIL_TAKEN', message: '${SENTENCE}' }; + if (ctx.input.name === 'no_message') throw { code: 'DUPLICATE_RECORD' }; + if (ctx.input.name === 'error') throw new Error('x'); + `, + capabilities: [], + }, +}; + +const MEMBER = { userId: 'u_member', isSystem: false }; + +let engine: ObjectQL; +let dispatcher: HttpDispatcher; + +beforeAll(async () => { + engine = new ObjectQL(); + engine.registerDriver(new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }), true); + await engine.init(); + for (const o of [PDOC, TDOC]) engine.registry.registerObject(o as any, TEST_PACKAGE_ID); + await engine.syncSchemas(); + + const ql = engine as any; + const bindBody = actionBodyRunnerFactory(new QuickJSScriptRunner(), { ql: engine, appId: 'pdoc_app' }); + for (const action of ACTIONS) ql.registerAction('pdoc', action.name, bindBody({ ...action, object: 'pdoc' })); + for (const k of HOOKED_BY_HANDLER) { + ql.registerAction('pdoc', `hooked_${k}`, async (ctx: any) => { + await ctx.engine.insert('tdoc', { id: 't_' + Math.random().toString(36).slice(2), name: k }); + return { written: true }; + }); + } + engine.setDefaultBodyRunner(hookBodyRunnerFactory(new QuickJSScriptRunner(), { ql: engine, appId: 'pdoc_app' })); + bindHooksToEngine(engine, [GUARD_HOOK as any], { packageId: TEST_PACKAGE_ID }); + + const protocol = new ObjectStackProtocolImplementation(engine as any); + const services: Record = { objectql: engine, data: engine, protocol }; + const get = (n: string) => services[n] ?? null; + const kernel: any = { context: { getService: get }, getService: get, getServiceAsync: async (n: string) => get(n) }; + dispatcher = new HttpDispatcher(kernel); + (dispatcher as any).timedResolveExecutionContext = async () => ({ ...MEMBER }); +}, 60_000); + +afterAll(async () => { + try { await engine?.destroy(); } catch { /* noop */ } +}); + +/** `POST /api/v1/actions/pdoc/` as `@objectstack/hono`'s catch-all hands it to `dispatch()`. */ +async function postAction(action: string) { + const path = `/actions/pdoc/${action}`; + const request = new Request(`http://localhost${PREFIX}${path}`, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: '{}', + }); + const res: any = await dispatcher.dispatch('POST', path, {}, {}, { request } as any, PREFIX); + return res.response as { status: number; body: any }; +} + +/** `{ code: 'EMAIL_TAKEN', message }`: the message verbatim, the unregistered code on `declaredCode` as before. */ +function expectMessageSentence(response: { status: number; body: any }) { + expect(response.status, JSON.stringify(response.body)).toBe(400); + const error = response.body?.error; + expect(error?.message).toBe(SENTENCE); + expect(error?.code).toBe('VALIDATION_ERROR'); + expect(error?.declaredCode).toBe('EMAIL_TAKEN'); +} + +/** `{ code: 'DUPLICATE_RECORD' }`: the no-business-message sentence, the registered code kept. */ +function expectNoMessageSentence(response: { status: number; body: any }) { + expect(response.status, JSON.stringify(response.body)).toBe(400); + const error = response.body?.error; + expect(error?.message).toBe(INTERNAL_ERROR_MESSAGE); + expect(error?.code).toBe('DUPLICATE_RECORD'); + expect(JSON.stringify(response.body)).not.toContain('[object Object]'); +} + +/** The error `engine.insert('tdoc')` rejects with when `guard` throws for `name`. */ +async function hookRefusal(name: string): Promise { + const err = await engine.insert('tdoc', { id: 'd_' + Math.random().toString(36).slice(2), name }).then( + () => undefined, + (e: unknown) => e, + ); + expect(err, `guard did not refuse '${name}'`).toBeDefined(); + return err; +} + +describe('[#23718] the action door answers a body\'s thrown plain object with its own message', () => { + it('{ code, message }: the message is the client sentence, and the code is kept', async () => { + expectMessageSentence(await postAction('throw_message')); + }); + + it('{ code } with no message: not [object Object], and the code is kept', async () => { + expectNoMessageSentence(await postAction('throw_no_message')); + }); + + it('CONTROL — a thrown Error answers as before', async () => { + const response = await postAction('throw_error'); + expect(response.status, JSON.stringify(response.body)).toBe(400); + expect(response.body?.error?.code).toBe('VALIDATION_ERROR'); + expect(response.body?.error?.message).toBe('x'); + }); +}); + +describe('[#23718] a sandboxed hook body\'s thrown plain object, at the action door behind a write', () => { + it('{ code, message }, behind a host handler\'s write', async () => { + expectMessageSentence(await postAction('hooked_message')); + expect(await engine.find('tdoc', {})).toHaveLength(0); + }); + + it('{ code } with no message, behind a host handler\'s write', async () => { + expectNoMessageSentence(await postAction('hooked_no_message')); + }); + + it('{ code, message }, behind a script body\'s write (two VM hops)', async () => { + expectMessageSentence(await postAction('write_hooked_message')); + expect(await engine.find('tdoc', {})).toHaveLength(0); + }); + + it('CONTROL — the hook\'s thrown Error answers as before', async () => { + const response = await postAction('hooked_error'); + expect(response.status, JSON.stringify(response.body)).toBe(400); + expect(response.body?.error?.message).toBe('x'); + }); +}); + +describe('[#23718] the /data classification of the same hook refusals', () => { + it('{ code, message }: the message is the sentence, the code on declaredCode', async () => { + const { status, body } = mapDataError(await hookRefusal('message'), 'tdoc'); + expect(status, JSON.stringify(body)).toBe(400); + expect(body.error).toBe(SENTENCE); + expect(body.declaredCode).toBe('EMAIL_TAKEN'); + }); + + it('{ code } with no message: not [object Object], and the code is kept', async () => { + const { status, body } = mapDataError(await hookRefusal('no_message'), 'tdoc'); + expect(status, JSON.stringify(body)).toBe(400); + expect(body.error).toBe(INTERNAL_ERROR_MESSAGE); + expect(body.code).toBe('DUPLICATE_RECORD'); + }); + + it('CONTROL — the hook\'s thrown Error is classified as before', async () => { + const { status, body } = mapDataError(await hookRefusal('error'), 'tdoc'); + expect(status, JSON.stringify(body)).toBe(400); + expect(body.error).toBe('x'); + }); +}); diff --git a/packages/runtime/src/sandbox/plain-object-throw-sentence.test.ts b/packages/runtime/src/sandbox/plain-object-throw-sentence.test.ts new file mode 100644 index 00000000000..660a0234261 --- /dev/null +++ b/packages/runtime/src/sandbox/plain-object-throw-sentence.test.ts @@ -0,0 +1,113 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#23718] A sandboxed body that throws a VALUE which is not an `Error` — the + * plain `{ code, message }` object metadata app bodies throw — crosses out of + * the VM with its own `message` as `innerMessage`, the field every door serves + * as the caller's sentence. + * + * ## What was wrong + * + * The wrapper's reject handler flattened every rejection as + * `: `, else `String(e)`. A plain object has no class name and + * no `toString` of its own, so a body throwing + * `{ code: 'EMAIL_TAKEN', message: 'That address is already on file' }` was + * answered `undefined: That address is already on file`, and one throwing + * `{ code: 'DUPLICATE_RECORD' }` was answered `[object Object]`. + * + * ## What holds now + * + * A non-`Error` object is flattened under the default name `Error`, so its + * `message` is the sentence byte for byte. With no usable `message` and no + * `toString` of its own, the sentence is `INTERNAL_ERROR_MESSAGE`, the one + * every door answers for a fault with no business message. `code` rides + * `__errorInfo` as before. A thrown `Error` (and subclass), a thrown primitive, + * and a value with a `toString` of its own are flattened exactly as before. + * + * The door rows live in `../sandbox-plain-object-throw-client-sentence.test.ts`. + */ + +import { describe, it, expect } from 'vitest'; +import { INTERNAL_ERROR_MESSAGE } from '@objectstack/types'; + +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 hookOpts: ScriptRunOptions = { origin: { kind: 'hook', name: 'guard' } }; + +const SENTENCE = 'That address is already on file'; + +/** Run `source` as a body and hand back the error it rejected with. */ +async function thrownBy(source: string, opts: ScriptRunOptions = actionOpts): Promise { + const err = await runner + .run({ language: 'js', source, capabilities: [] }, { input: {} } as ScriptContext, opts) + .then( + () => undefined, + (e: unknown) => e, + ); + expect(err).toBeInstanceOf(SandboxError); + return err as SandboxError; +} + +describe('[#23718] a thrown plain object crosses with its own message as the sentence', () => { + it('{ code, message }: the message is innerMessage, and the code is kept', async () => { + const err = await thrownBy(`throw { code: 'EMAIL_TAKEN', message: '${SENTENCE}' };`); + expect(err.innerMessage).toBe(SENTENCE); + expect(err.code).toBe('EMAIL_TAKEN'); + expect(err.message).not.toContain('undefined:'); + }); + + it('…on the hook origin too (both wrappers share the handler)', async () => { + const err = await thrownBy(`throw { code: 'EMAIL_TAKEN', message: '${SENTENCE}' };`, hookOpts); + expect(err.innerMessage).toBe(SENTENCE); + expect(err.code).toBe('EMAIL_TAKEN'); + }); + + it('{ code } with no message: the no-business-message sentence, never [object Object]', async () => { + const err = await thrownBy(`throw { code: 'DUPLICATE_RECORD' };`); + expect(err.innerMessage).toBe(INTERNAL_ERROR_MESSAGE); + expect(err.code).toBe('DUPLICATE_RECORD'); + expect(err.message).not.toContain('[object Object]'); + }); + + it('a value whose String() is empty gets the same sentence (it used to record no rejection)', async () => { + const err = await thrownBy(`throw [];`); + expect(err.innerMessage).toBe(INTERNAL_ERROR_MESSAGE); + }); + + it('a message that itself begins "Error: " crosses byte for byte', async () => { + const err = await thrownBy(`throw { message: 'Error: lead is incomplete' };`); + expect(err.innerMessage).toBe('Error: lead is incomplete'); + }); + + it('a name on a plain object is a field, never a frame: no prefix, and no crash reading', async () => { + const err = await thrownBy(`throw { name: 'TypeError', message: 'forged' };`); + expect(err.innerMessage).toBe('forged'); + }); +}); + +describe('[#23718] CONTROL — everything else is flattened exactly as before', () => { + it('a thrown Error still reads `name: message` in the log, and its message is the sentence', async () => { + const err = await thrownBy(`throw new Error('x');`); + expect(err.message).toBe("action 'submit' threw: Error: x"); + expect(err.innerMessage).toBe('x'); + }); + + it('a thrown Error subclass keeps its name in both', async () => { + const err = await thrownBy(`throw new TypeError('t');`); + expect(err.message).toBe("action 'submit' threw: TypeError: t"); + expect(err.innerMessage).toBe('TypeError: t'); + }); + + it('a thrown string is its own sentence', async () => { + const err = await thrownBy(`throw 'Name is required';`); + expect(err.innerMessage).toBe('Name is required'); + }); + + it('an object with a toString of its own keeps it', async () => { + const err = await thrownBy(`throw { toString() { return 'custom words'; } };`); + expect(err.innerMessage).toBe('custom words'); + }); +});