Skip to content

Commit d7b26df

Browse files
fix(core): an import row for a sandboxed body the door answers as a fault reads as that fault (#22740)
Fixes #22718 Clause-②: no ## What changes An import row for a sandboxed hook body that **crashed**, or for a sandboxed refusal that **declared a 5xx status**, now carries the same sentence `POST /data/:object` and `/createMany` answer for that error (`Internal server error`), instead of relaying the fault's text. The row still reads as a fault: `ok: false`, `action: 'failed'`, and its `code` is unchanged (`IMPORT_ROW_FAILED` for a crash, the declared code for a declared 5xx). A refusal with no status or a 4xx status keeps the hook's own sentence, as PR #22716 made it. One producer-side line in `toFailedResult` (`packages/core/src/utils/import-runner.ts`): ```ts const sandboxFault = isSandboxOrigin(err) && door.status >= 500 ? String(door.body.error) : undefined; const message = sandboxFault ?? sandboxBusinessMessage(err) ?? sanitizeRowError(e?.message); ``` `door` is the `mapDataError(err, objectName)` answer the function already computed for `adoptDoorVerdict`. No second withholding rule is added: - **Which errors are faults, and what their sentence is,** are both `mapDataError`'s verdict. For a sandbox-origin error it answers a 5xx in exactly two shapes: the `isSandboxCrash` terminal (`500 INTERNAL_ERROR`) and `declaredServerFaultAnswer` (the declared status, its code, the prose withheld). The row copies the door's `error` string, so a later change to what the door withholds reaches the row unchanged. - **The gate is the sandbox origin** (`isSandboxOrigin`, already exported from `@objectstack/types` for `rest`'s door). It is not the door's 5xx alone. A driver error also answers 5xx at the door, and its row text is the import's own reading through `sanitizeRowError`, which stays as it is. Ablation B below shows that dropping the gate changes that row. - **The row's code is not touched.** The card asks for "a failed row with a fault code". The crash row's `IMPORT_ROW_FAILED` is the code #22694's control already pinned as that reading, and the declared-5xx row's declared code is the code the doors keep too. ## Measured before the change (`origin/main` `a360cee92e`) These readings come from the real REST handlers and the real `runImport`, through the `rest-write-route-hook-refusal-sentence.ledger.test.ts` harness, with the `SandboxError` shape `quickjs-runner.ts` produces. | error | `POST /data/:object` | `/createMany` | sync `/import` row | `/import/jobs` stored row | |---|---|---|---|---| | crash (`innerMessage: 'TypeError: boom'`) | `500` `INTERNAL_ERROR` `Internal server error` | same | `IMPORT_ROW_FAILED` `hook 'mz_guard' threw: TypeError: boom` | same | | declared 5xx (`status: 503`, `code: 'SERVICE_UNAVAILABLE'`) | `503` `SERVICE_UNAVAILABLE` `Internal server error` | same | `SERVICE_UNAVAILABLE` `The ledger service is down.` | same | After the change, both import rows carry the doors' sentence in both rows of the table. PM mechanism assumption 1 (the `sandboxBusinessMessage(err) ?? sanitizeRowError(e?.message)` read) held. Assumptions 2 and 3 were re-measured in the table above. ## Pins - `packages/core/src/utils/import-runner-sandbox-refusal-row.test.ts`: - **§5 (new):** a crash row and a declared-5xx row on the inline `createData` path each equal `{ ok: false, action: 'failed', error: DOOR_SENTENCE, code }`. The door's own fault verdict is asserted first as anti-vacuity, and the native text and the prose are asserted absent. - **§4:** the crash control that pinned the old row text is removed; §5 now covers the crash. The driver-text control gains one line asserting the door answers that error with a 5xx too, so the control witnesses the sandbox gate. - `packages/rest/src/rest-write-route-hook-refusal-sentence.ledger.test.ts` §4 (rewritten): for the crash and the declared 5xx, the test drives `POST /data/:object` and `/createMany` and asserts that both answer a 5xx, that they agree, and that they withhold the text. The sync `/import` row and the `/import/jobs` stored row must then equal that answer. The refusal control is that file's §2, which drives both import doors and requires the hook's sentence. **File surface:** the rest file is `domain:cli` and outside the claim's declared surface. Only the test file changed there. Its §4 pinned the very crash row text this card changes (`row === sanitizeRowError(crash.message)`), so leaving it unchanged would have turned it red. The existing harness drives both import doors with the real handlers, which is the door-level pin triage asked for. No dogfood or verify file is touched. ## Evidence - **Red, then green, at the door level.** `@objectstack/rest` resolves `@objectstack/core` through `dist/` (no alias). With core's `dist/` still at BASE (the marker `door.status >= 500` counted 0 in `dist/index.js` and 1 in `src`), the rest ledger file ran `2 failed | 45 passed`. Both §4 cases failed at the sync `/import` door: `expected "hook 'mz_guard' threw: TypeError: boom" to be "Internal server error"` and `expected 'The ledger service is down.' to be 'Internal server error'`. After rebuilding core (`ablation-dist-preflight @objectstack/core 'door.status >= 500'`: marker present in `dist/index.js` and `dist/index.cjs`, tree clean) it ran `47 passed`. - **Ablation A** (remove `sandboxFault ??`, via `scripts/ablation-replace.mjs`, anchor 1 to 0, blob `8e511983ac2a` to `b05265a89971`): the core file ran `2 failed | 13 passed`, both §5 cases. The restore was proven: blob equal to HEAD and `git diff HEAD` empty. - **Ablation B** (drop the `isSandboxOrigin(err) &&` gate): the core file ran `1 failed | 14 passed`. The driver-text control received `Internal data error` where it expected `SQLITE_BUSY: database is locked`. The restore was proven the same way. The first attempt at B was refused by the tool before anything ran: the replacement was a substring of the anchor, so its count could not rise. I re-ran it with a distinct replacement, and only the second run counts. - **Tests at the merge commit `824d16b012`** (main merged once before opening; it touched `lint` and a spec migration string, and I rebuilt spec, lint and core after the merge): - `pnpm --filter @objectstack/core test`: 89 files, 2308 passed. - `pnpm --filter @objectstack/core typecheck`: exit 0. Its test layer passes through `check:test-typecheck`, with the 4 pinned debt signatures held. - `pnpm --filter @objectstack/rest typecheck`: exit 0, with 0 test-layer debt. - The rest ledger file: 47 passed. - Declared to CI: core's `repo` project (4 security pins, which do not touch the runner) and rest's full suite. The public face is unchanged (no export or type moved), so only the row value moved. ## Gates (at `824d16b012`) `node scripts/pm/dispatch-gates.mjs --commands` derived **65 commands** from this branch's change set (merge base `0f77ff520`): the dispatch's 51, plus these 14: - `check-adr-0087-registration` (base and self-test) - `check-empty-changeset` (base and self-test) - the `release-rehearsal-clone` and `release-pending-publish` self-tests - `check:engine-double-contract`, `check:objectql-double-limit`, `check:objectui-changeset`, `check:pm-changeset-deadline-census`, `check:query-options-erasure`, `check:type-check-coverage`, `check:type-check-debt`, `check:where-matcher` I also ran 3 roster gates whose rosters sit under one of these paths: `check-changeset-fixed`, `check:authz-resolver` and `check:filter-alias-parity`. Of the 68, **67 exited 0**. `pnpm check:dual-build-cjs-loads` exited 3 with `PREREQUISITE NOT MET` (it needs every package's `dist/`), so it is **NOT MEASURED**. As a narrowed reading, I loaded core's rebuilt `dist/index.cjs` directly (`runImport` and `sanitizeRowError` are functions); core is the only package whose `dist/` this diff changes. The `--ran` reconciliation, with exit codes recorded, reads `65 derived, 64 run, 1 NOT-MEASURED, 0 UNRUN`. The repo-wide `pnpm lint` is CI's and was not run here. ## Acceptance notes (noted, not filed here) - A crash whose body attached a `code` to the thrown native error keeps that code on the row, while the doors answer `INTERNAL_ERROR` for it (commit cf6e0a1's ruling at the doors). The row's code rule is unchanged by this card. Carrier: none. - Two neighbours of this card show the same row-versus-door gap. They are out of this card's scope and reported to the seat, not fixed here: - A `SandboxError` with no `innerMessage` (the sandbox's own CPU-budget or capability fault), where the door answers `500 Internal server error` and the row relays `.message`. - A non-sandbox native error (for example a code-defined hook's `TypeError`), where the row relays `x.trim is not a function`. Both were read through `mapDataError` and `sanitizeRowError` only, never at a public door. Rows relay unclassified non-sandbox text on purpose today, so the second one is a design question, not a defect reading. --- _Generated by [Claude Code](https://claude.ai/code/session_01JfJfBUC3cQ6hhgm9MQK76T)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 3b5475a commit d7b26df

4 files changed

Lines changed: 95 additions & 33 deletions

File tree

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
'@objectstack/core': patch
3+
---
4+
5+
An import row for a sandboxed hook body that crashed, or that refused with a declared 5xx status, reads as a server fault instead of relaying the fault's text
6+
7+
Clause-②: no
8+
9+
When a hook body crashed during `POST /api/v1/data/:object/import` (or the async `/import/jobs` job), the row's `error` read `hook 'NAME' threw: TypeError: …`, and a hook refusal that declared a 5xx status relayed its own message. `POST /api/v1/data/:object` and `/createMany` answer both as server faults and withhold that text. The row now carries the same sentence those routes answer (`Internal server error`), and its `code` is unchanged: `IMPORT_ROW_FAILED` for a crash, the declared code for a declared 5xx. A hook refusal with no status or a 4xx status still reads in the hook's own words, and a database error's row text is unchanged.

‎packages/core/src/utils/import-runner-sandbox-refusal-row.test.ts‎

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,13 @@
2929
* §3 pins door-to-row agreement directly: the row's sentence IS what
3030
* `mapDataError` (the create door) answers for the same error, including for a
3131
* sentence `sanitizeRowError` would have paraphrased. §4 holds the controls.
32+
* §5 (#22718) holds the two sandboxed failures the door answers as a fault, a
33+
* crashed body and a refusal that declared a 5xx: their row reads the door's
34+
* sentence.
3235
*/
3336

3437
import { describe, it, expect, vi } from 'vitest';
35-
import { mapDataError } from '@objectstack/types';
38+
import { mapDataError, INTERNAL_ERROR_MESSAGE } from '@objectstack/types';
3639
import { runImport, sanitizeRowError, type ImportProtocolLike } from './import-runner';
3740
import type { ExportFieldMeta } from './import-field-meta.js';
3841

@@ -176,17 +179,6 @@ describe('[#22694] §3 — the row answers what the create door answers', () =>
176179
});
177180

178181
describe('[#22694] §4 — controls: what the read must NOT change', () => {
179-
it('CONTROL — a hook body that CRASHED is not a refusal: its row is built from `.message` exactly as before', async () => {
180-
const crash = sandboxCrash();
181-
const row = await rowFor(sandboxCrash);
182-
183-
// `sandboxBusinessMessage` declines a crash, so the native error text is
184-
// never presented as the author's sentence.
185-
expect(row.error).not.toBe('TypeError: boom');
186-
expect(row.error).toBe(sanitizeRowError(crash.message));
187-
expect(row.code).toBe('IMPORT_ROW_FAILED');
188-
});
189-
190182
it('CONTROL — a plain error whose own text looks like the wrapper is relayed as it always was', async () => {
191183
// No `innerMessage`: nothing about this error came from the sandbox, so a
192184
// pattern-strip would be the only thing that could change it.
@@ -206,5 +198,28 @@ describe('[#22694] §4 — controls: what the read must NOT change', () => {
206198
const sql = "insert into `mz_locked` (`name`) values ('r1') - SQLITE_BUSY: database is locked";
207199
const row = await rowFor(() => new Error(sql));
208200
expect(row.error).toBe('SQLITE_BUSY: database is locked');
201+
// [#22718] The door answers this as a fault too, so §5's gate is the
202+
// sandbox origin, never the door's 5xx alone.
203+
expect(mapDataError(new Error(sql), OBJECT).status).toBeGreaterThanOrEqual(500);
204+
});
205+
});
206+
207+
/** A refusal that DECLARED a server-band status: the doors keep the status and code and withhold the prose. */
208+
const sandboxDeclared5xx = () => sandboxRefusal('The ledger service is down.', { status: 503, code: 'SERVICE_UNAVAILABLE' });
209+
210+
describe('[#22718] §5 — a sandboxed body the door answers as a fault reads as that fault', () => {
211+
it.each([
212+
['a body that CRASHED', sandboxCrash, 'IMPORT_ROW_FAILED', /TypeError|boom|threw:/],
213+
['a refusal that declared a 5xx', sandboxDeclared5xx, 'SERVICE_UNAVAILABLE', /ledger|threw:/],
214+
] as const)('%s', async (_label, err, code, leak) => {
215+
const door = mapDataError(err(), OBJECT);
216+
// Anti-vacuity: the door answers a fault and withholds the text.
217+
expect(door.status).toBeGreaterThanOrEqual(500);
218+
expect(door.body.error).toBe(INTERNAL_ERROR_MESSAGE);
219+
220+
const row = await rowFor(err);
221+
// The door's sentence, never the native text or the prose; the code still reads as a fault.
222+
expect(row).toEqual({ row: 1, ok: false, action: 'failed', error: door.body.error, code });
223+
expect(JSON.stringify(row)).not.toMatch(leak);
209224
});
210225
});

‎packages/core/src/utils/import-runner.ts‎

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import type { DroppedFieldsEvent } from '@objectstack/spec/data';
88
import type { ExecutionContext } from '@objectstack/spec/kernel';
99
import type { CreateDataRequest, FindDataRequest, UpdateDataRequest, ValidateDataIssue, ValidateDataRequest, ValidateDataResponse } from '@objectstack/spec/api';
1010
import { bulkWrite, withTransientRetry, defaultIsTransientError, type BulkWriteRowResult } from './bulk-write.js';
11-
import { isUniqueViolationError, uniqueViolationColumn, isEngineDuplicateRecordEnvelope, mapDataError, sandboxBusinessMessage } from '@objectstack/types';
11+
import { isUniqueViolationError, uniqueViolationColumn, isEngineDuplicateRecordEnvelope, mapDataError, sandboxBusinessMessage, isSandboxOrigin } from '@objectstack/types';
1212

1313
/**
1414
* import-runner — the shared row-processing core for bulk import.
@@ -459,22 +459,37 @@ export function sanitizeRowError(raw: unknown): string {
459459
* passed through {@link sanitizeRowError}. That function cleans DRIVER text,
460460
* and an author's sentence fed to it is paraphrased: one that opens with an
461461
* SQL verb ("Update the cost centre first.") reads as a leaked statement and
462-
* comes back as the generic database sentence. A body that CRASHED is not a
463-
* refusal: {@link sandboxBusinessMessage} declines it, so its row is built
464-
* exactly as before, from `.message` through {@link sanitizeRowError}. A
465-
* `code` the body declared still rides, read from the error as it always was.
462+
* comes back as the generic database sentence. A `code` the body declared
463+
* still rides, read from the error as it always was.
464+
*
465+
* ## A sandboxed body the door answers as a fault reads as that fault (#22718)
466+
*
467+
* Two sandboxed failures are not refusals, and the create door withholds
468+
* their text: a body that CRASHED (`TypeError: boom`, which
469+
* {@link sandboxBusinessMessage} declines) answers `500 INTERNAL_ERROR`, and a
470+
* refusal that declared a 5xx status answers that status with its prose
471+
* withheld. The row used to relay both: the crash as the wrapper through
472+
* {@link sanitizeRowError}, the declared 5xx as the body's prose. So for a
473+
* sandboxed error the door answers with a 5xx, the row's text is the door's
474+
* own sentence. Which errors are faults and what their sentence is are both
475+
* `mapDataError`'s verdict, ⛔ never a second withholding rule here. The row's
476+
* `code` is unchanged, so it still reads as a failed row with a fault code.
477+
* Every other error keeps its row text: driver text is still cleaned by
478+
* {@link sanitizeRowError}, which is the import's own reading of it.
466479
*/
467480
function toFailedResult(rowNo: number, err: unknown, objectName: string): ImportRowResult {
468481
const e = err as { code?: unknown; message?: unknown; fields?: unknown; field?: unknown } | null | undefined;
469482
const first = firstFinding(e?.fields);
483+
const door = mapDataError(err, objectName);
470484
if (first === undefined) {
471-
const adopted = adoptDoorVerdict(rowNo, mapDataError(err, objectName).body);
485+
const adopted = adoptDoorVerdict(rowNo, door.body);
472486
if (adopted !== undefined) return adopted;
473487
}
474488
const thrownCode = isEngineDuplicateRecordEnvelope(e) ? 'UNIQUE_VIOLATION' : e?.code;
475489
const code = first?.code ?? thrownCode ?? 'IMPORT_ROW_FAILED';
476490
const field = first?.field != null && first.field !== '' ? first.field : e?.field;
477-
const message = sandboxBusinessMessage(err) ?? sanitizeRowError(e?.message);
491+
const sandboxFault = isSandboxOrigin(err) && door.status >= 500 ? String(door.body.error) : undefined;
492+
const message = sandboxFault ?? sandboxBusinessMessage(err) ?? sanitizeRowError(e?.message);
478493
return {
479494
row: rowNo, ok: false, action: 'failed', error: message, code: String(code),
480495
...(field != null && field !== '' ? { field: String(field) } : {}),

‎packages/rest/src/rest-write-route-hook-refusal-sentence.ledger.test.ts‎

Lines changed: 37 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@
4242
*/
4343

4444
import { describe, it, expect } from 'vitest';
45-
import { sanitizeRowError } from '@objectstack/core';
4645
import { RestServer } from './rest-server.js';
4746
import { mountAndRecordDirectRoutes } from './direct-mount-composition.js';
4847
import { REST_ROUTE_LEDGER } from './rest-route-ledger.js';
@@ -391,19 +390,43 @@ describe('[#22694] §3 MEASURED, NOT REPAIRED: these doors answer the debug wrap
391390
}
392391
});
393392

394-
describe('[#22694] §4 CONTROL: a hook body that CRASHED still answers as a fault on both import doors', () => {
395-
const crash = sandboxCrash();
393+
/** A refusal that DECLARED a server-band status: the doors keep the status and withhold the prose. */
394+
function sandboxDeclared5xx(): Error {
395+
const err = new Error("hook 'mz_guard' threw: Error: The ledger service is down.");
396+
err.name = 'SandboxError';
397+
return Object.assign(err, { innerMessage: 'The ledger service is down.', status: 503, code: 'SERVICE_UNAVAILABLE' });
398+
}
399+
400+
/**
401+
* [#22718] A body that CRASHED and a refusal that declared a 5xx are answered
402+
* as faults by the data doors, with their text withheld. Both import rows are
403+
* held to that answer. §2 is the control: a refusal row keeps the hook's
404+
* sentence on both import doors.
405+
*/
406+
describe('[#22718] §4 a sandboxed body the doors answer as a fault reads as that fault on both import doors', () => {
396407
it.each([
397-
['POST /api/v1/data/:object/import', importRow, undefined],
398-
['POST /api/v1/data/:object/import/jobs', jobRow, IMPORT_JOB_PROTOCOL],
399-
] as const)('%s', async (route, answer, protocol) => {
400-
const b = boot(sandboxCrash, protocol?.(sandboxCrash));
401-
const res = await drive(b, route, IMPORT_BODY);
402-
403-
const row = await answer(res, b);
404-
// The native error text is never presented as the author's sentence;
405-
// the row is built from `.message` exactly as before this card.
406-
expect(row).not.toBe('TypeError: boom');
407-
expect(row).toBe(sanitizeRowError(crash.message));
408+
['a body that CRASHED', sandboxCrash, /TypeError|boom/],
409+
['a refusal that declared a 5xx', sandboxDeclared5xx, /ledger/],
410+
] as const)('%s', async (_label, error, leak) => {
411+
const doors: unknown[] = [];
412+
for (const [route, body] of [['POST /api/v1/data/:object', { name: 'x' }], ['POST /api/v1/data/:object/createMany', [{ name: 'x' }]]] as const) {
413+
const res = await drive(boot(error), route, body);
414+
expect(res.statusCode, route).toBeGreaterThanOrEqual(500);
415+
doors.push(sentenceOf(res.body));
416+
}
417+
expect(doors[1]).toBe(doors[0]);
418+
expect(String(doors[0])).not.toMatch(leak);
419+
420+
for (const [route, answer, protocol] of [
421+
['POST /api/v1/data/:object/import', importRow, undefined],
422+
['POST /api/v1/data/:object/import/jobs', jobRow, IMPORT_JOB_PROTOCOL],
423+
] as const) {
424+
const b = boot(error, protocol?.(error));
425+
const row = await answer(await drive(b, route, IMPORT_BODY), b);
426+
427+
expect(b.calls, route).toContain('protocol.insertManyData');
428+
expect(row, route).toBe(doors[0]);
429+
expect(String(row), route).not.toMatch(WRAPPER_RE);
430+
}
408431
}, 60_000);
409432
});

0 commit comments

Comments
 (0)