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
2 changes: 1 addition & 1 deletion .changeset/23491-patch-data-request-gate.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
---
'@objectstack/rest': minor
'@objectstack/runtime': minor
Expand All @@ -22,6 +22,6 @@

How it renders: `RestServer` answers `{ error, code: 'VALIDATION_FAILED', fields, object }`, the envelope its request gate already used. The runtime dispatcher returns its own `400 VALIDATION_FAILED` envelope, `fields` under `error.details`, instead of throwing, so a `@objectstack/hono` `createHonoApp` host renders it as `400`.

Unchanged: a record body, `{}` included; a string `expectedVersion` (judged; `409` on a mismatch); `If-Match` and the `DELETE`'s `?expectedVersion`, which are strings by transport; and a JSON `null` body on a `POST` create, which both doors still read as an empty record and answer `201`.
Unchanged: a record body, `{}` included; a string `expectedVersion` (judged; `409` on a mismatch); and `If-Match` and the `DELETE`'s `?expectedVersion`, which are strings by transport. A JSON `null` body on a `POST` create is refused too, on both doors, in the companion entry for the `/data` create and query requests.

To send a patch or a row, send the fields as a JSON object. To guard a write, send the version you read (the record's `updated_at`) as a string. The `@objectstack/client` SDK's `data.update` sends the version as an `If-Match` header string, so an SDK caller is affected only when the `data` it passes is not an object, or carries a number `expectedVersion`.
29 changes: 29 additions & 0 deletions .changeset/23574-data-request-contract-remainder.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
---
'@objectstack/rest': minor
'@objectstack/runtime': minor
---

fix(rest,runtime)!: a JSON `null` body on a `/data` create or query is refused `400 VALIDATION_FAILED` on both doors; the runtime dispatcher's `POST /data/:object/query` validates `FindDataRequestSchema`; and the dispatcher asks an object's API exposure before any request gate

`RestServer` read a JSON `null` body on `POST /data/:object` as an empty record and created an empty row, and on `POST /data/:object/query` as no query and answered the full record set. The runtime dispatcher's `/data` domain, the only answer on a host that mounts just the `@objectstack/hono` catch-all, did the same for the create, and validated no query at all: a mistyped clause or a retired key reached the engine, which dropped it and answered the full record set. `CreateDataRequestSchema.data` declares a record and `FindDataRequestSchema.query` declares a QueryAST or nothing, never `null`. Both doors now validate the assembled request as the caller sent it, forward that same request, and answer each case alike. On an object whose API exposure denies the verb, the dispatcher used to answer a `PATCH` body that is not a record, or a create body that is an array, a string or a number, with its request gate's `400`, where `RestServer` answered the exposure refusal; it now asks the exposure gate before each request gate, as `RestServer` does.

Clause-②: no (narrowing)

<!-- adr-0087: not-required (no-migration-prescription) No metadata moves: no spec key, authorable spelling, object definition or stored shape is added, removed, renamed or re-shaped, and no exported declaration is removed or narrowed, so there is nothing for `objectstack migrate meta` to rewrite. What narrows is the accept set of two HTTP routes on two doors: a create or query whose body is a JSON `null`, and on the dispatcher a query the request schema does not declare, is now refused by the request schema where it was applied (or, for a string or array query body, refused by the engine under another code), and the remedy is in the request a caller sends, never a rewrite of a package source or a stored row. The other categories are closed on facts: both packages publish (not unpublished); no ADR-0087 id is named or touched (not registered or already-registered); and nothing exported is removed or narrowed (not runtime-interface-only, type-surface-only or runtime-export-retired). -->

**BREAKING** (an accept-set narrowing), shipped as `minor` under the launch-window convention for breaking changes.

What changes for you, wherever `@objectstack/rest` or the runtime dispatcher serves these routes:

- Both doors, `POST /data/:object` with a JSON `null` body: FROM `201` and an empty row created, TO `400 VALIDATION_FAILED` naming `data`, with no row created.
- Both doors, `POST /data/:object/query` with a JSON `null` body: FROM `200` with the full record set, TO `400 VALIDATION_FAILED` naming `query`, the engine never asked.
- The runtime dispatcher, `POST /data/:object/query` with a mistyped clause (`{ "limit": "ten" }`), the retired `joins` key, or a number as the body: FROM `200` with the full record set, the clause dropped, TO `400 VALIDATION_FAILED` naming the member (`query.limit`, `query.joins`, `query`). `RestServer` already answered this.
- The runtime dispatcher, `POST /data/:object/query` with a string or an array as the body: FROM the engine's `400 INVALID_FIELD` for numbered field names, TO `400 VALIDATION_FAILED` naming `query`. `RestServer` already answered this.
- The runtime dispatcher, `POST /data/:object/query` with a body that nests the query under a `query` key (`{ "query": { "where": … } }`): FROM read as the nested query (`200`), TO read as `RestServer` reads it, the body itself being the query, so `query` is a field the object does not have: `400 INVALID_FIELD`.
- The runtime dispatcher, `PATCH /data/:object/:id` with a body that is not a record (an array, a string, a number or `null`), and `POST /data/:object` with an array, a string or a number as the body, on an object whose API exposure denies the verb: FROM `400 VALIDATION_FAILED`, TO the exposure refusal `RestServer` answers (`404` when the object sets `apiEnabled: false`, `405` when its `apiMethods` leaves the verb out). The request was refused before and is refused now; only the answer changes. A JSON `null` create on such an object already answered the exposure refusal (the old gate read it as `{}`), and so did every query on it: the query route had no request gate, and its new one runs after the exposure gate, as `RestServer` orders it.

How it renders: `RestServer` answers `{ error, code: 'VALIDATION_FAILED', fields, object }`, the envelope its request gates already use. The runtime dispatcher returns its own `400 VALIDATION_FAILED` envelope, `fields` under `error.details`, instead of throwing, so a `@objectstack/hono` `createHonoApp` host renders it as `400`.

Unchanged: an empty request body, which every host hands over as `{}` (an empty record creates a row; an empty query answers every record); a record body; a conforming query; and a body `object` key on a query, which the path object still overrides.

To create a row, send its fields as a JSON object, `{}` for all defaults. To query, send the QueryAST itself as the body, `{}` for every record. The `@objectstack/client` SDK's `data.create` and `data.query` send exactly these, so an SDK caller is affected only when the value it passes is `null`, or not the shape the request declares.
2 changes: 1 addition & 1 deletion content/docs/permissions/system-context.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ One of the largest consumers of the flag;

| # | Behaviour when `isSystem` | Package | What you get / what you lose | Anchor |
|:--|:---|:---|:---|:---|
| 48 | Object API-exposure gate bypassed (`apiEnabled` / `apiMethods`) | runtime | Get: internal self-writes ignore exposure declarations — these govern **external** exposure, not engine self-writes | `packages/runtime/src/action-execution.ts#callData` |
| 48 | Object API-exposure gate bypassed (`apiEnabled` / `apiMethods`) | runtime | Get: internal self-writes ignore exposure declarations — these govern **external** exposure, not engine self-writes. The gate is `refuseUnexposedDataAction`, which `callData` asks first and the `/data` domain asks before its request gate; `callData` reads the same bypass once more for each nested-relation condition's target | `packages/runtime/src/action-execution.ts#callData`, `#refuseUnexposedDataAction` |
| 49 | Action `requiredPermissions` bypassed | runtime | Get: engine self-invocation runs any action | `packages/runtime/src/action-execution.ts#actionPermissionError` |
| 50 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability (the permission-set clone door's save included) — and, because REST's save door and the stored-version read doors ask one save verdict, an app's full stored version on `/layers`, `?layers=true` and `/diff`, whatever its entry gates withhold | `packages/runtime/src/domains/meta.ts#handleMetadataRequest`, `packages/rest/src/rest-server.ts#registerMetadataEndpointsInner`, `packages/rest/src/rest-server.ts#metaSaveVerdict`, `packages/runtime/src/domains/permission-set-clone.ts#handlePermissionSetCloneWrite` |
| 51 | The shared metadata-write verdict itself returns `allowed` | metadata-core | Get: the one function all of row 50's doors consult answers yes before any capability is examined | `packages/metadata-core/src/meta-write-capability.ts#metaWriteCapabilityVerdict` |
Expand Down
7 changes: 6 additions & 1 deletion packages/rest/src/request-schema-gate.conformance.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,14 +117,19 @@ const CASES: GateCase[] = [
// fallback and an ordinary sum on every SQL datasource — one query, two
// numbers, both plausible.
{ aggregations: [{ function: 'sum', field: 'amount', alias: 'total', distinct: true }] },
// #23574 — `null` was read as no query (`req.body ?? {}`) and answered
// the full record set; `query` declares a QueryAST or nothing.
null,
],
valid: { where: { status: 'active' }, limit: 5 },
spy: 'findData',
},
{
route: 'POST /api/v1/data/:object',
params: PARAMS,
violating: ['garbage', 42, [{ name: 'x' }]],
// #23574 — `null` was read as an empty record (`req.body ?? {}`) and
// created an empty row.
violating: ['garbage', 42, [{ name: 'x' }], null],
valid: { name: 'ACME' },
spy: 'createData',
},
Expand Down
28 changes: 24 additions & 4 deletions packages/rest/src/rest-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9032,8 +9032,18 @@ export class RestServer {
// reaching the engine as `data`. Per-field checks stay
// downstream (object metadata / validation rules); this
// gate is about the SHAPE the contract declares.
//
// [#23574] …the body AS THE CALLER SENT IT. It was
// `req.body ?? {}`, so a JSON `null` body parsed as an
// empty record and created an empty row (`201`). `null`
// is not the record `data` declares, so it is refused
// like an array, a string or a number, and the runtime
// dispatcher's twin (`domains/data.ts`) asks this gate
// over the same assembled request. An empty request
// body is not `null`: the HTTP adapter hands it over
// as `{}`, a record.
const { CreateDataRequestSchema } = await import('@objectstack/spec/api');
const createInput = { object: req.params.object, data: req.body ?? {} };
const createInput = { object: req.params.object, data: req.body };
const parsedCreate = (CreateDataRequestSchema as any).safeParse(createInput);
if (!parsedCreate.success) {
res.status(400).json({
Expand All @@ -9046,7 +9056,7 @@ export class RestServer {
}
const createRequest: ServerScopedDataRequest<CreateDataRequest> = {
object: req.params.object,
data: req.body ?? {},
data: req.body,
...(environmentId ? { environmentId } : {}),
...(context ? { context } : {}),
};
Expand Down Expand Up @@ -9096,11 +9106,21 @@ export class RestServer {
// ORIGINAL body is forwarded, not the parse output, so the
// schema cannot inject defaults the engine did not receive
// before (the analytics-entry precedent, #3878).
//
// [#23574] …the body AS THE CALLER SENT IT. It was
// `req.body ?? {}`, so a JSON `null` body read as no
// query and answered the full record set (`200`).
// `query` declares a QueryAST or nothing, never `null`,
// so `null` is refused like a string or a number, and
// the runtime dispatcher's twin (`domains/data.ts`) now
// asks this gate over the same assembled request. An
// empty request body is not `null`: the HTTP adapter
// hands it over as `{}`, an empty query.
const { FindDataRequestSchema } = await import('@objectstack/spec/api');
const rawQuery = req.body ?? {};
const rawQuery = req.body;
const query = (rawQuery && typeof rawQuery === 'object' && !Array.isArray(rawQuery))
? { ...rawQuery, object: req.params.object }
: rawQuery; // non-object bodies go to the schema as-is and fail with `query: invalid_type`
: rawQuery; // non-object bodies (`null` included) go to the schema as-is and fail with `query: invalid_type`
const findInput = { object: req.params.object, query };
const parsedFind = (FindDataRequestSchema as any).safeParse(findInput);
if (!parsedFind.success) {
Expand Down
56 changes: 39 additions & 17 deletions packages/runtime/src/action-execution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,43 @@ async function resolveEngineQuietly(deps: ActionExecutionDeps, requestContext: H
}
}

/**
* ── Object-level API exposure gate (ADR-0049, #1889) ─────
* Honour the object's `apiEnabled` / `apiMethods` declarations for external
* traffic: throws the refusal ({@link checkApiExposure}'s `404` / `405`) when
* `object` does not expose `action`, and returns otherwise. System/internal
* contexts bypass — these flags govern API *exposure*, not internal engine
* self-writes.
*
* [#23574] Extracted from {@link callData}, which still asks it first for
* every caller, so that the runtime dispatcher's `/data` domain can ask the
* SAME question before its own request-schema gate (`domains/data.ts`): a
* caller the object refuses is answered the refusal, never a shape verdict
* about the body it sent. `RestServer` asks `enforceApiAccess` before its
* twin gate for the same reason. Moved, not copied: this is the one place the
* dispatcher's exposure decision is resolved.
*/
export async function refuseUnexposedDataAction(deps: ActionExecutionDeps,
requestContext: HttpProtocolContext,
action: string,
object: unknown,
scopeId?: string,
executionContext?: ExecutionContext,
): Promise<void> {
if (executionContext?.isSystem || !object) return;
let def: any;
try {
const meta = await deps.resolveService(requestContext, 'metadata', scopeId);
def = await (meta as any)?.getObject?.(object);
} catch {
def = undefined; // fall open to schema defaults (apiEnabled=true)
}
const gate = checkApiExposure(def, action);
if (!gate.allowed) {
throw { statusCode: gate.status ?? 403, message: gate.reason ?? 'API access denied' };
}
}

export async function callData(deps: ActionExecutionDeps,
requestContext: HttpProtocolContext,
action: string,
Expand All @@ -283,27 +320,12 @@ export async function callData(deps: ActionExecutionDeps,
scopeId?: string,
executionContext?: ExecutionContext,
): Promise<any> {
// ── Object-level API exposure gate (ADR-0049, #1889) ─────
// Honour the object's `apiEnabled` / `apiMethods` declarations for
// external traffic. System/internal contexts bypass — these flags
// govern API *exposure*, not internal engine self-writes.
// The object-level API exposure gate ({@link refuseUnexposedDataAction}).
// [#22823] Decided ONCE, here: the same bypass covers each nested-relation
// condition's target, asked below on the two arms that hand the engine a
// caller's `where` ({@link refuseUnservedRelationTargets}).
const exposureGated = !executionContext?.isSystem;
if (exposureGated && params?.object) {
let def: any;
try {
const meta = await deps.resolveService(requestContext, 'metadata', scopeId);
def = await (meta as any)?.getObject?.(params.object);
} catch {
def = undefined; // fall open to schema defaults (apiEnabled=true)
}
const gate = checkApiExposure(def, action);
if (!gate.allowed) {
throw { statusCode: gate.status ?? 403, message: gate.reason ?? 'API access denied' };
}
}
await refuseUnexposedDataAction(deps, requestContext, action, params?.object, scopeId, executionContext);

const protocol = await deps.resolveService(requestContext, 'protocol', scopeId);
const qlService = dataDriver ?? await deps.getObjectQL(requestContext, scopeId);
Expand Down
11 changes: 9 additions & 2 deletions packages/runtime/src/domains/data-path-object.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -222,11 +222,18 @@ describe('POST /data/:object/query binds to the object in the path (#3946)', ()
expect(req.query).toMatchObject({ where: { status: 'open' }, limit: 5 });
});

it('still honours an explicit `query` envelope', async () => {
// [#23574] Flipped. This pinned the body's `query` key being unwrapped as
// the query itself, a second spelling `RestServer` never read (there the
// body IS the QueryAST, as the catalog declares) and one the request gate
// cannot see: `{ query: { limit: 'ten' } }` would pass the gate and be
// served as the full set. The key now reaches the protocol as the field it
// names, which the real protocol refuses `400 INVALID_FIELD` on both doors
// (`data-write-request-gate.test.ts`, on a real store).
it('a body `query` key is the field it names, never a second spelling of the query', async () => {
const { deps, context, findData } = setup();
await post(deps, context, 'crm_account/query', { query: { where: { status: 'open' } } });

expect(findData.mock.calls[0][0].query).toEqual({ where: { status: 'open' } });
expect(findData.mock.calls[0][0].query).toEqual({ query: { where: { status: 'open' } } });
});

it('threads the caller execution context through unchanged', async () => {
Expand Down
Loading
Loading