Skip to content

Commit c54d4d3

Browse files
os-steveclaude
andauthored
fix(runtime): carry the producer's userMessage at the dispatcher's PERMISSION_DENIED door (#13811)
`HttpDispatcher.dispatch`'s foot catch recognises `isPermissionDeniedError` and answers the refusal itself, so a marked denial never reaches the throw-transparent exit #13241 repaired and lost its `userMessage` at this second door instead. The read is `declaredUserMessage` (`@objectstack/types`) — the one rule every boundary applies — and the mark rides as a declared top-level sibling via the `extra` bag, exactly as the sibling `errorFromThrown` carries it. #7450's withhold is untouched: `details` still carries only the ROUTE-derived object, and the gate's positions/permissionSets/cascade child stay server-side. The PR #13619 §2 carve-out pin is MOVED, not fixed green — its docblock asked for exactly that when the denial path stopped dropping the mark. Claude-Session: https://claude.ai/code/session_01UngCYXF98BVpYA9hfz6NYk Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 62a137b commit c54d4d3

4 files changed

Lines changed: 531 additions & 21 deletions

File tree

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
---
2+
'@objectstack/runtime': patch
3+
---
4+
5+
fix(runtime): carry the producer's `userMessage` at the dispatcher's PERMISSION_DENIED door (#13623)
6+
7+
`HttpDispatcher.dispatch`'s foot catch is not a pure rethrow: it recognises
8+
`isPermissionDeniedError``name === 'PermissionDeniedError'`, **or**
9+
`code === 'PERMISSION_DENIED'`, **or** a message starting `[Security] Access
10+
denied` — and answers the refusal itself from
11+
`packages/runtime/src/http-dispatcher.ts`. A marked denial therefore never
12+
reached `dispatcher-plugin`'s throw-transparent exit, which is the door #13241
13+
taught to carry the author-facing text channel, and lost the mark at this one
14+
instead. `ApiErrorSchema.userMessage` has declared the slot all along and
15+
`contract.zod.ts` states the invariant directly — *"`userMessage` on the thrown
16+
error; the boundaries carry it to the wire"* — so this is `declared ≠ enforced`
17+
at one more boundary, not a new field.
18+
19+
**Why this door matters more than its size.** #9934 made the mark
20+
status-agnostic precisely so a 403 could carry it, and a 403 is the refusal
21+
class most likely to carry deliberately-authored text ("You do not have access
22+
to this report; ask an admin for the Reporting role"). The one door that
23+
swallowed the mark was the one answering exactly those refusals.
24+
25+
**Transport parity, which the same ruling already asked for.**
26+
`@objectstack/rest`'s `mapDataError` has carried the field on this identical
27+
denial since #9934 (`withDeclaredUserMessage` over `classifyDataError`, whose
28+
`PERMISSION_DENIED` branch is pinned in
29+
`packages/rest/src/rest-user-facing-refusal-marking.test.ts`). The 2026-08-11
30+
ruling on #7450 makes REST's shape the contract for both transports; until now
31+
the dispatcher's 403 was the one that differed.
32+
33+
**#7450's disclosure withhold is untouched.** That ruling is about
34+
`error.details` — the gate's `positions` / `permissionSets` and the cascade
35+
child `object` the caller never addressed — and all of it is still dropped from
36+
the wire and still logged server-side. `userMessage` is the opposite by
37+
construction: it exists only because a producer wrote text *for* the caller,
38+
platform and driver code never set it, and it rides as a declared top-level
39+
sibling of `code`/`message`, never inside `details`. The read is
40+
`declaredUserMessage` (`@objectstack/types`), the one rule every boundary
41+
applies, so this door cannot fork its own answer to "what counts as marked".
42+
43+
**No behaviour change for an unmarked denial.** Its envelope is byte-identical
44+
to before — key set `code` / `details` / `httpStatus` / `message`, pinned — and
45+
the mark never moves the status or the `code`.

packages/runtime/src/dispatcher-plugin.declared-user-message.test.ts

Lines changed: 67 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,14 @@
1919
* — whose result already carries `userMessage` — and then did not read the
2020
* field. `declared ≠ enforced` at exactly one door.
2121
*
22+
* ⭐ [#13623] There was a SECOND door, and this file is where it was recorded:
23+
* `HttpDispatcher.dispatch`'s foot catch answers `isPermissionDeniedError`
24+
* itself, so a marked `PERMISSION_DENIED` 403 never reaches the exit above and
25+
* lost its mark there instead. §2's carve-out row pinned that absence as an
26+
* observation; #13623 closed the door and MOVED the row to assert presence —
27+
* see its docblock for why moving it (rather than fixing it green) is the
28+
* deliverable.
29+
*
2230
* ## Why this is the compensation channel and not a decoration
2331
*
2432
* The 2026-08-27 ruling on #12509 (option D), propagated to #12281, made this
@@ -225,7 +233,10 @@ describe('[#13241] the dispatcher throw-transparent exit carries `userMessage`',
225233
{ status: 400, code: 'VALIDATION_FAILED', prose: 'measure revenue not additive over stage', mark: 'Pick a measure that adds up across stages.' },
226234
// ⚠️ `FORBIDDEN`, deliberately NOT `PERMISSION_DENIED` — see the
227235
// carve-out pinned below this table: a throw spelling
228-
// `PERMISSION_DENIED` never arrives at this exit at all.
236+
// `PERMISSION_DENIED` never arrives at this exit at all. It is
237+
// answered by `HttpDispatcher.dispatch`'s foot catch, which since
238+
// #13623 carries the mark itself; this row keeps a status the foot
239+
// catch does NOT intercept, so it still measures THIS exit's 403.
229240
{ status: 403, code: 'FORBIDDEN', prose: 'principal lacks cube_read on pipeline', mark: 'You do not have access to this report. Ask an admin for the Reporting role.' },
230241
{ status: 409, code: 'RECORD_LOCKED', prose: 'cube pipeline locked by in-flight rebuild', mark: 'This report is being rebuilt. Try again shortly.' },
231242
{ status: 503, code: 'SERVICE_UNAVAILABLE', prose: 'warehouse pool exhausted', mark: 'Reporting is briefly offline.' },
@@ -247,25 +258,37 @@ describe('[#13241] the dispatcher throw-transparent exit carries `userMessage`',
247258
}
248259

249260
/**
250-
* ⚠️ MEASURED CARVE-OUT — the one 403 that is NOT throw-transparent, and
251-
* therefore NOT closed by this change.
261+
* ⭐ [#13623] THE CARVE-OUT MOVED — and this row is how a reader finds out.
262+
*
263+
* It was written as the opposite assertion (`expectNoUserMessageAnywhere`)
264+
* with a docblock saying, in terms: *"if a later change makes the denial
265+
* path throw-transparent, this test fails and tells the author that the
266+
* carve-out has moved"*. #13623 is that change, so the row is MOVED
267+
* deliberately rather than repaired green — the instrument worked, and
268+
* softening or deleting it would have thrown away the only signal.
269+
*
270+
* ⚠️ What moved is the MARK, not the route. The sentence the old
271+
* docblock opened with is still true: `HttpDispatcher.dispatch`'s foot
272+
* catch is not a pure rethrow — it recognises `isPermissionDeniedError`
273+
* (`name === 'PermissionDeniedError'` **or** `code === 'PERMISSION_DENIED'`
274+
* **or** a message starting `[Security] Access denied`) and answers it
275+
* itself via `this.error(…)` in
276+
* `packages/runtime/src/http-dispatcher.ts`. Such a throw still never
277+
* reaches `errorResponseBase`. What changed is that the OTHER door now
278+
* reads the same `declaredUserMessage` rule, so the mark survives both.
252279
*
253-
* `HttpDispatcher.dispatch`'s foot catch is not a pure rethrow: it
254-
* intercepts `isPermissionDeniedError` — `name === 'PermissionDeniedError'`
255-
* **or** `code === 'PERMISSION_DENIED'` **or** a message starting
256-
* `[Security] Access denied` — and answers it itself, via `this.error(…)`
257-
* in `packages/runtime/src/http-dispatcher.ts`. Such a throw never
258-
* reaches `errorResponseBase`, so its mark is dropped by a DIFFERENT
259-
* door than the one this card repairs.
280+
* ⇒ The row therefore pins a DIFFERENT door's behaviour from every other
281+
* row in this file, and is kept here on purpose: the two doors' answers
282+
* to one question belong in one place, and if either regresses the
283+
* failure names the door in its own title.
260284
*
261-
* ⛔ That door is #7898's on-hold trigger file and is out of this PR's
262-
* file surface, so the gap is recorded here rather than closed. Pinned
263-
* as an observation and not as a wish: if a later change makes the
264-
* denial path throw-transparent, this test fails and tells the author
265-
* that the carve-out has moved — which is the only way a reader finds
266-
* out that the sentence above went stale.
285+
* ⛔ Still NOT a claim that this exit became throw-transparent. A
286+
* regression that made it so would silently satisfy this row; the pin
287+
* against that is `expect(res.body.error.details)` below — the foot
288+
* catch derives `details.object` from the ROUTE (`permissionDeniedErrorDetails`)
289+
* and `errorResponseBase` does nothing of the kind.
267290
*/
268-
it('CARVE-OUT — a `PERMISSION_DENIED` 403 does not reach this exit, so its mark is still dropped', async () => {
291+
it('MOVED CARVE-OUT — a `PERMISSION_DENIED` 403 is answered by the foot catch, and now keeps its mark there', async () => {
269292
const res = await throwFromAnalyticsQuery(
270293
declaring(
271294
{ status: 403, code: 'PERMISSION_DENIED', userMessage: 'Ask an admin for the Reporting role.' },
@@ -275,8 +298,33 @@ describe('[#13241] the dispatcher throw-transparent exit carries `userMessage`',
275298

276299
expect(res.statusCode).toBe(403);
277300
expect(res.body.error.code).toBe('PERMISSION_DENIED');
278-
// Not a claim that dropping it is CORRECT — only that it is where
279-
// the field is dropped today, and that it is not this exit.
301+
// The half this card repaired: the author's channel now survives
302+
// the denial door too (#9934 is status-agnostic, and 403 is the
303+
// refusal class most likely to carry authored text).
304+
expect(res.body.error.userMessage).toBe('Ask an admin for the Reporting role.');
305+
// …verbatim, and never in place of the diagnostic channel.
306+
expect(JSON.stringify(res.body)).toContain('Ask an admin for the Reporting role.');
307+
expect(res.body.error.message).not.toBe('Ask an admin for the Reporting role.');
308+
// Still the foot catch answering, NOT this exit — the route-derived
309+
// `details` shape only that door produces. `/analytics/query` names
310+
// no object, so `permissionDeniedErrorDetails` contributes nothing
311+
// but the promoted `code`, and `details` is absent entirely.
312+
expect(res.body.error.details).toBeUndefined();
313+
// ⛔ #7450's withhold is untouched: the mark is a top-level sibling,
314+
// never `details` context.
315+
expect((res.body.error.details as any)?.userMessage).toBeUndefined();
316+
});
317+
318+
it('POSITIVE CONTROL (the denial door) — an UNMARKED `PERMISSION_DENIED` 403 still has no key', async () => {
319+
// Without this, the row above could pass on a door that always
320+
// reports a `userMessage`. Same route, same code, same status — the
321+
// ONLY difference is the mark.
322+
const res = await throwFromAnalyticsQuery(
323+
declaring({ status: 403, code: 'PERMISSION_DENIED' }, 'principal lacks cube_read on pipeline'),
324+
);
325+
326+
expect(res.statusCode).toBe(403);
327+
expect(res.body.error.code).toBe('PERMISSION_DENIED');
280328
expectNoUserMessageAnywhere(res);
281329
});
282330

0 commit comments

Comments
 (0)