diff --git a/.changeset/23723-sandbox-unreadable-rejection.md b/.changeset/23723-sandbox-unreadable-rejection.md new file mode 100644 index 00000000000..b52626524a5 --- /dev/null +++ b/.changeset/23723-sandbox-unreadable-rejection.md @@ -0,0 +1,24 @@ +--- +'@objectstack/runtime': patch +--- + +fix(runtime): a sandboxed body that throws a value no read can flatten (`throw ''`, a null-prototype object, an object whose `message` getter throws) now ends its run at once with the sentence `the script threw a value that could not be read`, instead of waiting for the wall-clock ceiling (#23723) + +**Latency and answer change, for these values only.** The QuickJS runner's reject handler records a body's rejection as a flattened string, and the run ends only once that string is non-empty. For these values the read threw or gave `''`, so nothing was recorded: the run waited for the sandbox's wall-clock ceiling (30 s by default) and ended as a budget error with no business message, which the doors answer as a crash. The handler now records the fixed sentence above for any value it cannot read, and the run ends on the same path as any other rejection. The sentence names nothing the value carried and no stack. + +Measured at `POST /api/v1/actions/:object/:action` on a `type: 'script'` action: + +- `throw { code: 'EMAIL_TAKEN', get message() { throw new Error('boom') } }` + - FROM, after the 30 s ceiling: `500 {"success":false,"error":{"code":"INTERNAL_ERROR","message":"Internal server error","httpStatus":500}}` + - TO, in about 0.1 s: `400 {"success":false,"error":{"code":"VALIDATION_ERROR","message":"the script threw a value that could not be read","httpStatus":400,"declaredCode":"EMAIL_TAKEN"}}` +- `throw ''` and `throw Object.create(null)` + - FROM, after the 30 s ceiling: the same `500 INTERNAL_ERROR` + - TO, in about 0.1 s: `400 {"success":false,"error":{"code":"VALIDATION_ERROR","message":"the script threw a value that could not be read","httpStatus":400}}` + +The status and envelope are the ones the door already gives a body that threw; no door changed. A `code` the value carries is kept, as for any other thrown value. Two more values that could not be read move the same way: an `Error` whose `name` and `message` are both empty, and an `Error` whose `message` getter throws. To give the caller words, throw a readable `message` (or an `Error`). + +Control, unchanged byte for byte: every value the handler could already read (a thrown `Error` or subclass, a plain `{ code, message }` object and the other plain-object shapes, primitives, the engine's relayed duplicate refusal, a nested hook refusal's sentence, the `code` / `fields` / `status` / `userMessage` fields, and a capability denial). A body that never settles still ends at the wall-clock ceiling. + +In-process: the `SandboxError` a caller catches for such a throw now reads `KIND 'NAME' threw: Error: the script threw a value that could not be read`, with that sentence as `innerMessage`, instead of the wall-clock budget error. + +Clause-②: no diff --git a/packages/runtime/src/sandbox-unreadable-rejection-door.test.ts b/packages/runtime/src/sandbox-unreadable-rejection-door.test.ts new file mode 100644 index 00000000000..5872a0155b2 --- /dev/null +++ b/packages/runtime/src/sandbox-unreadable-rejection-door.test.ts @@ -0,0 +1,158 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#23723] At `POST /api/v1/actions/:object/:action`, a `type: 'script'` + * action whose body throws a value no read can flatten answers at once, with + * the status and envelope the door already gives a body that threw, and the + * fixed sentence as the message. No door changed. + * + * ## What was wrong + * + * The wrapper's reject handler (`sandbox/quickjs-runner.ts`) left `__error` + * unset or empty for `throw ''`, a null-prototype object, and an object whose + * `message` getter throws. The run waited for the wall-clock ceiling (30 s by + * default) and ended as a budget `SandboxError` with no `innerMessage`, which + * the door answers `500 INTERNAL_ERROR`, naming no refusal. + * + * ## The rig + * + * PR #23722's: better-sqlite3 `:memory:` under the real `ObjectQL`, action + * bodies bound through `actionBodyRunnerFactory` over a real + * `QuickJSScriptRunner`, and the dispatcher reached through `dispatch()` over + * a real Fetch `Request`, the identity step the one stubbed seam. The runner + * has a 1000 ms wall ceiling (CPU budget the same), so an answer other than + * the ceiling's `500` is an answer within a second by construction. + * + * The sentence is the ruled one, so it is asserted exactly, beside the code + * and status. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { HttpDispatcher } from './http-dispatcher.js'; +import { actionBodyRunnerFactory } 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 CEILING_MS = 1000; +const UNREADABLE = 'the script threw a value that could not be read'; +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: [] }, +}); + +const ACTIONS = [ + scriptAction('throw_getter', `throw { code: 'EMAIL_TAKEN', get message() { throw new Error('boom at host'); } };`), + scriptAction('throw_empty_string', `throw '';`), + scriptAction('throw_null_proto', `throw Object.create(null);`), + // CONTROL — a readable `{ code, message }` keeps PR #23722's answer. + scriptAction('throw_message', `throw { code: 'EMAIL_TAKEN', message: '${SENTENCE}' };`), + // CONTROL — a body that never settles still ends at the ceiling. + scriptAction('never_settles', `await new Promise(() => {});`), +]; + +const RDOC = { + name: 'rdoc', + label: 'Unreadable-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, +}; + +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(); + engine.registry.registerObject(RDOC as any, TEST_PACKAGE_ID); + await engine.syncSchemas(); + + const runner = new QuickJSScriptRunner({ actionTimeoutMs: CEILING_MS, wallCeilingMs: CEILING_MS }); + const bindBody = actionBodyRunnerFactory(runner, { ql: engine, appId: 'rdoc_app' }); + for (const action of ACTIONS) (engine as any).registerAction('rdoc', action.name, bindBody({ ...action, object: 'rdoc' })); + + 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/rdoc/` as `@objectstack/hono`'s catch-all hands it to `dispatch()`. */ +async function postAction(action: string) { + const path = `/actions/rdoc/${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 }; +} + +describe('[#23723] the action door answers an unreadable rejection at once, as a body that threw', () => { + it('{ code, get message() { throw } }: 400, the code kept on declaredCode, the fixed sentence', async () => { + const response = await postAction('throw_getter'); + expect(response.status, JSON.stringify(response.body)).toBe(400); + const error = response.body?.error; + expect(error?.code).toBe('VALIDATION_ERROR'); + expect(error?.declaredCode).toBe('EMAIL_TAKEN'); + expect(error?.message).toBe(UNREADABLE); + expect(JSON.stringify(response.body)).not.toContain('boom'); + }); + + it("throw '': 400, the fixed sentence", async () => { + const response = await postAction('throw_empty_string'); + expect(response.status, JSON.stringify(response.body)).toBe(400); + expect(response.body?.error?.code).toBe('VALIDATION_ERROR'); + expect(response.body?.error?.message).toBe(UNREADABLE); + }); + + it('throw Object.create(null): 400, the fixed sentence', async () => { + const response = await postAction('throw_null_proto'); + expect(response.status, JSON.stringify(response.body)).toBe(400); + expect(response.body?.error?.code).toBe('VALIDATION_ERROR'); + expect(response.body?.error?.message).toBe(UNREADABLE); + }); +}); + +describe('[#23723] CONTROL — the door\'s other answers stand', () => { + it("{ code, message } keeps PR #23722's answer", async () => { + const response = await postAction('throw_message'); + expect(response.status, JSON.stringify(response.body)).toBe(400); + const error = response.body?.error; + expect(error?.code).toBe('VALIDATION_ERROR'); + expect(error?.declaredCode).toBe('EMAIL_TAKEN'); + expect(error?.message).toBe(SENTENCE); + }); + + it('a body that never settles still ends at the ceiling: 500, naming no refusal', async () => { + const response = await postAction('never_settles'); + expect(response.status, JSON.stringify(response.body)).toBe(500); + expect(response.body?.error?.code).toBe('INTERNAL_ERROR'); + }); +}); diff --git a/packages/runtime/src/sandbox/quickjs-runner.ts b/packages/runtime/src/sandbox/quickjs-runner.ts index bfc8615afe0..9870e8bc5b0 100644 --- a/packages/runtime/src/sandbox/quickjs-runner.ts +++ b/packages/runtime/src/sandbox/quickjs-runner.ts @@ -65,6 +65,11 @@ const DEFAULT_MEMORY_MB = 32; // interrupt handler can't fire while no VM code runs — so a separate, generous // wall ceiling cuts it off. 30s matches the spec cap on `ScriptBody.timeoutMs`. const DEFAULT_WALL_CEILING_MS = 30_000; +// [#23723] What the wrapper's reject handler records for a rejection no read +// can flatten (see REJECT_HANDLER), so the run still ends at once on the +// "threw" path instead of at the wall ceiling. A fixed sentence: it names no +// host detail and no stack, because there is nothing readable to name. +const UNREADABLE_REJECTION_SENTENCE = 'the script threw a value that could not be read'; export interface QuickJSScriptRunnerOptions { /** Default per-invocation **CPU-time** budget for hooks (ms). */ @@ -325,6 +330,18 @@ export class QuickJSScriptRunner implements ScriptRunner { // business message. `code` and the rest still ride `__errorInfo`. An // `Error`, a primitive, and a value whose reads throw keep the // flattening below unchanged. + // + // [#23723] …and every rejection is RECORDED, even one no read can + // flatten. The pump loop ends the run only when `__error` is a + // non-empty string; a `message` getter that throws, a null-prototype + // object whose `String()` throws, or a read that yields `''` + // (`throw ''`) used to leave it unset or empty, so the run waited out + // the wall-clock ceiling and every door answered a `500` naming no + // refusal. Such a value is recorded as the fixed + // {@link UNREADABLE_REJECTION_SENTENCE} instead, through the same + // "threw" path as any other rejection. It names no host detail and no + // stack. Every value a read can flatten keeps its flattening byte for + // byte: the fallback expression is unchanged, only guarded. const REJECT_HANDLER = `function(e){ var flat; @@ -338,7 +355,10 @@ export class QuickJSScriptRunner implements ScriptRunner { } } } catch (_) { flat = undefined; } - globalThis.__error = flat !== undefined ? flat : ((e && e.message) ? (e.name + ': ' + e.message) : String(e)); + try { + if (flat === undefined) flat = (e && e.message) ? (e.name + ': ' + e.message) : String(e); + } catch (_) { flat = undefined; } + globalThis.__error = (typeof flat === 'string' && flat !== '') ? flat : ${JSON.stringify(`Error: ${UNREADABLE_REJECTION_SENTENCE}`)}; 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 }) diff --git a/packages/runtime/src/sandbox/unreadable-rejection-recorded.test.ts b/packages/runtime/src/sandbox/unreadable-rejection-recorded.test.ts new file mode 100644 index 00000000000..f72d58cba3e --- /dev/null +++ b/packages/runtime/src/sandbox/unreadable-rejection-recorded.test.ts @@ -0,0 +1,119 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#23723] A sandboxed body whose rejection no read can flatten still ends its + * run at once, on the "threw" path, with a fixed sentence. + * + * ## What was wrong + * + * The wrapper's reject handler records a rejection into `__error`, and the + * pump loop ends the run only when that is a non-empty string. Three values + * defeated it: `throw ''` (the fallback `String(e)` is `''`), a + * null-prototype object (`String()` throws), and an object whose `message` + * getter throws. `__error` stayed unset or empty, so the run waited for the + * wall-clock ceiling and ended as a budget `SandboxError` with no + * `innerMessage`: 30 s at the action door, then `500` naming no refusal. + * + * ## What holds now + * + * The handler records the fixed sentence for such a value, so the run ends + * like any other rejection: ` '' threw: Error: `, with + * the sentence as `innerMessage`. The sentence is the ruled one, so it is + * asserted exactly. It names nothing the value carried (the getter's own + * error included). A `code` the value carries still rides `__errorInfo`. + * + * The runner here has a 1000 ms wall ceiling (CPU budget the same), so a + * named refusal from it is an answer within a second by construction; a + * regression answers the ceiling's budget error instead, at that second. + * The door rows live in `../sandbox-unreadable-rejection-door.test.ts`. + */ + +import { describe, it, expect } from 'vitest'; + +import { QuickJSScriptRunner, SandboxError } from './quickjs-runner.js'; +import type { ScriptContext, ScriptRunOptions } from './script-runner.js'; + +const CEILING_MS = 1000; +const runner = new QuickJSScriptRunner({ + actionTimeoutMs: CEILING_MS, + hookTimeoutMs: CEILING_MS, + wallCeilingMs: CEILING_MS, +}); +const actionOpts: ScriptRunOptions = { origin: { kind: 'action', name: 'submit' } }; +const hookOpts: ScriptRunOptions = { origin: { kind: 'hook', name: 'guard' } }; + +const UNREADABLE = 'the script threw a value that could not be read'; + +const GETTER_THROWS = `throw { code: 'EMAIL_TAKEN', get message() { throw new Error('boom at host'); } };`; + +/** 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; +} + +/** The run ended on the "threw" path with the fixed sentence, not at the ceiling. */ +function expectRecorded(err: SandboxError, frame = "action 'submit'") { + expect(err.message).toBe(`${frame} threw: Error: ${UNREADABLE}`); + expect(err.innerMessage).toBe(UNREADABLE); +} + +describe('[#23723] a rejection no read can flatten is recorded, so the run ends at once', () => { + it("throw '': the read yields an empty string", async () => { + const err = await thrownBy(`throw '';`); + expectRecorded(err); + expect(err.code).toBeUndefined(); + }); + + it('throw Object.create(null): String() throws', async () => { + const err = await thrownBy(`throw Object.create(null);`); + expectRecorded(err); + expect(err.code).toBeUndefined(); + }); + + it('{ code, get message() { throw } }: the code is kept, and the getter\'s own error names nothing', async () => { + const err = await thrownBy(GETTER_THROWS); + expectRecorded(err); + expect(err.code).toBe('EMAIL_TAKEN'); + expect(err.message).not.toContain('boom'); + }); + + it('…on the hook origin too (both wrappers share the handler)', async () => { + const err = await thrownBy(GETTER_THROWS, hookOpts); + expectRecorded(err, "hook 'guard'"); + expect(err.code).toBe('EMAIL_TAKEN'); + }); + + it('an Error whose name and message are both empty (its String() is empty)', async () => { + expectRecorded(await thrownBy(`const e = new Error(); e.name = ''; throw e;`)); + }); + + it('an Error whose message getter throws', async () => { + const err = await thrownBy( + `const e = new Error('m'); Object.defineProperty(e, 'message', { get() { throw new Error('boom at host'); } }); throw e;`, + ); + expectRecorded(err); + expect(err.message).not.toContain('boom'); + }); +}); + +describe('[#23723] CONTROL — what was readable, and what never settles', () => { + it("{ code, message } keeps PR #23722's answer: its own message, and the code", async () => { + const err = await thrownBy(`throw { code: 'EMAIL_TAKEN', message: 'That address is already on file' };`); + expect(err.message).toBe("action 'submit' threw: Error: That address is already on file"); + expect(err.innerMessage).toBe('That address is already on file'); + expect(err.code).toBe('EMAIL_TAKEN'); + }); + + it('a body that never settles still ends at the wall-clock ceiling, with no innerMessage', async () => { + const err = await thrownBy(`await new Promise(() => {});`); + expect(err.message).toMatch(new RegExp(`exceeded wall-clock ceiling of ${CEILING_MS}ms`)); + expect(err.innerMessage).toBeUndefined(); + }); +});