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 74327d3
Browse filesBrowse the repository at this point in the historyBrowse files
fix(runtime): a non-sandboxed crash at /api/v1/actions no longer ships its native error message verbatim (#18760)
Fixes#18540
Clause-②: no
A non-sandboxed crash at `/api/v1/actions` shipped its native error
message verbatim. The identical crash through the `/data` door was
already sanitised. **The status was already right; what leaked was the
sentence** — so only `error.message` moves here. No status code, no
`error.code`, no envelope key.
## Premise 1 — reproduced first, on today's `origin/main`
The card's own status note says the filing seat did not verify the
measurement at the tree, and that the first act is to reproduce it.
Driven through the real `HttpDispatcher.handleActions` door at base
`e77a23f02`, with `mapDataError` driven on the same throw as the
control:
| | `/actions` (before) | `/data` |
|---|---|---|
| `new TypeError("Cannot read properties of undefined (reading 'id')")`
| `500` `INTERNAL_ERROR` — message `Cannot read properties of undefined
(reading 'id')` | `500` `INTERNAL_ERROR` — message `Internal server
error` |
Byte-for-byte the envelope the card recorded. `premise_still_valid:
true`.
## Premise 2 — why #17273's terminal does not catch it
Confirmed at the tree, not copied from the card. The terminal reads:
```ts
const sandboxCrash = typeof inner === 'string' && inner.length > 0 && isNativeErrorName(inner.trim());
```
`inner` is `err.innerMessage`, which only the QuickJS runner populates.
An in-process registered handler never crosses the VM boundary, so
`innerMessage` is absent, `sandboxCrash` is `false`, and the throw falls
through to `unexpectedFault`.
⭐ The shape, carried into the code comment and the test docblock because
it generalises: **a predicate that classifies by HOW a crash arrived is
structurally blind to crashes that did not arrive that way — while
looking exhaustive.**
## Premise 3 — the width, with controls in both directions
Both doors driven on the same ten throws. Seven shapes leaked at
`/actions` and every one of them was **already** sanitised at `/data`:
| throw | `/actions` before | `/data` |
|---|---|---|
| `TypeError` | leaked the native sentence | `Internal server error` |
| `ReferenceError` | `x is not defined` | `Internal server error` |
| `RangeError` | `Maximum call stack size exceeded` | `Internal server
error` |
| `SyntaxError` | `Unexpected token }` | `Internal server error` |
| driver class, prose the heuristic does NOT recognise | `database disk
image is malformed at /srv/data/tenant_42.db` — a server **filesystem
path** | `Internal server error` |
| sandbox timeout | `action timed out after 5000ms` | `Internal server
error` |
| sandbox capability denial | `... capability 'api.read' not granted` |
`Internal server error` |
Controls in the other direction, unchanged by this PR:
| throw | `/actions` | note |
|---|---|---|
| driver class, leaky prose | already `Internal server error` | the 5xx
heuristic already fired |
| plain `Error` — a deliberate rejection | `400` + its own sentence |
the refusal channel, untouched |
| `TypeError` carrying `status: 403` | `403 FORBIDDEN` + `Not allowed` |
a declared answer, untouched |
⇒ the leaking population is **exactly** the `unexpectedFault` branch's
population, at one site. The other dispatcher doors (`/meta`, `/mcp`)
never make this classification at all, so their undeclared-5xx prose is
#5667's recorded decision rather than this defect — noted below, not
filed.
## The fix, and why it is not the heuristic
The two doors differ **structurally**, not by one missing phrasing:
- `/data` is **default-DENY**: `classifyDataError` ends in an
unconditional `UNCLASSIFIED_FAULT()`. Its `looksLikeInternalErrorLeak`
limb only chooses `DATABASE_ERROR` over `INTERNAL_ERROR` — that limb is
not what sanitises.
- the actions door was **default-ALLOW**: `unexpectedFault` relayed
`err.message` through a 5xx withhold gated on
`looksLikeInternalErrorLeak`, a driver-dump heuristic that reads FALSE
for stack-shaped prose.
`unexpectedFault` **is** this door's unclassified-fault terminal, so it
now answers the terminal's envelope — `INTERNAL_ERROR_MESSAGE`, through
the same `deps.error` seam #17273's terminal one branch above already
uses.
Both fences held:
- ⛔ `looksLikeInternalErrorLeak` is **not** relaxed or re-pointed at
stack-shaped prose. It guards a different question at every other
boundary.
- ⛔ `DomainHandlerDeps.error`'s seam is **not** widened. A fault's
`userMessage` and its non-string `details.code` do not ride this exit,
exactly as they do not ride `/data`'s — the same accepted difference
#17273's terminal already carries.
**The status provably does not move.** Reaching this branch already
proves `.status`/`.statusCode` are absent (the branch above serves them)
and that this is not a `ValidationError`. So in `resolveThrownHttpError`
the `declaredStatus` chain is empty, `status` is the `500` fallback, and
`code` is `standardErrorCodeForHttpStatus(500)` = `INTERNAL_ERROR` — the
code this exit emits. The STOP-AND-REPORT condition did not trigger: no
declared status code or envelope shape changes.
## Tests
`packages/runtime/src/domains/actions-fault-vs-rejection.test.ts` — the
file that already drove this exact door:
- the pre-existing `a TypeError from a buggy handler` pin used the
card's exact message and asserted status, `success` and `data` but
**nothing about `message`** — so the live leak sat green underneath it.
"Still 500" is exactly what the defect looked like. That pin now asserts
the sentence too.
- a new section adds both halves: two crashes whose prose the heuristic
does not recognise, and the two controls one property away on either
side (a rejection keeps its `400` and its words; a crash that declared
its own status keeps both).
⚠️ **Tier measured first, in both directions, before and after.**
`packages/runtime` has two vitest projects and `vitest.repo-tests.json`
names exactly two files. The repo's own filter preflight answers:
before, `local` ran the file (17 passed) and `repo` reported `FILTER
SELECTED NOTHING ... lives in the local project`; after, `local` runs 21
and `repo` still selects nothing. The file did not move, and its 17
existing pins did not move with it. The `/data` parity is therefore
asserted in **prose** rather than by importing `@objectstack/rest` into
this file — a cross-package import is what would have moved it.
Measured (`scripts/pm/os-verify-lock.sh`, shared box):
| run | result |
|---|---|
| `pnpm --filter @objectstack/runtime test` (project `local`) | 265
files, **3665 passed**, 1 skipped — exit 0 |
| `pnpm --filter @objectstack/runtime test:repo` | 2 files, **69
passed** — exit 0 (holds `error-envelope.conformance.test.ts`) |
| `pnpm --filter @objectstack/runtime typecheck` | exit 0 |
| `pnpm --filter '@objectstack/runtime^...' build` | exit 0 |
| `pnpm lint` — repo-wide `eslint . --no-inline-config`, full
population, no narrowing | exit 0 |
### Ablation — the pin is shown to go red when the leak is put back
Fix committed first, so a restore point existed. Mutation **proven on
disk before the run**, not from an editor's exit code: the target blob
moved `5aa9ba6dd…` → `a0178be61…`, the leak anchor went 1 → 2 and the
fixed anchor 2 → 1, at a printed offset.
⚠️ Recorded because it is the point of the proof: the **first** attempt
was a silent no-op — a `perl -0777` one-liner died on a quoting error,
changed nothing, and the anchor/blob check refused the run rather than
reporting a green. The reading below is from the second, verified
mutation.
With the leak restored: **3 failed | 18 passed**, the three failures
being exactly the disclosure assertions, carrying the real leaked
strings —
```
AssertionError: expected 'Cannot read properties of undefined (…' to be 'Internal server error'
AssertionError: expected 'database disk image is malformed at /…' to be 'Internal server error'
```
The four negative controls stayed green, which is what distinguishes
this fix from a blanket sweep that would have eaten the refusal channel.
Restore verified **by blob hash** — back to `5aa9ba6dd…`, `git diff
HEAD` empty — ⛔ not by an exit code, under a `trap ... EXIT INT TERM` on
an absolute path.
### Gates
Derived with `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack`. The first derivation reported a **STALE
TREE** (3 commits behind, and 2 files it derives from had changed);
`origin/main` was merged and the list re-derived from the merged tree —
byte-identical, 59 commands. **57 of 59 exit 0.** Reconciled with every
exit code recorded.
Also run because their `silent` verdict is a fact about a roster, not
about these paths — 5 of them keep that roster in a directory one of
these paths is in: `check:route-ledger-census`,
`check:error-code-casing`, `check:authz-resolver`,
`check:filter-alias-parity`, `check-changeset-fixed`, plus
`check:error-status-conformance` and `check:dispatcher-error-vocabulary`
as the two adjacent to this change. All exit 0 — the first confirms no
status row moved, the second that the `error.code` vocabulary is intact.
⚠️ `pnpm lint` is run although `dispatch-gates.mjs` does not name the
family — this lane's standing blind spot.
**NOT MEASURED, declared rather than counted as a pass:**
- `pnpm check:dual-build-cjs-loads` — `exit 3`, `PREREQUISITE NOT MET
... some package has no dist/`, naming ~36 unbuilt packages and `Run
pnpm build first`. A whole-farm build; CI's `Build Core` + `Lint & Repo
Gates` is where it is measured. ⛔ Not a pass and not a red.
- `pnpm check:type-check-debt` first answered `exit 3` for the same
class, naming one unbuilt dependency at a time. Both were built
(`@objectstack/runtime`, then `@objectstack/driver-mongodb`) and it now
reports `--re-measure: OK — 4 ledger entr(ies) re-measured, 53 raw tsc
error(s) total, none above its recorded number` — so this one **is**
measured.
### Docs-drift rider
Predicate stated **before** reading: *a document states, about the
`/api/v1/actions` door, that an unexpected fault's client-facing message
is or contains the thrown error's own text.* Swept by symbol
(`unexpectedFault`, `errorFromThrown`, `INTERNAL_ERROR`, crash) **and**
by input shape (the wire body, and the leaked sentences themselves).
**NOT FALSIFIED.** `content/docs/api/error-catalog.mdx`'s Action Errors
row declares the status and the envelope — `Handler crashed (TypeError,
driver error, sandbox timeout)` → `500` → `{ success: false, error: {
message, code } }` — never that `message` carries the thrown text; and
its `INTERNAL_ERROR` entry already reads *"An unexpected server-side
error occurred. Report the issue with the requestId and traceId"*, which
is what a withheld sentence means. Controls both ways: the same table's
rejection rows **are** statements about the message channel, were found
by the sweep, and describe branches this PR does not touch; the three
`content/docs/**` hits for the native sentences are authoring-time
validator prose, correctly not implicated.
`content/docs/protocol/kernel/http-protocol.mdx`, which `main` moved
during this change, was re-swept and says nothing about this surface.
One in-repo statement **was** falsified and is corrected here:
`packages/runtime/src/sandbox/capability-denial-is-a-fault.test.ts` said
of a capability denial that *"what the client sees is this text, minus
the prefix"*. Measured — that denial reaches `unexpectedFault`, so after
this change the client sees `Internal server error`. Its assertions are
all about the **thrown** error and are unchanged; only the justification
is corrected to name the log as their subject.
## Changeset
`patch` on `@objectstack/runtime`. ⛔ Not `skip-changeset`: the package
publishes (`files: ["dist", ...]`) and the wire answer of a published
HTTP surface moves. `Clause-②: no` — the diff adds or removes no schema
key, closed-set member, published export or registry row, and an
unexpected fault's message text is not a contract. Verified with the
repo's own `readClause2Line`, ⛔ not a hand-written regex:
`{"kind":"declared","value":"no","arm":null,"line":"Clause-②: no"}`,
re-checked against the newer copy of that reader after the merge.
## Acceptance notes
Out of scope for this card, ⛔ not filed and ⛔ not fixed here:
- **The two doors still disagree on STATUS for a missing-relation
throw.** A `SqliteError('no such table: crm_invoice')` answers `500
INTERNAL_ERROR` at `/actions` and `404 OBJECT_NOT_FOUND` at `/data`,
whose classifier has a missing-relation attribution arm the dispatcher
has no equivalent of. Measured here as a by-product. This is a
**status** difference, and this card is explicitly fenced from moving a
status. Successor: none of the open cards named in the dispatch faces
it.
- **The undeclared-5xx band at the other dispatcher doors.** `/meta` and
`/mcp` hand everything to `errorFromThrown(e, 500)` without ever
classifying a crash, so a `TypeError` there also ships its prose. That
is not this defect: it is #5667's recorded decision to keep undeclared
5xx legible, guarded only by `looksLikeInternalErrorLeak`, and
`resolveThrownHttpError`'s own docblock records #12281 as the separate
card for the *declared* limb with its own measurement-first step.
Changing it would reverse a recorded decision. Successor: #12281 for the
declared half; the undeclared half has no card and is a decision, not an
execution.
- **A phantom-risk note, not a defect.** `deps.error`'s seam carries no
`extra`, so this exit cannot pass `userMessage` even if a fault declared
one. Reachability is near-nil (a fault that also declares user-facing
text), the card fences the widening that would close it, and `/data` has
the same gap — recorded so the next reader does not mistake it for an
oversight.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3)_
Co-authored-by: Claude <noreply@anthropic.com>
0 commit comments