Skip to content

Commit 0041d1b

Browse files
committed
fix(rest): pass through explicit 4xx status+code in mapDataError (#2926 ⑦)
Record-scope authorization denials (plugin-sharing throws status=403, code=FORBIDDEN) degraded to a bare 400 with no code because the generic data routes call mapDataError directly, bypassing sendError's status passthrough. Add a guarded 4xx passthrough after the structured-code branches (409 envelopes keep their rich fields; 5xx still goes through the sanitizing heuristics; oversized messages fall back to generic text), and stop logging expected statuses as unhandled on the list route. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017bJWtKmZ2mFpRFAmmmoeqe
1 parent 0dbaee3 commit 0041d1b

3 files changed

Lines changed: 91 additions & 1 deletion

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@objectstack/rest': patch
3+
---
4+
5+
fix(rest): mapDataError now honors an explicit 4xx `error.status`/`error.code` carried by domain errors (#2926 ⑦). Record-scope authorization denials from plugin-sharing (status 403, code FORBIDDEN) previously degraded to a bare 400 with no machine-readable code because the generic data routes bypass sendError's status passthrough. Structured 409 envelopes (CONCURRENT_UPDATE, DELETE_RESTRICTED) keep their dedicated branches; 5xx statuses still go through the message-sanitizing heuristics.

packages/rest/src/rest-server.ts

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,27 @@ export function mapDataError(error: any, object?: string): { status: number; bod
143143
},
144144
};
145145
}
146+
// Generic passthrough for domain errors that already carry an explicit
147+
// HTTP status (e.g. plugin-sharing's record-scope denial: status 403 +
148+
// code FORBIDDEN) — mirrors sendError's `.status` handling, which the
149+
// generic data routes bypass by calling mapDataError directly (#2926 ⑦).
150+
// Placed AFTER the structured-code branches above (409s carry rich
151+
// fields this envelope would drop) and deliberately limited to 4xx:
152+
// 5xx messages keep going through the sanitizing heuristics below so
153+
// internal/SQL details never reach the client verbatim.
154+
if (typeof error?.status === 'number' && error.status >= 400 && error.status < 500) {
155+
const msg = typeof error?.message === 'string' && error.message.length > 0 && error.message.length < 500
156+
? error.message
157+
: 'Request failed';
158+
return {
159+
status: error.status,
160+
body: {
161+
error: msg,
162+
...(typeof error?.code === 'string' && error.code ? { code: error.code } : {}),
163+
...(object ? { object } : {}),
164+
},
165+
};
166+
}
146167

147168
const raw = String(error?.message ?? error ?? '');
148169
const lower = raw.toLowerCase();
@@ -3245,7 +3266,7 @@ export class RestServer {
32453266
res.json(result);
32463267
} catch (error: any) {
32473268
const mapped = mapDataError(error, req.params?.object);
3248-
if (mapped.status === 404 || mapped.status === 503 || mapped.status === 502) {
3269+
if (isExpectedDataStatus(mapped.status) || mapped.body?.code === 'UNSUPPORTED_QUERY_PARAM') {
32493270
res.status(mapped.status).json(mapped.body);
32503271
} else {
32513272
logError("[REST] Unhandled error:", error);

packages/rest/src/rest.test.ts

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2110,6 +2110,70 @@ describe('mapDataError — schema/constraint envelopes', () => {
21102110
expect(r.body.object).toBe('crm_lead');
21112111
});
21122112

2113+
// #2926 ⑦: plugin-sharing's record-scope denial throws with an explicit
2114+
// status 403 + code FORBIDDEN, but the generic data routes bypass
2115+
// sendError's `.status` passthrough — mapDataError must honor the
2116+
// explicit status itself or the 403 degrades to the catch-all 400.
2117+
it('passes through an explicit 4xx status + code (sharing FORBIDDEN → 403)', () => {
2118+
const r = mapDataError(
2119+
Object.assign(new Error('FORBIDDEN: insufficient privileges to update showcase_inquiry rec1'), {
2120+
code: 'FORBIDDEN',
2121+
status: 403,
2122+
}),
2123+
'showcase_inquiry',
2124+
);
2125+
expect(r.status).toBe(403);
2126+
expect(r.body.code).toBe('FORBIDDEN');
2127+
expect(r.body.object).toBe('showcase_inquiry');
2128+
expect(r.body.error).toContain('insufficient privileges');
2129+
});
2130+
2131+
it('does NOT pass through an explicit 5xx status (message stays sanitized)', () => {
2132+
const r = mapDataError(
2133+
Object.assign(new Error('connect ECONNREFUSED 10.0.0.5:5432 (internal pool)'), { status: 502 }),
2134+
);
2135+
// 5xx bypasses the passthrough and falls into the sanitizing heuristics.
2136+
expect(r.status).not.toBe(502);
2137+
expect(r.body.code).not.toBe('FORBIDDEN');
2138+
});
2139+
2140+
it('guards the passthrough message length (oversized → generic text)', () => {
2141+
const r = mapDataError(Object.assign(new Error('x'.repeat(600)), { status: 403, code: 'FORBIDDEN' }));
2142+
expect(r.status).toBe(403);
2143+
expect(r.body.error).toBe('Request failed');
2144+
});
2145+
2146+
it('keeps CONCURRENT_UPDATE 409 structured fields despite its own status property', () => {
2147+
const r = mapDataError(
2148+
Object.assign(new Error('Record was modified'), {
2149+
code: 'CONCURRENT_UPDATE',
2150+
status: 409,
2151+
currentVersion: 7,
2152+
currentRecord: { id: 'r1' },
2153+
}),
2154+
'sys_task',
2155+
);
2156+
expect(r.status).toBe(409);
2157+
expect(r.body.code).toBe('CONCURRENT_UPDATE');
2158+
expect(r.body.currentVersion).toBe(7);
2159+
expect(r.body.currentRecord).toEqual({ id: 'r1' });
2160+
});
2161+
2162+
it('keeps DELETE_RESTRICTED 409 structured fields despite its own status property', () => {
2163+
const r = mapDataError(
2164+
Object.assign(new Error('Cannot delete sys_position (p1): 1 dependent record'), {
2165+
code: 'DELETE_RESTRICTED',
2166+
status: 409,
2167+
dependentObject: 'sys_position_permission_set',
2168+
dependentCount: 1,
2169+
}),
2170+
'sys_position',
2171+
);
2172+
expect(r.status).toBe(409);
2173+
expect(r.body.code).toBe('DELETE_RESTRICTED');
2174+
expect(r.body.dependentCount).toBe(1);
2175+
});
2176+
21132177
it('maps SQLite "has no column named" → 400 INVALID_FIELD with the field', () => {
21142178
const r = mapDataError(
21152179
sqliteError(

0 commit comments

Comments
 (0)