Skip to content

Commit 3949a43

Browse files
os-zhuangclaude
andauthored
fix(metadata-protocol,rest): gate the data path on object existence — the 404 the exposure gate assumed (#3770) (#3866)
`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: `findData` (and every data entry point except `cloneData`) had no existence check; the engine does not reject unregistered names (`resolveObjectName` falls back to `resolveTableName({ name })`, so the object name IS the table name); and 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. 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. Follow-up filed as #3867: the same shape in a different subsystem (analytics `/query` on deployments running AnalyticsServicePlugin), plus a raw-SQL leak on that route's error path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 5d4de37 commit 3949a43

6 files changed

Lines changed: 553 additions & 22 deletions

File tree

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
---
2+
"@objectstack/metadata-protocol": minor
3+
"@objectstack/rest": minor
4+
---
5+
6+
fix(metadata-protocol,rest): the data path really 404s unknown objects now (#3770)
7+
8+
The REST API-exposure gate (`enforceApiAccess`) passes through any object it
9+
cannot find in metadata, and the comment there justified that with
10+
`// unknown object → let the data path 404`. That fallback did not exist.
11+
12+
- `findData` — and every other data entry point except `cloneData` — had **no
13+
existence check**. The repo's only `OBJECT_NOT_FOUND` throw was in `cloneData`.
14+
- The engine does not reject unregistered names either: `resolveObjectName`
15+
falls back to `StorageNameMapping.resolveTableName({ name })`, so the object
16+
name is used **as the table name**.
17+
- The 404 was therefore only ever a side effect of the **driver** erroring on a
18+
missing table, which the REST layer recognised by matching the driver's error
19+
string.
20+
21+
So the 404 held only when the table happened not to exist. When a physical table
22+
with that name **did** exist — out-of-band DDL, a registration that failed after
23+
`syncObjectSchema` had already run, a registration race — the exposure gate was
24+
silently skipped and the rows were served, with no layer turning it into a 404.
25+
(Since #3545 an authenticated caller on a plugin-security deployment is refused
26+
by the fail-closed posture check; anonymous callers and deployments without
27+
plugin-security were not.)
28+
29+
**The gate.** `ObjectStackProtocolImplementation` now runs a shared
30+
`assertObjectRegistered` before storage is touched, on `findData`, `getData`,
31+
`createData`, `cloneData`, `updateData`, `deleteData`, `batchData`,
32+
`createManyData`, `insertManyData`, `updateManyData`, `deleteManyData` and
33+
`analyticsQuery`. An object absent from the schema registry is rejected with
34+
`OBJECT_NOT_FOUND` / 404 — an authoritative answer from the registry, raised
35+
*before* the name becomes a table name, instead of an inference from driver
36+
prose. `cloneData`'s open-coded check is now that shared gate; its envelope is
37+
unchanged.
38+
39+
It sits at the protocol ingress, the same boundary `apiEnabled` guards: internal
40+
callers (hooks, flows, migrations, raw ObjectQL) go to the engine directly and
41+
are unaffected. When the engine exposes no schema registry at all there is
42+
nothing to consult, so the gate stands down and warns once per process —
43+
matching the tiering #3545 recorded in `api-exposure.ts` for a whole-registry
44+
outage.
45+
46+
**Behaviour change.** A REST data request for an object that is not in the
47+
schema registry now returns `404 object_not_found` even when a table of that
48+
name exists. Previously it returned that table's rows. If a deployment depended
49+
on reading a table with no registered object, register the object (its schema is
50+
what every other layer — exposure, RBAC/FLS/RLS, field projection — already
51+
needs in order to enforce anything at all).
52+
53+
**One wire code.** `mapDataError` maps the protocol's `OBJECT_NOT_FOUND` to the
54+
canonical `object_not_found` `ApiErrorCode` — byte-identical to the envelope the
55+
driver-string branch already produced — so a client keying on `code` sees *what
56+
happened*, not *which layer noticed*. The driver-string branch stays as the
57+
safety net for the other failure it actually covers: an object that IS registered
58+
but whose physical table is missing. Callers that were reading `cloneData`'s 404
59+
as `code: 'OBJECT_NOT_FOUND'` on the wire now get `object_not_found`; the status
60+
is 404 either way.
61+
62+
The misleading comment is replaced with what actually closes the hole — this
63+
gate for existence, plugin-security's `unresolved` posture (#3545) for
64+
authorization — and a note not to widen the exposure gate on the assumption that
65+
some other layer 404s.

packages/metadata-protocol/src/protocol.ts

Lines changed: 94 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,14 @@ import {
6363
*/
6464
const TYPE_TO_FORM: Readonly<Record<string, FormView>> = METADATA_FORM_REGISTRY;
6565

66+
/**
67+
* [#3770] One-shot flag for the "engine has no schema registry" warning emitted
68+
* by {@link ObjectStackProtocolImplementation.assertObjectRegistered}. The
69+
* condition is a property of how the host constructed the engine, so it is
70+
* constant for the process — warn once, not once per request.
71+
*/
72+
let warnedNoRegistryForDataGate = false;
73+
6674
/**
6775
* Convert a Zod schema to a JSON Schema, returning `undefined` if conversion
6876
* fails (e.g. unsupported constructs). Cached per schema reference.
@@ -2584,7 +2592,72 @@ export class ObjectStackProtocolImplementation implements
25842592
}
25852593
}
25862594

2595+
/**
2596+
* [#3770] Data-plane existence gate — the object MUST be in the schema
2597+
* registry before any data entry point below touches storage.
2598+
*
2599+
* ## Why this exists
2600+
*
2601+
* The REST API-exposure gate (`enforceApiAccess`, ADR-0049 / #1889) skips
2602+
* objects it cannot find in metadata, and justified that with "the data
2603+
* path will 404 anyway". It would not. `engine.find` resolves an
2604+
* UNREGISTERED name straight to a physical table name
2605+
* (`resolveObjectName` → `StorageNameMapping.resolveTableName({ name })`),
2606+
* so the request only 404'd as a *side effect* of the driver complaining
2607+
* about a missing table (which the REST layer recognises by matching the
2608+
* driver's error string) — and did not 404 at all when a table with that
2609+
* name happened to exist: out-of-band DDL, a registration that failed
2610+
* after `syncObjectSchema` had already run, a registration race. In that
2611+
* window the exposure gate was silently skipped and the rows were served.
2612+
*
2613+
* The gate lives HERE, at the protocol ingress, for the same reason
2614+
* `enforceApiAccess` does: this is the external API boundary. Internal
2615+
* callers (hooks, flows, migrations, raw ObjectQL) talk to the engine
2616+
* directly and are deliberately unaffected — `apiEnabled` and this check
2617+
* both control automatic API exposure, not data access.
2618+
*
2619+
* ## Tiering — mirrors the #3545 decision recorded in `api-exposure.ts`
2620+
*
2621+
* - **Registry present, object absent → fail CLOSED** (404
2622+
* `OBJECT_NOT_FOUND`). The registry is authoritative for objects:
2623+
* `object` is `allowOrgOverride: false` (ADR-0005), so no per-org
2624+
* overlay can legitimately exist outside the process-wide registry, and
2625+
* both boot hydration (`loadMetaFromDb`) and runtime authoring
2626+
* (`applyObjectRegistryMutation`) register the schema before its table
2627+
* is reachable.
2628+
* - **No registry on the engine at all → skip.** There is no source of
2629+
* truth to consult, so the check cannot answer; failing closed would
2630+
* break every registry-less host (edge/Lite embeddings, engine doubles)
2631+
* for no security gain. Warned once per process so a deployment in that
2632+
* state is observable rather than a silent blanket-allow — the lesson
2633+
* #3545 recorded for `loadObjectItems`.
2634+
*/
2635+
private assertObjectRegistered(object: string): void {
2636+
const registry: any = this.engine?.registry;
2637+
if (!registry || typeof registry.getObject !== 'function') {
2638+
if (!warnedNoRegistryForDataGate) {
2639+
warnedNoRegistryForDataGate = true;
2640+
console.warn(
2641+
'[Protocol] engine exposes no schema registry — the data-plane object-existence '
2642+
+ 'gate (#3770) is INACTIVE for this process; unregistered object names reach the '
2643+
+ 'driver as raw table names.',
2644+
);
2645+
}
2646+
return;
2647+
}
2648+
if (registry.getObject(object)) return;
2649+
const err: any = new Error(`Object '${object}' not found`);
2650+
err.code = 'OBJECT_NOT_FOUND';
2651+
err.status = 404;
2652+
err.object = object;
2653+
throw err;
2654+
}
2655+
25872656
async findData(request: { object: string, query?: any, context?: any }) {
2657+
// [#3770] Existence first: an unregistered object is a 404 before any
2658+
// query parameter is even parsed, so an unknown name can never be
2659+
// probed for query-shape validity (nor reach the driver as a table).
2660+
this.assertObjectRegistered(request.object);
25882661
const options: any = { ...request.query };
25892662
// Forward the dispatcher's ExecutionContext so RBAC/RLS middleware
25902663
// can apply per-request enforcement. The protocol layer is purely
@@ -2838,6 +2911,7 @@ export class ObjectStackProtocolImplementation implements
28382911
}
28392912

28402913
async getData(request: { object: string, id: string, expand?: string | string[], select?: string | string[], context?: any }) {
2914+
this.assertObjectRegistered(request.object); // [#3770]
28412915
const queryOptions: any = {
28422916
where: { id: request.id }
28432917
};
@@ -2883,6 +2957,7 @@ export class ObjectStackProtocolImplementation implements
28832957
}
28842958

28852959
async createData(request: { object: string, data: any, context?: any }) {
2960+
this.assertObjectRegistered(request.object); // [#3770]
28862961
// [#3043] Ingress-level static-`readonly` strip — a non-system caller
28872962
// cannot seed a read-only column (e.g. `approval_status`) on create.
28882963
const data = stripReadonlyForInsert(
@@ -2925,17 +3000,14 @@ export class ObjectStackProtocolImplementation implements
29253000
* clear a unique field, or reset status before insert.
29263001
*/
29273002
async cloneData(request: { object: string, id: string, overrides?: Record<string, any>, context?: any }) {
2928-
const schema: any = this.engine.registry.getObject(request.object);
2929-
if (!schema) {
2930-
const err: any = new Error(`Object '${request.object}' not found`);
2931-
err.code = 'OBJECT_NOT_FOUND';
2932-
err.status = 404;
2933-
err.object = request.object;
2934-
throw err;
2935-
}
3003+
// [#3770] This object-existence check used to be open-coded here and
3004+
// was the ONLY one on the whole data plane; it is now the shared gate
3005+
// every data entry point runs. Same error envelope as before.
3006+
this.assertObjectRegistered(request.object);
3007+
const schema: any = this.engine.registry?.getObject(request.object);
29363008
// `enable.clone` defaults to true in the spec; treat an absent block /
29373009
// absent flag as enabled and only block on an explicit `false`.
2938-
if (schema.enable?.clone === false) {
3010+
if (schema?.enable?.clone === false) {
29393011
const err: any = new Error(`Cloning is disabled for object '${request.object}'`);
29403012
err.code = 'CLONE_DISABLED';
29413013
err.status = 403;
@@ -2962,7 +3034,7 @@ export class ObjectStackProtocolImplementation implements
29623034
// path re-derives them rather than carrying the source's values over.
29633035
const data: Record<string, any> = { ...source };
29643036
for (const f of CLONE_STRIP_FIELDS) delete data[f];
2965-
const fields: Record<string, any> = schema.fields || {};
3037+
const fields: Record<string, any> = schema?.fields || {};
29663038
for (const [name, def] of Object.entries(fields)) {
29673039
if (!def) continue;
29683040
// Engine-/automation-owned values: injected system/audit columns,
@@ -2995,6 +3067,7 @@ export class ObjectStackProtocolImplementation implements
29953067
}
29963068

29973069
async updateData(request: { object: string, id: string, data: any, expectedVersion?: string, context?: any }) {
3070+
this.assertObjectRegistered(request.object); // [#3770]
29983071
await this.assertVersionMatch(request.object, request.id, request.expectedVersion, request.context);
29993072
const opts: any = { where: { id: request.id } };
30003073
if (request.context !== undefined) opts.context = request.context;
@@ -3016,6 +3089,7 @@ export class ObjectStackProtocolImplementation implements
30163089
}
30173090

30183091
async deleteData(request: { object: string, id: string, expectedVersion?: string, context?: any }) {
3092+
this.assertObjectRegistered(request.object); // [#3770]
30193093
await this.assertVersionMatch(request.object, request.id, request.expectedVersion, request.context);
30203094
const opts: any = { where: { id: request.id } };
30213095
if (request.context !== undefined) opts.context = request.context;
@@ -3325,6 +3399,7 @@ export class ObjectStackProtocolImplementation implements
33253399

33263400
async batchData(request: { object: string, request: BatchUpdateRequest, context?: any }): Promise<BatchUpdateResponse> {
33273401
const { object, request: batchReq, context } = request;
3402+
this.assertObjectRegistered(object); // [#3770]
33283403
const { operation, records, options } = batchReq;
33293404
const results: Array<{ id?: string; success: boolean; error?: string; record?: any; droppedFields?: DroppedFieldsEvent[] }> = [];
33303405
let succeeded = 0;
@@ -3428,6 +3503,7 @@ export class ObjectStackProtocolImplementation implements
34283503
}
34293504

34303505
async createManyData(request: { object: string, records: any[], context?: any }): Promise<any> {
3506+
this.assertObjectRegistered(request.object); // [#3770]
34313507
// [#3043] Ingress-level static-`readonly` strip (per row) — mirrors
34323508
// createData for the bulk-create / import surface.
34333509
const rows = stripReadonlyForInsert(
@@ -3470,6 +3546,7 @@ export class ObjectStackProtocolImplementation implements
34703546
* fall back to createManyData.
34713547
*/
34723548
async insertManyData(request: { object: string, records: any[], context?: any }): Promise<{ object: string; outcomes: Array<{ ok: boolean; record?: any; error?: unknown; droppedFields?: DroppedFieldsEvent[] }> }> {
3549+
this.assertObjectRegistered(request.object); // [#3770]
34733550
const engineInsertMany = (this.engine as any)?.insertMany;
34743551
if (typeof engineInsertMany !== 'function') {
34753552
throw new Error('insertManyData requires an engine with insertMany (framework#3172)');
@@ -3507,6 +3584,7 @@ export class ObjectStackProtocolImplementation implements
35073584

35083585
async updateManyData(request: UpdateManyDataRequest & { context?: any }): Promise<BatchUpdateResponse> {
35093586
const { object, records, options, context } = request;
3587+
this.assertObjectRegistered(object); // [#3770]
35103588
const results: Array<{ id?: string; success: boolean; error?: string; record?: any; droppedFields?: DroppedFieldsEvent[] }> = [];
35113589
let succeeded = 0;
35123590
let failed = 0;
@@ -3550,6 +3628,11 @@ export class ObjectStackProtocolImplementation implements
35503628
// cube name maps to object name; measures → aggregations; dimensions → groupBy.
35513629
const { query, cube } = request;
35523630
const object = cube;
3631+
// [#3770] A cube name IS an object name here (`getAnalyticsMeta` derives
3632+
// every cube from `registry.listItems('object')`), so this read surface
3633+
// needs the same existence gate as the CRUD ones — otherwise it stays a
3634+
// way to aggregate over an arbitrary physical table.
3635+
this.assertObjectRegistered(object);
35533636

35543637
// Build groupBy from dimensions
35553638
const groupBy = query.dimensions || [];
@@ -3723,6 +3806,7 @@ export class ObjectStackProtocolImplementation implements
37233806
}
37243807

37253808
async deleteManyData(request: DeleteManyDataRequest): Promise<any> {
3809+
this.assertObjectRegistered(request.object); // [#3770]
37263810
// This expects deleting by IDs.
37273811
return this.engine.delete(request.object, {
37283812
where: { id: { $in: request.ids } },

packages/objectql/src/protocol-data.test.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -593,4 +593,64 @@ describe('ObjectStackProtocolImplementation - Data Operations', () => {
593593
).rejects.toMatchObject({ code: 'OBJECT_NOT_FOUND', status: 404 });
594594
});
595595
});
596+
597+
// ═══════════════════════════════════════════════════════════════
598+
// [#3770] Object-existence gate — the tiering, at unit level
599+
//
600+
// The gap itself (an unregistered object whose physical table exists) is
601+
// pinned against a real engine in `protocol-unregistered-object.test.ts`.
602+
// What this block pins is the DECISION RULE, which an engine double is the
603+
// right tool for: registry present ⇒ the registry is the answer; no
604+
// registry at all ⇒ there is no answer, so the gate stands down.
605+
// ═══════════════════════════════════════════════════════════════
606+
607+
describe('object-existence gate (#3770)', () => {
608+
function makeGateProtocol(known: string[]) {
609+
const engine: any = {
610+
find: vi.fn().mockResolvedValue([]),
611+
findOne: vi.fn().mockResolvedValue(null),
612+
count: vi.fn().mockResolvedValue(0),
613+
insert: vi.fn().mockResolvedValue({ id: 'new-id' }),
614+
update: vi.fn().mockResolvedValue({ id: 'r1' }),
615+
delete: vi.fn().mockResolvedValue(undefined),
616+
registry: {
617+
getObject: vi.fn((name: string) =>
618+
known.includes(name) ? { name, fields: {} } : undefined),
619+
},
620+
};
621+
return { protocol: new ObjectStackProtocolImplementation(engine), engine };
622+
}
623+
624+
it('consults the registry, not the driver, and never reaches the engine on a miss', async () => {
625+
const { protocol, engine } = makeGateProtocol(['task']);
626+
await expect(
627+
protocol.findData({ object: 'ghost' }),
628+
).rejects.toMatchObject({ code: 'OBJECT_NOT_FOUND', status: 404, object: 'ghost' });
629+
expect(engine.registry.getObject).toHaveBeenCalledWith('ghost');
630+
expect(engine.find).not.toHaveBeenCalled();
631+
});
632+
633+
it('lets a registered object straight through', async () => {
634+
const { protocol, engine } = makeGateProtocol(['task']);
635+
await protocol.findData({ object: 'task' });
636+
expect(engine.find).toHaveBeenCalledOnce();
637+
});
638+
639+
it('stands down when the engine exposes no registry at all — nothing to consult', async () => {
640+
// The #3545 tiering: "whole registry unavailable" is a cold-start /
641+
// embedding shape, not a security decision. Failing closed here
642+
// would break every registry-less host for no gain, so the gate
643+
// skips (and warns once — see assertObjectRegistered).
644+
const engine: any = {
645+
find: vi.fn().mockResolvedValue([]),
646+
count: vi.fn().mockResolvedValue(0),
647+
};
648+
const protocol = new ObjectStackProtocolImplementation(engine);
649+
await expect(protocol.findData({ object: 'ghost' })).resolves.toMatchObject({
650+
object: 'ghost',
651+
records: [],
652+
});
653+
expect(engine.find).toHaveBeenCalledOnce();
654+
});
655+
});
596656
});

0 commit comments

Comments
 (0)