Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 65 additions & 0 deletions .changeset/data-path-object-existence-gate.md
Original file line numberDiff line numberDiff line change
@@ -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.
104 changes: 94 additions & 10 deletions packages/metadata-protocol/src/protocol.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -63,6 +63,14 @@ import {
*/
const TYPE_TO_FORM: Readonly<Record<string, FormView>> = 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.
Expand DownExpand Up@@ -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
Expand DownExpand Up@@ -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 }
};
Expand DownExpand Up@@ -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(
Expand DownExpand Up@@ -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<string, any>, 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;
Expand All@@ -2962,7 +3034,7 @@ export class ObjectStackProtocolImplementation implements
// path re-derives them rather than carrying the source's values over.
const data: Record<string, any> = { ...source };
for (const f of CLONE_STRIP_FIELDS) delete data[f];
const fields: Record<string, any> = schema.fields || {};
const fields: Record<string, any> = schema?.fields || {};
for (const [name, def] of Object.entries(fields)) {
if (!def) continue;
// Engine-/automation-owned values: injected system/audit columns,
Expand DownExpand Up@@ -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;
Expand All@@ -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;
Expand DownExpand Up@@ -3325,6 +3399,7 @@ export class ObjectStackProtocolImplementation implements

async batchData(request: { object: string, request: BatchUpdateRequest, context?: any }): Promise<BatchUpdateResponse> {
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;
Expand DownExpand Up@@ -3428,6 +3503,7 @@ export class ObjectStackProtocolImplementation implements
}

async createManyData(request: { object: string, records: any[], context?: any }): Promise<any> {
this.assertObjectRegistered(request.object); // [#3770]
// [#3043] Ingress-level static-`readonly` strip (per row) — mirrors
// createData for the bulk-create / import surface.
const rows = stripReadonlyForInsert(
Expand DownExpand Up@@ -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)');
Expand DownExpand Up@@ -3507,6 +3584,7 @@ export class ObjectStackProtocolImplementation implements

async updateManyData(request: UpdateManyDataRequest & { context?: any }): Promise<BatchUpdateResponse> {
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;
Expand DownExpand Up@@ -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 || [];
Expand DownExpand Up@@ -3723,6 +3806,7 @@ export class ObjectStackProtocolImplementation implements
}

async deleteManyData(request: DeleteManyDataRequest): Promise<any> {
this.assertObjectRegistered(request.object); // [#3770]
// This expects deleting by IDs.
return this.engine.delete(request.object, {
where: { id: { $in: request.ids } },
Expand Down
60 changes: 60 additions & 0 deletions packages/objectql/src/protocol-data.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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();
});
});
});
Loading
Loading