Skip to content

Commit e79107a

Browse files
os-trumpclaude
andauthored
test(rest): pin the approvals FORBIDDEN → 403 row here too — and correct the record that it was unpinned (#14838)
* test(rest): pin the approvals FORBIDDEN → 403 row's live emission `handleApprovalError`'s `[/^FORBIDDEN/, 403, 'FORBIDDEN']` row is the one every authorisation refusal rides, and it had no live-emission pin: the service suites assert the `FORBIDDEN:` message prefix at the throw site, which is a different fact from what the route answers on the wire. Adds one `it()` to `rest-approvals-wire-codes.test.ts` driving the real recall route with a service that throws the real refusal, asserting status 403, `code === 'FORBIDDEN'`, and that the [#13095] anchored strip removed the prefix. Losing the row fails closed (500 `APPROVAL_RECALL_FAILED` with the raw message), so the third assertion catches the strip half of the regression as well as the status half. Test-only; no production behaviour changed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza * test(rest): record that §7 already pins this row — the card's premise is false Measured, not read: deleting `[/^FORBIDDEN/, 403, 'FORBIDDEN']` from `handleApprovalError` reds THREE cases, not one — this new pin and both cases of `rest-data-door-code-prefix.test.ts` §7, which already drive the real approve route and already assert 403, `code: 'FORBIDDEN'` and the anchored strip. #14573 was filed and triaged on the reading that the row had no live-emission pin anywhere. That is wrong. What is true is narrower: the file that OWNS the approvals wire-code contract did not pin it, so an audit of wire codes here saw a gap a strip-contract file was silently covering. Naming §7 from here is half the fix — the unlabelled duplicate is what got the card mis-filed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 84199cb commit e79107a

1 file changed

Lines changed: 65 additions & 0 deletions

File tree

packages/rest/src/rest-approvals-wire-codes.test.ts

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,20 @@
2828
* route. The derivation mirrors the production template exactly
2929
* (single-occurrence `.replace('-', '_')` included), so a route name the
3030
* template would mangle into an invalid code also fails here.
31+
* [#14573] The file has since become the home for the approvals door's
32+
* live-emission pins generally, not only the #8885 population: the
33+
* `FORBIDDEN` → 403 case below pins a row that is registered vocabulary and
34+
* whose emission was — contrary to that card's premise — already observed
35+
* elsewhere. See its own comment for where, and why it is pinned here too.
36+
*
3137
* 3. The union stays CLOSED — the control case in
3238
* `rest-field-visibility-fault-envelope.test.ts` covers this file too (same
3339
* schema instance); membership green here is evidence, not vacuity.
3440
*/
3541

3642
import { describe, it, expect, vi } from 'vitest';
3743
import { ApiErrorSchema } from '@objectstack/spec/api';
44+
import { BUILTIN_OPERATION_MESSAGES } from '@objectstack/spec/system';
3845
// `.js` on purpose — NodeNext resolution requires the extension (#7248).
3946
import { RestServer } from './rest-server.js';
4047

@@ -115,6 +122,64 @@ describe('approvals wire codes are registered vocabulary (#8885)', () => {
115122
).toBe(true);
116123
});
117124

125+
// [#14573] The `FORBIDDEN` → 403 row is the one EVERY authorisation
126+
// refusal rides: recall by a non-submitter, decide by a non-approver,
127+
// reassign / remind / sendBack / resubmit by the wrong actor, and
128+
// `resolveActor`'s impersonation refusals all reach this table through
129+
// the same single row.
130+
//
131+
// ⚠️ THIS IS A SECOND PIN, NOT THE FIRST — read this before adding a
132+
// third. #14573 was filed and triaged on the reading that the row had NO
133+
// live-emission pin, and that reading is WRONG: §7 of
134+
// `rest-data-door-code-prefix.test.ts` ("[#13095] the approvals door
135+
// strips the code it answers") already drives the REAL approve route with
136+
// a `FORBIDDEN: …` throw and already asserts 403, `code: 'FORBIDDEN'` and
137+
// the strip. Measured, not read: deleting the row from `rest-server.ts`
138+
// reds THREE cases — this one and both of §7's. What was true is narrower
139+
// than the card: the file that OWNS the approvals wire-code contract did
140+
// not pin the row, so a reader auditing wire codes here saw a gap that a
141+
// strip-contract file was silently covering. That is the gap this case
142+
// closes, and naming §7 here is half the fix — an unlabelled duplicate is
143+
// what got the card mis-filed in the first place.
144+
//
145+
// The service suites (`recall-refusal-user-copy.test.ts`,
146+
// `approval-revise.test.ts`) are NOT pins on this row: they assert the
147+
// `FORBIDDEN:` MESSAGE PREFIX at the throw site, a different fact from
148+
// what the route answers.
149+
//
150+
// Losing the row FAILS CLOSED, which is why this is a contract pin and not
151+
// a security one: `handleApprovalError` returns false on no match, the
152+
// caller rethrows, and the terminal catch answers 500
153+
// `APPROVAL_RECALL_FAILED`. The caller is still refused — at the wrong
154+
// status, with the wrong code, and (because that arm forwards
155+
// `String(error?.message ?? error)` verbatim) with the raw `FORBIDDEN: `
156+
// token the [#13095] anchored strip exists to remove. Two contract
157+
// properties ride this one row, so the third assertion below is NOT
158+
// redundant with the first two: it is what catches the degraded shape's
159+
// unstripped message. (Ablation: all three reds read
160+
// `expected 500 to be 403`.)
161+
it('recall by a non-submitter answers 403 FORBIDDEN, prefix stripped — the row every authorisation refusal rides', async () => {
162+
// The message the real service throws: `approval-service.ts`'s recall
163+
// non-submitter branch is `FORBIDDEN: ${userFacingRefusal(...)}`.
164+
// Read from the catalog rather than transcribed so a [#11993] copy
165+
// edit cannot red this pin for a reason that is not the wire contract
166+
// — the same construction `approval-revise.test.ts` uses.
167+
const refusal = BUILTIN_OPERATION_MESSAGES.en.approval_recall_not_submitter;
168+
const rest = boot({
169+
recall: vi.fn().mockRejectedValue(new Error(`FORBIDDEN: ${refusal}`)),
170+
});
171+
const answer = await drive(rest, 'POST', `${REQ}/recall`);
172+
expect(answer.status).toBe(403);
173+
expect(answer.body?.code).toBe('FORBIDDEN');
174+
// [#13095] Exactly the `FORBIDDEN:` this row just answered comes off,
175+
// and the user-facing sentence reaches the wire whole.
176+
expect(answer.body?.error).toBe(refusal);
177+
expect(
178+
ApiErrorSchema.safeParse({ code: answer.body?.code, message: answer.body?.error }).success,
179+
'FORBIDDEN must be in StandardErrorCode ∪ ERROR_CODE_LEDGER',
180+
).toBe(true);
181+
});
182+
118183
// [#13182] `READ_BACK_FAILED` is a NAMED wire row (the RESUME_FAILED
119184
// precedent: a genuine server-side inconsistency, but named): the write is
120185
// recorded and NOT rolled back, the read-back is org-filtered, and the

0 commit comments

Comments
 (0)