diff --git a/.changeset/data-path-object-existence-gate.md b/.changeset/data-path-object-existence-gate.md new file mode 100644 index 0000000000..88e990ca43 --- /dev/null +++ b/.changeset/data-path-object-existence-gate.md @@ -0,0 +1,65 @@ +--- +"@objectstack/metadata-protocol": minor +"@objectstack/rest": minor +--- + +fix(metadata-protocol,rest): the data path really 404s unknown objects now (#3770) + +The REST API-exposure gate (`enforceApiAccess`) passes through any object it +cannot find in metadata, and the comment there justified that with +`// unknown object → let the data path 404`. That fallback did not exist. + +- `findData` — and every other data entry point except `cloneData` — had **no + existence check**. The repo's only `OBJECT_NOT_FOUND` throw was in `cloneData`. +- The engine does not reject unregistered names either: `resolveObjectName` + falls back to `StorageNameMapping.resolveTableName({ name })`, so the object + name is used **as the table name**. +- The 404 was therefore only ever a side effect of the **driver** erroring on a + missing table, which the REST layer recognised by matching the driver's error + string. + +So the 404 held only when the table happened not to exist. When a physical table +with that name **did** exist — out-of-band DDL, a registration that failed after +`syncObjectSchema` had already run, a registration race — the exposure gate was +silently skipped and the rows were served, with no layer turning it into a 404. +(Since #3545 an authenticated caller on a plugin-security deployment is refused +by the fail-closed posture check; anonymous callers and deployments without +plugin-security were not.) + +**The gate.** `ObjectStackProtocolImplementation` now runs a shared +`assertObjectRegistered` before storage is touched, on `findData`, `getData`, +`createData`, `cloneData`, `updateData`, `deleteData`, `batchData`, +`createManyData`, `insertManyData`, `updateManyData`, `deleteManyData` and +`analyticsQuery`. An object absent from the schema registry is rejected with +`OBJECT_NOT_FOUND` / 404 — an authoritative answer from the registry, raised +*before* the name becomes a table name, instead of an inference from driver +prose. `cloneData`'s open-coded check is now that shared gate; its envelope is +unchanged. + +It sits at the protocol ingress, the same boundary `apiEnabled` guards: internal +callers (hooks, flows, migrations, raw ObjectQL) go to the engine directly and +are unaffected. When the engine exposes no schema registry at all there is +nothing to consult, so the gate stands down and warns once per process — +matching the tiering #3545 recorded in `api-exposure.ts` for a whole-registry +outage. + +**Behaviour change.** A REST data request for an object that is not in the +schema registry now returns `404 object_not_found` even when a table of that +name exists. Previously it returned that table's rows. If a deployment depended +on reading a table with no registered object, register the object (its schema is +what every other layer — exposure, RBAC/FLS/RLS, field projection — already +needs in order to enforce anything at all). + +**One wire code.** `mapDataError` maps the protocol's `OBJECT_NOT_FOUND` to the +canonical `object_not_found` `ApiErrorCode` — byte-identical to the envelope the +driver-string branch already produced — so a client keying on `code` sees *what +happened*, not *which layer noticed*. The driver-string branch stays as the +safety net for the other failure it actually covers: an object that IS registered +but whose physical table is missing. Callers that were reading `cloneData`'s 404 +as `code: 'OBJECT_NOT_FOUND'` on the wire now get `object_not_found`; the status +is 404 either way. + +The misleading comment is replaced with what actually closes the hole — this +gate for existence, plugin-security's `unresolved` posture (#3545) for +authorization — and a note not to widen the exposure gate on the assumption that +some other layer 404s. diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index af63f633da..ab15ec3947 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -63,6 +63,14 @@ import { */ const TYPE_TO_FORM: Readonly> = METADATA_FORM_REGISTRY; +/** + * [#3770] One-shot flag for the "engine has no schema registry" warning emitted + * by {@link ObjectStackProtocolImplementation.assertObjectRegistered}. The + * condition is a property of how the host constructed the engine, so it is + * constant for the process — warn once, not once per request. + */ +let warnedNoRegistryForDataGate = false; + /** * Convert a Zod schema to a JSON Schema, returning `undefined` if conversion * fails (e.g. unsupported constructs). Cached per schema reference. @@ -2584,7 +2592,72 @@ export class ObjectStackProtocolImplementation implements } } + /** + * [#3770] Data-plane existence gate — the object MUST be in the schema + * registry before any data entry point below touches storage. + * + * ## Why this exists + * + * The REST API-exposure gate (`enforceApiAccess`, ADR-0049 / #1889) skips + * objects it cannot find in metadata, and justified that with "the data + * path will 404 anyway". It would not. `engine.find` resolves an + * UNREGISTERED name straight to a physical table name + * (`resolveObjectName` → `StorageNameMapping.resolveTableName({ name })`), + * so the request only 404'd as a *side effect* of the driver complaining + * about a missing table (which the REST layer recognises by matching the + * driver's error string) — and did not 404 at all when a table with that + * name happened to exist: out-of-band DDL, a registration that failed + * after `syncObjectSchema` had already run, a registration race. In that + * window the exposure gate was silently skipped and the rows were served. + * + * The gate lives HERE, at the protocol ingress, for the same reason + * `enforceApiAccess` does: this is the external API boundary. Internal + * callers (hooks, flows, migrations, raw ObjectQL) talk to the engine + * directly and are deliberately unaffected — `apiEnabled` and this check + * both control automatic API exposure, not data access. + * + * ## Tiering — mirrors the #3545 decision recorded in `api-exposure.ts` + * + * - **Registry present, object absent → fail CLOSED** (404 + * `OBJECT_NOT_FOUND`). The registry is authoritative for objects: + * `object` is `allowOrgOverride: false` (ADR-0005), so no per-org + * overlay can legitimately exist outside the process-wide registry, and + * both boot hydration (`loadMetaFromDb`) and runtime authoring + * (`applyObjectRegistryMutation`) register the schema before its table + * is reachable. + * - **No registry on the engine at all → skip.** There is no source of + * truth to consult, so the check cannot answer; failing closed would + * break every registry-less host (edge/Lite embeddings, engine doubles) + * for no security gain. Warned once per process so a deployment in that + * state is observable rather than a silent blanket-allow — the lesson + * #3545 recorded for `loadObjectItems`. + */ + private assertObjectRegistered(object: string): void { + const registry: any = this.engine?.registry; + if (!registry || typeof registry.getObject !== 'function') { + if (!warnedNoRegistryForDataGate) { + warnedNoRegistryForDataGate = true; + console.warn( + '[Protocol] engine exposes no schema registry — the data-plane object-existence ' + + 'gate (#3770) is INACTIVE for this process; unregistered object names reach the ' + + 'driver as raw table names.', + ); + } + return; + } + if (registry.getObject(object)) return; + const err: any = new Error(`Object '${object}' not found`); + err.code = 'OBJECT_NOT_FOUND'; + err.status = 404; + err.object = object; + throw err; + } + async findData(request: { object: string, query?: any, context?: any }) { + // [#3770] Existence first: an unregistered object is a 404 before any + // query parameter is even parsed, so an unknown name can never be + // probed for query-shape validity (nor reach the driver as a table). + this.assertObjectRegistered(request.object); const options: any = { ...request.query }; // Forward the dispatcher's ExecutionContext so RBAC/RLS middleware // can apply per-request enforcement. The protocol layer is purely @@ -2838,6 +2911,7 @@ export class ObjectStackProtocolImplementation implements } async getData(request: { object: string, id: string, expand?: string | string[], select?: string | string[], context?: any }) { + this.assertObjectRegistered(request.object); // [#3770] const queryOptions: any = { where: { id: request.id } }; @@ -2883,6 +2957,7 @@ export class ObjectStackProtocolImplementation implements } async createData(request: { object: string, data: any, context?: any }) { + this.assertObjectRegistered(request.object); // [#3770] // [#3043] Ingress-level static-`readonly` strip — a non-system caller // cannot seed a read-only column (e.g. `approval_status`) on create. const data = stripReadonlyForInsert( @@ -2925,17 +3000,14 @@ export class ObjectStackProtocolImplementation implements * clear a unique field, or reset status before insert. */ async cloneData(request: { object: string, id: string, overrides?: Record, context?: any }) { - const schema: any = this.engine.registry.getObject(request.object); - if (!schema) { - const err: any = new Error(`Object '${request.object}' not found`); - err.code = 'OBJECT_NOT_FOUND'; - err.status = 404; - err.object = request.object; - throw err; - } + // [#3770] This object-existence check used to be open-coded here and + // was the ONLY one on the whole data plane; it is now the shared gate + // every data entry point runs. Same error envelope as before. + this.assertObjectRegistered(request.object); + const schema: any = this.engine.registry?.getObject(request.object); // `enable.clone` defaults to true in the spec; treat an absent block / // absent flag as enabled and only block on an explicit `false`. - if (schema.enable?.clone === false) { + if (schema?.enable?.clone === false) { const err: any = new Error(`Cloning is disabled for object '${request.object}'`); err.code = 'CLONE_DISABLED'; err.status = 403; @@ -2962,7 +3034,7 @@ export class ObjectStackProtocolImplementation implements // path re-derives them rather than carrying the source's values over. const data: Record = { ...source }; for (const f of CLONE_STRIP_FIELDS) delete data[f]; - const fields: Record = schema.fields || {}; + const fields: Record = schema?.fields || {}; for (const [name, def] of Object.entries(fields)) { if (!def) continue; // Engine-/automation-owned values: injected system/audit columns, @@ -2995,6 +3067,7 @@ export class ObjectStackProtocolImplementation implements } async updateData(request: { object: string, id: string, data: any, expectedVersion?: string, context?: any }) { + this.assertObjectRegistered(request.object); // [#3770] await this.assertVersionMatch(request.object, request.id, request.expectedVersion, request.context); const opts: any = { where: { id: request.id } }; if (request.context !== undefined) opts.context = request.context; @@ -3016,6 +3089,7 @@ export class ObjectStackProtocolImplementation implements } async deleteData(request: { object: string, id: string, expectedVersion?: string, context?: any }) { + this.assertObjectRegistered(request.object); // [#3770] await this.assertVersionMatch(request.object, request.id, request.expectedVersion, request.context); const opts: any = { where: { id: request.id } }; if (request.context !== undefined) opts.context = request.context; @@ -3325,6 +3399,7 @@ export class ObjectStackProtocolImplementation implements async batchData(request: { object: string, request: BatchUpdateRequest, context?: any }): Promise { const { object, request: batchReq, context } = request; + this.assertObjectRegistered(object); // [#3770] const { operation, records, options } = batchReq; const results: Array<{ id?: string; success: boolean; error?: string; record?: any; droppedFields?: DroppedFieldsEvent[] }> = []; let succeeded = 0; @@ -3428,6 +3503,7 @@ export class ObjectStackProtocolImplementation implements } async createManyData(request: { object: string, records: any[], context?: any }): Promise { + this.assertObjectRegistered(request.object); // [#3770] // [#3043] Ingress-level static-`readonly` strip (per row) — mirrors // createData for the bulk-create / import surface. const rows = stripReadonlyForInsert( @@ -3470,6 +3546,7 @@ export class ObjectStackProtocolImplementation implements * fall back to createManyData. */ async insertManyData(request: { object: string, records: any[], context?: any }): Promise<{ object: string; outcomes: Array<{ ok: boolean; record?: any; error?: unknown; droppedFields?: DroppedFieldsEvent[] }> }> { + this.assertObjectRegistered(request.object); // [#3770] const engineInsertMany = (this.engine as any)?.insertMany; if (typeof engineInsertMany !== 'function') { throw new Error('insertManyData requires an engine with insertMany (framework#3172)'); @@ -3507,6 +3584,7 @@ export class ObjectStackProtocolImplementation implements async updateManyData(request: UpdateManyDataRequest & { context?: any }): Promise { const { object, records, options, context } = request; + this.assertObjectRegistered(object); // [#3770] const results: Array<{ id?: string; success: boolean; error?: string; record?: any; droppedFields?: DroppedFieldsEvent[] }> = []; let succeeded = 0; let failed = 0; @@ -3550,6 +3628,11 @@ export class ObjectStackProtocolImplementation implements // cube name maps to object name; measures → aggregations; dimensions → groupBy. const { query, cube } = request; const object = cube; + // [#3770] A cube name IS an object name here (`getAnalyticsMeta` derives + // every cube from `registry.listItems('object')`), so this read surface + // needs the same existence gate as the CRUD ones — otherwise it stays a + // way to aggregate over an arbitrary physical table. + this.assertObjectRegistered(object); // Build groupBy from dimensions const groupBy = query.dimensions || []; @@ -3723,6 +3806,7 @@ export class ObjectStackProtocolImplementation implements } async deleteManyData(request: DeleteManyDataRequest): Promise { + this.assertObjectRegistered(request.object); // [#3770] // This expects deleting by IDs. return this.engine.delete(request.object, { where: { id: { $in: request.ids } }, diff --git a/packages/objectql/src/protocol-data.test.ts b/packages/objectql/src/protocol-data.test.ts index 2df6b194b9..cb132c6c33 100644 --- a/packages/objectql/src/protocol-data.test.ts +++ b/packages/objectql/src/protocol-data.test.ts @@ -593,4 +593,64 @@ describe('ObjectStackProtocolImplementation - Data Operations', () => { ).rejects.toMatchObject({ code: 'OBJECT_NOT_FOUND', status: 404 }); }); }); + + // ═══════════════════════════════════════════════════════════════ + // [#3770] Object-existence gate — the tiering, at unit level + // + // The gap itself (an unregistered object whose physical table exists) is + // pinned against a real engine in `protocol-unregistered-object.test.ts`. + // What this block pins is the DECISION RULE, which an engine double is the + // right tool for: registry present ⇒ the registry is the answer; no + // registry at all ⇒ there is no answer, so the gate stands down. + // ═══════════════════════════════════════════════════════════════ + + describe('object-existence gate (#3770)', () => { + function makeGateProtocol(known: string[]) { + const engine: any = { + find: vi.fn().mockResolvedValue([]), + findOne: vi.fn().mockResolvedValue(null), + count: vi.fn().mockResolvedValue(0), + insert: vi.fn().mockResolvedValue({ id: 'new-id' }), + update: vi.fn().mockResolvedValue({ id: 'r1' }), + delete: vi.fn().mockResolvedValue(undefined), + registry: { + getObject: vi.fn((name: string) => + known.includes(name) ? { name, fields: {} } : undefined), + }, + }; + return { protocol: new ObjectStackProtocolImplementation(engine), engine }; + } + + it('consults the registry, not the driver, and never reaches the engine on a miss', async () => { + const { protocol, engine } = makeGateProtocol(['task']); + await expect( + protocol.findData({ object: 'ghost' }), + ).rejects.toMatchObject({ code: 'OBJECT_NOT_FOUND', status: 404, object: 'ghost' }); + expect(engine.registry.getObject).toHaveBeenCalledWith('ghost'); + expect(engine.find).not.toHaveBeenCalled(); + }); + + it('lets a registered object straight through', async () => { + const { protocol, engine } = makeGateProtocol(['task']); + await protocol.findData({ object: 'task' }); + expect(engine.find).toHaveBeenCalledOnce(); + }); + + it('stands down when the engine exposes no registry at all — nothing to consult', async () => { + // The #3545 tiering: "whole registry unavailable" is a cold-start / + // embedding shape, not a security decision. Failing closed here + // would break every registry-less host for no gain, so the gate + // skips (and warns once — see assertObjectRegistered). + const engine: any = { + find: vi.fn().mockResolvedValue([]), + count: vi.fn().mockResolvedValue(0), + }; + const protocol = new ObjectStackProtocolImplementation(engine); + await expect(protocol.findData({ object: 'ghost' })).resolves.toMatchObject({ + object: 'ghost', + records: [], + }); + expect(engine.find).toHaveBeenCalledOnce(); + }); + }); }); diff --git a/packages/objectql/src/protocol-unregistered-object.test.ts b/packages/objectql/src/protocol-unregistered-object.test.ts new file mode 100644 index 0000000000..fb8f980074 --- /dev/null +++ b/packages/objectql/src/protocol-unregistered-object.test.ts @@ -0,0 +1,225 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #3770 — the data path did NOT 404 for unknown objects. + * + * `enforceApiAccess` (rest-server) passes an object it cannot find in metadata + * straight through, and the comment there justified that with "let the data + * path 404". That premise was false: + * + * ① `findData` had no existence check — the only `OBJECT_NOT_FOUND` in the + * repo was thrown by `cloneData`. + * ② the engine does not reject unregistered names: `resolveObjectName` falls + * back to `StorageNameMapping.resolveTableName({ name })`, i.e. the object + * name IS used as the table name. + * ③ the 404 was therefore only ever a side effect of the DRIVER erroring on a + * missing table, recognised by string-matching that error in the REST layer. + * + * So the gap was case B below: when a physical table with that name exists but + * the object is not registered — out-of-band DDL, a registration that failed + * after `syncObjectSchema` ran, a registration race — the exposure gate was + * silently skipped AND nothing turned it into a 404. The rows were served. + * + * These tests drive a REAL {@link ObjectQL} engine + a minimal in-memory driver + * (the same shape `protocol-clone-real-engine.test.ts` uses), because the point + * is precisely what happens when metadata and physical storage disagree — which + * an engine double cannot show. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { ObjectQL } from './engine.js'; + +const registeredObject = { + name: 'gate_account', + label: 'Account', + fields: { + id: { name: 'id', label: 'ID', type: 'text' as const, primaryKey: true }, + name: { name: 'name', label: 'Name', type: 'text' as const, required: true }, + }, +}; + +/** The object nobody registered. A table by this name exists anyway (case B). */ +const UNREGISTERED = 'gate_ghost'; + +function makeMemoryDriver() { + const stores = new Map>>(); + const storeFor = (obj: string) => { + let s = stores.get(obj); + if (!s) { s = new Map(); stores.set(obj, s); } + return s; + }; + let nextId = 0; + const matchesWhere = (row: Record, where: any): boolean => { + if (!where || typeof where !== 'object') return true; + for (const [k, v] of Object.entries(where)) { + if (k.startsWith('$')) continue; + const expected = (v && typeof v === 'object' && '$eq' in (v as any)) ? (v as any).$eq : v; + const a = row[k] === undefined ? null : row[k]; + const b = expected === undefined ? null : expected; + if (a !== b) return false; + } + return true; + }; + const driver: any = { + name: 'memory', + version: '0.0.0', + supports: {} as any, + async connect() {}, + async disconnect() {}, + async checkHealth() { return true; }, + async execute() { return null; }, + async find(object: string, ast: any) { + return Array.from(storeFor(object).values()).filter((r) => matchesWhere(r, ast?.where)); + }, + findStream() { throw new Error('not implemented'); }, + async findOne(object: string, ast: any) { + for (const r of storeFor(object).values()) if (matchesWhere(r, ast?.where)) return r; + return null; + }, + async create(object: string, data: Record) { + nextId += 1; + const id = (data.id as string) ?? `r_${nextId}`; + const row = { ...data, id }; + storeFor(object).set(id, row); + return row; + }, + async update(object: string, id: string, data: Record) { + const s = storeFor(object); + const cur = s.get(id); + if (!cur) throw new Error(`not found: ${object}/${id}`); + const updated = { ...cur, ...data, id }; + s.set(id, updated); + return updated; + }, + async upsert(object: string, data: Record) { + const id = data.id as string | undefined; + if (id && storeFor(object).has(id)) return this.update(object, id, data); + return this.create(object, data); + }, + async delete(object: string, id: string) { return storeFor(object).delete(id); }, + async count(object: string, ast: any) { return (await this.find(object, ast)).length; }, + async aggregate(object: string, ast: any) { + return [{ count: (await this.find(object, ast)).length }]; + }, + async bulkCreate(object: string, rows: Record[]) { + return Promise.all(rows.map((r) => this.create(object, r))); + }, + async bulkUpdate() { return []; }, + async bulkDelete() {}, + async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, + async rollback() {}, + }; + return { driver, stores }; +} + +const OBJECT_NOT_FOUND = { code: 'OBJECT_NOT_FOUND', status: 404 }; + +describe('#3770 — data-plane object-existence gate (real ObjectQL engine)', () => { + let engine: ObjectQL; + let protocol: ObjectStackProtocolImplementation; + let stores: Map>>; + + beforeEach(async () => { + engine = new ObjectQL(); + const made = makeMemoryDriver(); + stores = made.stores; + engine.registerDriver(made.driver, true); + await engine.init(); + engine.registry.registerObject(registeredObject as any); + protocol = new ObjectStackProtocolImplementation(engine); + + // Case B setup: a physical table exists under the unregistered name, + // holding a row no API caller should ever see. + const ghost = new Map>(); + ghost.set('r1', { id: 'r1', secret: 'classified' }); + stores.set(UNREGISTERED, ghost); + }); + + // ───────────────────────────────────────────────────────────── + // The gap the issue reported + // ───────────────────────────────────────────────────────────── + + it('case B — an unregistered object whose physical table EXISTS is 404, not served', async () => { + // Pre-#3770 this resolved and returned `{ id: 'r1', secret: 'classified' }`. + await expect( + protocol.findData({ object: UNREGISTERED }), + ).rejects.toMatchObject(OBJECT_NOT_FOUND); + + // And the row really is reachable through the engine — i.e. the test + // fails for the right reason (the gate), not because storage was empty. + expect(await engine.find(UNREGISTERED, {})).toHaveLength(1); + }); + + it('case A — an unregistered object with no physical table is 404 from the gate, not the driver', async () => { + stores.delete(UNREGISTERED); + await expect( + protocol.findData({ object: 'gate_nothing_at_all' }), + ).rejects.toMatchObject(OBJECT_NOT_FOUND); + }); + + it('404s before the query is parsed — an unknown object cannot be probed for query-shape validity', async () => { + // A bad `$` param on a KNOWN object is a 400 (#2926 ⑩); on an unknown + // one the answer must stay 404 so the two are indistinguishable. + await expect( + protocol.findData({ object: 'gate_account', query: { $nope: '1' } }), + ).rejects.toMatchObject({ status: 400, code: 'UNSUPPORTED_QUERY_PARAM' }); + await expect( + protocol.findData({ object: UNREGISTERED, query: { $nope: '1' } }), + ).rejects.toMatchObject(OBJECT_NOT_FOUND); + }); + + // ───────────────────────────────────────────────────────────── + // Every data entry point, not just the read one + // ───────────────────────────────────────────────────────────── + + it('rejects every data entry point for an unregistered object', async () => { + const calls: Array<[string, () => Promise]> = [ + ['findData', () => protocol.findData({ object: UNREGISTERED })], + ['getData', () => protocol.getData({ object: UNREGISTERED, id: 'r1' })], + ['createData', () => protocol.createData({ object: UNREGISTERED, data: { secret: 'x' } })], + ['cloneData', () => protocol.cloneData({ object: UNREGISTERED, id: 'r1' })], + ['updateData', () => protocol.updateData({ object: UNREGISTERED, id: 'r1', data: { secret: 'x' } })], + ['deleteData', () => protocol.deleteData({ object: UNREGISTERED, id: 'r1' })], + ['createManyData', () => protocol.createManyData({ object: UNREGISTERED, records: [{ secret: 'x' }] })], + ['insertManyData', () => protocol.insertManyData({ object: UNREGISTERED, records: [{ secret: 'x' }] })], + ['updateManyData', () => protocol.updateManyData({ object: UNREGISTERED, records: [{ id: 'r1', data: {} }] } as any)], + ['deleteManyData', () => protocol.deleteManyData({ object: UNREGISTERED, ids: ['r1'] } as any)], + ['batchData', () => protocol.batchData({ object: UNREGISTERED, request: { operation: 'create', records: [{ data: {} }] } as any })], + ['analyticsQuery', () => protocol.analyticsQuery({ cube: UNREGISTERED, query: { measures: ['count'] } })], + ]; + for (const [name, call] of calls) { + await expect(call(), `${name} must reject`).rejects.toMatchObject(OBJECT_NOT_FOUND); + } + // Nothing was written to, or removed from, the ghost table. + expect(Array.from(stores.get(UNREGISTERED)!.values())).toEqual([{ id: 'r1', secret: 'classified' }]); + }); + + // ───────────────────────────────────────────────────────────── + // No regression for registered objects + // ───────────────────────────────────────────────────────────── + + it('leaves registered objects working end to end', async () => { + const created = await protocol.createData({ object: 'gate_account', data: { name: 'Acme' } }); + expect(created.record.name).toBe('Acme'); + + const listed = await protocol.findData({ object: 'gate_account' }); + expect(listed.records).toHaveLength(1); + + const got = await protocol.getData({ object: 'gate_account', id: created.id }); + expect(got.record.name).toBe('Acme'); + + await protocol.updateData({ object: 'gate_account', id: created.id, data: { name: 'Acme 2' } }); + expect((await protocol.getData({ object: 'gate_account', id: created.id })).record.name).toBe('Acme 2'); + + await protocol.deleteData({ object: 'gate_account', id: created.id }); + expect((await protocol.findData({ object: 'gate_account' })).records).toHaveLength(0); + }); + + it('still reports RECORD_NOT_FOUND (not OBJECT_NOT_FOUND) for a missing row on a registered object', async () => { + await expect( + protocol.getData({ object: 'gate_account', id: 'nope' }), + ).rejects.toMatchObject({ code: 'RECORD_NOT_FOUND', status: 404 }); + }); +}); diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index 8e5da1d5c6..02c89873ab 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -53,12 +53,22 @@ const TRANSLATABLE_META_TYPES = new Set(['view', 'action', 'object', 'app', 'das /** - * Map a data-layer error to a clean HTTP response. Unknown-object errors - * (SQLite "no such table", PG "relation does not exist", protocol - * "object not found", etc.) are surfaced as a 404 with `code: 'object_not_found'` - * so clients can distinguish "object isn't registered" from real server - * faults. Anything else becomes a 400 (bad request) preserving prior - * behavior. Genuine 500s are still logged. + * Map a data-layer error to a clean HTTP response. Unknown-object errors are + * surfaced as a 404 with `code: 'object_not_found'` so clients can distinguish + * "object isn't registered" from real server faults. Anything else becomes a + * 400 (bad request) preserving prior behavior. Genuine 500s are still logged. + * + * Two sources produce that 404, and since #3770 the FIRST one is the primary: + * - `code: 'OBJECT_NOT_FOUND'` from the protocol's registry gate + * (`assertObjectRegistered`) — an authoritative, driver-independent answer + * raised before the object name is ever turned into a table name. + * - Driver error strings (SQLite "no such table", PG "relation does not + * exist", …) — retained as the safety net for the *other* failure, an + * object that IS registered but whose physical table is missing (metadata / + * schema drift), plus engine-direct callers that bypass the protocol. + * Before #3770 this string match was the ONLY thing producing the 404, + * which is why an unregistered object whose table happened to exist was + * served instead of rejected. * * `PermissionDeniedError` (thrown by `SecurityPlugin`) MUST be caught * before the unknown-object heuristic, otherwise its message — @@ -200,6 +210,25 @@ export function mapDataError(error: any, object?: string): { status: number; bod }, }; } + // [#3770] Object does not exist — thrown by the protocol's registry gate + // (`assertObjectRegistered`, which covers every data entry point) and by + // `cloneData`. Mapped to the SAME envelope the driver-string branch below + // produces, so one condition has exactly one wire code (`object_not_found`, + // the canonical `ApiErrorCode`) no matter which layer detected it — the + // point of #3770 is that this 404 no longer depends on a driver erroring + // on a missing table. Must precede the generic 4xx passthrough, which + // would otherwise ship the internal SCREAMING_CASE code verbatim. + if (error?.code === 'OBJECT_NOT_FOUND') { + const name = error?.object ?? object; + return { + status: 404, + body: { + error: name ? `Object '${name}' is not registered` : 'Object not found', + code: 'object_not_found', + ...(name ? { object: name } : {}), + }, + }; + } // Generic passthrough for domain errors that already carry an explicit // HTTP status (e.g. plugin-sharing's record-scope denial: status 403 + // code FORBIDDEN) — mirrors sendError's `.status` handling, which the @@ -414,7 +443,13 @@ export function mapDataError(error: any, object?: string): { status: number; bod * uniformly across CRUD, batch, metadata, UI and discovery routes. */ function sendError(res: any, error: any, object?: string): void { - if (typeof error?.status === 'number' && error.status >= 400 && error.status < 600) { + // [#3770] `OBJECT_NOT_FOUND` is deliberately excluded from this + // status-passthrough: `mapDataError` owns its canonical envelope + // (`object_not_found`), and short-circuiting here would ship a second wire + // code for the same condition depending on which route caught it. + const passThroughStatus = error?.code !== 'OBJECT_NOT_FOUND' + && typeof error?.status === 'number' && error.status >= 400 && error.status < 600; + if (passThroughStatus) { const safeMsg = typeof error.message === 'string' && error.message.length < 500 ? error.message : 'Request failed'; @@ -1139,10 +1174,32 @@ export class RestServer { * - `enable.apiMethods` (non-empty whitelist) → unlisted operations rejected (405). * * Default-allow: objects with no `enable` block (or `apiEnabled` unset/true and - * no `apiMethods` whitelist) behave exactly as before — no regression. Unknown - * objects fall through to the normal 404 path. A metadata-read failure does not - * block (the data call itself needs the same metadata and will surface the - * error). Returns `true` when the request was blocked (response already sent). + * no `apiMethods` whitelist) behave exactly as before — no regression. A + * metadata-read failure does not block (the data call itself needs the same + * metadata and will surface the error). Returns `true` when the request was + * blocked (response already sent). + * + * ## Unknown objects (#3770) + * + * An object this gate cannot find in metadata is passed through — there is no + * declared exposure policy to enforce on it, so there is nothing for this gate + * to decide. What CLOSES it is downstream, and it is worth naming precisely + * because the previous note here named the wrong thing ("let the data path + * 404" — a fallback that did not exist): + * + * 1. `protocol.assertObjectRegistered` (#3770) rejects every data entry point + * for an object absent from the schema registry with 404 + * `OBJECT_NOT_FOUND`, BEFORE the engine turns the name into a table name. + * That is the real 404, and unlike the old assumption it does not depend + * on a driver happening to error on a missing table — which is why an + * unregistered object whose physical table DID exist used to be served. + * 2. plugin-security's `getObjectSecurityMeta` (#3545) reports an + * `unresolved` posture for the same object, and the engine middleware, + * `canExport` and `getReadableFields` fail CLOSED on it. + * + * Neither is a reason to widen this gate: (1) is the existence answer and (2) + * is the authorization answer. Do not relax the pass-through on the assumption + * that some other layer 404s — verify which one, as #3770 did. * * See ADR-0049 (#1889): shipping a non-enforcing `apiEnabled` is false security. */ @@ -1158,7 +1215,9 @@ export class RestServer { if (!objectName) return false; const items = await this.loadObjectItems(p, environmentId); const obj = items.find((o: any) => o?.name === objectName); - if (!obj) return false; // unknown object → let the data path 404 + // [#3770] Unknown object → no declared exposure policy to enforce here; + // the data path's registry gate 404s it. See the doc comment above. + if (!obj) return false; const denial = apiAccessDenialFromEnable(obj.enable, objectName, operation, opts); if (denial) { res.status(denial.status).json(denial.body); diff --git a/packages/rest/src/rest.test.ts b/packages/rest/src/rest.test.ts index 7374d2a074..1b2b2cbe03 100644 --- a/packages/rest/src/rest.test.ts +++ b/packages/rest/src/rest.test.ts @@ -2365,6 +2365,44 @@ describe('mapDataError — schema/constraint envelopes', () => { expect(r.body.error).toBe('线索信息不完整'); expect(r.body.object).toBeUndefined(); }); + + // [#3770] The protocol's registry gate is now the PRIMARY producer of the + // unknown-object 404 — it answers from the schema registry before the name + // ever becomes a table name, instead of the REST layer inferring it from a + // driver's error string. Both producers must land on ONE wire envelope, or + // a client keying on `code` sees which layer noticed rather than what + // happened. + const objectNotFound = (object: string) => + Object.assign(new Error(`Object '${object}' not found`), { + code: 'OBJECT_NOT_FOUND', + status: 404, + object, + }); + + it('maps the protocol registry gate OBJECT_NOT_FOUND → 404 object_not_found', () => { + const r = mapDataError(objectNotFound('ghost'), 'ghost'); + expect(r.status).toBe(404); + expect(r.body.code).toBe('object_not_found'); + expect(r.body.object).toBe('ghost'); + }); + + it('emits the identical envelope whether the gate or the driver detected it', () => { + const fromGate = mapDataError(objectNotFound('ghost'), 'ghost'); + const fromDriver = mapDataError(sqliteError('no such table: ghost'), 'ghost'); + expect(fromGate).toEqual(fromDriver); + }); + + it('names the object from the error itself when the caller passes none', () => { + const r = mapDataError(objectNotFound('ghost')); + expect(r.status).toBe(404); + expect(r.body.code).toBe('object_not_found'); + expect(r.body.object).toBe('ghost'); + }); + + it('does not let the generic 4xx passthrough ship the internal SCREAMING_CASE code', () => { + const r = mapDataError(objectNotFound('ghost'), 'ghost'); + expect(r.body.code).not.toBe('OBJECT_NOT_FOUND'); + }); }); // ---------------------------------------------------------------------------