Skip to content

fix(rest,runtime): a sandboxed crash is a fault at the doors mapDataError does not decide - #18533

Merged
os-support-ai merged 3 commits into
mainfrom
claude/issue-17273-sandbox-declared-4xx-sanitisation
Sep 16, 2026
Merged

os-support-ai merged 3 commits into
mainfrom
claude/issue-17273-sandbox-declared-4xx-sanitisation

Conversation

@os-support-ai

@os-support-ai os-support-ai commented Sep 16, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #17273

Clause-②: no

#15071 converged ONE /data door on the ruling that a sandboxed crash is a
fault, and wrote the boundary into a pin (ACCEPTED DIVERGENCE) rather than
pretending to be done. This closes both faces that pin named, and flips the
spelling with them.

The two faces, and the count

The card names exactly two, counted by its own ### subsections under
"Two faces still answer a sandboxed crash from the pre-#15071 world" — face 1
resolveErrorResponse's declared-4xx passthrough, face 2 the /api/v1/actions
door reading innerMessage as intent. Both are converged here, in one delivery.

Face 1 — packages/rest/src/error-response.ts

resolveErrorResponse now asks the same isSandboxCrash gate #15071 put above
classifyDataError's code-gated arms, before its declared-status passthrough,
and answers through mapDataError so the two doors are identical by
construction rather than by a second opinion.

Deliberately not band-scoped: scoping the gate to 4xx would have converged
the named shape while minting a new divergence one band over (the single door
has answered 500 INTERNAL_ERROR for a crash declaring a 5xx since #15071, and
the 5xx arm would have kept 503 plus the declared code). The 5xx arm's
unconditional prose-drop is not narrowed — the fault terminal withholds
prose too; what moves for that shape is the status and the declared code, both
toward the sanitised pair. Pinned as its own case beside the 4xx one.

Face 2 — packages/runtime/src/domains/actions.ts

unexpectedFault required !innerMessage, reading the presence of that field as
"user code threw this deliberately" — but the runner sets the same field for a
CRASH, to the native error text. So a crash was neither a deliberate throw nor an
unexpected fault, and fell to the 400 terminal carrying TypeError: …. A crash
terminal now sits above every branch that reads a producer declaration as
intent (including the one that serves err.status), and answers the sanitised
500. The crash reading is isNativeErrorName from @objectstack/types — the ONE
reader @objectstack/rest composes into isScriptFaultMessage, so the doors
cannot drift.

Driven, not read — the bytes before and after

The card warned that face 2 was a source reading, never driven. It reproduces.
A body that really crashes inside QuickJS (return ctx.input.title.trim(); with
a numeric title), through the real doors:

Bulk / metadata / UI routes (handleRouteError):

BEFORE  409 {"error":"hook 'guard' threw: TypeError: ctx.input.title.trim is not a function",
             "code":"DELETE_RESTRICTED","object":"account"}
AFTER   500 {"error":"Internal server error","code":"INTERNAL_ERROR"}

POST /api/v1/actions/:object/:action:

BEFORE  400 {"success":false,"error":{"code":"VALIDATION_ERROR",
             "message":"TypeError: not a function","httpStatus":400}}
AFTER   500 {"success":false,"error":{"code":"INTERNAL_ERROR",
             "message":"Internal server error","httpStatus":500}}

…and the same crash carrying a declared 409 DELETE_RESTRICTED answered 409
with the TypeError: message before, the sanitised 500 now. Note the BEFORE
code on the undeclared row: a crash was being reported to clients as
VALIDATION_ERROR. That is one step worse than the card predicted, and it is
gone.

The published page was right and the code was wrong: content/docs/api/error-catalog.mdx
already states "a TypeError / a ReferenceError / a driver's own error class
is a crash (500)" for this very route, and this module's own header says
crash → 500. No docs edit is owed — the code now matches the page.

