fix(rest): serve a sandboxed hook's own refusal sentence on the bulk write and analytics routes, not the QuickJS debug wrapper

.changeset/bulk-write-refusal-message-parity.md

@@ -0,0 +1,69 @@
+---
+'@objectstack/rest': patch
+---
+
+Serve a sandboxed hook's own refusal sentence on the bulk write routes, instead
+of the QuickJS debug wrapper
+
+A hook's `throw new Error('删除被阻断…')` is a deliberate business rule, and
+`classifyDataError`'s sandbox unwrap door exists precisely so the end user sees
+only that sentence — the `<kind> '<name>' threw: <msg>` prefix "belongs in
+server logs", in the door's own words. Six write routes never reached the door.
+Measured against the real route handlers: `PATCH /api/v1/data/:object/:id`
+answered `Opportunity is closed.` while `POST …/batch`, `…/createMany`,
+`…/updateMany`, `…/deleteMany` and `…/:id/clone` answered
+`hook 'guard' threw: Error: Opportunity is closed.` — one hook, one refusal, a
+different sentence depending on which route the caller happened to use.
+
+The branch is `resolveErrorResponse`'s declared-status passthrough, which is
+checked *before* it delegates to `mapDataError` and answered its 4xx arm from
+`error.message`. It now reads the business text through `sandboxBusinessMessage`
+— the unwrap door's own two conditions (a non-empty string `.innerMessage`, and
+not a `isScriptFaultMessage` crash) named once so the two doors ask the same
+question.
+
+**Not a reorder.** The passthrough's own docblock argues the ordering: handing a
+declared 5xx to `mapDataError` re-labels it from the message TEXT (the
+overlay-delete fault comes back `404 OBJECT_NOT_FOUND` and stops being logged),
+so the arm stays exactly where it is and keeps deciding the status. Only the
+sentence it reads changes. #5437/#5582's unconditional 5xx prose withhold is
+untouched — a sandbox refusal declaring a 5xx still answers with the generic
+text, pinned on both spellings.
+
+What this restores is an invariant the same docblock already asserts. Its #7525
+paragraph says an error declaring `statusCode` instead falls to `mapDataError`,
+"So the two doors already agree on the wire answer." For a sandbox refusal that
+was false — `statusCode` was unwrapped and `status` was not — which is the
+two-spellings asymmetry this card was filed on. The doors agree again, pinned
+door-to-door across the whole 4xx band rather than asserted in a comment.
+
+**Bump level: `patch`, argued rather than defaulted.** The change is to message
+TEXT on shipped routes, so the level is not automatic. It is a patch because
+nothing about the envelope's contract moves: same status, same `code`, same
+field set, no request newly accepted or refused. The delta is that one string
+loses a debug prefix that this boundary already declares must never be on the
+wire, and that the single-row routes never emitted — so no client could have
+been reading it uniformly in the first place. Keying on the prefix would mean
+substring-matching prose that is localised and deliberately reworded over time,
+which is the practice the ADR-0112 `code` vocabulary exists to remove.
+
+`POST /api/v1/analytics/dataset/query` — the seventh row — needed its own
+repair: it builds a `{ code, message }` envelope inline and touches neither
+door. It now imports the same `sandboxBusinessMessage` rather than re-deriving
+the unwrap, so the analytics face and the `/data` face cannot answer one refusal
+two ways. Both of its client emissions are covered (the declared-4xx envelope
+and the `500 ANALYTICS_QUERY_FAILED` fallback); `logError` still receives the
+whole error and `looksLikeInternalErrorLeak` still reads the raw text, so the
+operator's copy and the leak heuristic are untouched.
+
+`POST …/import` and `GET …/export` exit through `handleRouteError` like the bulk
+routes, so they are repaired by the same change — measured rather than assumed.
+
+**Measured and deliberately NOT repaired here**, each recorded so it is not
+rediscovered as new: the record-share routes (`…/:id/shares`, list/grant/revoke)
+are a third branch again — `respondSharingError` classifies by
+`message.startsWith(CODE)` and its fallback interpolates `error.message` into a
+hand-built `500`, ignoring a declared `status`/`code` entirely. And on the
+analytics route an *undeclared* hook refusal answers `500` where `/data` answers
+`400`; only the sentence was corrected, the status disagreement is a separate
+defect. Both are filed as their own issues.

packages/rest/src/error-response.ts

@@ -232,6 +232,48 @@ function isScriptFaultMessage(message: string): boolean {
  return NATIVE_ERROR_NAME_RE.test(message.trim());
 }
 
