diff --git a/.changeset/22741-sandbox-fault-import-row.md b/.changeset/22741-sandbox-fault-import-row.md new file mode 100644 index 00000000000..8dd68a4dae7 --- /dev/null +++ b/.changeset/22741-sandbox-fault-import-row.md @@ -0,0 +1,13 @@ +--- +'@objectstack/core': patch +'@objectstack/types': minor +--- + +An import row for the sandbox's own fault reads the create door's fault sentence, not the runner's diagnostic text (#22741) + +Clause-②: yes (widening) + +The QuickJS runner raises some `SandboxError`s itself: a hook or action body that runs past its CPU budget or past the wall-clock ceiling, calls a capability it was not granted, or meets a failed install or marshal. These carry no `innerMessage`. `POST /api/v1/data/:object` and `/createMany` answer them `500 INTERNAL_ERROR` `Internal server error`. The sync `/import` row and the `/import/jobs` stored row answered the runner's text instead, for example `hook 'guard' exceeded CPU budget of 50ms (after 3 pump iterations)`. + +- **`@objectstack/types`** exports `isSandboxOwnFault(error)`. It is `true` for an error whose `name` is `'SandboxError'` and whose `innerMessage` is not a non-empty string, which is how the runner marks the faults it raises itself. A body's refusal or crash carries `innerMessage` and answers `false`. +- **`@objectstack/core`**'s import row applies the rule a crashed body already gets: when the sandbox raised the error and the create door answers it with a 5xx, the row's `error` is the door's sentence. The row's `code` is unchanged (`IMPORT_ROW_FAILED`). A driver error, and an error a code-defined hook threw, keep the row text they had. diff --git a/packages/core/src/utils/import-runner-sandbox-fault-row.test.ts b/packages/core/src/utils/import-runner-sandbox-fault-row.test.ts new file mode 100644 index 00000000000..1ba1573f846 --- /dev/null +++ b/packages/core/src/utils/import-runner-sandbox-fault-row.test.ts @@ -0,0 +1,357 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#22741] An import row for the sandbox's OWN fault reads the create door's + * fault sentence, never the runner's diagnostic text. This is the enumeration + * pin that closes the import-row versus data-door fault-text family (#22694 the + * refusal, #22718 the crash and the declared 5xx, this card the rest). + * + * ## What was measured broken + * + * The REAL `QuickJSScriptRunner`, at `eed637c09f`, fed to the real + * `mapDataError` and `runImport`: a hook body that burns its CPU budget, one + * stuck past the wall-clock ceiling, one denied `api.read`, one denied `log` + * and one whose `ctx.api` method is missing each threw a `SandboxError` with no + * `innerMessage`. The create door answered every one `500 INTERNAL_ERROR` + * `Internal server error`; the row answered the runner's text, e.g. + * `hook 'mz_lock_insert' exceeded CPU budget of 50ms (after 0 pump iterations)`. + * `toFailedResult` takes the door's sentence only for a sandbox ORIGIN (a + * non-empty `innerMessage`), and these errors have none. + * + * ## Why an enumeration, and what it iterates + * + * The population is read from the producer, not listed here. + * `packages/runtime/src/sandbox/quickjs-runner.ts` is parsed with the + * TypeScript compiler, and every `new SandboxError(…)` that passes no + * `innerMessage` is a site, as is every `throwSandboxFault(vm, …)` call, whose + * `SandboxError` the pump loop rebuilds with no `innerMessage` once the fault + * has crossed the VM. Each site's message is rendered from its own template + * literal, with a fixed sample per interpolation. So: + * + * - a new fault site is in the population the day it lands, and turns this + * file red if its row does not read the door's sentence; + * - a site this file cannot render (an interpolation with no sample, a + * message that is not a literal and not a declared relay) is red too, and + * the failure names it. + * + * Comments are not code: the two `throw new SandboxError(` examples inside the + * runner's doc comments are not sites, because the parser never sees them. + * + * The class is REPRODUCED, because `@objectstack/core` sits below + * `@objectstack/runtime`, but its `name` is READ from the producer's + * constructor (`this.name = '…'`). A rename there reaches every case below. + * + * §2 runs the relay half: a construction that DOES set `innerMessage`, after + * its body crashed one VM hop down, arrives at the outer pump as a fault with + * that construction's `.message` and no `innerMessage`. §3 holds the controls, + * which stay unchanged by this card: driver text, and errors the sandbox did + * not raise. `import-runner-sandbox-refusal-row.test.ts` §4 is the driver-text + * control #22718's ablation B pinned, and it is not edited here. + * + * The sample object name in a rendered message is deliberately NOT the + * import's object. `mapDataError`'s unknown-object heuristic answers `404 + * OBJECT_NOT_FOUND` for a text that names the requested object and contains + * `not`, which a capability denial on that object does. That is a door + * defect outside this card; the row does not copy a 4xx sentence. + */ + +import { readFileSync } from 'node:fs'; +import { dirname, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import ts from 'typescript'; +import { describe, it, expect, vi } from 'vitest'; +import { mapDataError, INTERNAL_ERROR_MESSAGE, isSandboxOwnFault } from '@objectstack/types'; +import { runImport, sanitizeRowError, type ImportProtocolLike } from './import-runner'; +import type { ExportFieldMeta } from './import-field-meta.js'; + +const HERE = dirname(fileURLToPath(import.meta.url)); +const RUNNER = resolve(HERE, '../../../runtime/src/sandbox/quickjs-runner.ts'); +const SOURCE = ts.createSourceFile(RUNNER, readFileSync(RUNNER, 'utf8'), ts.ScriptTarget.Latest, true); + +/** + * One sample per interpolation the runner's fault messages use. A message + * naming an expression not listed here is unrenderable, and §1's census says + * which. `objectName` is not the import's object (see the header). + */ +const SAMPLES: Readonly> = { + 'args.origin.kind': 'hook', + 'args.origin.name': 'mz_lock_insert', + 'origin.kind': 'hook', + 'origin.name': 'mz_lock_insert', + cpuBudget: '50', + wallCeiling: '30000', + pumps: '3', + 'formatErr(msg)': 'SyntaxError: unexpected token', + objectName: 'mz_other', + method: 'find', + required: 'api.read', + fieldName: 'owner', +}; + +/** §2: the error a nested body threw, so the construction it lands in is a CRASH. */ +const CRASH_SAMPLES: Readonly> = { + ...SAMPLES, + 'formatErr(err)': 'TypeError: boom', + errStr: 'TypeError: boom', +}; + +/** + * The single-argument constructions whose message is not a literal: each one + * re-raises a fault that other sites produce. A relay missing from the + * runner, or a new non-literal one, fails §1's census. + */ +const RELAYS: Readonly> = { + message: '`throwSandboxFault` builds the error for each of its call sites, which are sites here in their own right', + 'withoutSandboxErrorPrefix(String(errStr))': + 'the pump loop rebuilds a fault marked across the VM hop: the `throwSandboxFault` messages, the host-side faults thrown inside a host call (sites here), and a nested body\'s crash (§2)', +}; + +/** Each named fault kind, with the number of sites measured at `eed637c09f` as its floor. */ +const KINDS: ReadonlyArray = [ + ['CPU budget', /exceeded CPU budget/, 1], + ['wall-clock ceiling', /exceeded wall-clock ceiling/, 1], + ['capability denial', /^capability '/, 5], + ['ctx.api unavailable', /^ctx\.api unavailable/, 1], + ['install failure', /^failed to install/, 4], + ['marshal failure', /^failed to marshal/, 1], + ['nested transaction refusal', /^nested ctx\.api\.transaction/, 1], + ['missing ctx.api method', /not implemented$/, 1], +]; + +interface Site { + label: string; + message: string; +} + +function lineOf(node: ts.Node): number { + return SOURCE.getLineAndCharacterOfPosition(node.getStart()).line + 1; +} + +function render(node: ts.Expression, samples: Readonly>): string { + if (ts.isStringLiteral(node) || ts.isNoSubstitutionTemplateLiteral(node)) return node.text; + if (ts.isParenthesizedExpression(node)) return render(node.expression, samples); + if (ts.isTemplateExpression(node)) { + let out = node.head.text; + for (const span of node.templateSpans) { + const expr = span.expression.getText(); + if (!(expr in samples)) throw new Error(`no sample for the interpolation \`${expr}\``); + out += samples[expr] + span.literal.text; + } + return out; + } + if (ts.isBinaryExpression(node) && node.operatorToken.kind === ts.SyntaxKind.PlusToken) { + return render(node.left, samples) + render(node.right, samples); + } + throw new Error(`a message that is neither a literal nor a declared relay: \`${node.getText()}\``); +} + +/** The `name` the producer's constructor assigns, read from the source. */ +function producerName(): string | undefined { + let found: string | undefined; + const visit = (node: ts.Node): void => { + if (ts.isClassDeclaration(node) && node.name?.text === 'SandboxError') { + for (const member of node.members) { + if (!ts.isConstructorDeclaration(member) || member.body === undefined) continue; + for (const st of member.body.statements) { + if (!ts.isExpressionStatement(st) || !ts.isBinaryExpression(st.expression)) continue; + const { left, right, operatorToken } = st.expression; + if ( + operatorToken.kind === ts.SyntaxKind.EqualsToken && + ts.isPropertyAccessExpression(left) && + left.expression.kind === ts.SyntaxKind.ThisKeyword && + left.name.text === 'name' && + ts.isStringLiteral(right) + ) { + found = right.text; + } + } + } + } + ts.forEachChild(node, visit); + }; + visit(SOURCE); + return found; +} + +const census = (() => { + const faults: Site[] = []; + const crashes: Site[] = []; + const relays: string[] = []; + const unrenderable: string[] = []; + const visit = (node: ts.Node): void => { + if (ts.isNewExpression(node) && ts.isIdentifier(node.expression) && node.expression.text === 'SandboxError') { + const args = node.arguments ?? []; + const at = `quickjs-runner.ts:${lineOf(node)}`; + if (args.length === 1) { + const text = args[0].getText(); + if (text in RELAYS) { + relays.push(text); + } else { + try { + faults.push({ label: `${at} new SandboxError(…)`, message: render(args[0], SAMPLES) }); + } catch (e) { + unrenderable.push(`${at}: ${(e as Error).message}`); + } + } + } else if (args.length >= 2) { + try { + crashes.push({ label: `${at} new SandboxError(…, innerMessage), relayed`, message: render(args[0], CRASH_SAMPLES) }); + } catch (e) { + unrenderable.push(`${at} (relayed): ${(e as Error).message}`); + } + } else { + unrenderable.push(`${at}: a construction with no message`); + } + } + if (ts.isCallExpression(node) && ts.isIdentifier(node.expression) && node.expression.text === 'throwSandboxFault') { + const at = `quickjs-runner.ts:${lineOf(node)}`; + try { + faults.push({ label: `${at} throwSandboxFault(…)`, message: render(node.arguments[1], SAMPLES) }); + } catch (e) { + unrenderable.push(`${at}: ${(e as Error).message}`); + } + } + ts.forEachChild(node, visit); + }; + visit(SOURCE); + return { faults, crashes, relays, unrenderable }; +})(); + +const PRODUCER_NAME = producerName(); + +/** The producer's own fault: its declared `name`, and no `innerMessage`. */ +function ownFault(message: string): Error { + const err = new Error(message); + err.name = PRODUCER_NAME ?? 'unread'; + return err; +} + +type CreateArgs = Parameters[0]; + +const OBJECT = 'mz_locked'; + +const baseOpts = { + objectName: OBJECT, + metaMap: new Map([['name', { name: 'name', type: 'text' }]]), + writeMode: 'insert' as const, + matchFields: [] as string[], + dryRun: false, + runAutomations: false, + trimWhitespace: true, + createMissingOptions: false, + skipBlankMatchKey: false, +}; + +/** Inline path: no bulk primitive, so each row is one `createData`. */ +function inlineProtocol(fail: () => Error): ImportProtocolLike { + return { + findData: vi.fn(async () => []), + createData: vi.fn(async (args: CreateArgs) => { + if (args.data.name === 'r1') throw fail(); + return { id: `id_${String(args.data.name)}` }; + }), + updateData: vi.fn(), + }; +} + +async function rowFor(fail: () => Error) { + const summary = await runImport({ ...baseOpts, p: inlineProtocol(fail), rows: [{ name: 'r1' }] }); + expect(summary.errors).toBe(1); + return summary.results[0]; +} + +/** The door's verdict FIRST: a fault, its text withheld. Anti-vacuity for every row assertion below. */ +function expectDoorWithholds(err: Error, message: string) { + const door = mapDataError(err, OBJECT); + expect(door.status).toBe(500); + expect(door.body).toEqual({ error: INTERNAL_ERROR_MESSAGE, code: 'INTERNAL_ERROR' }); + expect(String(door.body.error)).not.toContain(message); + return door; +} + +describe('[#22741] §1 — every fault the runner raises itself reads the door\'s sentence on the row', () => { + it('census: the producer is read, every construction is classified, and each named kind is present', () => { + expect(PRODUCER_NAME, 'the `this.name = …` assignment in SandboxError\'s constructor').toBeTypeOf('string'); + expect(census.unrenderable, 'sites this pin cannot render; add a sample or a relay').toEqual([]); + expect([...new Set(census.relays)].sort()).toEqual(Object.keys(RELAYS).sort()); + for (const [kind, pattern, floor] of KINDS) { + const n = census.faults.filter((s) => pattern.test(s.message)).length; + expect(n, `${kind}: sites found`).toBeGreaterThanOrEqual(floor); + } + // §2's population: the constructions that set `innerMessage` (crash and refusal sites). + expect(census.crashes.length).toBeGreaterThanOrEqual(4); + }); + + it.each(census.faults.map((s) => [s.label, s.message] as const))('%s', async (_label, message) => { + const err = ownFault(message); + expect(isSandboxOwnFault(err)).toBe(true); + const door = expectDoorWithholds(err, message); + + const row = await rowFor(() => ownFault(message)); + expect(row).toEqual({ row: 1, ok: false, action: 'failed', error: door.body.error, code: 'IMPORT_ROW_FAILED' }); + }); + + it('the batched write paths reach the same answer (the budget fault)', async () => { + const budget = census.faults.find((s) => /exceeded CPU budget/.test(s.message))!.message; + const insertManyData = vi.fn(async (args: { records: Array> }) => ({ + outcomes: args.records.map((r) => (r.name === 'r1' + ? { ok: false, error: ownFault(budget) } + : { ok: true, record: { id: `id_${String(r.name)}` } })), + })); + const p = { ...inlineProtocol(() => ownFault(budget)), createManyData: vi.fn(), insertManyData } as unknown as ImportProtocolLike; + const summary = await runImport({ ...baseOpts, p, rows: [{ name: 'r0' }, { name: 'r1' }, { name: 'r2' }] }); + + expect(insertManyData).toHaveBeenCalledTimes(1); + expect(summary.created).toBe(2); + expect(summary.results[1]).toEqual({ row: 2, ok: false, action: 'failed', error: INTERNAL_ERROR_MESSAGE, code: 'IMPORT_ROW_FAILED' }); + expect(JSON.stringify(summary)).not.toMatch(/exceeded CPU budget|mz_lock_insert/); + }); +}); + +describe('[#22741] §2 — a nested body\'s crash, relayed one VM hop up as the sandbox\'s fault', () => { + it.each(census.crashes.map((s) => [s.label, s.message] as const))('%s', async (_label, message) => { + const err = ownFault(message); + expect(isSandboxOwnFault(err)).toBe(true); + const door = expectDoorWithholds(err, message); + + const row = await rowFor(() => ownFault(message)); + expect(row).toEqual({ row: 1, ok: false, action: 'failed', error: door.body.error, code: 'IMPORT_ROW_FAILED' }); + expect(JSON.stringify(row)).not.toMatch(/TypeError|boom|threw:/); + }); +}); + +describe('[#22741] §3 — controls: what the predicate must NOT change', () => { + const budget = () => census.faults.find((s) => /exceeded CPU budget/.test(s.message))!.message; + + it('CONTROL — the same text on an error the sandbox did not raise keeps the row\'s own reading', async () => { + // The predicate reads the producer's class identity, never the message. + const err = () => new Error(budget()); + expect(isSandboxOwnFault(err())).toBe(false); + expect(mapDataError(err(), OBJECT).status).toBe(500); + expect((await rowFor(err)).error).toBe(sanitizeRowError(budget())); + }); + + it('CONTROL — a code-defined hook\'s native TypeError keeps its text, as the filing says', async () => { + const text = "Cannot read properties of undefined (reading 'owner')"; + const err = () => new TypeError(text); + expect(isSandboxOwnFault(err())).toBe(false); + expect((await rowFor(err)).error).toBe(text); + }); + + it('CONTROL — driver text keeps `sanitizeRowError`\'s reading although its door answers 5xx', async () => { + const sql = "insert into `mz_locked` (`name`) values ('r1') - SQLITE_BUSY: database is locked"; + expect(isSandboxOwnFault(new Error(sql))).toBe(false); + expect(mapDataError(new Error(sql), OBJECT).status).toBeGreaterThanOrEqual(500); + expect((await rowFor(() => new Error(sql))).error).toBe('SQLITE_BUSY: database is locked'); + }); + + it('CONTROL — a sandboxed body\'s refusal or crash is the origin rule\'s, not this predicate\'s', () => { + const refusal = Object.assign(ownFault("hook 'g' threw: Error: Locked."), { innerMessage: 'Locked.' }); + const crash = Object.assign(ownFault("hook 'g' threw: TypeError: boom"), { innerMessage: 'TypeError: boom' }); + expect(isSandboxOwnFault(refusal)).toBe(false); + expect(isSandboxOwnFault(crash)).toBe(false); + for (const value of [undefined, null, 'SandboxError', { name: 'SandboxErrorX' }]) { + expect(isSandboxOwnFault(value), String(value)).toBe(false); + } + }); +}); diff --git a/packages/core/src/utils/import-runner.ts b/packages/core/src/utils/import-runner.ts index e8b4eb660e0..41a09d4a4bc 100644 --- a/packages/core/src/utils/import-runner.ts +++ b/packages/core/src/utils/import-runner.ts @@ -8,7 +8,7 @@ import { canServeApiOperation, type DroppedFieldsEvent, type EnableLike } from ' import type { ExecutionContext } from '@objectstack/spec/kernel'; import type { CreateDataRequest, FindDataRequest, GetMetaItemRequest, UpdateDataRequest, ValidateDataIssue, ValidateDataRequest, ValidateDataResponse } from '@objectstack/spec/api'; import { bulkWrite, withTransientRetry, defaultIsTransientError, type BulkWriteRowResult } from './bulk-write.js'; -import { isUniqueViolationError, uniqueViolationColumn, isEngineDuplicateRecordEnvelope, mapDataError, sandboxBusinessMessage, isSandboxOrigin } from '@objectstack/types'; +import { isUniqueViolationError, uniqueViolationColumn, isEngineDuplicateRecordEnvelope, mapDataError, sandboxBusinessMessage, isSandboxOrigin, isSandboxOwnFault } from '@objectstack/types'; /** * import-runner — the shared row-processing core for bulk import. @@ -485,6 +485,18 @@ export function sanitizeRowError(raw: unknown): string { * `code` is unchanged, so it still reads as a failed row with a fault code. * Every other error keeps its row text: driver text is still cleaned by * {@link sanitizeRowError}, which is the import's own reading of it. + * + * ## The sandbox's own fault reads as that fault too (#22741) + * + * The runner also raises `SandboxError`s of its own: the CPU budget, the + * wall-clock ceiling, a denied capability, a failed install or marshal. They + * carry no `innerMessage`, so the rule above, keyed on a sandbox origin, did not + * see them, and the row relayed `.message` (`hook '' exceeded CPU budget + * of …`) where the create door answers `Internal server error`. The row + * recognises them by `isSandboxOwnFault`, read off the producer, and the same + * rule applies: a sandboxed error the door answers with a 5xx takes the door's + * sentence. ⛔ Still not "every door 5xx": a driver error is neither a sandbox + * origin nor a sandbox fault, and its row keeps {@link sanitizeRowError}'s text. */ function toFailedResult(rowNo: number, err: unknown, objectName: string): ImportRowResult { const e = err as { code?: unknown; message?: unknown; fields?: unknown; field?: unknown } | null | undefined; @@ -497,7 +509,8 @@ function toFailedResult(rowNo: number, err: unknown, objectName: string): Import const thrownCode = isEngineDuplicateRecordEnvelope(e) ? 'UNIQUE_VIOLATION' : e?.code; const code = first?.code ?? thrownCode ?? 'IMPORT_ROW_FAILED'; const field = first?.field != null && first.field !== '' ? first.field : e?.field; - const sandboxFault = isSandboxOrigin(err) && door.status >= 500 ? String(door.body.error) : undefined; + const sandboxed = isSandboxOrigin(err) || isSandboxOwnFault(err); + const sandboxFault = sandboxed && door.status >= 500 ? String(door.body.error) : undefined; const message = sandboxFault ?? sandboxBusinessMessage(err) ?? sanitizeRowError(e?.message); return { row: rowNo, ok: false, action: 'failed', error: message, code: String(code), diff --git a/packages/core/vitest.repo-tests.json b/packages/core/vitest.repo-tests.json index d0db050bdd1..02fe9b15e9b 100644 --- a/packages/core/vitest.repo-tests.json +++ b/packages/core/vitest.repo-tests.json @@ -3,5 +3,6 @@ "src/security/granted-permissions-not-enforced.pin.test.ts", "src/security/operation-private-keys.pin.test.ts", "src/security/row-serving-door-exposure.pin.test.ts", - "src/security/second-object-read-exposure.pin.test.ts" + "src/security/second-object-read-exposure.pin.test.ts", + "src/utils/import-runner-sandbox-fault-row.test.ts" ] diff --git a/packages/types/src/data-error-classification.ts b/packages/types/src/data-error-classification.ts index 61b0d967685..97f36d1190a 100644 --- a/packages/types/src/data-error-classification.ts +++ b/packages/types/src/data-error-classification.ts @@ -774,6 +774,38 @@ function isSandboxOrigin(error: any): boolean { return typeof error?.innerMessage === 'string' && error.innerMessage.length > 0; } +/** + * [#22741] Is this the sandbox's OWN fault: a `SandboxError` the runner raised + * itself, rather than one a sandboxed body threw? + * + * Both halves are read off the producer, `runtime/src/sandbox/quickjs-runner.ts`: + * + * - its class names itself `'SandboxError'` (`this.name`), and + * - its contract leaves `innerMessage` undefined for "the sandbox's own + * internal errors (capability denials, timeouts, marshalling failures)", + * so {@link isSandboxOrigin} answers `false` for them. + * + * So the CPU budget, the wall-clock ceiling, a denied capability, an + * unavailable `ctx.api`, a failed install or marshal, and a nested body's fault + * relayed across a VM hop all answer `true`. A body's refusal or crash carries + * `innerMessage` and answers `false`; it is {@link isSandboxOrigin}'s. + * + * The class is recognised by its declared name rather than by `instanceof`, + * for the reason {@link isEngineDuplicateRecordEnvelope} gives: the import + * runner's row report (`@objectstack/core`) sits below `@objectstack/runtime` + * and never sees the class. + * + * ⛔ Not a withholding rule. {@link mapDataError} asks nothing here: it answers + * these errors from their message like any other unclassified throw, which for + * the runner's fault texts is its sanitised `500 INTERNAL_ERROR` terminal. This + * predicate lets the import row recognise the error so it can take that door's + * sentence, as it does for a crash; the verdict stays the door's. + */ +export function isSandboxOwnFault(error: unknown): boolean { + const name = (error as { name?: unknown } | null | undefined)?.name; + return name === 'SandboxError' && !isSandboxOrigin(error); +} + /** * [commit cf6e0a193] Did a sandboxed body CRASH — as opposed to reporting a refusal? *