Skip to content

Commit 775d552

Browse files
committed
fix(rest): enforce per-object API access on the cross-object batch route (#1604)
The POST {basePath}/batch cross-object transactional batch (issue #1604 / ADR-0034) wraps N create/update/delete ops in one engine transaction but skipped the per-object API-exposure gate every single-record write applies — so an authenticated caller could write to an apiEnabled:false object, or run an operation outside an object's apiMethods whitelist, straight through the batch surface (ADR-0049 / #1889; the same declared-not-enforced hole closed for the generic write path in #3220 / #3213). - Validate the request against a new CrossObjectBatchRequestSchema (@objectstack/spec/api, Zod-First); a malformed op / unknown action / missing object is now a 400, not a 500. - Enforce enable.apiEnabled / apiMethods for EVERY op (metadata fetched once, each distinct object+action checked once) BEFORE opening the transaction → 404 OBJECT_API_DISABLED / 405 OBJECT_API_METHOD_NOT_ALLOWED. - Require an id for update/delete; reject an unresolvable {$ref} with 400 BATCH_UNRESOLVED_REF instead of writing a silent null FK; reject an explicit atomic:false (400 BATCH_NOT_ATOMIC). - Refactor enforceApiAccess to share the pure apiAccessDenialFromEnable check + a loadObjectItems helper with the batch route (single-record behavior unchanged). - Add rest-batch-endpoint.test.ts — the REST-boundary coverage ADR-0034 flagged as missing (commit, $ref, rollback surfacing, API-access denial, validation). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015DgfsaEfmwVAY1SsPtQJ6U
1 parent 06cb319 commit 775d552

5 files changed

Lines changed: 460 additions & 44 deletions

File tree

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
---
2+
'@objectstack/rest': patch
3+
'@objectstack/spec': minor
4+
---
5+
6+
fix(rest): gate the cross-object transactional batch by the same per-object API rules as single-record writes (#1604)
7+
8+
The `POST {basePath}/batch` route (issue #1604 / ADR-0034) wraps N cross-object
9+
create/update/delete ops in one engine transaction, but it skipped the
10+
per-object API-exposure gate every single-record route applies — an
11+
authenticated caller could write to an `apiEnabled: false` object, or run an
12+
operation outside an object's `apiMethods` whitelist, straight through the batch
13+
surface (ADR-0049 / #1889 — the same "declared ≠ enforced" hole closed for the
14+
generic write path in #3220 / #3213).
15+
16+
The route now:
17+
18+
- validates the body against a new `CrossObjectBatchRequestSchema`
19+
(`@objectstack/spec/api`, Zod-First) — a malformed op, an unknown action, or a
20+
missing `object` is a `400` instead of a `500`;
21+
- enforces `enable.apiEnabled` / `enable.apiMethods` for **every** op (metadata
22+
fetched once, each distinct `(object, action)` checked) BEFORE opening the
23+
transaction — `404 OBJECT_API_DISABLED` / `405 OBJECT_API_METHOD_NOT_ALLOWED`;
24+
- requires an `id` for `update` / `delete` (`400`);
25+
- rejects an unresolvable `{ $ref }` with `400 BATCH_UNRESOLVED_REF` instead of
26+
silently writing a `null` FK;
27+
- rejects an explicit `atomic: false` (`400 BATCH_NOT_ATOMIC`) rather than
28+
silently applying atomically — non-atomic per-object batches stay on
29+
`POST /data/:object/batch`.
30+
31+
`enforceApiAccess` is refactored to share the pure `apiAccessDenialFromEnable`
32+
check + a `loadObjectItems` helper with the batch route (single-record behavior
33+
unchanged). Adds `rest-batch-endpoint.test.ts` — the REST-boundary coverage
34+
ADR-0034 flagged as missing (commit, `$ref`, rollback surfacing, API-access
35+
denial, request validation).
Lines changed: 232 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,232 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// Cross-object transactional batch route (POST {basePath}/batch, issue #1604 /
4+
// ADR-0034). Engine-level atomicity is covered in
5+
// objectql/src/engine-ambient-transaction.test.ts; this suite is the
6+
// REST-boundary contract ADR-0034 flagged as missing: request validation,
7+
// per-op API-exposure enforcement, $ref resolution, and error surfacing.
8+
9+
import { describe, it, expect, vi } from 'vitest';
10+
import { RestServer } from './rest-server';
11+
12+
// ── helpers ──────────────────────────────────────────────────────────────────
13+
14+
function mockServer() {
15+
return {
16+
get: vi.fn(), post: vi.fn(), put: vi.fn(), delete: vi.fn(), patch: vi.fn(),
17+
use: vi.fn(), listen: vi.fn().mockResolvedValue(undefined), close: vi.fn().mockResolvedValue(undefined),
18+
};
19+
}
20+
21+
function mockRes() {
22+
const res: any = { statusCode: 200, body: undefined };
23+
res.status = vi.fn((c: number) => { res.statusCode = c; return res; });
24+
res.json = vi.fn((b: any) => { res.body = b; return res; });
25+
res.end = vi.fn(() => res);
26+
return res;
27+
}
28+
29+
/**
30+
* A fake ObjectQL whose `transaction(cb)` runs the callback and rethrows on
31+
* failure (mirroring commit-on-success / rollback-on-throw). `insert` returns a
32+
* freshly-minted id so `$ref` resolution and index-aligned results are testable.
33+
*/
34+
function makeQl(overrides: Partial<Record<'insert' | 'update' | 'delete', any>> = {}) {
35+
let seq = 0;
36+
const ql: any = {
37+
transaction: vi.fn(async (cb: any, ctx: any) => cb({ __trx: true, ctx })),
38+
insert: overrides.insert ?? vi.fn(async (_object: string, data: any) => ({ id: `id_${++seq}`, ...data })),
39+
update: overrides.update ?? vi.fn(async (_object: string, data: any) => ({ ...data })),
40+
delete: overrides.delete ?? vi.fn(async (_object: string, arg: any) => ({ success: true, id: arg?.where?.id })),
41+
};
42+
return ql;
43+
}
44+
45+
/** Build a RestServer with an optional ObjectQL provider and object metadata. */
46+
function buildServer(opts: { ql?: any; objects?: any[] } = {}) {
47+
const server = mockServer();
48+
const protocol: any = {
49+
getDiscovery: vi.fn().mockResolvedValue({ version: 'v0', endpoints: {} }),
50+
getMetaTypes: vi.fn().mockResolvedValue([]),
51+
getMetaItems: vi.fn().mockResolvedValue(opts.objects ?? []),
52+
};
53+
const objectQLProvider = opts.ql ? async () => opts.ql : undefined;
54+
const rest = new RestServer(
55+
server as any, protocol as any, { api: { requireAuth: false } } as any,
56+
undefined, undefined, undefined, undefined, // kernelManager, envRegistry, defaultEnvIdProvider, authServiceProvider
57+
objectQLProvider, // objectQLProvider (positional arg #8)
58+
);
59+
rest.registerRoutes();
60+
const route = rest.getRoutes().find(
61+
(r) => r.method === 'POST' && (r.metadata?.summary ?? '').startsWith('Cross-object'),
62+
);
63+
return { route, protocol };
64+
}
65+
66+
/** POST a body at the cross-object batch route and return the mock response. */
67+
async function post(route: any, body: any) {
68+
const res = mockRes();
69+
await route!.handler({ method: 'POST', params: {}, headers: {}, body } as any, res);
70+
return res;
71+
}
72+
73+
// ── registration ─────────────────────────────────────────────────────────────
74+
75+
describe('POST {basePath}/batch — cross-object transactional batch', () => {
76+
it('registers the route under the data/batch tags', () => {
77+
const { route } = buildServer({ ql: makeQl() });
78+
expect(route).toBeTruthy();
79+
expect(route!.metadata?.tags).toEqual(expect.arrayContaining(['data', 'batch']));
80+
});
81+
82+
it('returns 501 when the runtime has no transactional ObjectQL', async () => {
83+
const { route } = buildServer({}); // no ql provider
84+
const res = await post(route, { operations: [{ object: 'account', data: {} }] });
85+
expect(res.statusCode).toBe(501);
86+
});
87+
88+
// ── happy path ───────────────────────────────────────────────────────────
89+
90+
it('commits multiple ops in one transaction and returns index-aligned results', async () => {
91+
const ql = makeQl();
92+
const { route } = buildServer({ ql });
93+
const res = await post(route, {
94+
operations: [
95+
{ object: 'project', action: 'create', data: { name: 'Apollo' } },
96+
{ object: 'task', action: 'create', data: { title: 'Kickoff' } },
97+
],
98+
});
99+
expect(res.statusCode).toBe(200);
100+
expect(ql.transaction).toHaveBeenCalledTimes(1);
101+
expect(ql.insert).toHaveBeenCalledTimes(2);
102+
expect(res.body.results).toHaveLength(2);
103+
expect(res.body.results[0]).toMatchObject({ id: 'id_1', name: 'Apollo' });
104+
expect(res.body.results[1]).toMatchObject({ id: 'id_2', title: 'Kickoff' });
105+
});
106+
107+
it('resolves { $ref: <opIndex> } to an earlier create\'s id (master-detail)', async () => {
108+
const ql = makeQl();
109+
const { route } = buildServer({ ql });
110+
const res = await post(route, {
111+
operations: [
112+
{ object: 'project', action: 'create', data: { name: 'Apollo' } },
113+
{ object: 'task', action: 'create', data: { title: 'Kickoff', project: { $ref: 0 } } },
114+
],
115+
});
116+
expect(res.statusCode).toBe(200);
117+
// The child's FK was rewritten from { $ref: 0 } to the parent's generated id.
118+
const childData = ql.insert.mock.calls[1][1];
119+
expect(childData.project).toBe('id_1');
120+
});
121+
122+
it('defaults a missing action to create', async () => {
123+
const ql = makeQl();
124+
const { route } = buildServer({ ql });
125+
const res = await post(route, { operations: [{ object: 'account', data: { name: 'X' } }] });
126+
expect(res.statusCode).toBe(200);
127+
expect(ql.insert).toHaveBeenCalledTimes(1);
128+
});
129+
130+
it('returns an empty result set for zero operations without opening a transaction', async () => {
131+
const ql = makeQl();
132+
const { route } = buildServer({ ql });
133+
const res = await post(route, { operations: [] });
134+
expect(res.statusCode).toBe(200);
135+
expect(res.body).toEqual({ results: [] });
136+
expect(ql.transaction).not.toHaveBeenCalled();
137+
});
138+
139+
// ── atomicity / rollback surfacing ────────────────────────────────────────
140+
141+
it('surfaces a failing op with its mapped status (atomic rollback, not partial success)', async () => {
142+
const insert = vi.fn()
143+
.mockResolvedValueOnce({ id: 'id_1', name: 'Apollo' })
144+
.mockRejectedValueOnce(Object.assign(new Error('bad'), { code: 'VALIDATION_FAILED', name: 'ValidationError' }));
145+
const ql = makeQl({ insert });
146+
const { route } = buildServer({ ql });
147+
const res = await post(route, {
148+
operations: [
149+
{ object: 'project', action: 'create', data: { name: 'Apollo' } },
150+
{ object: 'task', action: 'create', data: {} },
151+
],
152+
});
153+
expect(res.statusCode).toBe(400);
154+
expect(res.body.code).toBe('VALIDATION_FAILED');
155+
// No success envelope is returned when the transaction threw.
156+
expect(res.body.results).toBeUndefined();
157+
});
158+
159+
it('rejects an unresolvable $ref with 400 BATCH_UNRESOLVED_REF', async () => {
160+
const ql = makeQl();
161+
const { route } = buildServer({ ql });
162+
const res = await post(route, {
163+
operations: [{ object: 'task', action: 'create', data: { project: { $ref: 5 } } }],
164+
});
165+
expect(res.statusCode).toBe(400);
166+
expect(res.body.code).toBe('BATCH_UNRESOLVED_REF');
167+
});
168+
169+
// ── per-object API-exposure enforcement (ADR-0049) ────────────────────────
170+
171+
it('blocks a write to an API-disabled object with 404 before opening a transaction', async () => {
172+
const ql = makeQl();
173+
const { route } = buildServer({ ql, objects: [{ name: 'secret', enable: { apiEnabled: false } }] });
174+
const res = await post(route, { operations: [{ object: 'secret', action: 'create', data: {} }] });
175+
expect(res.statusCode).toBe(404);
176+
expect(res.body.code).toBe('OBJECT_API_DISABLED');
177+
expect(ql.transaction).not.toHaveBeenCalled();
178+
});
179+
180+
it('blocks an operation absent from the apiMethods whitelist with 405', async () => {
181+
const ql = makeQl();
182+
const { route } = buildServer({ ql, objects: [{ name: 'ledger', enable: { apiMethods: ['read'] } }] });
183+
const res = await post(route, { operations: [{ object: 'ledger', action: 'create', data: {} }] });
184+
expect(res.statusCode).toBe(405);
185+
expect(res.body.code).toBe('OBJECT_API_METHOD_NOT_ALLOWED');
186+
expect(ql.transaction).not.toHaveBeenCalled();
187+
});
188+
189+
it('allows an operation that IS in the apiMethods whitelist', async () => {
190+
const ql = makeQl();
191+
const { route } = buildServer({ ql, objects: [{ name: 'ledger', enable: { apiMethods: ['create', 'read'] } }] });
192+
const res = await post(route, { operations: [{ object: 'ledger', action: 'create', data: { amount: 1 } }] });
193+
expect(res.statusCode).toBe(200);
194+
expect(ql.transaction).toHaveBeenCalledTimes(1);
195+
});
196+
197+
// ── request validation ────────────────────────────────────────────────────
198+
199+
it('rejects a non-atomic request (this endpoint is always atomic)', async () => {
200+
const ql = makeQl();
201+
const { route } = buildServer({ ql });
202+
const res = await post(route, { operations: [{ object: 'account', data: {} }], atomic: false });
203+
expect(res.statusCode).toBe(400);
204+
expect(res.body.code).toBe('BATCH_NOT_ATOMIC');
205+
expect(ql.transaction).not.toHaveBeenCalled();
206+
});
207+
208+
it('rejects an update op with no id', async () => {
209+
const ql = makeQl();
210+
const { route } = buildServer({ ql });
211+
const res = await post(route, { operations: [{ object: 'account', action: 'update', data: { name: 'X' } }] });
212+
expect(res.statusCode).toBe(400);
213+
expect(res.body.code).toBe('VALIDATION_FAILED');
214+
expect(ql.transaction).not.toHaveBeenCalled();
215+
});
216+
217+
it('rejects a malformed operation (missing object) with 400', async () => {
218+
const ql = makeQl();
219+
const { route } = buildServer({ ql });
220+
const res = await post(route, { operations: [{ action: 'create', data: {} }] });
221+
expect(res.statusCode).toBe(400);
222+
expect(res.body.code).toBe('VALIDATION_FAILED');
223+
});
224+
225+
it('rejects an unknown action with 400', async () => {
226+
const ql = makeQl();
227+
const { route } = buildServer({ ql });
228+
const res = await post(route, { operations: [{ object: 'account', action: 'frobnicate', data: {} }] });
229+
expect(res.statusCode).toBe(400);
230+
expect(res.body.code).toBe('VALIDATION_FAILED');
231+
});
232+
});

0 commit comments

Comments
 (0)