diff --git a/docs/qa/platform-checklist/areas/api-backend.json b/docs/qa/platform-checklist/areas/api-backend.json index 8f4f85bf772..7a6a84564ec 100644 --- a/docs/qa/platform-checklist/areas/api-backend.json +++ b/docs/qa/platform-checklist/areas/api-backend.json @@ -2258,7 +2258,7 @@ "source": [ "packages/spec/liveness/batch_endpoints.json (the ADR-0049 ledger this item answers: maxBatchSize, enableBatchEndpoint and three operations.* live, upsertMany and defaultAtomic dead)", "packages/spec/src/api/rest-server.zod.ts#BatchEndpointsConfigSchema (declared bounds 1..1000 int, default 200, and the two retiredKey tombstones)", - "packages/rest/src/rest-server.ts#registerBatchEndpoints (the four per-object gates, each ANDed with its protocol member, and the unconditionally-mounted cross-object door), (enforceBatchSize — the one envelope), (normalizeConfig), (parseDeclaredSubConfig)", + "packages/rest/src/batch-routes.ts#registerBatchEndpoints (the four per-object gates, each ANDed with its protocol member, and the unconditionally-mounted cross-object door) + packages/rest/src/rest-server.ts#enforceBatchSize (enforceBatchSize — the one envelope), (normalizeConfig), (parseDeclaredSubConfig)", "packages/spec/src/api/error-code-ledger.zod.ts#BATCH_TOO_LARGE (the code the cap answers with)", "#3939 (one cap, one envelope, every door) · #11984 (maxBatchSize 0 was the live cap under the old cast) · #14691 (the two retirements) · ADR-0119 D4 (per-request atomicity)", "sibling clause: api-backend.bulk-write-contract and api-backend.batch-transactional-discovery (they drive the doors' BEHAVIOUR on the default config; this item covers the config that decides the doors exist)" diff --git a/packages/qa/dogfood/test/authz-probe-blind-spot.census.ts b/packages/qa/dogfood/test/authz-probe-blind-spot.census.ts index e28fa932312..d82b09cac92 100644 --- a/packages/qa/dogfood/test/authz-probe-blind-spot.census.ts +++ b/packages/qa/dogfood/test/authz-probe-blind-spot.census.ts @@ -512,7 +512,8 @@ export const PROBE_FILE_CENSUS: readonly ProbeFileReading[] = [ '`registerPerItemRoute` (the shared forwarder, not a route; its extent is bounded by the declaration\'s own ' + 'indentation and the subtrahend is pinned at 1 by the `forwarder slice:` control, never inferred from a ' + 'terminator spelling), PLUS `registerPerItemRoute(` call sites; counted over this file AND ' + - '`data-action-routes.ts`, the module `registerDataActionEndpoints`\' body moved to; ' + + '`data-action-routes.ts` and `batch-routes.ts`, the modules the bodies of `registerDataActionEndpoints` ' + + 'and `registerBatchEndpoints` moved to; ' + 'reachable = those inside registerMetadataEndpoints', // [#15542 / #15854] ⭐ THE POPULATION RULE LEARNED A SECOND SPELLING, and // the numbers it produces did NOT move: 80 / 19 / 61, exactly as before. @@ -645,10 +646,22 @@ export const PROBE_FILE_CENSUS: readonly ProbeFileReading[] = [ // mention, and left `data-action-routes.ts` untouched. No call site was // added. ⚠️ The dogfood shards skip on PRs and in the merge queue, so this // move was seen only by the hourly full run on `main`. + // [#23050 slice 2] 70 / 19 / 51 UNCHANGED, the same measurement as slice 1: + // `registerBatchEndpoints`' body (five routes, all outside + // `registerMetadataEndpoints`, so all five in the blind spot) moved to + // `batch-routes.ts`, and the population rule reads that module too. + // `this.routeManager.register(` stays 63 over the three files (49 here + 9 + // + 5, the new control), `private register*Endpoints(` stays 16 (the + // registrar is still declared here), and `enforceAuth` 64 -> 68 is + // RE-ANCHORED, not relaxed: the five call sites moved with their routes and + // are counted in both readings; the four new occurrences are that slice's + // seam, the same two spellings as slice 1's (the host literal's forwarder, + // two; the host interface's member, two). No call site was added. controls: { 'private register*Endpoints(': 16, 'this.routeManager.register(': 63, 'data-action-routes.ts: this.routeManager.register(': 9, + 'batch-routes.ts: this.routeManager.register(': 5, // Both halves of the new rule carry their own control, so neither can go // silently to zero: a helper deleted and its routes inlined back would // still read population 80, and only these two controls would notice the @@ -656,7 +669,7 @@ export const PROBE_FILE_CENSUS: readonly ProbeFileReading[] = [ 'registerPerItemRoute(': 8, 'const registerPerItemRoute =': 1, 'forwarder slice: this.routeManager.register(': 1, - enforceAuth: 64, + enforceAuth: 68, }, note: 'The single non-tripwire probe names ONE registrar of 16. The other 15 can never mint a key: ' + @@ -1033,7 +1046,12 @@ export function deriveProbeFileCensus(): { // no `registerPerItemRoute`, so appending it moves neither the forwarder // slice nor the `reachable` slice; its own control below proves it is read. const moved = read('packages/rest/src/data-action-routes.ts'); - const all = `${src}\n${moved}`; + // [#23050 slice 2] The same lesson, again: `registerBatchEndpoints`' body + // moved to `batch-routes.ts` with its five registration sites. Appended for + // the same reason and on the same terms — it declares no registrar and no + // `registerPerItemRoute` either — with its own control below. + const movedBatch = read('packages/rest/src/batch-routes.ts'); + const all = `${src}\n${moved}\n${movedBatch}`; const registrarRe = /^\s*private\s+register[A-Za-z]*Endpoints\s*\(/gm; const mountRe = /this\.routeManager\.register\(/g; // Matches the CALL sites only. The declaration reads @@ -1137,6 +1155,7 @@ export function deriveProbeFileCensus(): { // [#23050 slice 1] The moved module's own share, so a reading that // silently stopped appending it cannot hide behind the total above. 'data-action-routes.ts: this.routeManager.register(': occurrences(moved, /this\.routeManager\.register\(/g), + 'batch-routes.ts: this.routeManager.register(': occurrences(movedBatch, /this\.routeManager\.register\(/g), 'registerPerItemRoute(': occurrences(src, /registerPerItemRoute\(/g), 'const registerPerItemRoute =': occurrences(src, /const\s+registerPerItemRoute\s*=/g), // ⭐ The one control here that is NOT a whole-file count, deliberately: diff --git a/packages/rest/src/batch-routes.ts b/packages/rest/src/batch-routes.ts new file mode 100644 index 00000000000..8634e23bc36 --- /dev/null +++ b/packages/rest/src/batch-routes.ts @@ -0,0 +1,584 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#23050 slice 2] The batch routes of `RestServer` — the cross-object + * transactional `POST {basePath}/batch`, and the per-object + * `POST /data/:object/batch`, `createMany`, `updateMany` and `deleteMany` — + * moved out of `rest-server.ts` whole. + * + * A MOVE, not a redesign. The function below is the body of + * `RestServer#registerBatchEndpoints` as it stood, re-indented one level and + * otherwise unedited: the same routes, registered in the same order, behind + * the same mount conditions, by the same handlers, answering the same bodies. + * + * ## How the moved body reaches the class + * + * The shape slice 1 set, repeated (`data-action-routes.ts`' header carries the + * full reasoning). The class keeps its private `registerBatchEndpoints` — the + * published `dist/*.d.ts` lists its name, docblock and position — and its body + * is one {@link BatchRouteHost} literal plus `registerBatchEndpoints.call(host, + * basePath)`. A method is handed over as a closure that calls it on the server + * at call time, so a test double or spy set on the server is still the one the + * body reaches. + * + * Each field was checked on its own, not inherited from slice 1's sentence: + * + * - `config`, `routeManager` and `protocol` go BY VALUE. The body reads each + * one only while registration runs (`crud` / `batch` off the config, the four + * mount conditions off the protocol, every `register` call), and the class + * assigns `config` and `protocol` only in its constructor. Its one + * `routeManager` reassignment wraps `registerMetadataEndpointsInner`, never + * this registrar. + * - `objectQLProvider` goes as a GETTER. The cross-object door reads it at + * REQUEST time (`wiredEngineOrLoud(Boolean(this.objectQLProvider), …)`), and + * the field is not `readonly`, so a value copied at registration could part + * from it. The getter reads the field the handler read, when it read it. + * + * Module-level helpers stay in `rest-server.ts` and are imported from it; + * `wiredEngineOrLoud` is exported there for this module, and the package entry + * re-exports neither it nor `apiAccessDenialFromEnable`, so the published + * surface does not move. `ServerScopedDataRequest` comes from + * `data-action-routes.ts`, where slice 1 moved it. + */ + +import type { + CreateDataRequest, + BatchDataRequest, + CreateManyDataRequest, + UpdateManyDataRequest, + DeleteManyDataRequest, +} from '@objectstack/spec/api'; +import { zodIssuesToFields } from '@objectstack/spec/api'; +import type { DroppedFieldsEvent } from '@objectstack/spec/data'; +// The write-response rules (credential-class mask, `internal` omit) for +// the cross-object batch's direct-engine update arm. +import { omitInternalFieldsFromWriteResponse } from '@objectstack/core'; +import { handleRouteError } from './error-response.js'; +import type { ServerScopedDataRequest } from './data-action-routes.js'; +import { + apiAccessDenialFromEnable, + wiredEngineOrLoud, + type RestServer, +} from './rest-server.js'; + +/** + * The `RestServer` members {@link registerBatchEndpoints} reads — every one of + * them, and no other. Each is typed AS the class member (an indexed access on + * the class), so a change to a member's signature on the class is a change + * here too, and naming a member the class does not have is a compile error. + * See the module header for which field is a getter and why. + */ +export interface BatchRouteHost { + readonly config: RestServer['config']; + readonly routeManager: RestServer['routeManager']; + readonly protocol: RestServer['protocol']; + readonly objectQLProvider: RestServer['objectQLProvider']; + resolveProtocol: RestServer['resolveProtocol']; + resolveExecCtx: RestServer['resolveExecCtx']; + enforceAuth: RestServer['enforceAuth']; + enforceApiAccess: RestServer['enforceApiAccess']; + enforceBatchSize: RestServer['enforceBatchSize']; + loadObjectItems: RestServer['loadObjectItems']; +} + +/** + * Register batch operation endpoints — `RestServer#registerBatchEndpoints`' + * body, called by that method with the server as `this`. The method's own + * docblock, which the published declaration carries, names it. + */ +export function registerBatchEndpoints(this: BatchRouteHost, basePath: string): void { + const { crud, batch } = this.config; + const dataPath = `${basePath}${crud.dataPrefix}`; + const isScoped = basePath.includes('/environments/:environmentId'); + + const operations = batch.operations; + // [#3939] One cap, read once, applied by every bulk route below via + // {@link enforceBatchSize} — see that method for why it lives here and + // not in the Zod schemas. + const maxBatch = batch.maxBatchSize ?? 200; + + // 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). 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`, + handler: async (req: any, res: any) => { + try { + const environmentId = isScoped ? req.params?.environmentId : undefined; + const context = await this.resolveExecCtx(environmentId, req); + if (this.enforceAuth(req, res, context)) return; + // [#18559] The engine seam, reached the way its two SIBLING + // consumers of this same slot already reach it — + // `wiredEngineOrLoud` — so "no engine is wired" and "the + // engine WAS wired and could not be resolved" stay two facts + // instead of being told apart only by accident. The retired + // spelling this replaces was a plain read: + // + // this.objectQLProvider ? await this.objectQLProvider(environmentId) : undefined + // + // ⚠️ It did NOT re-collapse them — that is why this is not a + // regression and was not a blocker for #14251's decidable + // test. A rejection escaped the plain read, missed the 501 + // arm below (which tests `!ql || typeof ql.transaction !== + // 'function'`, and a rejection never reaches it), and was + // caught by this handler's GENERIC outer catch + // (`handleRouteError`). So the two facts did differ on the + // wire — 500 INTERNAL_ERROR against 501 NOT_IMPLEMENTED — + // but through a catch-all that knows nothing about this + // seam, at a status this slot's other two consumers do not + // use for the same fact. + // + // ⭐ What decided it, measured on a real `RestServer` over a + // real `ObjectKernel` rather than argued: THIS DOOR ALREADY + // ANSWERS 503 on the single-kernel wiring. There + // `computeExecCtx` takes its PROVIDER branch, which is + // `wiredEngineOrLoud`, and raises before this line runs. The + // 500 was reachable only on the MULTI-KERNEL wiring, where + // the gate's kernel branch absorbs by design (see + // `wiredEngineOrLoud`'s RESIDUE note) and hands the engine + // question down to this line: + // + // | wiring, engine wired and FAILING | before | after | + // |:--|:--|:--| + // | single-kernel (gate raises first)| 503 | 503 — unchanged | + // | multi-kernel (gate absorbs) | **500**| **503** | + // + // ⇒ the repair does not choose a new wire answer for this + // door; it removes a WIRING-DEPENDENT divergence, leaving + // the answer this door already gave on the composition the + // open core boots. + // + // ⛔ NOT `seamOrUndefined`. That helper SWALLOWS, and its own + // docblock forbids routing the data-engine seam back through + // it "to make the seams uniform". + // + // The wiring fact is the provider's PRESENCE, asked once and + // never inferred from what it returned, so both ABSENCE + // shapes reach the 501 below byte-for-byte as before: no + // provider wired at all, and a provider that RESOLVES + // `undefined`, which is the seam contract declaring absence + // rather than failing. `wiredEngineOrLoud` also invokes the + // provider SYNCHRONOUSLY, so a host wiring a non-`async` + // provider — which the seam's declared type cannot prevent — + // reaches the same answer as one that rejects (commit add6a1b1c). + const ql = await wiredEngineOrLoud( + Boolean(this.objectQLProvider), + () => this.objectQLProvider!(environmentId), + ); + if (!ql || typeof ql.transaction !== 'function') { + // Typed like every other 501 on this server (clone/search, + // #4067) so a client can key on the code, not the prose. + res.status(501).json({ error: 'Transactional batch not supported by this runtime', code: 'NOT_IMPLEMENTED' }); + return; + } + + // 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; + } + if (ops.length === 0) { res.json({ results: [] }); return; } + // [#3939] Same check, same envelope as every other bulk route + // now — this one used to be the only one that capped at all, + // and it answered without a `code` for clients to key on. + if (this.enforceBatchSize(res, ops.length, maxBatch)) 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 + // [#3391] Cross-object batch is a bulk surface: gate each op + // as `bulk ∧ child(op.action)` — the object must grant the + // `bulk` primitive AND the specific write it performs. + const denial = apiAccessDenialFromEnable(obj.enable, op.object, 'bulk', { bulkChild: 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 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; + } + } + return result; + }; + + // [#3794] Write-observability on THIS surface too. The engine + // strips `readonly` / `readonlyWhen` writes silently, and every + // other write path already reports what it dropped (#3431/#3455) + // — but this one did not, and it is precisely the path the + // console's record form takes for a master-detail save. Result: + // a user edited a `readonlyWhen`-locked field, got "updated + // successfully", and the value never changed with nothing said + // (#3794 problem 2). Each event is tagged with its operation + // index, since `results` entries are bare record echoes with no + // envelope to hang a per-row list on. + const dropped: Array = []; + const results = await ql.transaction(async (trxCtx: any) => { + const out: any[] = []; + for (const [index, op] of ops.entries()) { + const data = resolveRefs(op.data, out); + if (op.action === 'create') { + // [#3835] Go through the protocol's create ingress — + // the SAME one `POST /data/:object` uses — rather than + // calling `ql.insert` directly. When this was written the + // #3043 static-`readonly` strip lived at that ingress and + // a direct `ql.insert` bypassed it, so `readonly` meant + // two different things on two create paths. Since the + // maintainer ruling of 2026-09-03 (option C, #14147) the + // strip runs inside `engine.insert` for every non-system + // caller, so both routes are stripped identically, and the + // platform-object carve-out (a `sys_`/`managedBy` object's + // own guard must REJECT a forged value, not silently + // swallow it) is the engine's as well. The routing stands + // on what the ingress still owns: the #3770 object- + // existence gate, the #7823 `internal: true` response + // strip and the `droppedFields` relay — one create + // ingress, one response contract, and a future change to + // its policy covers the batch for free. `trxCtx` carries + // the caller's context (including `isSystem`) plus the + // open transaction, so the engine's strip decides exactly + // as it does on the single route and the insert still + // joins this transaction. + const batchCreateRequest: ServerScopedDataRequest = { + object: op.object, data, context: trxCtx, + }; + const created: any = await p.createData(batchCreateRequest); + for (const e of (created?.droppedFields ?? []) as DroppedFieldsEvent[]) { + dropped.push({ ...e, index }); + } + out.push(created?.record); + } else if (op.action === 'update') { + // Update needs no ingress detour for the WRITE half: + // the engine enforces both static `readonly` (#2948) + // and `readonlyWhen` (#3042) on its own update path, + // and reports them through this listener. + const onFieldsDropped = (e: DroppedFieldsEvent) => { dropped.push({ ...e, index }); }; + const id = op.id ?? data?.id; + const updated = await ql.update(op.object, { ...data, id }, { context: trxCtx, onFieldsDropped }); + // [#7823] …but the RESPONSE half moved to the ingress + // (A-prime ruling, 2026-08-13): the engine keeps its + // write results whole, so this direct-`ql.update` + // mouth applies the shared write-response rules + // itself (credential-class mask, then `internal` + // omit) before the row rides `results` out. Called + // from `@objectstack/core` directly — never through + // an optional protocol method, which a protocol + // without it would skip silently. FAIL CLOSED: with + // no registered schema to judge the fields by, only + // the id is echoed. + const updateSchema = (ql as any).registry?.getObject?.(op.object); + if (!updateSchema) { + out.push(updated && typeof updated === 'object' ? { id: (updated as any).id ?? id } : updated); + } else { + omitInternalFieldsFromWriteResponse(updateSchema, updated); + out.push(updated); + } + } else { // 'delete' + out.push(await ql.delete(op.object, { where: { id: op.id }, context: trxCtx })); + } + } + return out; + }, context); + + res.json({ results, ...(dropped.length > 0 ? { droppedFields: dropped } : {}) }); + } catch (error: any) { + // Log only genuine server faults; client 4xx (validation, + // unresolved ref, atomic rollback of a bad op) are expected. + // This site used to judge on `status >= 500` alone, which + // also swallowed the un-coded 400 `mapDataError` degrades an + // UNRECOGNISED error to — a handler `TypeError` inside a + // batch transaction vanished here. The shared predicate + // keeps that one loud while staying quiet on the coded 4xx. + handleRouteError(res, error); + } + }, + metadata: { + summary: 'Cross-object transactional batch (atomic create/update/delete across objects)', + tags: ['data', 'batch'], + }, + }); + + // POST /data/:object/batch - Generic batch endpoint + if (batch.enableBatchEndpoint && this.protocol.batchData) { + this.routeManager.register({ + method: 'POST', + path: `${dataPath}/:object/batch`, + handler: async (req: any, res: any) => { + try { + const environmentId = isScoped ? req.params?.environmentId : undefined; + const p = await this.resolveProtocol(environmentId, req); + const context = await this.resolveExecCtx(environmentId, req); + if (this.enforceAuth(req, res, context)) return; + // [#3391] bulk ∧ child(body.operation) — the object must grant + // the `bulk` primitive AND the batched write kind. + if (await this.enforceApiAccess(req, res, p, environmentId, 'bulk', { bulkChild: req.body?.operation })) return; + // [#3899] Validate against the declared contract + // (`BatchUpdateRequestSchema`, catalog `requestSchema`) — + // this route used to hand the body straight to the + // protocol, so `{ operation: 'updat', records: {} }` + // reached the engine as-is. Validation only: the ORIGINAL + // body is forwarded, not the parse output, so + // `BatchOptionsSchema`'s defaults (e.g. `atomic: true`) + // are not injected into a request that never sent them. + const { BatchUpdateRequestSchema } = await import('@objectstack/spec/api'); + const batchInput = req.body ?? {}; + const parsedBatch = (BatchUpdateRequestSchema as any).safeParse(batchInput); + if (!parsedBatch.success) { + res.status(400).json({ + error: 'Invalid batch request', + code: 'VALIDATION_FAILED', + fields: zodIssuesToFields(parsedBatch.error?.issues, batchInput), + object: req.params?.object, + }); + return; + } + // [#3939] Cap AFTER the shape check, so a caller gets the + // more specific answer first. + if (this.enforceBatchSize(res, parsedBatch.data.records.length, maxBatch, req.params?.object)) return; + const batchRequest: ServerScopedDataRequest = { + object: req.params.object, + request: req.body, + ...(environmentId ? { environmentId } : {}), + ...(context ? { context } : {}), + }; + const result = await p.batchData!(batchRequest); + res.json(result); + } catch (error: any) { + handleRouteError(res, error, req.params?.object); + } + }, + metadata: { + summary: 'Batch operations', + tags: ['data', 'batch'], + }, + }); + } + + // POST /data/:object/createMany - Bulk create + if (operations.createMany && this.protocol.createManyData) { + this.routeManager.register({ + method: 'POST', + path: `${dataPath}/:object/createMany`, + handler: async (req: any, res: any) => { + try { + const environmentId = isScoped ? req.params?.environmentId : undefined; + const p = await this.resolveProtocol(environmentId, req); + const context = await this.resolveExecCtx(environmentId, req); + if (this.enforceAuth(req, res, context)) return; + // [#3391] bulk ∧ create — createMany requires the `bulk` primitive. + if (await this.enforceApiAccess(req, res, p, environmentId, 'bulk', { bulkChild: 'create' })) return; + // [#3899] Body IS the records array on this route (what + // `client.data.createMany` posts). Validate the assembled + // protocol request (`CreateManyDataRequestSchema`, catalog + // `requestSchema`) so `{ records: [...] }` — updateMany's + // envelope, an easy cross-route slip — or any other + // non-array body 400s instead of reaching the engine as a + // single garbage "record". + const { CreateManyDataRequestSchema } = await import('@objectstack/spec/api'); + const createManyInput = { object: req.params.object, records: req.body ?? [] }; + const parsedCreateMany = (CreateManyDataRequestSchema as any).safeParse(createManyInput); + if (!parsedCreateMany.success) { + res.status(400).json({ + error: 'Invalid createMany request — the body must be a JSON array of record objects', + code: 'VALIDATION_FAILED', + fields: zodIssuesToFields(parsedCreateMany.error?.issues, createManyInput), + object: req.params?.object, + }); + return; + } + // [#3939] Cap AFTER the shape check. + if (this.enforceBatchSize(res, parsedCreateMany.data.records.length, maxBatch, req.params?.object)) return; + const createManyRequest: ServerScopedDataRequest = { + object: req.params.object, + records: req.body || [], + ...(environmentId ? { environmentId } : {}), + ...(context ? { context } : {}), + }; + const result = await p.createManyData!(createManyRequest); + res.status(201).json(result); + } catch (error: any) { + handleRouteError(res, error, req.params?.object); + } + }, + metadata: { + summary: 'Create multiple records', + tags: ['data', 'batch'], + }, + }); + } + + // POST /data/:object/updateMany - Bulk update + if (operations.updateMany && this.protocol.updateManyData) { + this.routeManager.register({ + method: 'POST', + path: `${dataPath}/:object/updateMany`, + handler: async (req: any, res: any) => { + try { + const environmentId = isScoped ? req.params?.environmentId : undefined; + const p = await this.resolveProtocol(environmentId, req); + const context = await this.resolveExecCtx(environmentId, req); + if (this.enforceAuth(req, res, context)) return; + // [#3391] bulk ∧ update — updateMany requires the `bulk` primitive. + if (await this.enforceApiAccess(req, res, p, environmentId, 'bulk', { bulkChild: 'update' })) return; + // [#3933] Validate against the spec contract, and write the + // PATH object last. The body used to be spread over + // `object: req.params.object`, so `{"object":"other", …}` + // moved the write to a different object than the one + // `enforceApiAccess` had just cleared — that gate reads + // `req.params.object`, so `enable.apiEnabled` / `apiMethods` + // (ADR-0049) was enforced on A while B was written. Zod also + // strips unknown keys, which keeps a body `context` from + // becoming the execution context on a deployment where none + // resolves (e.g. an anonymous public-book read, #3963). + const { UpdateManyDataRequestSchema } = await import('@objectstack/spec/api'); + const updateManyInput = { ...(req.body ?? {}), object: req.params.object }; + const parsedUpdate = UpdateManyDataRequestSchema.safeParse(updateManyInput); + if (!parsedUpdate.success) { + res.status(400).json({ + error: 'Invalid updateMany request', + code: 'VALIDATION_FAILED', + fields: zodIssuesToFields(parsedUpdate.error?.issues, updateManyInput), + object: req.params?.object, + }); + return; + } + // [#3939] Cap AFTER the shape check, so a caller gets the + // more specific answer first. + if (this.enforceBatchSize(res, parsedUpdate.data.records.length, maxBatch, req.params?.object)) return; + const updateManyRequest: ServerScopedDataRequest = { + ...parsedUpdate.data, + ...(environmentId ? { environmentId } : {}), + ...(context ? { context } : {}), + }; + const result = await p.updateManyData!(updateManyRequest); + res.json(result); + } catch (error: any) { + handleRouteError(res, error, req.params?.object); + } + }, + metadata: { + summary: 'Update multiple records', + tags: ['data', 'batch'], + }, + }); + } + + // POST /data/:object/deleteMany - Bulk delete + if (operations.deleteMany && this.protocol.deleteManyData) { + this.routeManager.register({ + method: 'POST', + path: `${dataPath}/:object/deleteMany`, + handler: async (req: any, res: any) => { + try { + const environmentId = isScoped ? req.params?.environmentId : undefined; + const p = await this.resolveProtocol(environmentId, req); + const context = await this.resolveExecCtx(environmentId, req); + if (this.enforceAuth(req, res, context)) return; + // [#3391] bulk ∧ delete — deleteMany requires the `bulk` primitive. + if (await this.enforceApiAccess(req, res, p, environmentId, 'bulk', { bulkChild: 'delete' })) return; + // [#3897] Validate against the spec contract instead of + // splatting the raw body into the protocol request. Zod + // object schemas STRIP unknown keys, which is what makes + // this a security boundary and not just an error message: + // `options` is narrowed to `BatchOptions`, so a body key + // (`options.where`, `options.multi`) can no longer ride + // into the engine's delete options, and a top-level + // `context` can no longer forge the caller's principal on + // a route reachable without auth. The protocol layer + // refuses the same shapes independently (defence in + // depth) — this stops them one hop earlier, with a 400 + // the caller can act on. + // [#3933] The PATH object is written LAST for the same + // reason: `enforceApiAccess` gates on `req.params.object`, + // so a body `object` would move the delete to an object + // whose exposure policy was never checked. + const { DeleteManyDataRequestSchema } = await import('@objectstack/spec/api'); + const deleteManyInput = { ...(req.body ?? {}), object: req.params.object }; + const parsed = DeleteManyDataRequestSchema.safeParse(deleteManyInput); + if (!parsed.success) { + res.status(400).json({ + error: 'Invalid deleteMany request', + code: 'VALIDATION_FAILED', + fields: zodIssuesToFields(parsed.error?.issues, deleteManyInput), + object: req.params?.object, + }); + return; + } + // [#3939] The cap that matters most: since #3897 this + // route deletes per id, so the list length IS the engine + // round-trip count. + if (this.enforceBatchSize(res, parsed.data.ids.length, maxBatch, req.params?.object)) return; + const deleteManyRequest: ServerScopedDataRequest = { + ...parsed.data, + ...(environmentId ? { environmentId } : {}), + ...(context ? { context } : {}), + }; + const result = await p.deleteManyData!(deleteManyRequest); + res.json(result); + } catch (error: any) { + handleRouteError(res, error, req.params?.object); + } + }, + metadata: { + summary: 'Delete multiple records', + tags: ['data', 'batch'], + }, + }); + } +} diff --git a/packages/rest/src/execctx-consumer-census.test.ts b/packages/rest/src/execctx-consumer-census.test.ts index 802784d362d..1ab21854cda 100644 --- a/packages/rest/src/execctx-consumer-census.test.ts +++ b/packages/rest/src/execctx-consumer-census.test.ts @@ -76,30 +76,35 @@ const SOURCE = readFileSync(resolve(HERE, 'rest-server.ts'), 'utf8'); * are still `this.resolveExecCtx(environmentId, req)` on a `RestServer` route, so * they are still this census's population. A site is therefore identified by * `file:line`, never by a line alone. + * + * [#23050 slice 2] `RestServer#registerBatchEndpoints`' body followed, unedited, to + * `batch-routes.ts`, and its five sites with it — on the same terms. */ -const SITE_FILES = ['rest-server.ts', 'data-action-routes.ts'] as const; +const SITE_FILES = ['rest-server.ts', 'data-action-routes.ts', 'batch-routes.ts'] as const; const SOURCES = new Map(SITE_FILES.map((file) => [ file, file === 'rest-server.ts' ? SOURCE : readFileSync(resolve(HERE, file), 'utf8'), ])); const siteKey = (file: string, line: number) => `${file}:${line}`; /** - * The one frame in `rest-server.ts` that is NOT a consumer: the host literal - * `registerDataActionEndpoints` hands the moved body forwards each member to the - * server with `(...a) => this.(...a)`, so a moved site's stack shows that - * forwarder above the site itself. The site is the first censused frame that is - * not it. Found by its spelling, so a respelt forwarder is a control failure in - * §2 rather than every moved site silently attributed to it. + * The frames in `rest-server.ts` that are NOT consumers: the host literal each + * moved registrar (`registerDataActionEndpoints`, `registerBatchEndpoints`) hands + * its moved body forwards each member to the server with + * `(...a) => this.(...a)`, so a moved site's stack shows that forwarder + * above the site itself. The site is the first censused frame that is not one. + * Found by their spelling, and §2 pins ONE per moved module, so a respelt + * forwarder — or a module moved without joining `SITE_FILES` — is a control + * failure there rather than every moved site silently attributed to it. */ -const FORWARDER_LINE = SOURCE.split('\n').findIndex((l) => - l.includes('resolveExecCtx: (...a) => this.resolveExecCtx(...a),')) + 1; -const FORWARDER = siteKey('rest-server.ts', FORWARDER_LINE); +const FORWARDER_LINES = SOURCE.split('\n').flatMap((l, i) => + l.includes('resolveExecCtx: (...a) => this.resolveExecCtx(...a),') ? [i + 1] : []); +const FORWARDERS = new Set(FORWARDER_LINES.map((line) => siteKey('rest-server.ts', line))); /** The census site a `resolveExecCtx` call came from, read off its stack. */ function siteOfStack(stack: string): string | undefined { - for (const m of stack.matchAll(/(rest-server|data-action-routes)\.ts:(\d+):\d+/g)) { + for (const m of stack.matchAll(/(rest-server|data-action-routes|batch-routes)\.ts:(\d+):\d+/g)) { const key = siteKey(`${m[1]}.ts`, Number(m[2])); - if (key !== FORWARDER) return key; + if (!FORWARDERS.has(key)) return key; } return undefined; } @@ -534,9 +539,18 @@ describe('[#13160] §2 the consumer surface, counted from the tree', () => { // RestServer['resolveExecCtx']`, two). The forwarder is the one // `rest-server.ts` frame `siteOfStack` steps over; the control below // proves it is still where that spelling says it is. + // + // [#23050 slice 2] 62 sites UNCHANGED / 81 → 85 mentions — the same MOVE. + // `registerBatchEndpoints`' body left for `batch-routes.ts` with its five + // BARE sites, so this census reads three files: 48 + 9 + 5 = 62 sites. + // The four new mentions are that slice's seam, the same two spellings + // (its host literal's forwarder, two; its host interface's member, two). + // There are now two forwarders, one per moved module, and the control + // pins that pairing. expect(SITES.length).toBe(62); - expect([...SOURCES.values()].reduce((n, text) => n + text.split('resolveExecCtx').length - 1, 0)).toBe(81); - expect(FORWARDER_LINE, 'the host literal\'s resolveExecCtx forwarder was respelt or removed').toBeGreaterThan(0); + expect([...SOURCES.values()].reduce((n, text) => n + text.split('resolveExecCtx').length - 1, 0)).toBe(85); + expect(FORWARDER_LINES, 'one host-literal resolveExecCtx forwarder per moved module: one was respelt or removed, or a module moved without joining SITE_FILES') + .toHaveLength(SITE_FILES.length - 1); }); it('the split is 16 locally caught / 46 bare — NOT 12 / 46, which does not add to 62', () => { diff --git a/packages/rest/src/objectql-slot-consumer-census.test.ts b/packages/rest/src/objectql-slot-consumer-census.test.ts index e351c615d17..2baeb0d7e00 100644 --- a/packages/rest/src/objectql-slot-consumer-census.test.ts +++ b/packages/rest/src/objectql-slot-consumer-census.test.ts @@ -107,6 +107,27 @@ const HERE = dirname(fileURLToPath(import.meta.url)); const SOURCE = readFileSync(resolve(HERE, 'rest-server.ts'), 'utf8'); const PLUGIN = readFileSync(resolve(HERE, 'rest-api-plugin.ts'), 'utf8'); +/** + * [#23050 slice 2] The files this census reads. `RestServer#registerBatchEndpoints`' + * body — the cross-object batch door, the THIRD consumer — moved, unedited, to + * `batch-routes.ts`. It still reads `this.objectQLProvider`, now off the host + * literal the class hands it, whose `objectQLProvider` is a getter onto the + * class's own field: so it is still this slot's consumer, and still this + * census's population. A site is identified by `file:line`, never by a line alone. + */ +const SLOT_FILES = ['rest-server.ts', 'batch-routes.ts'] as const; +const SOURCES = new Map(SLOT_FILES.map((file) => [ + file, file === 'rest-server.ts' ? SOURCE : readFileSync(resolve(HERE, file), 'utf8'), +])); + +/** Every invocation site of the slot over every censused file, keyed `file:line`. */ +function censusInvocationSites(field: string): Array<{ file: string; line: number; text: string; lines: string[] }> { + return [...SOURCES].flatMap(([file, source]) => { + const lines = source.split('\n'); + return invocationSites(source, field).map((s) => ({ file, ...s, lines })); + }); +} + const BATCH_PATH = '/api/v1/batch'; // --------------------------------------------------------------------------- @@ -153,25 +174,28 @@ describe('[#18546] §1 the `objectQLProvider` consumer census is mechanical', () }); it('⭐ THREE consumers, not two — the count the two prose censuses got wrong', () => { - const sites = invocationSites(SOURCE, 'objectQLProvider'); + // [#23050 slice 2] 3 UNCHANGED, over two files: 2 in `rest-server.ts` + // and the batch door's 1 in `batch-routes.ts`. + const sites = censusInvocationSites('objectQLProvider'); // The message carries the sites, so a failure here hands the next // reader the new census instead of a bare number. - expect(sites.map((s) => `L${s.line}: ${s.text}`).join('\n')).toBeTruthy(); + expect(sites.map((s) => `${s.file}:${s.line}: ${s.text}`).join('\n')).toBeTruthy(); expect(sites).toHaveLength(3); }); it('⭐ [#18559] ALL THREE consumers now reach the slot through `wiredEngineOrLoud` — none reads it directly', () => { - const sites = invocationSites(SOURCE, 'objectQLProvider'); + const sites = censusInvocationSites('objectQLProvider'); const lines = SOURCE.split('\n'); const viaSeam = sites.filter((s) => { // The helper is named on the line that OPENS the call, two lines up - // from the invocation in all three spellings. - return lines.slice(Math.max(0, s.line - 4), s.line).join('\n').includes('wiredEngineOrLoud('); + // from the invocation in all three spellings — read in the site's + // OWN file. + return s.lines.slice(Math.max(0, s.line - 4), s.line).join('\n').includes('wiredEngineOrLoud('); }); const direct = sites.filter((s) => !viaSeam.includes(s)); // The message carries the offenders, so a failure hands the next reader // the site rather than a bare number. - expect(direct.map((s) => `L${s.line}: ${s.text}`).join('\n')).toBe(''); + expect(direct.map((s) => `${s.file}:${s.line}: ${s.text}`).join('\n')).toBe(''); expect(direct).toHaveLength(0); expect(viaSeam).toHaveLength(3); @@ -193,16 +217,35 @@ describe('[#18546] §1 the `objectQLProvider` consumer census is mechanical', () // `undefined` path produces a few lines later — so "3 via the seam" is // pinned to include the consumer #18559 moved, not just its two siblings. const batch = viaSeam.filter((s) => - lines.slice(s.line, s.line + 6).join('\n').includes('Transactional batch not supported by this runtime')); + s.lines.slice(s.line, s.line + 6).join('\n').includes('Transactional batch not supported by this runtime')); expect(batch).toHaveLength(1); + // [#23050 slice 2] …and it is the one that moved: read in `batch-routes.ts`. + expect(batch[0]?.file).toBe('batch-routes.ts'); }); it('⛔ the field is read only as `this.objectQLProvider` — no alias keeps a consumer out of this census', () => { // A dynamic read would be invisible to the scanner above, and TypeScript's // `private` is compile-time only, so the absence of these spellings is // what bounds the enumeration to what §1 can see. - expect(SOURCE).not.toMatch(/\[\s*['"]objectQLProvider['"]\s*\]/); - expect(SOURCE).not.toMatch(/as any\s*\)\s*\.objectQLProvider/); + // + // [#23050 slice 2] Over every censused file. `batch-routes.ts` spells the + // slot in brackets exactly ONCE, and as a TYPE that reads nothing: its + // host interface's member, typed `RestServer['objectQLProvider']`. That + // one spelling is set aside before the check and pinned at exactly one + // there (zero here), so setting it aside cannot grow into an exemption. + const HOST_MEMBER_TYPE = "RestServer['objectQLProvider']"; + for (const [file, source] of SOURCES) { + expect(source.split(HOST_MEMBER_TYPE).length - 1, file).toBe(file === 'rest-server.ts' ? 0 : 1); + const code = source.split(HOST_MEMBER_TYPE).join(''); + expect(code, file).not.toMatch(/\[\s*['"]objectQLProvider['"]\s*\]/); + expect(code, file).not.toMatch(/as any\s*\)\s*\.objectQLProvider/); + } + // [#23050 slice 2] The one way the slot leaves `RestServer` for another + // file: a moved registrar's host literal hands it on as a GETTER onto + // the class's own field. One getter per censused module beyond + // `rest-server.ts`, so a slot handed to a module that never joined + // `SLOT_FILES` reds here instead of hiding its consumer from §1. + expect(SOURCE.match(/\bget objectQLProvider\(\)/g) ?? []).toHaveLength(SLOT_FILES.length - 1); // And the shipped closure reaches `RestServer` through exactly one // construction site, so `rest-api-plugin.ts` cannot be wiring a second // consumer this file never looked at. diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index 330f581b476..bee3d9b7bf5 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -26,9 +26,8 @@ import { // [#7678] ADR-0090 D5/D9 suggested-binding `?status=` vocabulary — the one // owner, shared with the runtime dispatcher's `/security` domain. isAudienceBindingSuggestionStatus, unknownAudienceBindingSuggestionStatusMessage, - // The write-response rules (credential-class mask, `internal` omit) for - // the cross-object batch's direct-engine update arm. - omitInternalFieldsFromWriteResponse, + // `omitInternalFieldsFromWriteResponse`, the cross-object batch's update-arm + // write-response rules, moved with that door to `batch-routes.ts` (#23050 slice 2). } from '@objectstack/core'; import { isMcpServerEnabled, @@ -164,11 +163,10 @@ import type { CreateDataRequest, UpdateDataRequest, DeleteDataRequest, - BatchDataRequest, - CreateManyDataRequest, - UpdateManyDataRequest, - DeleteManyDataRequest, } from '@objectstack/spec/api'; +// The four bulk request types (`BatchDataRequest`, `CreateManyDataRequest`, +// `UpdateManyDataRequest`, `DeleteManyDataRequest`) moved with their doors to +// `batch-routes.ts` (#23050 slice 2). // [#8073] The closed ADR-0112 error vocabulary, so the explain family's single // refusal emitter types its `code` parameter as the vocabulary rather than as // `string` — an invented code is a compile error at the call site instead of a @@ -182,7 +180,6 @@ import type { ErrorCode } from '@objectstack/spec/api'; import { PUBLIC_FORM_SERVER_MANAGED_FIELDS } from '@objectstack/spec/security'; import { PLURAL_TO_SINGULAR, canonicalMetaUrlType } from '@objectstack/spec/shared'; import { stripReadDecorations } from '@objectstack/spec/kernel'; -import type { DroppedFieldsEvent } from '@objectstack/spec/data'; // [#20193] THE per-caller read gate of a `/meta/:type/:name` document, and the // docs-audience and app-nav gates it is built from — one implementation, which // this server and the runtime dispatcher's `/meta` domain both call. The @@ -320,6 +317,8 @@ import { } from './export-format.js'; // [#23050 slice 1] The data-action routes' body moved to its own module — see its header. import { registerDataActionEndpoints, type DataActionRouteHost, type ServerScopedDataRequest } from './data-action-routes.js'; +// [#23050 slice 2] The batch routes' body moved to its own module — see its header. +import { registerBatchEndpoints, type BatchRouteHost } from './batch-routes.js'; // [#17551] The `POST …/analytics/dataset/query` door parse — the half of the // analytics family this route never had. See the module header for the // measurement that decides its shape. @@ -1542,7 +1541,7 @@ async function seamOrUndefined(call: () => T | PromiseLike): Promise( +export async function wiredEngineOrLoud( wired: boolean, call: () => T | PromiseLike, ): Promise { @@ -12352,500 +12351,24 @@ export class RestServer { * Register batch operation endpoints */ private registerBatchEndpoints(basePath: string): void { - const { crud, batch } = this.config; - const dataPath = `${basePath}${crud.dataPrefix}`; - const isScoped = basePath.includes('/environments/:environmentId'); - - const operations = batch.operations; - // [#3939] One cap, read once, applied by every bulk route below via - // {@link enforceBatchSize} — see that method for why it lives here and - // not in the Zod schemas. - const maxBatch = batch.maxBatchSize ?? 200; - - // 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). 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`, - handler: async (req: any, res: any) => { - try { - const environmentId = isScoped ? req.params?.environmentId : undefined; - const context = await this.resolveExecCtx(environmentId, req); - if (this.enforceAuth(req, res, context)) return; - // [#18559] The engine seam, reached the way its two SIBLING - // consumers of this same slot already reach it — - // `wiredEngineOrLoud` — so "no engine is wired" and "the - // engine WAS wired and could not be resolved" stay two facts - // instead of being told apart only by accident. The retired - // spelling this replaces was a plain read: - // - // this.objectQLProvider ? await this.objectQLProvider(environmentId) : undefined - // - // ⚠️ It did NOT re-collapse them — that is why this is not a - // regression and was not a blocker for #14251's decidable - // test. A rejection escaped the plain read, missed the 501 - // arm below (which tests `!ql || typeof ql.transaction !== - // 'function'`, and a rejection never reaches it), and was - // caught by this handler's GENERIC outer catch - // (`handleRouteError`). So the two facts did differ on the - // wire — 500 INTERNAL_ERROR against 501 NOT_IMPLEMENTED — - // but through a catch-all that knows nothing about this - // seam, at a status this slot's other two consumers do not - // use for the same fact. - // - // ⭐ What decided it, measured on a real `RestServer` over a - // real `ObjectKernel` rather than argued: THIS DOOR ALREADY - // ANSWERS 503 on the single-kernel wiring. There - // `computeExecCtx` takes its PROVIDER branch, which is - // `wiredEngineOrLoud`, and raises before this line runs. The - // 500 was reachable only on the MULTI-KERNEL wiring, where - // the gate's kernel branch absorbs by design (see - // `wiredEngineOrLoud`'s RESIDUE note) and hands the engine - // question down to this line: - // - // | wiring, engine wired and FAILING | before | after | - // |:--|:--|:--| - // | single-kernel (gate raises first)| 503 | 503 — unchanged | - // | multi-kernel (gate absorbs) | **500**| **503** | - // - // ⇒ the repair does not choose a new wire answer for this - // door; it removes a WIRING-DEPENDENT divergence, leaving - // the answer this door already gave on the composition the - // open core boots. - // - // ⛔ NOT `seamOrUndefined`. That helper SWALLOWS, and its own - // docblock forbids routing the data-engine seam back through - // it "to make the seams uniform". - // - // The wiring fact is the provider's PRESENCE, asked once and - // never inferred from what it returned, so both ABSENCE - // shapes reach the 501 below byte-for-byte as before: no - // provider wired at all, and a provider that RESOLVES - // `undefined`, which is the seam contract declaring absence - // rather than failing. `wiredEngineOrLoud` also invokes the - // provider SYNCHRONOUSLY, so a host wiring a non-`async` - // provider — which the seam's declared type cannot prevent — - // reaches the same answer as one that rejects (commit add6a1b1c). - const ql = await wiredEngineOrLoud( - Boolean(this.objectQLProvider), - () => this.objectQLProvider!(environmentId), - ); - if (!ql || typeof ql.transaction !== 'function') { - // Typed like every other 501 on this server (clone/search, - // #4067) so a client can key on the code, not the prose. - res.status(501).json({ error: 'Transactional batch not supported by this runtime', code: 'NOT_IMPLEMENTED' }); - return; - } - - // 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; - } - if (ops.length === 0) { res.json({ results: [] }); return; } - // [#3939] Same check, same envelope as every other bulk route - // now — this one used to be the only one that capped at all, - // and it answered without a `code` for clients to key on. - if (this.enforceBatchSize(res, ops.length, maxBatch)) 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 - // [#3391] Cross-object batch is a bulk surface: gate each op - // as `bulk ∧ child(op.action)` — the object must grant the - // `bulk` primitive AND the specific write it performs. - const denial = apiAccessDenialFromEnable(obj.enable, op.object, 'bulk', { bulkChild: 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 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; - } - } - return result; - }; - - // [#3794] Write-observability on THIS surface too. The engine - // strips `readonly` / `readonlyWhen` writes silently, and every - // other write path already reports what it dropped (#3431/#3455) - // — but this one did not, and it is precisely the path the - // console's record form takes for a master-detail save. Result: - // a user edited a `readonlyWhen`-locked field, got "updated - // successfully", and the value never changed with nothing said - // (#3794 problem 2). Each event is tagged with its operation - // index, since `results` entries are bare record echoes with no - // envelope to hang a per-row list on. - const dropped: Array = []; - const results = await ql.transaction(async (trxCtx: any) => { - const out: any[] = []; - for (const [index, op] of ops.entries()) { - const data = resolveRefs(op.data, out); - if (op.action === 'create') { - // [#3835] Go through the protocol's create ingress — - // the SAME one `POST /data/:object` uses — rather than - // calling `ql.insert` directly. When this was written the - // #3043 static-`readonly` strip lived at that ingress and - // a direct `ql.insert` bypassed it, so `readonly` meant - // two different things on two create paths. Since the - // maintainer ruling of 2026-09-03 (option C, #14147) the - // strip runs inside `engine.insert` for every non-system - // caller, so both routes are stripped identically, and the - // platform-object carve-out (a `sys_`/`managedBy` object's - // own guard must REJECT a forged value, not silently - // swallow it) is the engine's as well. The routing stands - // on what the ingress still owns: the #3770 object- - // existence gate, the #7823 `internal: true` response - // strip and the `droppedFields` relay — one create - // ingress, one response contract, and a future change to - // its policy covers the batch for free. `trxCtx` carries - // the caller's context (including `isSystem`) plus the - // open transaction, so the engine's strip decides exactly - // as it does on the single route and the insert still - // joins this transaction. - const batchCreateRequest: ServerScopedDataRequest = { - object: op.object, data, context: trxCtx, - }; - const created: any = await p.createData(batchCreateRequest); - for (const e of (created?.droppedFields ?? []) as DroppedFieldsEvent[]) { - dropped.push({ ...e, index }); - } - out.push(created?.record); - } else if (op.action === 'update') { - // Update needs no ingress detour for the WRITE half: - // the engine enforces both static `readonly` (#2948) - // and `readonlyWhen` (#3042) on its own update path, - // and reports them through this listener. - const onFieldsDropped = (e: DroppedFieldsEvent) => { dropped.push({ ...e, index }); }; - const id = op.id ?? data?.id; - const updated = await ql.update(op.object, { ...data, id }, { context: trxCtx, onFieldsDropped }); - // [#7823] …but the RESPONSE half moved to the ingress - // (A-prime ruling, 2026-08-13): the engine keeps its - // write results whole, so this direct-`ql.update` - // mouth applies the shared write-response rules - // itself (credential-class mask, then `internal` - // omit) before the row rides `results` out. Called - // from `@objectstack/core` directly — never through - // an optional protocol method, which a protocol - // without it would skip silently. FAIL CLOSED: with - // no registered schema to judge the fields by, only - // the id is echoed. - const updateSchema = (ql as any).registry?.getObject?.(op.object); - if (!updateSchema) { - out.push(updated && typeof updated === 'object' ? { id: (updated as any).id ?? id } : updated); - } else { - omitInternalFieldsFromWriteResponse(updateSchema, updated); - out.push(updated); - } - } else { // 'delete' - out.push(await ql.delete(op.object, { where: { id: op.id }, context: trxCtx })); - } - } - return out; - }, context); - - res.json({ results, ...(dropped.length > 0 ? { droppedFields: dropped } : {}) }); - } catch (error: any) { - // Log only genuine server faults; client 4xx (validation, - // unresolved ref, atomic rollback of a bad op) are expected. - // This site used to judge on `status >= 500` alone, which - // also swallowed the un-coded 400 `mapDataError` degrades an - // UNRECOGNISED error to — a handler `TypeError` inside a - // batch transaction vanished here. The shared predicate - // keeps that one loud while staying quiet on the coded 4xx. - handleRouteError(res, error); - } - }, - metadata: { - summary: 'Cross-object transactional batch (atomic create/update/delete across objects)', - tags: ['data', 'batch'], - }, - }); - - // POST /data/:object/batch - Generic batch endpoint - if (batch.enableBatchEndpoint && this.protocol.batchData) { - this.routeManager.register({ - method: 'POST', - path: `${dataPath}/:object/batch`, - handler: async (req: any, res: any) => { - try { - const environmentId = isScoped ? req.params?.environmentId : undefined; - const p = await this.resolveProtocol(environmentId, req); - const context = await this.resolveExecCtx(environmentId, req); - if (this.enforceAuth(req, res, context)) return; - // [#3391] bulk ∧ child(body.operation) — the object must grant - // the `bulk` primitive AND the batched write kind. - if (await this.enforceApiAccess(req, res, p, environmentId, 'bulk', { bulkChild: req.body?.operation })) return; - // [#3899] Validate against the declared contract - // (`BatchUpdateRequestSchema`, catalog `requestSchema`) — - // this route used to hand the body straight to the - // protocol, so `{ operation: 'updat', records: {} }` - // reached the engine as-is. Validation only: the ORIGINAL - // body is forwarded, not the parse output, so - // `BatchOptionsSchema`'s defaults (e.g. `atomic: true`) - // are not injected into a request that never sent them. - const { BatchUpdateRequestSchema } = await import('@objectstack/spec/api'); - const batchInput = req.body ?? {}; - const parsedBatch = (BatchUpdateRequestSchema as any).safeParse(batchInput); - if (!parsedBatch.success) { - res.status(400).json({ - error: 'Invalid batch request', - code: 'VALIDATION_FAILED', - fields: zodIssuesToFields(parsedBatch.error?.issues, batchInput), - object: req.params?.object, - }); - return; - } - // [#3939] Cap AFTER the shape check, so a caller gets the - // more specific answer first. - if (this.enforceBatchSize(res, parsedBatch.data.records.length, maxBatch, req.params?.object)) return; - const batchRequest: ServerScopedDataRequest = { - object: req.params.object, - request: req.body, - ...(environmentId ? { environmentId } : {}), - ...(context ? { context } : {}), - }; - const result = await p.batchData!(batchRequest); - res.json(result); - } catch (error: any) { - handleRouteError(res, error, req.params?.object); - } - }, - metadata: { - summary: 'Batch operations', - tags: ['data', 'batch'], - }, - }); - } - - // POST /data/:object/createMany - Bulk create - if (operations.createMany && this.protocol.createManyData) { - this.routeManager.register({ - method: 'POST', - path: `${dataPath}/:object/createMany`, - handler: async (req: any, res: any) => { - try { - const environmentId = isScoped ? req.params?.environmentId : undefined; - const p = await this.resolveProtocol(environmentId, req); - const context = await this.resolveExecCtx(environmentId, req); - if (this.enforceAuth(req, res, context)) return; - // [#3391] bulk ∧ create — createMany requires the `bulk` primitive. - if (await this.enforceApiAccess(req, res, p, environmentId, 'bulk', { bulkChild: 'create' })) return; - // [#3899] Body IS the records array on this route (what - // `client.data.createMany` posts). Validate the assembled - // protocol request (`CreateManyDataRequestSchema`, catalog - // `requestSchema`) so `{ records: [...] }` — updateMany's - // envelope, an easy cross-route slip — or any other - // non-array body 400s instead of reaching the engine as a - // single garbage "record". - const { CreateManyDataRequestSchema } = await import('@objectstack/spec/api'); - const createManyInput = { object: req.params.object, records: req.body ?? [] }; - const parsedCreateMany = (CreateManyDataRequestSchema as any).safeParse(createManyInput); - if (!parsedCreateMany.success) { - res.status(400).json({ - error: 'Invalid createMany request — the body must be a JSON array of record objects', - code: 'VALIDATION_FAILED', - fields: zodIssuesToFields(parsedCreateMany.error?.issues, createManyInput), - object: req.params?.object, - }); - return; - } - // [#3939] Cap AFTER the shape check. - if (this.enforceBatchSize(res, parsedCreateMany.data.records.length, maxBatch, req.params?.object)) return; - const createManyRequest: ServerScopedDataRequest = { - object: req.params.object, - records: req.body || [], - ...(environmentId ? { environmentId } : {}), - ...(context ? { context } : {}), - }; - const result = await p.createManyData!(createManyRequest); - res.status(201).json(result); - } catch (error: any) { - handleRouteError(res, error, req.params?.object); - } - }, - metadata: { - summary: 'Create multiple records', - tags: ['data', 'batch'], - }, - }); - } - - // POST /data/:object/updateMany - Bulk update - if (operations.updateMany && this.protocol.updateManyData) { - this.routeManager.register({ - method: 'POST', - path: `${dataPath}/:object/updateMany`, - handler: async (req: any, res: any) => { - try { - const environmentId = isScoped ? req.params?.environmentId : undefined; - const p = await this.resolveProtocol(environmentId, req); - const context = await this.resolveExecCtx(environmentId, req); - if (this.enforceAuth(req, res, context)) return; - // [#3391] bulk ∧ update — updateMany requires the `bulk` primitive. - if (await this.enforceApiAccess(req, res, p, environmentId, 'bulk', { bulkChild: 'update' })) return; - // [#3933] Validate against the spec contract, and write the - // PATH object last. The body used to be spread over - // `object: req.params.object`, so `{"object":"other", …}` - // moved the write to a different object than the one - // `enforceApiAccess` had just cleared — that gate reads - // `req.params.object`, so `enable.apiEnabled` / `apiMethods` - // (ADR-0049) was enforced on A while B was written. Zod also - // strips unknown keys, which keeps a body `context` from - // becoming the execution context on a deployment where none - // resolves (e.g. an anonymous public-book read, #3963). - const { UpdateManyDataRequestSchema } = await import('@objectstack/spec/api'); - const updateManyInput = { ...(req.body ?? {}), object: req.params.object }; - const parsedUpdate = UpdateManyDataRequestSchema.safeParse(updateManyInput); - if (!parsedUpdate.success) { - res.status(400).json({ - error: 'Invalid updateMany request', - code: 'VALIDATION_FAILED', - fields: zodIssuesToFields(parsedUpdate.error?.issues, updateManyInput), - object: req.params?.object, - }); - return; - } - // [#3939] Cap AFTER the shape check, so a caller gets the - // more specific answer first. - if (this.enforceBatchSize(res, parsedUpdate.data.records.length, maxBatch, req.params?.object)) return; - const updateManyRequest: ServerScopedDataRequest = { - ...parsedUpdate.data, - ...(environmentId ? { environmentId } : {}), - ...(context ? { context } : {}), - }; - const result = await p.updateManyData!(updateManyRequest); - res.json(result); - } catch (error: any) { - handleRouteError(res, error, req.params?.object); - } - }, - metadata: { - summary: 'Update multiple records', - tags: ['data', 'batch'], - }, - }); - } - - // POST /data/:object/deleteMany - Bulk delete - if (operations.deleteMany && this.protocol.deleteManyData) { - this.routeManager.register({ - method: 'POST', - path: `${dataPath}/:object/deleteMany`, - handler: async (req: any, res: any) => { - try { - const environmentId = isScoped ? req.params?.environmentId : undefined; - const p = await this.resolveProtocol(environmentId, req); - const context = await this.resolveExecCtx(environmentId, req); - if (this.enforceAuth(req, res, context)) return; - // [#3391] bulk ∧ delete — deleteMany requires the `bulk` primitive. - if (await this.enforceApiAccess(req, res, p, environmentId, 'bulk', { bulkChild: 'delete' })) return; - // [#3897] Validate against the spec contract instead of - // splatting the raw body into the protocol request. Zod - // object schemas STRIP unknown keys, which is what makes - // this a security boundary and not just an error message: - // `options` is narrowed to `BatchOptions`, so a body key - // (`options.where`, `options.multi`) can no longer ride - // into the engine's delete options, and a top-level - // `context` can no longer forge the caller's principal on - // a route reachable without auth. The protocol layer - // refuses the same shapes independently (defence in - // depth) — this stops them one hop earlier, with a 400 - // the caller can act on. - // [#3933] The PATH object is written LAST for the same - // reason: `enforceApiAccess` gates on `req.params.object`, - // so a body `object` would move the delete to an object - // whose exposure policy was never checked. - const { DeleteManyDataRequestSchema } = await import('@objectstack/spec/api'); - const deleteManyInput = { ...(req.body ?? {}), object: req.params.object }; - const parsed = DeleteManyDataRequestSchema.safeParse(deleteManyInput); - if (!parsed.success) { - res.status(400).json({ - error: 'Invalid deleteMany request', - code: 'VALIDATION_FAILED', - fields: zodIssuesToFields(parsed.error?.issues, deleteManyInput), - object: req.params?.object, - }); - return; - } - // [#3939] The cap that matters most: since #3897 this - // route deletes per id, so the list length IS the engine - // round-trip count. - if (this.enforceBatchSize(res, parsed.data.ids.length, maxBatch, req.params?.object)) return; - const deleteManyRequest: ServerScopedDataRequest = { - ...parsed.data, - ...(environmentId ? { environmentId } : {}), - ...(context ? { context } : {}), - }; - const result = await p.deleteManyData!(deleteManyRequest); - res.json(result); - } catch (error: any) { - handleRouteError(res, error, req.params?.object); - } - }, - metadata: { - summary: 'Delete multiple records', - tags: ['data', 'batch'], - }, - }); - } + // [#23050 slice 2] The body moved to `batch-routes.ts`. This literal + // hands it every member it reads — the list IS its dependency on this + // class (see `BatchRouteHost`, and that module's header for why + // `objectQLProvider` alone is a getter). + const readObjectQLProvider = () => this.objectQLProvider; + const host: BatchRouteHost = { + config: this.config, + routeManager: this.routeManager, + protocol: this.protocol, + get objectQLProvider() { return readObjectQLProvider(); }, + resolveProtocol: (...a) => this.resolveProtocol(...a), + resolveExecCtx: (...a) => this.resolveExecCtx(...a), + enforceAuth: (...a) => this.enforceAuth(...a), + enforceApiAccess: (...a) => this.enforceApiAccess(...a), + enforceBatchSize: (...a) => this.enforceBatchSize(...a), + loadObjectItems: (...a) => this.loadObjectItems(...a), + }; + registerBatchEndpoints.call(host, basePath); } diff --git a/packages/rest/src/ui-view-route-identity.measurement.test.ts b/packages/rest/src/ui-view-route-identity.measurement.test.ts index bc57eae5fd6..bbd8c37f620 100644 --- a/packages/rest/src/ui-view-route-identity.measurement.test.ts +++ b/packages/rest/src/ui-view-route-identity.measurement.test.ts @@ -79,8 +79,10 @@ const SOURCE = readFileSync(resolve(HERE, 'rest-server.ts'), 'utf8'); // [#23050 slice 1] The reverse checks below count a term's spelling over the // files RestServer's registrars live in, which since that slice include the // module `registerDataActionEndpoints`' body moved to. Sliced readings of ONE -// registrar still read `SOURCE` alone. -const REGISTRAR_SOURCES = SOURCE + '\n' + readFileSync(resolve(HERE, 'data-action-routes.ts'), 'utf8'); +// registrar still read `SOURCE` alone. [#23050 slice 2] …and the module +// `registerBatchEndpoints`' body moved to. +const REGISTRAR_SOURCES = [SOURCE, ...['data-action-routes.ts', 'batch-routes.ts'] + .map((file) => readFileSync(resolve(HERE, file), 'utf8'))].join('\n'); const BASE = '/api/v1'; const UI_ROUTE = `GET ${BASE}/ui/view/:object/:type`; diff --git a/packages/rest/src/ui-view-route-tenancy.measurement.test.ts b/packages/rest/src/ui-view-route-tenancy.measurement.test.ts index 78476c8774d..5909090a990 100644 --- a/packages/rest/src/ui-view-route-tenancy.measurement.test.ts +++ b/packages/rest/src/ui-view-route-tenancy.measurement.test.ts @@ -100,8 +100,10 @@ const SOURCE = readFileSync(resolve(HERE, 'rest-server.ts'), 'utf8'); // [#23050 slice 1] The reverse checks below count a term's spelling over the // files RestServer's registrars live in, which since that slice include the // module `registerDataActionEndpoints`' body moved to. Sliced readings of ONE -// registrar still read `SOURCE` alone. -const REGISTRAR_SOURCES = SOURCE + '\n' + readFileSync(resolve(HERE, 'data-action-routes.ts'), 'utf8'); +// registrar still read `SOURCE` alone. [#23050 slice 2] …and the module +// `registerBatchEndpoints`' body moved to. +const REGISTRAR_SOURCES = [SOURCE, ...['data-action-routes.ts', 'batch-routes.ts'] + .map((file) => readFileSync(resolve(HERE, file), 'utf8'))].join('\n'); const BASE = '/api/v1'; const UI_ROUTE = `GET ${BASE}/ui/view/:object/:type`; diff --git a/scripts/check-route-envelope.mjs b/scripts/check-route-envelope.mjs index 4fb0bbcd8c4..971bf6530af 100644 --- a/scripts/check-route-envelope.mjs +++ b/scripts/check-route-envelope.mjs @@ -329,7 +329,12 @@ const MODULES = { // string `error` bodies of `registerDataActionEndpoints`' handlers left this // file with that body for `data-action-routes.ts`, which declares them // below at the count they had here; the sum is unchanged (26 + 13 = 39). - stringError: 26, + // + // 26 → 18 (#23050 slice 2): NOT progress — the same MOVE. The eight literal + // string `error` bodies of `registerBatchEndpoints`' handlers left this file + // with that body for `batch-routes.ts`, declared below at the count they had + // here; the sum is unchanged (18 + 13 + 8 = 39). + stringError: 18, // 77 → 75 (#7981): registerSecurityEndpoints' two `handleError` arms moved // off the `{ code, error }` sibling-code literal onto the shared // `respondError` helper, banking that progress per the ratchet's own rule. @@ -371,7 +376,11 @@ const MODULES = { // 54 → 39 (#23050 slice 1): NOT progress — the same MOVE as `stringError` // above. Fifteen `{ code, error }` bodies left with the data-action body for // `data-action-routes.ts`, declared below; the sum is unchanged (39 + 15 = 54). - siblingCode: 39, + // + // 39 → 31 (#23050 slice 2): NOT progress — the same MOVE as `stringError` + // above. Eight `{ code, error }` bodies left with the batch body for + // `batch-routes.ts`, declared below; the sum is unchanged (31 + 15 + 8 = 54). + siblingCode: 31, }, // [#8850] The ADR-0112 error/fault-classification prologue, extracted from @@ -427,6 +436,23 @@ const MODULES = { stringError: 13, siblingCode: 15, }, + + // [#23050 slice 2] `RestServer#registerBatchEndpoints`' body — the + // cross-object transactional batch and the four per-object bulk doors — + // moved out of `rest-server.ts` whole, on exactly slice 1's terms above: a + // MOVE, no wire answer changed, so it inherits `dialectOnly` and the two + // dialect facts, and no write-site count. Both counts were measured on the + // moved file and equal what left `rest-server.ts` in the same PR (stringError + // 26 → 18, siblingCode 39 → 31), so each sum is unchanged. Both only ever tick + // down. Discovered by the `*-routes.ts` convention and declared in the same PR + // that created the module, so the gap never exists. + 'packages/rest/src/batch-routes.ts': { + dialectOnly: + 'moved out of `rest-server.ts` by #23050 slice 2 and held to exactly the guarantee it had there — the two dialect facts, never a write-site count', + ratchet: '#9559 (option 1: convert onto the shared sendOk/sendError)', + stringError: 8, + siblingCode: 8, + }, }; /** Identifiers whose `.json()` READS a request rather than writing a response. */