The pins, flipped rather than left saying an old thing

  • error-response-structured-arm-door-parity.test.ts §4 — ACCEPTED DIVERGENCE (#15071 widens it) → CONVERGED (#17273), asserting both doors and the
    whole body. Two cases added beside it: the 5xx band this repair also reaches,
    and a sandboxed-REFUSAL negative control one innerMessage away.
  • rest-hook-refusal-message-parity.test.ts §7 — the MEASURED AND NOT REPAIRED
    section is INVERTED, not deleted, the discipline §8b in that same file
    already applies: a pin that records a defect is the only thing that would
    notice it coming back.
  • error-response.ts — the two docblock paragraphs that asserted the divergence
    in prose (the armSentence note and the "measured and NOT repaired here"
    paragraph inside the 4xx arm) are corrected in the same edit.

⚠️ The card's third premise — "the parity pin is the only one asserting this
divergence" — is FALSIFIED. Four carriers, not one; all four moved. The sweep
was grep -rn "ACCEPTED DIVERGENCE" and grep -rni "not repaired" over the tree
with control words known present (9 and 30 hits respectively, so a zero would
have been a reading).

Ablation — the pins fail on the wrong fix

Both crash terminals were removed on disk (mutation proven by git hash-object
against the HEAD blob, and by anchor occurrence counts falling 1 → 0), the
suites re-run, then restored (git checkout HEAD -- …, restoration proven by
blob equality and an empty git diff HEAD).

ablated  rest    :  3 failed | 78 passed   (409→500, 503→500, 409→500)
ablated  runtime :  3 failed | 17 passed   (400→500, 409→500, 400→500 on the real crash)
restored rest    :  153 passed (3 files)
restored runtime :   20 passed (2 files)

Every negative control stayed green in the ablated run — the flip cannot be
green because the surface was deleted.

Verification

what verdict
pnpm --filter '@objectstack/rest^...' --filter '@objectstack/runtime^...' build exit 0
pnpm exec turbo run build --filter='./packages/*' --filter='./packages/*/*' exit 0, 72/72
pnpm --filter @objectstack/rest test 192 files, 3218 passed, 1 skipped, 0 failed
pnpm --filter @objectstack/runtime test 264 files, 3654 passed, 0 failed
pnpm --filter @objectstack/rest --filter @objectstack/runtime typecheck exit 0 (both, source + test layer)
pnpm lint — repo-wide, eslint . --no-inline-config exit 0
node scripts/pm/dispatch-gates.mjs --commands → run → --ran 61 derived / 61 run / 0 NOT-MEASURED / 0 UNRUN
pnpm check:error-status-conformance exit 0 — every derivable runtime status documented, every documented status reachable
node scripts/pm/check-clause2-carriers.mjs --pair 18533 exit 0 — both carriers agree on no, no widening tell in the diff

One gate exits non-zero and it is not this PR's: pnpm check:cross-package-test-inputs
(exit 1), rooted in packages/cli/test/init-created-files-summary.e2e.test.ts —
pre-existing, filed as #18353 / #18348, and it names zero paths under
packages/rest or packages/runtime. Two gates first answered exit 3
(check:dual-build-cjs-loads, check:type-check-debt — PREREQUISITE NOT MET,
neither pass nor fail); the full package build above satisfied both and they were
re-run to exit 0, which is what the reconciliation counts.

Docs — swept, nothing falsified, three pages become accurate

The drift check listed content/docs/protocol/kernel/error-handling.mdx (via
resolveErrorResponse) and content/docs/permissions/system-context.mdx (via
handleActionsRequest). Predicate declared before reading: a page is falsified
iff it (a) says a sandboxed crash's own message or the <kind> '<name>' threw:
wrapper reaches the caller, (b) documents the declared-status passthrough as
answering a crashed body with the producer's status and prose, or (c) prints an
example body / table row this change now renders differently. Merely naming the
symbol, or documenting the sanitised shape #15071 already declared, is not
falsification.

Neither listed page is falsified. error-handling.mdx names
resolveErrorResponse once, in a sentence about which writer builds the flat
envelope, and its only worked example is a UNIQUE_VIOLATION refusal; its
INTERNAL_ERROR section already states the 5xx withhold-by-construction rule
this change extends rather than contradicts. system-context.mdx matches on an
unrelated sandbox row and says nothing about error status.

The check warns that a page stating a rule by its inputs shares no identifier
with the emitter, so I swept past the list. Zeros taken with controls:
grep -rn "threw:" content/docs/ → 0 hits (control: 144 files mention
hook), so no page prints the wrapper anywhere. content/docs/releases/** → 0
hits
for the crash rule (control: 14 files mention action); nothing there is
falsified and nothing there is touched.

Three pages state the crash rule by its inputs, and all three said 500 while
the code said 400/409 — they were describing the contract this change makes
true, so they go from aspirational to accurate and no docs edit is owed:

  • content/docs/api/error-catalog.mdx — "a TypeError / a ReferenceError / a
    driver's own error class is a crash (500)";
  • content/docs/ui/actions.mdx — "500 — it crashed: a TypeError in
    your handler … A deliberate throw new Error('…') is a 400 rejection";
  • content/docs/api/client-sdk.mdx — "(400 rejection, 404 / 403 / 503 dispatch
    failures, 500 crash)".

Scope fences honoured

⛔ No packages/spec, no docs/adr/**, no .claude/**, no skills/**, no
content/docs/releases/, no packages/cli/test/commands.test.ts. No label,
assignee, ready flip, auto-merge arm or enqueue — landing is the PM seat's act.
No new export, no new error code, no new accepted input: the wire loses the
crash text and the producer's code and gains nothing, which is why this is
declared Clause-②: no.

⚠️ The card body predicted Clause-②: yes ("it moves a status a passthrough
decided, on a published door") while the claim comment of record declares no
on triage's reasoning (a tightening back to the sanitisation contract #15071
already declared; the leaked surface shrinks). Both readings are on the record
and this PR follows the claim's declaration, which is the machine-read carrier —
flagged here rather than picked silently.

维护者速读(草稿)

改了什么 — 沙箱里的脚本体崩溃(TypeError 这类)时,两个已发布的 HTTP 门
本来会把 QuickJS 的调试文本连同一个业务状态码一起发给调用方。现在这两个门都回
标准的 500 / INTERNAL_ERROR / Internal server error,崩溃原文只留在服务端日志。

为什么改 — #15071 已经裁定「崩溃是故障,不是作者声明的那个拒绝」,但只收敛了
一个门,并把边界诚实地写进 pin 再开卡。这张卡就是那两个残余的门;不收敛的话,
同一次崩溃在不同路由上给出两种答案,其中一种把内部异常文本泄露给了 API 调用方。

风险与代价(含回滚) — 属于破坏性行为变更:原来拿 4xx / 400 的客户端分支现在
会走 5xx 分支。这正是本次改动的目的(崩溃本来就不是那个业务拒绝)。普通的沙箱
业务拒绝一个字节都没变,已有负向对照测试钉住。回滚 = revert 本 PR 的两个 commit;
无数据迁移、无 schema 变更、无导出变更。

席位意见 — (留空,由席位定稿)

你要做的 — 判断「崩溃一律回 500」这个方向是否需要在 5xx band 上也生效(本 PR
认为需要,理由写在代码注释和上面的 Face 1 段);以及确认 Clause-②: no 的判定
(卡面正文曾预期 yes,分诊与认领判 no)。


🤖 Generated with Claude Code

https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3


Generated by Claude Code

…rror does not decide

#15071 converged the single `/data` door and named the residue. Both faces it
left are closed here:

- `resolveErrorResponse` asks the same `isSandboxCrash` gate above its
  declared-status passthrough, so the bulk/metadata/UI routes stop answering a
  crash at the declared status with the QuickJS debug wrapper as the
  client-facing sentence.
- the `/api/v1/actions` door stops reading `innerMessage` as intent: a crashed
  sandbox body answers the sanitised 500 its own published catalog row states,
  instead of a 400 carrying the native-error text.

Claude-Session: https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3
Co-authored-by: Claude <noreply@anthropic.com>
…to end

- `error-response-structured-arm-door-parity.test.ts` §4: the sandboxed-CRASH
  case flips from `ACCEPTED DIVERGENCE (#15071 widens it)` to `CONVERGED`,
  with the 5xx band and a refusal negative control added beside it.
- `rest-hook-refusal-message-parity.test.ts` §7 is INVERTED, not deleted — the
  bound it recorded is closed and the section is what would notice it coming
  back.
- `actions-fault-vs-rejection.test.ts` gains the crash line at the `/actions`
  door, and a new file drives a body that really crashes inside QuickJS into
  that door so the card's source reading becomes a measurement.

Claude-Session: https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Sep 16, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/rest, @objectstack/runtime, touching 2 documentable anchor(s).

2 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/permissions/system-context.mdx (via handleActionsRequest (symbol, a top-level function))
  • content/docs/protocol/kernel/error-handling.mdx (via resolveErrorResponse (symbol, a top-level function))
What this run could not see
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 33 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 1e496f9796126b0398c026c96fb8d240f1982e91 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 7debcc4f2ee98d7d19ebdd173268697837b58f76 — the merge of head 8e37306aa32ddfebf3d714fef312ed33cad34ee3 into base 1e496f9796126b0398c026c96fb8d240f1982e91, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 7debcc4f2ee98d7d19ebdd173268697837b58f76 && git checkout 7debcc4f2ee98d7d19ebdd173268697837b58f76
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 1e496f9796126b0398c026c96fb8d240f1982e91 8e37306aa32ddfebf3d714fef312ed33cad34ee3 && git checkout -B drift-repro 1e496f9796126b0398c026c96fb8d240f1982e91 && git merge --no-ff 8e37306aa32ddfebf3d714fef312ed33cad34ee3

node scripts/docs-audit/affected-docs.mjs --json 1e496f9796126b0398c026c96fb8d240f1982e91

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 1e496f9796126b0398c026c96fb8d240f1982e91 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

2 participants