Skip to content

Commit e1478fe

Browse files
xuyushun441-sysos-zhuangclaude
authored
fix(rest): map schema-mismatch & not-null driver errors to structured 4xx (#1594)
* fix(rest): map schema-mismatch & not-null driver errors to structured 4xx `mapDataError` collapsed any SQL-looking driver error into a generic 500 `DATABASE_ERROR`, so a bad write payload — e.g. `POST /data/sys_team` with an unknown `label` field, or omitting the required `organization_id` — leaked a 500 instead of a fixable 4xx. (Found running LOCAL-E2E-CHECKLIST B7: per-env data API CRUD.) Add two structured branches before the unknown-object / SQL-leak fallbacks: - unknown column → 400 INVALID_FIELD { field } (SQLite "has no column named" / "no such column"; Postgres 'column "c" of relation ... does not exist'; MySQL "Unknown column 'c'"). Placed before the unknown-object branch so the Postgres phrasing isn't mis-mapped to 404 object_not_found. - not-null → 400 VALIDATION_FAILED { fields:[{required}] } (SQLite "NOT NULL constraint failed: t.c"; Postgres "null value in column"; MySQL "Column 'c' cannot be null"). Genuine driver faults (e.g. SQLITE_IOERR) and unique violations are unchanged (500 DATABASE_ERROR / 409 UNIQUE_VIOLATION). This is a last-resort safety net; the durable fix is upstream validation (unknown-field rejection + provenance-aware required checks) — tracked separately. Export `mapDataError` (package-internal; not added to index) + 8 tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: add changeset for rest 4xx error mapping Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent d5a8161 commit e1478fe

3 files changed

Lines changed: 159 additions & 2 deletions

File tree

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
'@objectstack/rest': patch
3+
---
4+
5+
fix(rest): map schema-mismatch & not-null driver errors to structured 4xx
6+
7+
`mapDataError` collapsed any SQL-looking driver error into a generic
8+
`500 DATABASE_ERROR`, so a bad write payload to the data API leaked a 500
9+
instead of a fixable 4xx (e.g. `POST /data/sys_team` with an unknown field,
10+
or omitting a required column). It now maps unknown-column errors to
11+
`400 INVALID_FIELD { field }` and not-null violations to
12+
`400 VALIDATION_FAILED { fields:[{required}] }` across SQLite/Postgres/MySQL
13+
phrasings, placed before the unknown-object branch so Postgres
14+
`column … of relation … does not exist` is not mis-mapped to 404. Genuine
15+
driver faults still return 500; unique violations still return 409.

packages/rest/src/rest-server.ts

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ const TRANSLATABLE_META_TYPES = new Set(['view', 'action', 'object', 'app', 'das
2929
* not permitted …" — trips the `'<obj>' … not` substring check and
3030
* returns a misleading 404.
3131
*/
32-
function mapDataError(error: any, object?: string): { status: number; body: Record<string, unknown> } {
32+
export function mapDataError(error: any, object?: string): { status: number; body: Record<string, unknown> } {
3333
// Optimistic-Concurrency-Control mismatch → 409 with current state.
3434
// Surfaced FIRST so the structured fields (`currentVersion`,
3535
// `currentRecord`) are preserved instead of being squashed into the
@@ -122,6 +122,59 @@ function mapDataError(error: any, object?: string): { status: number; body: Reco
122122
};
123123
}
124124

125+
// Schema-mismatch & required-field violations are CLIENT errors (a bad
126+
// payload the caller can fix), not server faults — so map them to a
127+
// structured 4xx BEFORE the unknown-object / SQL-leak branches, which
128+
// would otherwise bury them in a generic 404 or 500. Driver phrasing
129+
// varies by dialect; cover SQLite / Postgres / MySQL:
130+
// unknown column → SQLite "table X has no column named c" /
131+
// "no such column: c"; Postgres 'column "c" of
132+
// relation "X" does not exist'; MySQL "Unknown
133+
// column 'c' in 'field list'".
134+
// not-null → SQLite "NOT NULL constraint failed: X.c";
135+
// Postgres 'null value in column "c" ... violates
136+
// not-null constraint'; MySQL "Column 'c' cannot
137+
// be null".
138+
// NOTE: this is a last-resort safety net — the validation layer should
139+
// ideally reject these before they reach the driver (see follow-ups on
140+
// unknown-field rejection + provenance-aware required checks).
141+
const unknownColumn =
142+
/has no column named\s+["'`]?([a-z0-9_]+)/i.exec(raw) ||
143+
/no such column:\s*["'`]?([a-z0-9_.]+)/i.exec(raw) ||
144+
/unknown column\s+["'`]([a-z0-9_]+)["'`]/i.exec(raw) ||
145+
/column\s+["'`]([a-z0-9_]+)["'`]\s+of relation\s+\S+\s+does not exist/i.exec(raw);
146+
if (unknownColumn) {
147+
const field = unknownColumn[1]?.split('.').pop();
148+
return {
149+
status: 400,
150+
body: {
151+
error: field
152+
? `Unknown field '${field}'${object ? ` on object '${object}'` : ''}`
153+
: 'Request references a field that does not exist',
154+
code: 'INVALID_FIELD',
155+
...(field ? { field } : {}),
156+
...(object ? { object } : {}),
157+
},
158+
};
159+
}
160+
161+
const notNull =
162+
/not null constraint failed:\s*\S*?\.([a-z0-9_]+)/i.exec(raw) ||
163+
/null value in column\s+["'`]([a-z0-9_]+)["'`]/i.exec(raw) ||
164+
/column\s+["'`]([a-z0-9_]+)["'`]\s+cannot be null/i.exec(raw);
165+
if (notNull) {
166+
const field = notNull[1];
167+
return {
168+
status: 400,
169+
body: {
170+
error: `${field} is required`,
171+
code: 'VALIDATION_FAILED',
172+
fields: [{ field, code: 'required', message: `${field} is required` }],
173+
...(object ? { object } : {}),
174+
},
175+
};
176+
}
177+
125178
const looksLikeUnknownObject =
126179
lower.includes('no such table') ||
127180
lower.includes('relation') && lower.includes('does not exist') ||

packages/rest/src/rest.test.ts

Lines changed: 90 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import { describe, it, expect, beforeEach, vi } from 'vitest';
44
import { RouteManager, RouteGroupBuilder } from './route-manager';
5-
import { RestServer } from './rest-server';
5+
import { RestServer, mapDataError } from './rest-server';
66
import { createRestApiPlugin } from './rest-api-plugin';
77
import type { RestApiPluginConfig } from './rest-api-plugin';
88

@@ -1463,3 +1463,92 @@ describe('RestServer.resolveProtocol', () => {
14631463
expect(f.kernelManager.getOrCreate).not.toHaveBeenCalled();
14641464
});
14651465
});
1466+
1467+
// ---------------------------------------------------------------------------
1468+
// mapDataError — schema-mismatch & required-field violations must surface as
1469+
// structured 4xx, never a leaked 500 DATABASE_ERROR (repro: B7 POST
1470+
// /data/sys_team with a body whose fields don't match the table).
1471+
// ---------------------------------------------------------------------------
1472+
describe('mapDataError — schema/constraint envelopes', () => {
1473+
const sqliteError = (message: string, code = 'SQLITE_ERROR') =>
1474+
Object.assign(new Error(message), { code });
1475+
1476+
it('maps SQLite "has no column named" → 400 INVALID_FIELD with the field', () => {
1477+
const r = mapDataError(
1478+
sqliteError(
1479+
"insert into `sys_team` (`id`, `label`, `name`) values (?, ?, ?) returning * - table sys_team has no column named label",
1480+
),
1481+
'sys_team',
1482+
);
1483+
expect(r.status).toBe(400);
1484+
expect(r.body.code).toBe('INVALID_FIELD');
1485+
expect(r.body.field).toBe('label');
1486+
expect(r.body.object).toBe('sys_team');
1487+
expect(String(r.body.error)).not.toMatch(/insert into|sqlite/i);
1488+
});
1489+
1490+
it('maps SQLite "no such column" → 400 INVALID_FIELD', () => {
1491+
const r = mapDataError(sqliteError('no such column: bogus'), 'widget');
1492+
expect(r.status).toBe(400);
1493+
expect(r.body.code).toBe('INVALID_FIELD');
1494+
expect(r.body.field).toBe('bogus');
1495+
});
1496+
1497+
it('maps MySQL "Unknown column" → 400 INVALID_FIELD', () => {
1498+
const r = mapDataError(sqliteError("Unknown column 'label' in 'field list'", 'ER_BAD_FIELD_ERROR'), 'sys_team');
1499+
expect(r.status).toBe(400);
1500+
expect(r.body.code).toBe('INVALID_FIELD');
1501+
expect(r.body.field).toBe('label');
1502+
});
1503+
1504+
it('maps Postgres "column ... of relation ... does not exist" → 400 INVALID_FIELD (not 404 object_not_found)', () => {
1505+
const r = mapDataError(
1506+
sqliteError('column "label" of relation "sys_team" does not exist', '42703'),
1507+
'sys_team',
1508+
);
1509+
expect(r.status).toBe(400);
1510+
expect(r.body.code).toBe('INVALID_FIELD');
1511+
expect(r.body.field).toBe('label');
1512+
});
1513+
1514+
it('maps SQLite NOT NULL constraint → 400 VALIDATION_FAILED with required field', () => {
1515+
const r = mapDataError(
1516+
sqliteError(
1517+
"insert into `sys_team` ... - NOT NULL constraint failed: sys_team.organization_id",
1518+
'SQLITE_CONSTRAINT_NOTNULL',
1519+
),
1520+
'sys_team',
1521+
);
1522+
expect(r.status).toBe(400);
1523+
expect(r.body.code).toBe('VALIDATION_FAILED');
1524+
expect(r.body.fields).toEqual([
1525+
{ field: 'organization_id', code: 'required', message: 'organization_id is required' },
1526+
]);
1527+
});
1528+
1529+
it('maps Postgres not-null violation → 400 VALIDATION_FAILED', () => {
1530+
const r = mapDataError(
1531+
sqliteError('null value in column "organization_id" of relation "sys_team" violates not-null constraint', '23502'),
1532+
'sys_team',
1533+
);
1534+
expect(r.status).toBe(400);
1535+
expect(r.body.code).toBe('VALIDATION_FAILED');
1536+
expect((r.body.fields as any[])[0].field).toBe('organization_id');
1537+
});
1538+
1539+
it('still hides genuine SQL leaks (unrecognized driver dump) behind a generic 500', () => {
1540+
const r = mapDataError(sqliteError('SQLITE_IOERR: disk I/O error', 'SQLITE_IOERR'), 'sys_team');
1541+
expect(r.status).toBe(500);
1542+
expect(r.body.code).toBe('DATABASE_ERROR');
1543+
expect(String(r.body.error)).not.toMatch(/disk i\/o/i);
1544+
});
1545+
1546+
it('still maps unique-constraint violations to 409 (unchanged)', () => {
1547+
const r = mapDataError(
1548+
sqliteError('UNIQUE constraint failed: sys_team.name, sys_team.organization_id', 'SQLITE_CONSTRAINT_UNIQUE'),
1549+
'sys_team',
1550+
);
1551+
expect(r.status).toBe(409);
1552+
expect(r.body.code).toBe('UNIQUE_VIOLATION');
1553+
});
1554+
});

0 commit comments

Comments
 (0)