+/**
+ * [#11588] The caller-addressed BUSINESS text a sandboxed hook/action body
+ * threw, or `undefined` when this error is not a sandbox refusal.
+ *
+ * QuickJS bodies throw a `SandboxError` whose `.message` is the
+ * `<kind> '<name>' threw: <msg>` debug wrapper and whose `.innerMessage` is the
+ * text the author addressed to the end user (see
+ * `runtime/src/sandbox/quickjs-runner.ts`). The wrapper "belongs in server
+ * logs" — {@link classifyDataError}'s unwrap door exists precisely to keep it
+ * off the wire. This is that door's read, named once so the door ABOVE it in
+ * {@link resolveErrorResponse} can ask the same question instead of shipping
+ * `error.message` raw.
+ *
+ * Both conditions are the door's, in the door's order:
+ *
+ * - a non-empty string `.innerMessage`, which is what makes this a sandbox
+ * error at all;
+ * - NOT {@link isScriptFaultMessage}. A body that CRASHED arrives with the
+ * same shape, and its `TypeError: not a function` is an internal fault
+ * rather than a business message (#7543). This answers `undefined` there,
+ * so a crash is never mistaken for authored text.
+ *
+ * ⛔ It is deliberately a READ of a field the sandbox populated, never a
+ * pattern-strip of the wrapper off `.message`. Stripping prose by regex would
+ * also rewrite a plain error whose own text happens to contain `threw:`, and
+ * the caller's message is the remedy on a 4xx (#5423) — the one thing this
+ * boundary must not paraphrase. `rest-hook-refusal-message-parity.test.ts` §5
+ * is the control that keeps it a read.
+ *
+ * Exported for the SECOND boundary that has to ask the same question:
+ * `/analytics/dataset/query` builds its own `{ code, message }` envelope inline
+ * in `rest-server.ts` and shares no branch with either door here. It reads this
+ * rather than re-deriving the unwrap, so the analytics face and the `/data`
+ * face cannot drift into two answers for one refusal — the door-disagreement
+ * shape #7525/#8016 keeps producing when a boundary open-codes the read.
+ */
+export function sandboxBusinessMessage(error: any): string | undefined {
+ if (typeof error?.innerMessage !== 'string' || !error.innerMessage) return undefined;
+ if (isScriptFaultMessage(error.innerMessage)) return undefined;
+ return error.innerMessage;
+}
+
 /**
  * [#5462] Does a driver's missing-relation message name the very object this
  * request asked for?
@@ -661,6 +703,15 @@ function classifyDataError(error: any, object?: string): { status: number; body:
  // VOCABULARY either — an unregistered spelling is demoted to
  // `declaredCode` by the same shared resolver, so this door stops being the
  // one flat exit #9232 could not reach.
+ //
+ // [#11588] The same two reads, in the same order, are named as
+ // {@link sandboxBusinessMessage} for the declared-status passthrough in
+ // {@link resolveErrorResponse}, which sits ABOVE this door and used to ship
+ // the wrapper verbatim. This door keeps its own spelling because its crash
+ // case is a TERMINAL (the sanitised 500) rather than a fall-through, which
+ // is a different answer to the same question; the two are held together by
+ // a door-to-door pin (`rest-hook-refusal-message-parity.test.ts` §4) rather
+ // than by this comment.
  if (typeof error?.innerMessage === 'string' && error.innerMessage) {
  // [#7543] …but only when the body REPORTED something. A body that
  // CRASHED arrives here too, and its `TypeError: not a function` is an
@@ -1495,9 +1546,52 @@ function resolveErrorResponse(error: any, object?: string): { status: number; bo
  // [#5423] 4xx keeps the bound as a TRUNCATION, not a replacement: a 4xx
  // message is addressed TO the caller and is the remedy. Unchanged by
  // #5437 — see {@link truncateClientMessage}.
- const safeMsg = typeof error.message !== 'string'
- ? 'Request failed'
- : truncateClientMessage(error.message);
+ //
+ // [#11588] …and for a SANDBOX refusal the text addressed to the caller
+ // is `.innerMessage`, not `.message` — see
+ // {@link sandboxBusinessMessage}. Without this read, every route that
+ // reports through `handleRouteError` (batch, createMany, updateMany,
+ // deleteMany, clone, and the metadata/UI/import/export families that
+ // share the exit) shipped the QuickJS DEBUG WRAPPER to the end user:
+ // `hook 'guard' threw: Error: Opportunity is closed.` where the
+ // single-row `PATCH` on the same object answered `Opportunity is
+ // closed.` One hook, one refusal, two different sentences depending on
+ // which route the caller happened to use.
+ //
+ // ⛔ This is NOT the reorder it looks like from the card. The unwrap
+ // door lives in `mapDataError`, BELOW this arm, and moving it above is
+ // ruled out by this arm's own argument two paragraphs up: `mapDataError`
+ // derives a status from the message TEXT, so a declared 5xx handed to
+ // it comes back re-labelled (`404 OBJECT_NOT_FOUND` for the
+ // overlay-delete fault) and stops being logged. The passthrough stays
+ // exactly where it is and keeps deciding the STATUS; only the sentence
+ // it reads for the caller changes. Nothing about the 5xx arm above —
+ // #5437/#5582's unconditional prose withhold — moves, and a sandbox
+ // refusal declaring a 5xx still exits there with the prose dropped.
+ //
+ // What this restores is an invariant THIS DOCBLOCK already asserts. The
+ // #7525 paragraph at the top of the arm says an error declaring
+ // `statusCode` instead "falls to `mapDataError` below … So the two
+ // doors already agree on the wire answer". For a sandbox refusal that
+ // sentence was false: `statusCode` fell through and was unwrapped,
+ // `status` was answered here from the wrapper, and one hook produced
+ // two message shapes on one route depending on the spelling its author
+ // picked. The two doors agree again now — pinned door-to-door rather
+ // than asserted, in `rest-hook-refusal-message-parity.test.ts` §4.
+ //
+ // Recorded because it is measured and NOT repaired here: a body that
+ // CRASHED while carrying a declared 4xx `status` still answers with
+ // that status and the wrapper, where `mapDataError` would sanitise it
+ // to a 500. `sandboxBusinessMessage` declines the crash (#7543) so this
+ // arm's answer for it is byte-identical to before. Closing that gap
+ // means moving the STATUS this arm decided, which is the contract
+ // question this card was fenced away from — filed separately.
+ const businessMessage = sandboxBusinessMessage(error);
+ const safeMsg = businessMessage !== undefined
+ ? truncateClientMessage(businessMessage)
+ : typeof error.message !== 'string'
+ ? 'Request failed'
+ : truncateClientMessage(error.message);
  // [#9232] Narrowed, same as the three arms above.
  return withDeclaredUserMessage(error, {
  status: error.status,