You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit 3fdb92d
Browse filesBrowse the repository at this point in the historyBrowse files
fix(rest): hand-built write routes withhold the sandbox's own fault as the data door does (#22885) (#22896)
Fixes#22885
Clause-②: no
## What this does
The QuickJS runner raises some `SandboxError`s itself, with no
`innerMessage`: a denied capability, a missing or unavailable `ctx.api`
method, a CPU budget or wall-clock ceiling, an install or marshal
failure, a nested body's fault relayed one VM hop up.
`isSandboxOwnFault` (`@objectstack/types`) recognises them off the
producer, and the `/data` door answers them `500 INTERNAL_ERROR` /
`Internal server error`. Twenty-three hand-built REST write rows relayed
the runner's diagnostic text instead.
- `packages/rest/src/error-response.ts`: `sandboxFaultAnswer` answers
`isSandboxOwnFault` with `resolveErrorResponse`, the same classification
door and the same withheld `500` it gives a crash. One predicate, one
terminal, no own-fault branch anywhere else.
- `packages/rest/src/rest-server.ts`, record-share arms only:
`sharingFaultMessage` asks `sandboxFaultAnswer` for its sentence instead
of keeping its own sandbox read. The family keeps its own `500
SHARE_*_FAILED` status and code, as it does for a crash (see "The
record-share rows" below).
-
`packages/rest/src/rest-write-route-hook-refusal-sentence.ledger.test.ts`
§6: nine own-fault shapes, one per kind the runner raises, over every
row the ledger already drives (`ANSWERS_THE_SENTENCE`, held to
`REST_ROUTE_LEDGER` by §1, not hand-listed). The crash and declared-5xx
shapes stay as the CONTROL, unchanged. Each own-fault case asserts the
ADR-0112 `status` + `code`, the doors' withheld sentence, no trace of
the runner's text in the body, and that the whole `SandboxError` reaches
`console.error` (the file now declares `OS_REST_LOG: 'info'` inside §6,
as `check:rest-log-spy-declared` requires).
- `.changeset/22885-sandbox-own-fault-rest-write-routes.md`:
`@objectstack/rest` `patch`, `Clause-②: no`.
## Measured at RED (the card's "Not measured"), BASE `2e11d9006c`
The new §6 cases, run before the fix: **207 failed / 370 passed**, which
is exactly 23 rows x 9 shapes. Every named downstream was called, and no
upstream arm (a prefix map, a 403/404 arm, `classifiedRefusalAnswer`)
intercepted any shape. Every row reached its own terminal with the
runner's text:
| Rows | Status and code at RED |
|---|---|
| `POST /analytics/dataset/query` | `500 ANALYTICS_QUERY_FAILED` |
| `POST /security/explain` | `500 EXPLAIN_FAILED` |
| `POST` / `DELETE /data/:object/:id/shares` | `500 SHARE_GRANT_FAILED`
/ `SHARE_REVOKE_FAILED` |
| the three sharing-rule writes | `500 RULE_*_FAILED` |
| suggested-binding `confirm` / `dismiss` | `500 SUGGESTION_*_FAILED` |
| the nine `POST /approvals/requests/:id/*` writes | `500
APPROVAL_*_FAILED` |
| `POST /packages/publish` | `500 INTERNAL_ERROR` (text relayed) |
| the four `POST /datasources/:name/external/*` routes | `400
EXTERNAL_DATASOURCE_ERROR` / `EXTERNAL_IMPORT_ERROR` |
The other 20 driven rows already answered every shape `500
INTERNAL_ERROR` (the import rows per row inside `200` / `201`), and all
86 control readings already withheld. Before any edit, `sendThrownError`
(the classification door) answered all eight runner kinds `500
INTERNAL_ERROR` / `Internal server error`, while `sandboxFaultAnswer`
and `classifiedRefusalAnswer` both answered `undefined`. So the change
is one line at the function every family already asks.
After: 21 rows answer `500 INTERNAL_ERROR` / `Internal server error`.
The two record-share write rows answer `500 SHARE_*_FAILED` / `Internal
server error`. `GET /data/:object/:id/shares`, which the write ledger
does not drive, was measured by a throwaway probe (deleted): `500
SHARES_LIST_FAILED` / `Internal server error`; the same words on a plain
`Error` keep the relayed text there.
## The record-share rows: one reading of the triage text
Triage says the record-share rows "go through the same answer" and "Do
not patch their own terminal separately", and also that "The crash rows
stay as the CONTROL". The dispatch's reading was that these arms ask
`sandboxFaultAnswer` as the other families do. Measured, that literal
shape moves the CONTROL: on the two share write rows a crash would go
from `500 SHARE_*_FAILED` to `500 INTERNAL_ERROR`, and a sandboxed
declared 5xx from `500 SHARE_*_FAILED` to `503 SERVICE_UNAVAILABLE`,
four existing assertions, and it would delete the ledger's
`OWN_FAULT_TERMINAL` exception that #22764 recorded as "a decision this
card does not take".
So the share arms' sentence is `sandboxFaultAnswer`'s (one predicate, no
own-fault check of their own), and their status and code stay their own,
exactly as for a crash. `OWN_FAULT_TERMINAL` keeps its two rows; its
reason now names the shared read. If the family's status and code should
move too, that is a four-line change plus deleting `OWN_FAULT_TERMINAL`,
and it moves the crash and declared-5xx answers on these routes with it.
## Files outside the claim's surface, declared
`packages/rest/src/package-routes.ts` and
`packages/rest/src/external-datasource-routes.ts`, comment-only. Both
docblocks said sentences this change makes false ("`sandboxFaultAnswer`
is the doors' answer for exactly those two", "Every other throw keeps
this family's own `400`"). No code there changes; both callers already
ask `sandboxFaultAnswer` first.
## Reverse verification (one-time; all from committed `24f3dc6f46`, via
`scripts/ablation-replace.mjs`, each restore proven by blob == HEAD and
an empty `git diff HEAD`)
The tested subjects resolve by relative `src` import, not through a
dependency's `exports`, so no `dist` rebuild leg applies.
- **A**, delete the own-fault line in `sandboxFaultAnswer`: anchor 1 to
0, blob `1c91226dfd37` to `aa8e4dc71f19`. Ledger **207 failed / 370
passed**, the RED population exactly (the share rows go red too, since
their sentence reads the same answer).
- **B**, delete the share arm's `sandboxFaultAnswer` read: blob
`8e11bff9ffa9` to `e31a3df9f2c5`. **18 failed**, only the two
record-share write rows (9 shapes each).
- **C**, delete the approvals family's fault-arm `logError`: blob
`8e11bff9ffa9` to `96834aaee48f`. **81 failed** (9 approval rows x 9
shapes), each on "the runner's error reaches the server log". The log
assertion can fail.
## Tests, at `24f3dc6f46`
- `pnpm --filter @objectstack/rest exec vitest run --maxWorkers=2
src/rest-write-route-hook-refusal-sentence.ledger.test.ts`: 577 passed
(189 before this change).
- `pnpm --filter @objectstack/rest exec vitest run --project local
--maxWorkers=2`: 273 files, 5721 passed, 326 skipped. This includes
`sharing-envelope.test.ts`.
- `pnpm --filter @objectstack/rest exec vitest run --project repo
--maxWorkers=2`: 5 files, 209 passed, 1 skipped. This includes the three
`error-response-*` suites.
- `pnpm --filter @objectstack/rest typecheck`: exit 0,
`check:test-typecheck: OK`. `--listFiles` puts the ledger in the
`tsconfig.test.json` program (1 hit).
- Gates: `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` with no paths, on this tree. Reconciled with
`--ran`: **65 derived, 64 run at exit 0, 1 NOT-MEASURED, 0 unrun**.
- **NOT MEASURED: `pnpm check:dual-build-cjs-loads`**, reason: exit 3,
`PREREQUISITE NOT MET`. 42 packages this diff does not touch have no
`dist/` in this container. Narrower reading: `@objectstack/rest` is
built and `require('./dist/index.cjs')` loads, with 34 exports. CI runs
the full gate.
- Lint, a proven narrowing rather than `pnpm lint`: `eslint
--no-inline-config --format json` over the five touched `.ts` files. ①
Population, read from eslint's own config: `--print-config` resolves a
config for each of the five (6/6/6/6/5 rules), and the changeset `.md`
matches none. ② The JSON result holds 5 files, 0 errors, 0 warnings. ③
Invariance: `eslint.config.mjs` sets no `parserOptions.project` and no
`projectService` (its own comment: it "never enables type-aware
linting"), and this diff touches no config or data file a rule reads, so
it cannot move any untouched file's verdict.
## Acceptance notes
- The other `sandboxFaultAnswer` callers in `rest-server.ts` (analytics,
explain, sharing rules, suggested bindings, approvals) still describe
the answer as "(a crash, or a refusal declaring a 5xx)". That is now
incomplete but not false, and it was left alone to keep `rest-server.ts`
to the record-share arms. Carrier: none.
- `resolveErrorResponse` asks `isSandboxCrash` at its top but not
`isSandboxOwnFault`. An own fault reaches `classifyDataError`'s
own-fault terminal only because it carries no `status` or `code`. That
holds for every raise site in `quickjs-runner.ts` today (no own-fault
construction passes `SandboxErrorInfo`), so it is dormant. Carrier:
none.
- The import rows answer an own fault per row with the withheld
sentence. This PR adds no log assertion on them, because their contract
is a per-row outcome handed to the caller.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01VoSxBQujKLZKPwK2u5ehQ6)_
Co-authored-by: Claude <noreply@anthropic.com>
A sandboxed hook or action that fails on the sandbox's own fault (a capability it was not granted, a `ctx.api` method that is missing or unavailable, a CPU budget or wall-clock ceiling, an install or marshal failure) is now answered by the hand-built write routes the way `POST /api/v1/data/:object` answers it: a withheld `500` with `Internal server error`. The runner's diagnostic text, which names the hook, the object and the capability, goes to the server log only.
6
+
7
+
Clause-②: no
8
+
9
+
Before, 23 write routes relayed that text to the caller: `POST /analytics/dataset/query`, `POST /security/explain`, `POST` and `DELETE /data/:object/:id/shares`, the three sharing-rule writes, the two suggested-binding writes, the nine approval-request writes, `POST /packages/publish` and the four `POST /datasources/:name/external/*` routes. Most of them answered `500` with their own fault code, and the four external routes answered `400`. Twenty-one of them now answer `500 INTERNAL_ERROR`, the answer each already gave a sandboxed body that crashed. The record-share routes (and the share list, `GET /data/:object/:id/shares`) keep their own `500` and `SHARE_*_FAILED` code, as they do for a crash, and withhold the sentence the same way.
10
+
11
+
A refusal a sandboxed body throws keeps its own sentence and status on every route. No key, default, enum member or export moves.
0 commit comments