Skip to content

Commit d6bc477

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-18733-combined-status-pending-trap
2 parents 55c8e84 + 447e2e8 commit d6bc477

10 files changed

Lines changed: 453 additions & 51 deletions
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
---
2+
"@objectstack/runtime": patch
3+
---
4+
5+
fix(runtime): a NON-sandboxed crash at `/api/v1/actions` no longer ships its native error message verbatim (#18540)
6+
7+
Clause-②: no
8+
9+
A plain `TypeError` thrown by an in-process registered action handler answered
10+
`500 INTERNAL_ERROR` carrying the native sentence on the wire:
11+
12+
```
13+
{"success":false,"error":{"code":"INTERNAL_ERROR",
14+
"message":"Cannot read properties of undefined (reading 'id')","httpStatus":500}}
15+
```
16+
17+
The identical crash through the `/data` door answered `"Internal server error"`
18+
(#7543 / #15071). One repository, two doors, one already meeting the contract.
19+
20+
**The status was already right; what leaked was the sentence.** No status code,
21+
no `error.code` and no envelope key moves — reaching this branch already proves
22+
the throw declared no `status`/`statusCode` (the branch above serves those) and
23+
is not a `ValidationError`, so the resolver's status was the 500 fallback and its
24+
code was the status-derived `INTERNAL_ERROR`. Only `error.message` changes.
25+
26+
**Why neither existing guard caught it.** #17273's crash terminal is keyed on the
27+
SANDBOX — `isNativeErrorName` read over the `innerMessage` the QuickJS runner
28+
fills — and this face never crosses a VM boundary, so nothing sets `innerMessage`
29+
and that terminal never fires. *A predicate that classifies by HOW a crash
30+
arrived is structurally blind to crashes that did not arrive that way, while
31+
looking exhaustive.* The other guard, the dispatcher's 5xx withhold, is gated on
32+
`looksLikeInternalErrorLeak`, which recognises DRIVER DUMPS and reads FALSE for
33+
stack-shaped prose.
34+
35+
**The structural difference, which is the fix.** The `/data` door is default-DENY:
36+
`classifyDataError` ends in an unconditional `UNCLASSIFIED_FAULT()`, and its
37+
`looksLikeInternalErrorLeak` limb only picks `DATABASE_ERROR` over
38+
`INTERNAL_ERROR` — that limb is not what sanitises. The actions door's
39+
`unexpectedFault` exit relayed `err.message` and was therefore default-ALLOW:
40+
prose shipped unless a heuristic recognised it. That exit is this door's
41+
unclassified-fault terminal, so it now answers the terminal's envelope —
42+
`INTERNAL_ERROR_MESSAGE`, through the same `deps.error` seam #17273's terminal
43+
uses.
44+
45+
`looksLikeInternalErrorLeak` is NOT re-pointed at stack-shaped prose. It guards
46+
a different question at every other boundary, and widening it would change what
47+
each of them withholds.
48+
49+
**Measured population.** Driven through the real `HttpDispatcher.handleActions`
50+
door against `mapDataError` on the same throws: seven shapes leaked at `/actions`
51+
and were already sanitised at `/data``TypeError`, `ReferenceError`,
52+
`RangeError`, `SyntaxError`, a driver class whose prose the heuristic does not
53+
recognise (this one shipped a server **filesystem path**), a sandbox timeout and
54+
a sandbox capability denial. All seven now answer the same sentence at both
55+
doors. Two controls are unchanged in both directions: a deliberate rejection
56+
keeps its `400` and its own words, and a crash that DECLARED its own status keeps
57+
that status and that sentence.
58+
59+
**Who is affected.** Any caller reading `error.message` off a `500` from
60+
`/api/v1/actions` to tell one crash from another. That text was never a contract
61+
— it is the thrown error's own prose — and the full text still reaches the
62+
operator: the `console.error` on the line above keeps it, the same
63+
"the client does not read it, the log keeps it" split `rest` already draws.

packages/core/src/security/resolve-authz-context.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1601,7 +1601,7 @@ describe('the in-memory ObjectQL double honours `limit` (#10978)', () => {
16011601
*
16021602
* ## What was measured, before the guard existed
16031603
*
1604-
* On a live `isolated` boot with the real cloud-private `Organizations` plugin
1604+
* On a live `isolated` boot with the real licence-gated `Organizations` plugin
16051605
* and a file-backed sqlite store, a session whose owner had been removed
16061606
* through better-auth's OWN `/organization/remove-member` — driven by the org
16071607
* owner, 200, the `sys_member` row really deleted — went on READING that

packages/core/src/security/resolve-authz-context.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -480,7 +480,7 @@ export async function resolveAuthzContext(input: ResolveAuthzInput): Promise<Res
480480
// The block above asks "is this stamped organization still backed by a
481481
// membership?" and, until this card, asked it ONLY of an API key. A browser
482482
// session's `activeOrganizationId` reached `ctx.tenantId` unread: measured on
483-
// a live `isolated` boot with the real cloud-private `Organizations` plugin,
483+
// a live `isolated` boot with the real licence-gated `Organizations` plugin,
484484
// a session whose owner had been removed through better-auth's OWN
485485
// `/organization/remove-member` (driven by the org owner, 200, the
486486
// `sys_member` row really deleted) went on READING that organization's rows

packages/rest/src/rest-api-plugin.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -297,9 +297,9 @@ export function createRestApiPlugin(config: RestApiPluginConfig = {}): Plugin {
297297
// organization's rows (GET 200) and WROTE a new one into it (POST
298298
// 201, the row read back from the store carrying the other
299299
// organization's id). objectstack#15163 measured it on the
300-
// framework; cloud#1982 reproduced it with the real, cloud-private
301-
// `@objectstack/organizations` mounted, which adds no request-time
302-
// refusal of its own.
300+
// framework; cloud#1982 reproduced it with the real, licence-gated
301+
// `@objectstack/organizations` subclass mounted, which adds no
302+
// request-time refusal of its own.
303303
//
304304
// ## Why this is NOT `authServiceProvider`'s catch-all
305305
//

packages/rest/src/single-kernel-isolated-api-key-matrix.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,10 @@
1818
* | organization-less key | **200 total 0 (silent) · 403** | **401** |
1919
*
2020
* objectstack#15163 measured it on the framework; cloud#1982 reproduced it on
21-
* `apps/objectos-ee` with the REAL cloud-private `@objectstack/organizations`
22-
* mounted, reading the written row back out of the sqlite file — the enterprise
23-
* plugin adds no request-time refusal, so the blast radius was every walled
24-
* deployment.
21+
* `apps/objectos-ee` with the REAL licence-gated `@objectstack/organizations`
22+
* subclass mounted, reading the written row back out of the sqlite file — the
23+
* enterprise plugin adds no request-time refusal, so the blast radius was
24+
* every walled deployment.
2525
*
2626
* ## Why the fixture is shaped the way it is
2727
*

packages/rest/src/single-kernel-isolated-session-org-claim-matrix.test.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,10 @@
88
* ## What was measured, before this guard existed
99
*
1010
* On a real `objectstack serve` of cloud's `apps/objectos-ee` — 44 plugins, the
11-
* REAL cloud-private `@objectstack/organizations`, `Tenancy: isolated`,
12-
* `SqlDriver(better-sqlite3)` on a FILE — a browser session whose
13-
* `activeOrganizationId` pointed at an organization its owner had LEFT:
11+
* REAL licence-gated `@objectstack/organizations` subclass,
12+
* `Tenancy: isolated`, `SqlDriver(better-sqlite3)` on a FILE — a browser
13+
* session whose `activeOrganizationId` pointed at an organization its owner
14+
* had LEFT:
1415
*
1516
* | after the membership ended | GET | POST | the row, read back from the sqlite file |
1617
* |:--|:--|:--|:--|

packages/runtime/src/domains/actions-fault-vs-rejection.test.ts

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,11 @@ describe('an unexpected FAULT is a 500', () => {
145145
// No `{success:true, data:{...}}` wrapper — this is the dispatcher's
146146
// error exit, so monitoring sees a 5xx.
147147
expect(response.body.data).toBeUndefined();
148+
// [#18540] …and the SENTENCE is withheld. This pin asserted the three
149+
// lines above and nothing about `message`, so the live leak sat green
150+
// underneath it — "still 500" is exactly what the defect looked like.
151+
// The disclosure half is pinned in full in its own section below.
152+
expect(response.body.error.message).toBe('Internal server error');
148153
});
149154

150155
it('a ReferenceError from a buggy handler', async () => {
@@ -314,3 +319,101 @@ describe('[#17273] a sandboxed body that CRASHED is a fault, not a rejection', (
314319
expect(response.body.error.message).toBe('Import failed with a TypeError in row 4');
315320
});
316321
});
322+
323+
/**
324+
* [#18540] A NON-sandboxed crash's native sentence is withheld — the face of
325+
* this question that needs no sandbox at all.
326+
*
327+
* #17273 put a crash terminal above every branch of this door that reads a
328+
* producer declaration as intent, but its predicate is keyed on the SANDBOX:
329+
* `isNativeErrorName` over the `innerMessage` the QuickJS runner fills. A plain
330+
* `TypeError` from an in-process registered handler never crosses a VM
331+
* boundary, so nothing sets `innerMessage`, that terminal never fires, and the
332+
* throw fell to `unexpectedFault` → `errorFromThrown`, which relays
333+
* `err.message`. Measured on the wire before this change:
334+
*
335+
* {"success":false,"error":{"code":"INTERNAL_ERROR",
336+
* "message":"Cannot read properties of undefined (reading 'id')","httpStatus":500}}
337+
*
338+
* The same crash through the `/data` door answered `"Internal server error"`
339+
* (#7543 / #15071). ⇒ the status was already right; what leaked was the
340+
* sentence.
341+
*
342+
* **The shape worth carrying: a predicate that classifies by HOW a crash
343+
* arrived is structurally blind to crashes that did not arrive that way —
344+
* while looking exhaustive.** Same family as a ratchet with no row for the
345+
* case, and as a slot whose third consumer nobody reached.
346+
*
347+
* The other guard misses it for a second, independent reason, and that is why
348+
* the fix is not a new phrasing: the dispatcher's 5xx withhold is gated on
349+
* `looksLikeInternalErrorLeak`, which recognises DRIVER DUMPS and reads FALSE
350+
* for stack-shaped prose. So the relay is DEFAULT-ALLOW, while `/data` is
351+
* default-DENY — `classifyDataError` ends in an unconditional
352+
* `UNCLASSIFIED_FAULT()`, and its `looksLikeInternalErrorLeak` limb only picks
353+
* `DATABASE_ERROR` over `INTERNAL_ERROR`. The fix answers this door's terminal
354+
* with the terminal's envelope; ⛔ it does not re-point the heuristic, which
355+
* guards a different question at every other boundary.
356+
*
357+
* The cases below are BOTH halves, because a disclosure pin that only asserts
358+
* the withheld case cannot tell a fix from a blanket sweep that ate the
359+
* refusal channel: two crashes whose text the heuristic does NOT recognise,
360+
* then the two controls one property away on either side. The `/data` parity
361+
* is deliberately asserted in prose rather than by importing
362+
* `@objectstack/rest` here — a cross-package import would move this file, and
363+
* the pins above it, into the `repo` vitest project.
364+
*/
365+
describe('[#18540] a NON-sandboxed crash is answered with the sanitised sentence', () => {
366+
it("the card's exact repro — a bare TypeError, no sandbox anywhere in the path", async () => {
367+
const response = await invoke(new TypeError("Cannot read properties of undefined (reading 'id')"));
368+
369+
// Unmoved: the status and the code were already right, and this card is
370+
// fenced from touching them.
371+
expect(response.status).toBe(500);
372+
expect(response.body.error.code).toBe('INTERNAL_ERROR');
373+
// The whole of the change: the sentence, matching what `/data` answers.
374+
expect(response.body.error.message).toBe('Internal server error');
375+
expect(String(response.body.error.message)).not.toContain('Cannot read properties');
376+
expect(response.body.success).toBe(false);
377+
});
378+
379+
it('a fault whose prose the leak heuristic does NOT recognise — a driver class naming a FILE PATH', async () => {
380+
// `looksLikeInternalErrorLeak` reads FALSE here: no SQL keyword, no
381+
// dialect template, nothing quoted. Before this change the tenant
382+
// received a server filesystem path. This case is why the fix cannot be
383+
// "teach the heuristic about TypeError" — the leaking population is not
384+
// a phrasing family, it is everything the terminal was relaying.
385+
const err: any = new Error('database disk image is malformed at /srv/data/tenant_42.db');
386+
err.name = 'SqliteError';
387+
const response = await invoke(err);
388+
389+
expect(response.status).toBe(500);
390+
expect(response.body.error.message).toBe('Internal server error');
391+
expect(String(response.body.error.message)).not.toContain('/srv/data');
392+
});
393+
394+
it('negative control: a deliberate rejection keeps its 400 AND its own sentence', async () => {
395+
// One `name` away from the first case. An implementation that withheld
396+
// every message at this catch — or that moved the fault terminal above
397+
// the rejection exit — would turn the cases above green while deleting
398+
// the channel a business rule speaks through.
399+
const response = await invoke(new Error('Lead is already converted'));
400+
401+
expect(response.status).toBe(400);
402+
expect(response.body.error.message).toBe('Lead is already converted');
403+
});
404+
405+
it('negative control: a crash that DECLARED its own status keeps that status and that sentence', async () => {
406+
// The branch serving `.status` sits above `unexpectedFault`, so this
407+
// throw never reaches the terminal. It is the control for the fence on
408+
// this card: ⛔ no declared status moves, and a producer that composed
409+
// an answer still speaks.
410+
const err: any = new TypeError('Not allowed');
411+
err.status = 403;
412+
err.code = 'FORBIDDEN';
413+
const response = await invoke(err);
414+
415+
expect(response.status).toBe(403);
416+
expect(response.body.error.code).toBe('FORBIDDEN');
417+
expect(response.body.error.message).toBe('Not allowed');
418+
});
419+
});

packages/runtime/src/domains/actions.ts

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -969,7 +969,58 @@ export async function handleActionsRequest(deps: DomainHandlerDeps, path: string
969969
&& !validationFailureDetails(err);
970970
if (unexpectedFault) {
971971
console.error(`[action ${objectName}/${actionName}] unexpected fault (${name}): ${full}`);
972-
return { handled: true, response: deps.errorFromThrown(err, 500) };
972+
// [#18540] This branch is this door's UNCLASSIFIED-FAULT TERMINAL, so
973+
// it answers the terminal's envelope — `INTERNAL_ERROR_MESSAGE`, the
974+
// same sentence `/data` answers — instead of relaying `err.message`.
975+
//
976+
// The status was already right; what leaked was the SENTENCE. A
977+
// non-sandboxed crash — a plain `TypeError` from an in-process
978+
// registered handler — reached the wire as `500 INTERNAL_ERROR`
979+
// carrying `Cannot read properties of undefined (reading 'id')`
980+
// verbatim, while the IDENTICAL throw through the `/data` door
981+
// answered `Internal server error`.
982+
//
983+
// Why neither existing guard caught it, both measured on this tree:
984+
//
985+
// - #17273's crash terminal above is keyed on the SANDBOX
986+
// (`isNativeErrorName` over the `innerMessage` the QuickJS runner
987+
// fills). This face never crosses a VM boundary, so nothing sets
988+
// `innerMessage` and that terminal never fires. A predicate that
989+
// classifies by HOW a crash arrived is structurally blind to
990+
// crashes that did not arrive that way — while looking exhaustive.
991+
// - `errorFromThrown` relays `err.message` through the dispatcher's
992+
// 5xx withhold, which is gated on `looksLikeInternalErrorLeak` —
993+
// a DRIVER-DUMP heuristic that reads FALSE for stack-shaped prose
994+
// (#17273's changeset records the same reading). So that relay is
995+
// DEFAULT-ALLOW: prose ships unless the heuristic recognises it.
996+
//
997+
// `/data` is default-DENY by construction: `classifyDataError` ends
998+
// in an unconditional `UNCLASSIFIED_FAULT()`, and its
999+
// `looksLikeInternalErrorLeak` limb only chooses `DATABASE_ERROR`
1000+
// over `INTERNAL_ERROR` — that limb is not what sanitises. Aligning
1001+
// therefore means answering the terminal here too, ⛔ never teaching
1002+
// the heuristic a new phrasing: re-pointing `looksLikeInternalErrorLeak`
1003+
// at stack-shaped prose would change what every OTHER boundary
1004+
// withholds, and it guards a different question.
1005+
//
1006+
// Nothing about the ANSWER moves but the sentence. Reaching here
1007+
// already proves `.status`/`.statusCode` are absent (the branch above
1008+
// serves them) and that this is not a `ValidationError`, so
1009+
// `resolveThrownHttpError` had no declared status either: its `status`
1010+
// was the 500 fallback and its `code` was
1011+
// `standardErrorCodeForHttpStatus(500)` — `INTERNAL_ERROR`, the code
1012+
// this exit emits. Same status, same code, same envelope shape.
1013+
//
1014+
// ⛔ Deliberately the SAME `deps.error` seam #17273's terminal uses,
1015+
// never a widened one: a fault's `userMessage` and its non-string
1016+
// `details.code` (a driver errno — the backend-naming disclosure
1017+
// `demotedDeclaredCode` already withholds on an undeclared 5xx) do not
1018+
// ride this exit, exactly as they do not ride `/data`'s.
1019+
//
1020+
// The words are not lost: the `console.error` above keeps the full
1021+
// text — the same "the client does not read it, the log keeps it"
1022+
// split #5437 draws in `rest`.
1023+
return { handled: true, response: deps.error(INTERNAL_ERROR_MESSAGE, 500) };
9731024
}
9741025

9751026
// [#3962] A deliberate REJECTION is a 400. The 200-with-inner-envelope

packages/runtime/src/sandbox/capability-denial-is-a-fault.test.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -67,12 +67,16 @@ describe('[#4431] an in-VM capability denial reaches the classifier as a FAULT',
6767
// invisible to gateway error rates, APM and alerting.
6868
expect(err.innerMessage).toBeUndefined();
6969

70-
// The debug prefix must not reach the client. The runner's own doc says
71-
// only the business message should — and a sandbox fault has none, so what
72-
// the client sees is this text, minus the prefix.
70+
// The debug prefix must not reach the OPERATOR'S LOG line either. Every
71+
// assertion in this block is about the THROWN error — what the runner hands
72+
// the classifier and what `domains/actions.ts` prints — never about the wire.
73+
// [#18540] Since the door's unexpected-fault terminal answers
74+
// `INTERNAL_ERROR_MESSAGE`, what the CLIENT sees for this denial is
75+
// `Internal server error`, the same sentence `/data` answers; this text is
76+
// the diagnostic half of that split, and it has to stay readable.
7377
expect(err.message).not.toContain('SandboxError:');
74-
// …while the actionable content survives: which capability, whose, and the
75-
// call that tripped the gate.
78+
// …while the actionable content survives for the log: which capability,
79+
// whose, and the call that tripped the gate.
7680
expect(err.message).toContain("capability 'api.read' not granted");
7781
expect(err.message).toContain("action 'rc1_crash_probe'");
7882
expect(err.message).toContain("ctx.api.object('showcase_task').count");

0 commit comments

Comments
 (0)