diff --git a/.changeset/cross-object-batch-authz-hardening.md b/.changeset/cross-object-batch-authz-hardening.md new file mode 100644 index 0000000000..1fc4b36f93 --- /dev/null +++ b/.changeset/cross-object-batch-authz-hardening.md @@ -0,0 +1,35 @@ +--- +'@objectstack/rest': patch +'@objectstack/spec': minor +--- + +fix(rest): gate the cross-object transactional batch by the same per-object API rules as single-record writes (#1604) + +The `POST {basePath}/batch` route (issue #1604 / ADR-0034) wraps N cross-object +create/update/delete ops in one engine transaction, but it skipped the +per-object API-exposure gate every single-record route applies — 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 ≠ enforced" hole closed for the +generic write path in #3220 / #3213). + +The route now: + +- validates the body against a new `CrossObjectBatchRequestSchema` + (`@objectstack/spec/api`, Zod-First) — a malformed op, an unknown action, or a + missing `object` is a `400` instead of a `500`; +- enforces `enable.apiEnabled` / `enable.apiMethods` for **every** op (metadata + fetched once, each distinct `(object, action)` checked) BEFORE opening the + transaction — `404 OBJECT_API_DISABLED` / `405 OBJECT_API_METHOD_NOT_ALLOWED`; +- requires an `id` for `update` / `delete` (`400`); +- rejects an unresolvable `{ $ref }` with `400 BATCH_UNRESOLVED_REF` instead of + silently writing a `null` FK; +- rejects an explicit `atomic: false` (`400 BATCH_NOT_ATOMIC`) rather than + silently applying atomically — non-atomic per-object batches stay on + `POST /data/:object/batch`. + +`enforceApiAccess` is refactored to share the pure `apiAccessDenialFromEnable` +check + a `loadObjectItems` helper with the batch route (single-record behavior +unchanged). Adds `rest-batch-endpoint.test.ts` — the REST-boundary coverage +ADR-0034 flagged as missing (commit, `$ref`, rollback surfacing, API-access +denial, request validation). diff --git a/content/docs/protocol/kernel/http-protocol.mdx b/content/docs/protocol/kernel/http-protocol.mdx index b158ecb9e7..999ed1579a 100644 --- a/content/docs/protocol/kernel/http-protocol.mdx +++ b/content/docs/protocol/kernel/http-protocol.mdx @@ -655,7 +655,18 @@ Content-Type: application/json **Behavior:** - Maximum batch size: 200 operations by default (configurable via `maxBatchSize`) - The entire batch runs inside **one engine transaction** — if any operation - fails, all are rolled back (commit-all-or-nothing) + fails, all are rolled back (commit-all-or-nothing). The batch is always + atomic; an explicit `"atomic": false` is rejected with `400 BATCH_NOT_ATOMIC` + (use `POST /data/{object}/batch` for a non-atomic per-object batch) +- Every operation is subject to the **same per-object API-exposure gate** as the + single-record routes, enforced before the transaction opens: an object with + `enable.apiEnabled: false` returns `404 OBJECT_API_DISABLED`, and an action + outside an object's `enable.apiMethods` whitelist returns + `405 OBJECT_API_METHOD_NOT_ALLOWED` +- The request shape is validated: a malformed operation, an unknown action, or a + missing `object` returns `400`; `update` / `delete` require an `id` +- A `{ "$ref": }` that does not resolve to an earlier create's id + returns `400 BATCH_UNRESOLVED_REF` (never a silently-written null value) - Returns HTTP 501 if the underlying runtime does not support transactions ## Metadata API diff --git a/content/docs/references/api/batch.mdx b/content/docs/references/api/batch.mdx index 63d64f1558..ec01c8c79d 100644 --- a/content/docs/references/api/batch.mdx +++ b/content/docs/references/api/batch.mdx @@ -30,8 +30,8 @@ Industry alignment: Salesforce Bulk API, Microsoft Dynamics Bulk Operations ## TypeScript Usage ```typescript -import { BatchConfig, BatchOperationResult, BatchOperationType, BatchOptions, BatchRecord, BatchUpdateRequest, BatchUpdateResponse, DeleteManyRequest, UpdateManyRequest } from '@objectstack/spec/api'; -import type { BatchConfig, BatchOperationResult, BatchOperationType, BatchOptions, BatchRecord, BatchUpdateRequest, BatchUpdateResponse, DeleteManyRequest, UpdateManyRequest } from '@objectstack/spec/api'; +import { BatchConfig, BatchOperationResult, BatchOperationType, BatchOptions, BatchRecord, BatchUpdateRequest, BatchUpdateResponse, CrossObjectBatchOperation, CrossObjectBatchRequest, CrossObjectBatchResponse, DeleteManyRequest, UpdateManyRequest } from '@objectstack/spec/api'; +import type { BatchConfig, BatchOperationResult, BatchOperationType, BatchOptions, BatchRecord, BatchUpdateRequest, BatchUpdateResponse, CrossObjectBatchOperation, CrossObjectBatchRequest, CrossObjectBatchResponse, DeleteManyRequest, UpdateManyRequest } from '@objectstack/spec/api'; // Validate data const result = BatchConfig.parse(data); @@ -135,6 +135,43 @@ const result = BatchConfig.parse(data); | **results** | `{ id?: string; success: boolean; errors?: { code: string; message: string; category?: string; details?: any; … }[]; data?: Record; … }[]` | ✅ | Detailed results for each record | +--- + +## CrossObjectBatchOperation + +### Properties + +| Property | Type | Required | Description | +| :--- | :--- | :--- | :--- | +| **object** | `string` | ✅ | Target object (table) name | +| **action** | `Enum<'create' \| 'update' \| 'delete'>` | ✅ | Operation to perform (default: create) | +| **id** | `string` | optional | Target record id — required for update and delete | +| **data** | `Record` | optional | Record payload for create/update; a value may be `{ $ref: }` to reference an earlier op's created id | + + +--- + +## CrossObjectBatchRequest + +### Properties + +| Property | Type | Required | Description | +| :--- | :--- | :--- | :--- | +| **operations** | `{ object: string; action: Enum<'create' \| 'update' \| 'delete'>; id?: string; data?: Record }[]` | ✅ | Ordered operations executed in one transaction | +| **atomic** | `boolean` | ✅ | Always true — the cross-object batch is all-or-nothing | + + +--- + +## CrossObjectBatchResponse + +### Properties + +| Property | Type | Required | Description | +| :--- | :--- | :--- | :--- | +| **results** | `any[]` | ✅ | Per-operation result, index-aligned with the request operations | + + --- ## DeleteManyRequest diff --git a/content/docs/releases/implementation-status.mdx b/content/docs/releases/implementation-status.mdx index 362461ed92..50f8d207a4 100644 --- a/content/docs/releases/implementation-status.mdx +++ b/content/docs/releases/implementation-status.mdx @@ -184,6 +184,7 @@ The data (`/data`), metadata (`/meta`), and batch endpoints are implemented in ` | `/data/{object}/deleteMany` | POST | Batch delete | ✅ | | `/data/{object}/{id}/clone` | POST | Clone a record (gated by `enable.clone`) | ✅ | | `/data/{object}/batch` | POST | Atomic batch operations | ✅ | +| `/batch` | POST | Cross-object atomic batch (parent + children in one transaction, `$ref`) | ✅ | ### Advanced Protocols diff --git a/packages/rest/src/rest-batch-endpoint.test.ts b/packages/rest/src/rest-batch-endpoint.test.ts new file mode 100644 index 0000000000..906e766c94 --- /dev/null +++ b/packages/rest/src/rest-batch-endpoint.test.ts @@ -0,0 +1,232 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// Cross-object transactional batch route (POST {basePath}/batch, issue #1604 / +// ADR-0034). Engine-level atomicity is covered in +// objectql/src/engine-ambient-transaction.test.ts; this suite is the +// REST-boundary contract ADR-0034 flagged as missing: request validation, +// per-op API-exposure enforcement, $ref resolution, and error surfacing. + +import { describe, it, expect, vi } from 'vitest'; +import { RestServer } from './rest-server'; + +// ── helpers ────────────────────────────────────────────────────────────────── + +function mockServer() { + return { + get: vi.fn(), post: vi.fn(), put: vi.fn(), delete: vi.fn(), patch: vi.fn(), + use: vi.fn(), listen: vi.fn().mockResolvedValue(undefined), close: vi.fn().mockResolvedValue(undefined), + }; +} + +function mockRes() { + const res: any = { statusCode: 200, body: undefined }; + res.status = vi.fn((c: number) => { res.statusCode = c; return res; }); + res.json = vi.fn((b: any) => { res.body = b; return res; }); + res.end = vi.fn(() => res); + return res; +} + +/** + * A fake ObjectQL whose `transaction(cb)` runs the callback and rethrows on + * failure (mirroring commit-on-success / rollback-on-throw). `insert` returns a + * freshly-minted id so `$ref` resolution and index-aligned results are testable. + */ +function makeQl(overrides: Partial> = {}) { + let seq = 0; + const ql: any = { + transaction: vi.fn(async (cb: any, ctx: any) => cb({ __trx: true, ctx })), + insert: overrides.insert ?? vi.fn(async (_object: string, data: any) => ({ id: `id_${++seq}`, ...data })), + update: overrides.update ?? vi.fn(async (_object: string, data: any) => ({ ...data })), + delete: overrides.delete ?? vi.fn(async (_object: string, arg: any) => ({ success: true, id: arg?.where?.id })), + }; + return ql; +} + +/** Build a RestServer with an optional ObjectQL provider and object metadata. */ +function buildServer(opts: { ql?: any; objects?: any[] } = {}) { + const server = mockServer(); + const protocol: any = { + getDiscovery: vi.fn().mockResolvedValue({ version: 'v0', endpoints: {} }), + getMetaTypes: vi.fn().mockResolvedValue([]), + getMetaItems: vi.fn().mockResolvedValue(opts.objects ?? []), + }; + const objectQLProvider = opts.ql ? async () => opts.ql : undefined; + const rest = new RestServer( + server as any, protocol as any, { api: { requireAuth: false } } as any, + undefined, undefined, undefined, undefined, // kernelManager, envRegistry, defaultEnvIdProvider, authServiceProvider + objectQLProvider, // objectQLProvider (positional arg #8) + ); + rest.registerRoutes(); + const route = rest.getRoutes().find( + (r) => r.method === 'POST' && (r.metadata?.summary ?? '').startsWith('Cross-object'), + ); + return { route, protocol }; +} + +/** POST a body at the cross-object batch route and return the mock response. */ +async function post(route: any, body: any) { + const res = mockRes(); + await route!.handler({ method: 'POST', params: {}, headers: {}, body } as any, res); + return res; +} + +// ── registration ───────────────────────────────────────────────────────────── + +describe('POST {basePath}/batch — cross-object transactional batch', () => { + it('registers the route under the data/batch tags', () => { + const { route } = buildServer({ ql: makeQl() }); + expect(route).toBeTruthy(); + expect(route!.metadata?.tags).toEqual(expect.arrayContaining(['data', 'batch'])); + }); + + it('returns 501 when the runtime has no transactional ObjectQL', async () => { + const { route } = buildServer({}); // no ql provider + const res = await post(route, { operations: [{ object: 'account', data: {} }] }); + expect(res.statusCode).toBe(501); + }); + + // ── happy path ─────────────────────────────────────────────────────────── + + it('commits multiple ops in one transaction and returns index-aligned results', async () => { + const ql = makeQl(); + const { route } = buildServer({ ql }); + const res = await post(route, { + operations: [ + { object: 'project', action: 'create', data: { name: 'Apollo' } }, + { object: 'task', action: 'create', data: { title: 'Kickoff' } }, + ], + }); + expect(res.statusCode).toBe(200); + expect(ql.transaction).toHaveBeenCalledTimes(1); + expect(ql.insert).toHaveBeenCalledTimes(2); + expect(res.body.results).toHaveLength(2); + expect(res.body.results[0]).toMatchObject({ id: 'id_1', name: 'Apollo' }); + expect(res.body.results[1]).toMatchObject({ id: 'id_2', title: 'Kickoff' }); + }); + + it('resolves { $ref: } to an earlier create\'s id (master-detail)', async () => { + const ql = makeQl(); + const { route } = buildServer({ ql }); + const res = await post(route, { + operations: [ + { object: 'project', action: 'create', data: { name: 'Apollo' } }, + { object: 'task', action: 'create', data: { title: 'Kickoff', project: { $ref: 0 } } }, + ], + }); + expect(res.statusCode).toBe(200); + // The child's FK was rewritten from { $ref: 0 } to the parent's generated id. + const childData = ql.insert.mock.calls[1][1]; + expect(childData.project).toBe('id_1'); + }); + + it('defaults a missing action to create', async () => { + const ql = makeQl(); + const { route } = buildServer({ ql }); + const res = await post(route, { operations: [{ object: 'account', data: { name: 'X' } }] }); + expect(res.statusCode).toBe(200); + expect(ql.insert).toHaveBeenCalledTimes(1); + }); + + it('returns an empty result set for zero operations without opening a transaction', async () => { + const ql = makeQl(); + const { route } = buildServer({ ql }); + const res = await post(route, { operations: [] }); + expect(res.statusCode).toBe(200); + expect(res.body).toEqual({ results: [] }); + expect(ql.transaction).not.toHaveBeenCalled(); + }); + + // ── atomicity / rollback surfacing ──────────────────────────────────────── + + it('surfaces a failing op with its mapped status (atomic rollback, not partial success)', async () => { + const insert = vi.fn() + .mockResolvedValueOnce({ id: 'id_1', name: 'Apollo' }) + .mockRejectedValueOnce(Object.assign(new Error('bad'), { code: 'VALIDATION_FAILED', name: 'ValidationError' })); + const ql = makeQl({ insert }); + const { route } = buildServer({ ql }); + const res = await post(route, { + operations: [ + { object: 'project', action: 'create', data: { name: 'Apollo' } }, + { object: 'task', action: 'create', data: {} }, + ], + }); + expect(res.statusCode).toBe(400); + expect(res.body.code).toBe('VALIDATION_FAILED'); + // No success envelope is returned when the transaction threw. + expect(res.body.results).toBeUndefined(); + }); + + it('rejects an unresolvable $ref with 400 BATCH_UNRESOLVED_REF', async () => { + const ql = makeQl(); + const { route } = buildServer({ ql }); + const res = await post(route, { + operations: [{ object: 'task', action: 'create', data: { project: { $ref: 5 } } }], + }); + expect(res.statusCode).toBe(400); + expect(res.body.code).toBe('BATCH_UNRESOLVED_REF'); + }); + + // ── per-object API-exposure enforcement (ADR-0049) ──────────────────────── + + it('blocks a write to an API-disabled object with 404 before opening a transaction', async () => { + const ql = makeQl(); + const { route } = buildServer({ ql, objects: [{ name: 'secret', enable: { apiEnabled: false } }] }); + const res = await post(route, { operations: [{ object: 'secret', action: 'create', data: {} }] }); + expect(res.statusCode).toBe(404); + expect(res.body.code).toBe('OBJECT_API_DISABLED'); + expect(ql.transaction).not.toHaveBeenCalled(); + }); + + it('blocks an operation absent from the apiMethods whitelist with 405', async () => { + const ql = makeQl(); + const { route } = buildServer({ ql, objects: [{ name: 'ledger', enable: { apiMethods: ['read'] } }] }); + const res = await post(route, { operations: [{ object: 'ledger', action: 'create', data: {} }] }); + expect(res.statusCode).toBe(405); + expect(res.body.code).toBe('OBJECT_API_METHOD_NOT_ALLOWED'); + expect(ql.transaction).not.toHaveBeenCalled(); + }); + + it('allows an operation that IS in the apiMethods whitelist', async () => { + const ql = makeQl(); + const { route } = buildServer({ ql, objects: [{ name: 'ledger', enable: { apiMethods: ['create', 'read'] } }] }); + const res = await post(route, { operations: [{ object: 'ledger', action: 'create', data: { amount: 1 } }] }); + expect(res.statusCode).toBe(200); + expect(ql.transaction).toHaveBeenCalledTimes(1); + }); + + // ── request validation ──────────────────────────────────────────────────── + + it('rejects a non-atomic request (this endpoint is always atomic)', async () => { + const ql = makeQl(); + const { route } = buildServer({ ql }); + const res = await post(route, { operations: [{ object: 'account', data: {} }], atomic: false }); + expect(res.statusCode).toBe(400); + expect(res.body.code).toBe('BATCH_NOT_ATOMIC'); + expect(ql.transaction).not.toHaveBeenCalled(); + }); + + it('rejects an update op with no id', async () => { + const ql = makeQl(); + const { route } = buildServer({ ql }); + const res = await post(route, { operations: [{ object: 'account', action: 'update', data: { name: 'X' } }] }); + expect(res.statusCode).toBe(400); + expect(res.body.code).toBe('VALIDATION_FAILED'); + expect(ql.transaction).not.toHaveBeenCalled(); + }); + + it('rejects a malformed operation (missing object) with 400', async () => { + const ql = makeQl(); + const { route } = buildServer({ ql }); + const res = await post(route, { operations: [{ action: 'create', data: {} }] }); + expect(res.statusCode).toBe(400); + expect(res.body.code).toBe('VALIDATION_FAILED'); + }); + + it('rejects an unknown action with 400', async () => { + const ql = makeQl(); + const { route } = buildServer({ ql }); + const res = await post(route, { operations: [{ object: 'account', action: 'frobnicate', data: {} }] }); + expect(res.statusCode).toBe(400); + expect(res.body.code).toBe('VALIDATION_FAILED'); + }); +}); diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index ff89cf1e18..6c4a68604f 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -412,6 +412,44 @@ function isExpectedDataStatus(status: number): boolean { return status === 403 || status === 404 || status === 409 || status === 502 || status === 503; } +/** + * Pure per-object API-exposure check: given an object's `enable` block, decide + * whether `operation` is denied on the *external* REST surface (ADR-0049 / + * #1889). Returns the `{ status, body }` to send, or `null` when allowed. + * Shared by the single-record routes (`enforceApiAccess`) and the cross-object + * batch route so both honour the SAME gate. A missing/loose `enable` block is + * default-allow — objects with no `enable` behave exactly as before. + */ +function apiAccessDenialFromEnable( + enable: any, + objectName: string, + operation: string, +): { status: number; body: Record } | null { + if (!enable) return null; + if (enable.apiEnabled === false) { + return { + status: 404, + body: { + error: `Object '${objectName}' is not exposed via the API`, + code: 'OBJECT_API_DISABLED', + object: objectName, + }, + }; + } + if (Array.isArray(enable.apiMethods) && enable.apiMethods.length > 0 && !enable.apiMethods.includes(operation)) { + return { + status: 405, + body: { + error: `API operation '${operation}' is not allowed on object '${objectName}'`, + code: 'OBJECT_API_METHOD_NOT_ALLOWED', + object: objectName, + allowed: enable.apiMethods, + }, + }; + } + return null; +} + /** Platform object backing async import jobs (see sys-import-job.object.ts). */ const IMPORT_JOB_OBJECT = 'sys_import_job'; /** Hard ceiling on rows per async import job (mirrors spec IMPORT_JOB_MAX_ROWS). */ @@ -1014,38 +1052,34 @@ export class RestServer { ): Promise { const objectName = req?.params?.object; if (!objectName) return false; - let enable: any; + 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 + const denial = apiAccessDenialFromEnable(obj.enable, objectName, operation); + if (denial) { + res.status(denial.status).json(denial.body); + return true; + } + return false; + } + + /** + * Load the object metadata items for the current protocol/environment, + * coerced to a plain array. Returns `[]` when metadata is unavailable so + * callers fail OPEN (the data call itself needs the same metadata and will + * surface any real error). Shared by `enforceApiAccess` (one object) and the + * cross-object batch route (all ops, fetched once). + */ + private async loadObjectItems(p: RestProtocol, environmentId: string | undefined): Promise { try { const r: any = await (p as any).getMetaItems?.({ type: 'object', ...(environmentId ? { environmentId } : {}), }); - const items: any[] = Array.isArray(r?.items) ? r.items : Array.isArray(r) ? r : []; - const obj = items.find((o: any) => o?.name === objectName); - if (!obj) return false; // unknown object → let the data path 404 - enable = obj.enable; + return Array.isArray(r?.items) ? r.items : Array.isArray(r) ? r : []; } catch { - return false; // metadata unavailable → don't block (data call needs it too) - } - if (!enable) return false; - if (enable.apiEnabled === false) { - res.status(404).json({ - error: `Object '${objectName}' is not exposed via the API`, - code: 'OBJECT_API_DISABLED', - object: objectName, - }); - return true; + return []; } - if (Array.isArray(enable.apiMethods) && enable.apiMethods.length > 0 && !enable.apiMethods.includes(operation)) { - res.status(405).json({ - error: `API operation '${operation}' is not allowed on object '${objectName}'`, - code: 'OBJECT_API_METHOD_NOT_ALLOWED', - object: objectName, - allowed: enable.apiMethods, - }); - return true; - } - return false; } /** @@ -6007,11 +6041,14 @@ export class RestServer { const operations = batch.operations; - // POST /batch — cross-object transactional batch (issue #1604). + // POST /batch — cross-object transactional batch (issue #1604 / ADR-0034). // Runs heterogeneous create/update/delete across objects in ONE engine - // transaction (commit all or roll back all). Intra-batch references: - // a field value of `{ $ref: }` resolves to that op's - // created id, so a child can reference its parent (master-detail). + // transaction (commit all or roll back all). Intra-batch references: a + // field value of `{ $ref: }` resolves to that op's + // created id, so a child can reference its parent (master-detail). The + // request is validated against the spec contract, and each op is gated by + // the SAME per-object API-exposure rules (enable.apiEnabled / apiMethods) + // as the single-record routes before any transaction is opened. this.routeManager.register({ method: 'POST', path: `${basePath}/batch`, @@ -6025,18 +6062,75 @@ export class RestServer { res.status(501).json({ error: 'Transactional batch not supported by this runtime' }); return; } - const ops: any[] = Array.isArray(req.body?.operations) ? req.body.operations : []; + + // Validate the request against the spec contract (Zod-First). + const { CrossObjectBatchRequestSchema } = await import('@objectstack/spec/api'); + const parsed = (CrossObjectBatchRequestSchema as any).safeParse(req.body ?? {}); + if (!parsed.success) { + res.status(400).json({ error: 'Invalid batch request', code: 'VALIDATION_FAILED', issues: parsed.error?.issues }); + return; + } + const ops: Array<{ object: string; action: 'create' | 'update' | 'delete'; id?: string; data?: Record }> = parsed.data.operations; + // All-or-nothing by construction: refuse a request that asks for + // non-atomic semantics rather than silently applying atomically + // (honest contract). Per-object partial batches use the + // POST /data/:object/batch route instead. + if (parsed.data.atomic === false) { + res.status(400).json({ error: 'Cross-object batch is always atomic; use POST /data/:object/batch for non-atomic per-object batches', code: 'BATCH_NOT_ATOMIC' }); + return; + } const max = batch.maxBatchSize ?? 200; if (ops.length === 0) { res.json({ results: [] }); return; } if (ops.length > max) { res.status(400).json({ error: `Batch too large (max ${max})` }); return; } + // update/delete need a target id — the schema can't express this + // conditionally, so surface it as a 400 up front. + for (const op of ops) { + if ((op.action === 'update' || op.action === 'delete') && op.id == null && op.data?.id == null) { + res.status(400).json({ error: `Operation '${op.action}' on '${op.object}' requires an id`, code: 'VALIDATION_FAILED' }); + return; + } + } + + // Enforce object-level API exposure (enable.apiEnabled / + // apiMethods) for EVERY op BEFORE opening the transaction — the + // batch write surface must honour the same per-object gate as the + // single-record routes (ADR-0049 / #1889). Metadata is fetched + // once; each distinct (object, action) is checked once. + const p = await this.resolveProtocol(environmentId, req); + const items = await this.loadObjectItems(p, environmentId); + if (items.length > 0) { + const byName = new Map(items.map((o: any) => [o?.name, o])); + const checked = new Set(); + for (const op of ops) { + const key = `${op.object}\u0000${op.action}`; + if (checked.has(key)) continue; + checked.add(key); + const obj = byName.get(op.object); + if (!obj) continue; // unknown object → surfaced by the op inside the tx + const denial = apiAccessDenialFromEnable(obj.enable, op.object, op.action); + if (denial) { res.status(denial.status).json(denial.body); return; } + } + } + + // Resolve `{ $ref: }` values against results collected + // so far. A ref MUST point at an earlier create whose id is known; + // anything else is a 400 (never a silent null FK). const resolveRefs = (data: any, out: any[]): any => { if (!data || typeof data !== 'object') return data; const result: any = Array.isArray(data) ? [] : {}; for (const [k, v] of Object.entries(data)) { if (v && typeof v === 'object' && '$ref' in (v as any)) { - const ref = out[(v as any).$ref]; - result[k] = (ref && (ref.id ?? ref._id)) ?? null; + const idx = (v as any).$ref; + const ref = typeof idx === 'number' ? out[idx] : undefined; + const refId = ref && (ref.id ?? ref._id); + if (refId == null) { + const err: any = new Error(`Unresolved $ref ${JSON.stringify(idx)} on field '${k}' — must reference an earlier create in the same batch`); + err.status = 400; + err.code = 'BATCH_UNRESOLVED_REF'; + throw err; + } + result[k] = refId; } else { result[k] = v; } @@ -6047,19 +6141,14 @@ export class RestServer { const results = await ql.transaction(async (trxCtx: any) => { const out: any[] = []; for (const op of ops) { - const action = String(op?.action || 'create'); - const object = String(op?.object || ''); - if (!object) throw new Error('Each operation requires an `object`'); const data = resolveRefs(op.data, out); - if (action === 'create') { - out.push(await ql.insert(object, data, { context: trxCtx })); - } else if (action === 'update') { + if (op.action === 'create') { + out.push(await ql.insert(op.object, data, { context: trxCtx })); + } else if (op.action === 'update') { const id = op.id ?? data?.id; - out.push(await ql.update(object, { ...data, id }, { context: trxCtx })); - } else if (action === 'delete') { - out.push(await ql.delete(object, { where: { id: op.id }, context: trxCtx })); - } else { - throw new Error(`Unknown batch action: ${action}`); + out.push(await ql.update(op.object, { ...data, id }, { context: trxCtx })); + } else { // 'delete' + out.push(await ql.delete(op.object, { where: { id: op.id }, context: trxCtx })); } } return out; @@ -6067,7 +6156,10 @@ export class RestServer { res.json({ results }); } catch (error: any) { - logError('[REST] Unhandled error:', error); + // Log only genuine server faults; client 4xx (validation, + // unresolved ref, atomic rollback of a bad op) are expected. + const status = typeof error?.status === 'number' ? error.status : mapDataError(error).status; + if (status >= 500) logError('[REST] Unhandled error:', error); sendError(res, error); } }, diff --git a/packages/spec/api-surface.json b/packages/spec/api-surface.json index 5b564e2adf..ba40e8cf7f 100644 --- a/packages/spec/api-surface.json +++ b/packages/spec/api-surface.json @@ -2275,6 +2275,12 @@ "CreateViewRequestSchema (const)", "CreateViewResponse (type)", "CreateViewResponseSchema (const)", + "CrossObjectBatchOperation (type)", + "CrossObjectBatchOperationSchema (const)", + "CrossObjectBatchRequest (type)", + "CrossObjectBatchRequestSchema (const)", + "CrossObjectBatchResponse (type)", + "CrossObjectBatchResponseSchema (const)", "CrudEndpointPattern (type)", "CrudEndpointPatternSchema (const)", "CrudEndpointsConfig (type)", diff --git a/packages/spec/json-schema.manifest.json b/packages/spec/json-schema.manifest.json index 3e4fb37a21..957b368432 100644 --- a/packages/spec/json-schema.manifest.json +++ b/packages/spec/json-schema.manifest.json @@ -152,6 +152,9 @@ "api/CreateRequest", "api/CreateViewRequest", "api/CreateViewResponse", + "api/CrossObjectBatchOperation", + "api/CrossObjectBatchRequest", + "api/CrossObjectBatchResponse", "api/CrudEndpointPattern", "api/CrudEndpointsConfig", "api/CrudOperation", diff --git a/packages/spec/src/api/batch.zod.ts b/packages/spec/src/api/batch.zod.ts index 5af89cebc5..c13865354a 100644 --- a/packages/spec/src/api/batch.zod.ts +++ b/packages/spec/src/api/batch.zod.ts @@ -243,3 +243,57 @@ export const BatchConfigSchema = lazySchema(() => z.object({ }).passthrough()); // Allow additional properties export type BatchConfig = z.infer; + +// ========================================== +// Cross-Object Transactional Batch (issue #1604) +// ========================================== + +/** + * A single operation in a cross-object transactional batch. Targets one object + * with a create/update/delete action. A value inside `data` may carry an + * intra-batch reference `{ $ref: }` that the server resolves + * to that op's created id — so a child row can point at a parent created earlier + * in the SAME transaction (master-detail). See ADR-0034 / #1604. + */ +export const CrossObjectBatchOperationSchema = lazySchema(() => z.object({ + object: z.string().min(1).describe('Target object (table) name'), + action: z.enum(['create', 'update', 'delete']).optional().default('create').describe('Operation to perform (default: create)'), + id: z.string().optional().describe('Target record id — required for update and delete'), + data: RecordDataSchema.optional().describe('Record payload for create/update; a value may be { $ref: } to reference an earlier op\'s created id'), +})); + +export type CrossObjectBatchOperation = z.input; + +/** + * Request payload for the cross-object transactional batch + * (`POST {basePath}/batch`). Every operation runs in ONE engine transaction — + * commit all or roll back all. `atomic` is accepted for contract symmetry but + * MUST be true (the endpoint is all-or-nothing by construction); a non-atomic + * per-object batch is served by `POST /data/:object/batch` instead. + * + * @example + * // POST /api/v1/batch — parent + child in one transaction (master-detail) + * { + * "operations": [ + * { "object": "project", "action": "create", "data": { "name": "Apollo" } }, + * { "object": "task", "action": "create", "data": { "title": "Kickoff", "project": { "$ref": 0 } } } + * ] + * } + */ +export const CrossObjectBatchRequestSchema = lazySchema(() => z.object({ + operations: z.array(CrossObjectBatchOperationSchema).max(1000).describe('Ordered operations executed in one transaction'), + atomic: z.boolean().optional().default(true).describe('Always true — the cross-object batch is all-or-nothing'), +})); + +export type CrossObjectBatchRequest = z.input; + +/** + * Response for the cross-object transactional batch — one result per operation, + * index-aligned with the request `operations` (create/update echo the record, + * delete echoes the driver's delete result). + */ +export const CrossObjectBatchResponseSchema = lazySchema(() => z.object({ + results: z.array(z.unknown()).describe('Per-operation result, index-aligned with the request operations'), +})); + +export type CrossObjectBatchResponse = z.infer;