From 746ca4f80fd7bdc975543f24b904da6662c1187b Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Fri, 5 Jun 2026 09:52:12 +0800 Subject: [PATCH 1/2] fix(rest): map schema-mismatch & not-null driver errors to structured 4xx MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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) --- packages/rest/src/rest-server.ts | 55 ++++++++++++++++++- packages/rest/src/rest.test.ts | 91 +++++++++++++++++++++++++++++++- 2 files changed, 144 insertions(+), 2 deletions(-) diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index 89f22712e..ba7569e44 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -29,7 +29,7 @@ const TRANSLATABLE_META_TYPES = new Set(['view', 'action', 'object', 'app', 'das * not permitted …" — trips the `'' … not` substring check and * returns a misleading 404. */ -function mapDataError(error: any, object?: string): { status: number; body: Record } { +export function mapDataError(error: any, object?: string): { status: number; body: Record } { // Optimistic-Concurrency-Control mismatch → 409 with current state. // Surfaced FIRST so the structured fields (`currentVersion`, // `currentRecord`) are preserved instead of being squashed into the @@ -122,6 +122,59 @@ function mapDataError(error: any, object?: string): { status: number; body: Reco }; } + // Schema-mismatch & required-field violations are CLIENT errors (a bad + // payload the caller can fix), not server faults — so map them to a + // structured 4xx BEFORE the unknown-object / SQL-leak branches, which + // would otherwise bury them in a generic 404 or 500. Driver phrasing + // varies by dialect; cover SQLite / Postgres / MySQL: + // unknown column → SQLite "table X has no column named c" / + // "no such column: c"; Postgres 'column "c" of + // relation "X" does not exist'; MySQL "Unknown + // column 'c' in 'field list'". + // not-null → SQLite "NOT NULL constraint failed: X.c"; + // Postgres 'null value in column "c" ... violates + // not-null constraint'; MySQL "Column 'c' cannot + // be null". + // NOTE: this is a last-resort safety net — the validation layer should + // ideally reject these before they reach the driver (see follow-ups on + // unknown-field rejection + provenance-aware required checks). + const unknownColumn = + /has no column named\s+["'`]?([a-z0-9_]+)/i.exec(raw) || + /no such column:\s*["'`]?([a-z0-9_.]+)/i.exec(raw) || + /unknown column\s+["'`]([a-z0-9_]+)["'`]/i.exec(raw) || + /column\s+["'`]([a-z0-9_]+)["'`]\s+of relation\s+\S+\s+does not exist/i.exec(raw); + if (unknownColumn) { + const field = unknownColumn[1]?.split('.').pop(); + return { + status: 400, + body: { + error: field + ? `Unknown field '${field}'${object ? ` on object '${object}'` : ''}` + : 'Request references a field that does not exist', + code: 'INVALID_FIELD', + ...(field ? { field } : {}), + ...(object ? { object } : {}), + }, + }; + } + + const notNull = + /not null constraint failed:\s*\S*?\.([a-z0-9_]+)/i.exec(raw) || + /null value in column\s+["'`]([a-z0-9_]+)["'`]/i.exec(raw) || + /column\s+["'`]([a-z0-9_]+)["'`]\s+cannot be null/i.exec(raw); + if (notNull) { + const field = notNull[1]; + return { + status: 400, + body: { + error: `${field} is required`, + code: 'VALIDATION_FAILED', + fields: [{ field, code: 'required', message: `${field} is required` }], + ...(object ? { object } : {}), + }, + }; + } + const looksLikeUnknownObject = lower.includes('no such table') || lower.includes('relation') && lower.includes('does not exist') || diff --git a/packages/rest/src/rest.test.ts b/packages/rest/src/rest.test.ts index f68b6e342..37e0546e4 100644 --- a/packages/rest/src/rest.test.ts +++ b/packages/rest/src/rest.test.ts @@ -2,7 +2,7 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; import { RouteManager, RouteGroupBuilder } from './route-manager'; -import { RestServer } from './rest-server'; +import { RestServer, mapDataError } from './rest-server'; import { createRestApiPlugin } from './rest-api-plugin'; import type { RestApiPluginConfig } from './rest-api-plugin'; @@ -1426,3 +1426,92 @@ describe('RestServer.resolveProtocol', () => { expect(f.kernelManager.getOrCreate).not.toHaveBeenCalled(); }); }); + +// --------------------------------------------------------------------------- +// mapDataError — schema-mismatch & required-field violations must surface as +// structured 4xx, never a leaked 500 DATABASE_ERROR (repro: B7 POST +// /data/sys_team with a body whose fields don't match the table). +// --------------------------------------------------------------------------- +describe('mapDataError — schema/constraint envelopes', () => { + const sqliteError = (message: string, code = 'SQLITE_ERROR') => + Object.assign(new Error(message), { code }); + + it('maps SQLite "has no column named" → 400 INVALID_FIELD with the field', () => { + const r = mapDataError( + sqliteError( + "insert into `sys_team` (`id`, `label`, `name`) values (?, ?, ?) returning * - table sys_team has no column named label", + ), + 'sys_team', + ); + expect(r.status).toBe(400); + expect(r.body.code).toBe('INVALID_FIELD'); + expect(r.body.field).toBe('label'); + expect(r.body.object).toBe('sys_team'); + expect(String(r.body.error)).not.toMatch(/insert into|sqlite/i); + }); + + it('maps SQLite "no such column" → 400 INVALID_FIELD', () => { + const r = mapDataError(sqliteError('no such column: bogus'), 'widget'); + expect(r.status).toBe(400); + expect(r.body.code).toBe('INVALID_FIELD'); + expect(r.body.field).toBe('bogus'); + }); + + it('maps MySQL "Unknown column" → 400 INVALID_FIELD', () => { + const r = mapDataError(sqliteError("Unknown column 'label' in 'field list'", 'ER_BAD_FIELD_ERROR'), 'sys_team'); + expect(r.status).toBe(400); + expect(r.body.code).toBe('INVALID_FIELD'); + expect(r.body.field).toBe('label'); + }); + + it('maps Postgres "column ... of relation ... does not exist" → 400 INVALID_FIELD (not 404 object_not_found)', () => { + const r = mapDataError( + sqliteError('column "label" of relation "sys_team" does not exist', '42703'), + 'sys_team', + ); + expect(r.status).toBe(400); + expect(r.body.code).toBe('INVALID_FIELD'); + expect(r.body.field).toBe('label'); + }); + + it('maps SQLite NOT NULL constraint → 400 VALIDATION_FAILED with required field', () => { + const r = mapDataError( + sqliteError( + "insert into `sys_team` ... - NOT NULL constraint failed: sys_team.organization_id", + 'SQLITE_CONSTRAINT_NOTNULL', + ), + 'sys_team', + ); + expect(r.status).toBe(400); + expect(r.body.code).toBe('VALIDATION_FAILED'); + expect(r.body.fields).toEqual([ + { field: 'organization_id', code: 'required', message: 'organization_id is required' }, + ]); + }); + + it('maps Postgres not-null violation → 400 VALIDATION_FAILED', () => { + const r = mapDataError( + sqliteError('null value in column "organization_id" of relation "sys_team" violates not-null constraint', '23502'), + 'sys_team', + ); + expect(r.status).toBe(400); + expect(r.body.code).toBe('VALIDATION_FAILED'); + expect((r.body.fields as any[])[0].field).toBe('organization_id'); + }); + + it('still hides genuine SQL leaks (unrecognized driver dump) behind a generic 500', () => { + const r = mapDataError(sqliteError('SQLITE_IOERR: disk I/O error', 'SQLITE_IOERR'), 'sys_team'); + expect(r.status).toBe(500); + expect(r.body.code).toBe('DATABASE_ERROR'); + expect(String(r.body.error)).not.toMatch(/disk i\/o/i); + }); + + it('still maps unique-constraint violations to 409 (unchanged)', () => { + const r = mapDataError( + sqliteError('UNIQUE constraint failed: sys_team.name, sys_team.organization_id', 'SQLITE_CONSTRAINT_UNIQUE'), + 'sys_team', + ); + expect(r.status).toBe(409); + expect(r.body.code).toBe('UNIQUE_VIOLATION'); + }); +}); From c055ef6c2b1a20ca0f066f2276b69485c7153aff Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Fri, 5 Jun 2026 10:13:40 +0800 Subject: [PATCH 2/2] chore: add changeset for rest 4xx error mapping Co-Authored-By: Claude Opus 4.8 (1M context) --- .changeset/rest-map-schema-errors.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 .changeset/rest-map-schema-errors.md diff --git a/.changeset/rest-map-schema-errors.md b/.changeset/rest-map-schema-errors.md new file mode 100644 index 000000000..a94e6a8bc --- /dev/null +++ b/.changeset/rest-map-schema-errors.md @@ -0,0 +1,15 @@ +--- +'@objectstack/rest': patch +--- + +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 to the data API leaked a 500 +instead of a fixable 4xx (e.g. `POST /data/sys_team` with an unknown field, +or omitting a required column). It now maps unknown-column errors to +`400 INVALID_FIELD { field }` and not-null violations to +`400 VALIDATION_FAILED { fields:[{required}] }` across SQLite/Postgres/MySQL +phrasings, placed before the unknown-object branch so Postgres +`column … of relation … does not exist` is not mis-mapped to 404. Genuine +driver faults still return 500; unique violations still return 409.