Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions .changeset/share-user-message-bypass-exits.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
---
"@objectstack/rest": patch
---

fix(rest): the record-share family carries a producer-marked `userMessage` on its two non-classified exits (#12693)

`respondSharingError` learned to carry the producer's caller-facing sentence at
its classified re-dress (#12669 fork (a)). The family has **two other exits**
that never reach that classification and still dropped it:

- the **500 fault terminal** (`SHARES_LIST_FAILED` / `SHARE_GRANT_FAILED` /
`SHARE_REVOKE_FAILED`) — `classifiedRefusalAnswer` deliberately hands a
declared or resolved 5xx back to "the catching route's own terminal", so a
marked fault never had a classification to ride;
- the **ADR-0111 message-prefix arm** — it runs precisely when the
classification answered nothing.

Measured on `15bf9e859` before the repair, one marked producer per exit driven
through the real routes on both doors:

```text
throw { code: 'SHARE_STORE_DOWN', status: 503, userMessage: '…' }
share door : 500 SHARES_LIST_FAILED — no mark
/data door : 503 SERVICE_UNAVAILABLE — mark carried
throw Error('NOT_FOUND: no such record …') + userMessage
share door : 404 NOT_FOUND — no mark
/data door : 500 INTERNAL_ERROR — mark carried
```

Nothing invalid shipped — every body parsed as `ApiErrorSchema` — which is what
made the loss silent and one-directional: a console told by ADR-0112 to render
`userMessage` verbatim found nothing at these two exits and fell back to its
generic substitution, for the same throw the twin door rendered.

Neither exit holds a `refusal.body`, so the classified arm's expression is not
reusable at either. `error-response.ts` now exports
`boundedDeclaredUserMessage` — `declaredUserMessage`'s presence answer with
#5423's bound applied, lifted out of the private `withDeclaredUserMessage`
wrapper so a caller with no body to merge into can ask the same rule rather
than open-code it. The flat `/data` door is unchanged and goes on calling it
through that wrapper.

⛔ Only the mark is added. Every existing key keeps its value and position at
both doors (measured: 32 route/door answers before and after, 0 statuses moved,
0 existing keys moved or changed, 24 gaining exactly `userMessage`). The three
deliberate share-vs-`/data` differences visible in the same measurement stay
exactly as they are — the family still folds a declared `503` into its own
`500`, still interpolates the caught message where `/data` withholds 5xx prose
per #5437, and `/data` is still not taught this service's local prefix idiom.

No in-tree producer sets `userMessage` at this seam today
(`plugin-sharing` = 0 hits; positive control `throw ` = 25 files), so this
wires a declared channel rather than repairing a live loss.
39 changes: 37 additions & 2 deletions packages/rest/src/error-response.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -566,9 +566,44 @@ function withDeclaredUserMessage(
error: any,
mapped: { status: number; body: Record<string, unknown> },
): { status: number; body: Record<string, unknown> } {
const userMessage = declaredUserMessage(error);
const userMessage = boundedDeclaredUserMessage(error);
if (userMessage === undefined) return mapped;
return { status: mapped.status, body: { ...mapped.body, userMessage: truncateClientMessage(userMessage) } };
return { status: mapped.status, body: { ...mapped.body, userMessage } };
}

/**
* [#12693] The wire VALUE of {@link withDeclaredUserMessage}'s rule: the
* sentence a producer marked, with #5423's bound already applied — or
* `undefined` when it marked none.
*
* The rule itself is unchanged and still stated once, in the docblock above:
* `declaredUserMessage` (`@objectstack/types`) decides PRESENCE and
* {@link truncateClientMessage} decides the BOUND. This is that same pair
* lifted out of the body-merging wrapper so a caller that has no body to merge
* into can ask it.
*
* ## Why an export rather than the wrapper
*
* The record-share family in `rest-server.ts` has two exits that never reach
* {@link classifiedRefusalAnswer} — its 500 fault terminal, and its ADR-0111
* message-prefix arm — and therefore hold no classification `{ status, body }`
* to ride the mark onto. They hold the raw thrown error and build their
* envelope by hand. Handing them the wrapper would mean inventing a flat body
* for them to merge into and then unpicking it, and open-coding
* `declaredUserMessage(error)` at the exits instead would leave #5423's bound
* applied at some marks and not others — a per-exit answer to a question this
* file already owns, which is the drift {@link classifiedRefusalAnswer}'s own
* docblock was written against.
*
* ⛔ What this does NOT decide is the ENVELOPE, and that stays true for every
* future caller: it answers a string, and where that string lands — the flat
* body's top level, the nested ADR-0112 `ApiError.userMessage` — is the
* caller's dialect decision, exactly as #9232 keeps vocabulary and position
* apart.
*/
export function boundedDeclaredUserMessage(error: unknown): string | undefined {
const userMessage = declaredUserMessage(error);
return userMessage === undefined ? undefined : truncateClientMessage(userMessage);
}

function classifyDataError(error: any, object?: string): { status: number; body: Record<string, unknown> } {
Expand Down
74 changes: 71 additions & 3 deletions packages/rest/src/rest-server.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -206,6 +206,7 @@ import {
mapDataError,
sandboxBusinessMessage,
classifiedRefusalAnswer,
boundedDeclaredUserMessage,
declaredServerFaultAnswer,
sendThrownError,
sendDeclaredFault,
Expand DownExpand Up@@ -9594,6 +9595,59 @@ export class RestServer {
res, 501, 'NOT_IMPLEMENTED',
'Sharing service is not configured on this deployment',
);
/**
* [#12693] The `extra` this family's TWO NON-CLASSIFIED exits owe a
* producer that marked its refusal: the 500 fault terminal below, and
* the ADR-0111 prefix arm inside {@link respondSharingError}.
*
* Neither reaches {@link classifiedRefusalAnswer} — the 500 terminal
* because a declared server fault is deliberately not a refusal
* (that function's own docblock rules it back to "the catching route's
* own terminal", which is these three arms), the prefix arm because it
* runs precisely when the classification answered `undefined`. So
* neither holds a `refusal.body` to re-dress, and the one line the
* classified arm uses (`refusal.body.userMessage`, #12669) is NOT
* reusable here. What is reusable is the RULE, and
* {@link boundedDeclaredUserMessage} is that rule asked of the raw
* thrown error instead of the classification: `declaredUserMessage`'s
* presence answer with #5423's bound applied, one definition, shared
* with the `/data` door rather than copied beside it.
*
* Measured on `15bf9e859` before the repair, one producer per exit
* through the real routes on both doors:
*
* ```text
* throw { code: 'SHARE_STORE_DOWN', status: 503, userMessage: '…' }
* share door : 500 SHARES_LIST_FAILED — no mark
* /data door : 503 SERVICE_UNAVAILABLE — mark carried
* throw Error('NOT_FOUND: no such record …') + userMessage
* share door : 404 NOT_FOUND — no mark
* /data door : 500 INTERNAL_ERROR — mark carried
* ```
*
* ⛔ The mark is the ONLY thing this adds, and the two doors' other
* disagreements visible in that measurement stay exactly as they are:
* the share family folds a declared 503 into its own 500 terminal, and
* it interpolates the caught message where `/data` withholds 5xx prose
* unconditionally (#5437). Both are deliberate and argued in
* {@link sharingFaultMessage} and the #11683 docblock below; the
* `/data` door's status for the prefix arm differs for a third
* deliberate reason — the prefix idiom is this service's local
* convention and no shared classifier can read it (ADR-0111).
*
* ⛔ Riding the mark across a FAULT terminal is not a re-opening of
* #5437 either, and the reason is `withDeclaredUserMessage`'s, not a
* second one: the withheld text is prose the producer never addressed
* to the caller, while this field exists only because an author wrote
* caller-facing text onto it. A genuine crash carries no mark and its
* envelope is byte-identical to before.
*/
const sharingDeclaredExtra = (
error: any,
): { userMessage: string } | undefined => {
const userMessage = boundedDeclaredUserMessage(error);
return userMessage === undefined ? undefined : { userMessage };
};
// [ADR-0111] The service enforces authorization (D1/D4/D5/D7) and
// signals the verdict via message prefixes, the plugin's established
// error idiom — this maps them onto HTTP. Returns true when handled.
Expand DownExpand Up@@ -9768,9 +9822,14 @@ export class RestServer {
];
for (const [code, status] of map) {
if (msg.startsWith(code)) {
// [#12693] …and the producer's own sentence to the caller
// rides this arm too. ⛔ Only the sentence: the PREFIX
// read, the status it decides and the stripping below are
// untouched — see {@link sharingDeclaredExtra}.
respondError(
res, status, code,
msg.replace(new RegExp(`^${code}:\\s*`), ''),
sharingDeclaredExtra(error),
);
return true;
}
Expand DownExpand Up@@ -9823,7 +9882,10 @@ export class RestServer {
// The 500 arms keep their 500-char cap: an unexpected
// fault's message is not a contract, and truncating it
// stays a sanitization step — only the position moves.
respondError(res, 500, 'SHARES_LIST_FAILED', sharingFaultMessage(error));
respondError(
res, 500, 'SHARES_LIST_FAILED', sharingFaultMessage(error),
sharingDeclaredExtra(error),
);
}
},
metadata: { summary: 'List per-record sharing grants', tags: ['sharing'] },
Expand DownExpand Up@@ -9857,7 +9919,10 @@ export class RestServer {
} catch (error: any) {
if (respondSharingError(res, error)) return;
logError('[REST] Grant share error:', error);
respondError(res, 500, 'SHARE_GRANT_FAILED', sharingFaultMessage(error));
respondError(
res, 500, 'SHARE_GRANT_FAILED', sharingFaultMessage(error),
sharingDeclaredExtra(error),
);
}
},
metadata: { summary: 'Grant a per-record share to a principal', tags: ['sharing'] },
Expand DownExpand Up@@ -9886,7 +9951,10 @@ export class RestServer {
} catch (error: any) {
if (respondSharingError(res, error)) return;
logError('[REST] Revoke share error:', error);
respondError(res, 500, 'SHARE_REVOKE_FAILED', sharingFaultMessage(error));
respondError(
res, 500, 'SHARE_REVOKE_FAILED', sharingFaultMessage(error),
sharingDeclaredExtra(error),
);
}
},
metadata: { summary: 'Revoke a per-record share by id', tags: ['sharing'] },
Expand Down
Loading
Loading