From 346c62148c2b00534c90942fe595546793786a5d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 12:33:11 +0000 Subject: [PATCH] =?UTF-8?q?fix(metadata-protocol,rest):=20gate=20the=20dat?= =?UTF-8?q?a=20path=20on=20object=20existence=20=E2=80=94=20the=20404=20th?= =?UTF-8?q?e=20exposure=20gate=20assumed=20(#3770)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `enforceApiAccess` passes through any object it cannot find in metadata, and justified that with `// unknown object → let the data path 404`. That fallback did not exist: 1. `findData` — and every data entry point except `cloneData` — had no existence check; the repo's only `OBJECT_NOT_FOUND` throw was in `cloneData`. 2. The engine does not reject unregistered names: `resolveObjectName` falls back to `StorageNameMapping.resolveTableName({ name })`, so the object name is used as the table name. 3. The 404 was 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 404 held only while the table happened not to exist. When one did — out-of-band DDL, a registration that failed after `syncObjectSchema` had run, a registration race — the exposure gate was silently skipped and the rows were served. Verified live: `GET /api/v1/data/sqlite_sequence` used to reach the driver as a table name; it now returns 404 `object_not_found`. Adds `ObjectStackProtocolImplementation.assertObjectRegistered`, run before storage is touched by 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. `cloneData`'s open-coded check becomes that shared gate; its envelope is unchanged. The gate sits at the protocol ingress, the same boundary `apiEnabled` guards: internal callers (hooks, flows, migrations, raw ObjectQL) reach the engine directly and are unaffected. When the engine exposes no schema registry there is nothing to consult, so it stands down and warns once per process — the tiering #3545 recorded in `api-exposure.ts` for a whole-registry outage. `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 one condition has one wire code regardless of which layer noticed. That branch stays as the safety net for the other failure it actually covers: an object that IS registered but whose physical table is missing. `sendError` defers to it so both funnels agree. 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. Tests: `protocol-unregistered-object.test.ts` drives a real ObjectQL engine with a table present under an unregistered name (4 of its cases fail without this change, including the row being served); unit coverage pins the tiering; REST coverage pins the single wire envelope. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EWHv5StvA4KDkHhx19xufX --- .changeset/data-path-object-existence-gate.md | 65 +++++ packages/metadata-protocol/src/protocol.ts | 104 +++++++- packages/objectql/src/protocol-data.test.ts | 60 +++++ .../src/protocol-unregistered-object.test.ts | 225 ++++++++++++++++++ packages/rest/src/rest-server.ts | 83 ++++++- packages/rest/src/rest.test.ts | 38 +++ 6 files changed, 553 insertions(+), 22 deletions(-) create mode 100644 .changeset/data-path-object-existence-gate.md create mode 100644 packages/objectql/src/protocol-unregistered-object.test.ts 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'); + }); }); // ---------------------------------------------------------------------------