diff --git a/.changeset/api-derivation-batch-alias-row-retired.md b/.changeset/api-derivation-batch-alias-row-retired.md new file mode 100644 index 0000000000..b8362dd70a --- /dev/null +++ b/.changeset/api-derivation-batch-alias-row-retired.md @@ -0,0 +1,43 @@ +--- +"@objectstack/spec": patch +--- + +fix(spec): drop the producer-less `batch` row from `DATA_ACTION_TO_API_OPERATION` (#6259) + +`DATA_ACTION_TO_API_OPERATION` normalizes the action vocabularies its callers +speak onto canonical `ApiOperation` names. One row, `batch: 'bulk'`, had no +producer on either side, and the table's own TSDoc still taught readers that it +did — calling `batch` a "runtime `callData` action". + +Both consumers were re-enumerated at `origin/main` before the row was removed: + +- `packages/runtime/src/api-exposure.ts` (`checkApiExposure`) is reached only + from `callData`, which branches on a closed set — `create`/`get`/`update`/ + `delete`/`query`/`find`/`aggregate` — and every call site passes one of those + as a string literal. Its `batch` arm was retired in #5856, so no caller has + been able to send the word since. +- `packages/rest/src/rest-server.ts` (`apiAccessDenialFromEnable`) is fed only + canonical literals by `enforceApiAccess`: `import`, `bulk`, `create`, + `update`, `list`, `delete`, `get`, `export`. The cross-object `POST /batch` + route is the trap worth naming — it spells `batch` in the **URL** and gates on + `'bulk'`, so the route is untouched by this change. + +FROM → TO: `batch` → `bulk`. If you read this table directly, spell the bulk +surface `bulk`; `DATA_ACTION_TO_API_OPERATION['batch']` is now `undefined`. + +**Nothing on a live path changes**, because nothing sent `batch`. What changes +is the answer waiting for anyone who does: the lookup misses, the consumers' +`?? action` pass-through hands `batch` through unmapped, and an unmapped action +is *ungated* by `apiMethods` (it still respects `apiEnabled`) — the same +treatment every custom action gets. That last point is why the row was worth +removing rather than leaving as harmless: while it existed, one unreachable +word silently bought a real `bulk ∧ child` permission verdict, and a reader — +or an AI author — would reasonably conclude `batch` was a supported spelling +and write consumer-side tolerance for it. Prime Directive #12 forbids exactly +that: an alias with no producer belongs at the producer or nowhere. + +The export itself is unchanged — same name, same `Record` +type — so `check:api-surface` records no delta (that snapshot prints type +references, not expanded shapes; #3883 is the precedent for a key-level change +being invisible to it). No authorable metadata key is involved, so there is no +tombstone, no conversion and no liveness-ledger row: nothing parses this table. diff --git a/packages/rest/src/rest-api-derivation-gates.test.ts b/packages/rest/src/rest-api-derivation-gates.test.ts index 9f5eead269..86a6117d83 100644 --- a/packages/rest/src/rest-api-derivation-gates.test.ts +++ b/packages/rest/src/rest-api-derivation-gates.test.ts @@ -128,6 +128,25 @@ describe('REST gate — action alias normalization (#3391)', () => { const ro = { apiMethods: ['get', 'list'] }; expect(allowed(ro, 'query')).toBe(true); // query → list expect(allowed(ro, 'find')).toBe(true); // find → list - expect(allowed(ro, 'batch', { bulkChild: 'create' })).toBe(false); // batch → bulk + // [#6259] Was `allowed(ro, 'batch', …)` — the third alias this table + // claimed to normalize, and the only one no producer ever sends. Every + // `enforceApiAccess` call site passes a canonical literal, and the + // cross-object `POST /batch` route (rest-server.ts, `registerBatchEndpoints`) + // gates on `'bulk'` — the URL spells `batch`, the operation never does. + // Re-spelled to `'bulk'`, which is the assertion this line always meant: + // an object granting only get/list does not get the bulk surface. + expect(allowed(ro, 'bulk', { bulkChild: 'create' })).toBe(false); + }); + + // [#6259] The REST half of the absence pin. Stated as the fork it is: + // `batch` is not DENIED, it is unrecognized — and an unrecognized action is + // ungated by `apiMethods` (custom actions never were). That is precisely why + // a producer-less row could not be dismissed as harmless: while it existed, + // this word silently bought a real bulk∧child verdict. + it('`batch` is no longer normalized — it is an unknown, ungated action', () => { + const ro = { apiMethods: ['get', 'list'] }; + expect(allowed(ro, 'batch', { bulkChild: 'create' })).toBe(true); + // The canonical spelling the /batch route actually sends stays gated. + expect(denial(ro, 'bulk', { bulkChild: 'create' })?.status).toBe(405); }); }); diff --git a/packages/runtime/src/api-exposure.test.ts b/packages/runtime/src/api-exposure.test.ts index 7548d56461..0b3ebda877 100644 --- a/packages/runtime/src/api-exposure.test.ts +++ b/packages/runtime/src/api-exposure.test.ts @@ -1,5 +1,8 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. +import fs from 'node:fs'; +import path from 'node:path'; +import url from 'node:url'; import { describe, it, expect } from 'vitest'; import { checkApiExposure } from './api-exposure.js'; @@ -108,11 +111,46 @@ describe('checkApiExposure (#1889)', () => { expect(checkApiExposure(createOnly, 'import', { writeMode: 'update' }).allowed).toBe(false); }); + // [#6259] Was spelled `'batch'`, which reached this rule only through the + // `batch: 'bulk'` alias row — a spelling no producer sends (`callData` lost + // its `batch` arm in #5856; REST gates `/batch` on `'bulk'`). With the row + // gone `'batch'` is an unknown operation and falls to the ungated + // pass-through, so the negative assertion below flipped to `true` and this + // test went red — the one place in the repo that still depended on the row. + // Re-spelled to the canonical `'bulk'`, which is what the rule is about and + // what every real caller passes; the assertions themselves are unchanged. it('bulk requires the bulk primitive AND the child op', () => { const bulkCreate = { apiMethods: ['create', 'bulk'] }; - expect(checkApiExposure(bulkCreate, 'batch', { bulkChild: 'create' }).allowed).toBe(true); + expect(checkApiExposure(bulkCreate, 'bulk', { bulkChild: 'create' }).allowed).toBe(true); const createOnly = { apiMethods: ['create'] }; - expect(checkApiExposure(createOnly, 'batch', { bulkChild: 'create' }).allowed).toBe(false); + expect(checkApiExposure(createOnly, 'bulk', { bulkChild: 'create' }).allowed).toBe(false); + }); + + // [#6259] The absence pin's runtime half: `batch` is no longer a spelling + // this gate understands. It is NOT denied — an unmapped action respects + // `apiEnabled` only — which is exactly why the row could not be left in + // place as "harmless": it silently bought a bulk∧child judgement for a + // word no producer emits. + it('`batch` is no longer an alias for `bulk` — it is an ungated unknown action', () => { + const createOnly = { apiMethods: ['create'] }; + expect(checkApiExposure(createOnly, 'batch', { bulkChild: 'create' }).allowed).toBe(true); + // The object still hides entirely when the API is off, alias or not. + expect(checkApiExposure({ apiEnabled: false }, 'batch').status).toBe(404); + }); + + // [#6259] The prose half of the same finding: this function's `@param` + // listed `batch` among the runtime data actions its only caller sends. + it('the `@param action` TSDoc does not advertise `batch` as a live action', () => { + const source = fs.readFileSync( + path.join(path.dirname(url.fileURLToPath(import.meta.url)), 'api-exposure.ts'), + 'utf8', + ); + const param = source.match(/@param action[\s\S]*?@param opts/); + expect(param, 'the `@param action` block vanished').toBeTruthy(); + // The note recording the removal is expected to name `batch`; the + // enumeration of what `callData` can send must not. + const [enumeration] = param![0].split('#6259'); + expect(enumeration).not.toMatch(/batch/); }); }); }); diff --git a/packages/runtime/src/api-exposure.ts b/packages/runtime/src/api-exposure.ts index a388bf32f9..e0a4087e46 100644 --- a/packages/runtime/src/api-exposure.ts +++ b/packages/runtime/src/api-exposure.ts @@ -102,8 +102,12 @@ function resolveEnableBlock(def: ObjectApiDef): EnableLike { * Decide whether a data `action` is permitted for `def`'s declared exposure. * * @param def Object definition (nested `getObject` shape or flat). - * @param action Runtime data action (`create`/`get`/`query`/`find`/`aggregate`/ - * `batch`/…); normalized to a canonical operation internally. + * @param action Runtime data action; normalized to a canonical operation + * internally. The only caller is `callData`, whose closed set is + * `create`/`get`/`update`/`delete`/`query`/`find`/`aggregate`. + * (#6259 removed one more name from this list: `callData` has had + * no `batch` arm since #5856, so no caller could ever send that + * word. Batching reaches this gate from REST as canonical `bulk`.) * @param opts Optional `writeMode` (import precision) / `bulkChild` (bulk∧child). */ export function checkApiExposure( diff --git a/packages/spec/src/data/api-derivation.test.ts b/packages/spec/src/data/api-derivation.test.ts index 0662242c4b..bf0fae8357 100644 --- a/packages/spec/src/data/api-derivation.test.ts +++ b/packages/spec/src/data/api-derivation.test.ts @@ -1,5 +1,8 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. +import fs from 'node:fs'; +import path from 'node:path'; +import url from 'node:url'; import { describe, it, expect } from 'vitest'; import { resolveEffectiveApiMethods, @@ -194,8 +197,45 @@ describe('api-derivation (#3391)', () => { it('maps runtime action vocabulary to canonical operations', () => { expect(DATA_ACTION_TO_API_OPERATION.query).toBe('list'); expect(DATA_ACTION_TO_API_OPERATION.find).toBe('list'); - expect(DATA_ACTION_TO_API_OPERATION.batch).toBe('bulk'); expect(DATA_ACTION_TO_API_OPERATION.get).toBe('get'); + // The canonical bulk spelling — the one every REST caller actually sends, + // including the cross-object `POST /batch` route. + expect(DATA_ACTION_TO_API_OPERATION.bulk).toBe('bulk'); + }); + + // [#6259] `batch: 'bulk'` was a producer-less row: `callData` has had no + // `batch` arm since #5856, and REST gates `/batch` on the literal `'bulk'`. + // Two pins, because the finding had two halves — the row AND the prose + // that told readers `batch` was a live runtime action. + it('has no `batch` row — a lookup is undefined, not an alias for `bulk`', () => { + expect(DATA_ACTION_TO_API_OPERATION.batch).toBeUndefined(); + expect(Object.keys(DATA_ACTION_TO_API_OPERATION)).not.toContain('batch'); + // Absence is not a denial: an unmapped action falls to the consumers' + // `?? action` pass-through and is judged as itself, exactly like any + // other unrecognized/custom action. + const eff = resolveEffectiveApiMethods({ apiMethods: ['list'] }); + expect(isApiOperationAllowed(eff, 'batch')).toBe(true); + // …while the real bulk surface stays gated on the `bulk` primitive. + expect(isApiOperationAllowed(eff, 'bulk')).toBe(false); + }); + + it('its TSDoc no longer describes `batch` as part of the live vocabulary', () => { + const source = fs.readFileSync( + path.join(path.dirname(url.fileURLToPath(import.meta.url)), 'api-derivation.ts'), + 'utf8', + ); + const doc = source.match( + /\/\*\*(?:[^*]|\*(?!\/))*?\*\/\s*export const DATA_ACTION_TO_API_OPERATION\b/, + ); + expect(doc, 'DATA_ACTION_TO_API_OPERATION lost its TSDoc block').toBeTruthy(); + // The block has two halves and only the first is a claim about today: + // the vocabulary description, then a `[#6259]` note recording what was + // removed and why. The note is EXPECTED to say `batch`; the description + // saying it is the drift this issue is about ("runtime `callData` + // actions (`query`/`find`→`list`, `batch`→`bulk`)"). + const [description, history] = doc![0].split('[#6259]'); + expect(history, 'the `[#6259]` removal note vanished from the TSDoc').toBeTruthy(); + expect(description).not.toMatch(/batch/); }); }); diff --git a/packages/spec/src/data/api-derivation.ts b/packages/spec/src/data/api-derivation.ts index 2b3f193d7d..6bee23c122 100644 --- a/packages/spec/src/data/api-derivation.ts +++ b/packages/spec/src/data/api-derivation.ts @@ -145,17 +145,31 @@ export const API_METHOD_DERIVATION: Record = { }; /** - * Alias table normalizing the two runtime vocabularies onto the canonical + * Alias table normalizing the two producer vocabularies onto the canonical * {@link ApiOperation} names: - * - runtime `callData` actions (`query`/`find`→`list`, `batch`→`bulk`); + * - runtime `callData` actions — of that closed set only `query`/`find` need + * normalizing (both → `list`); `get`/`create`/`update`/`delete`/`aggregate` + * are already canonical and map to themselves; * - REST operation literals (already canonical, listed for completeness). * * Actions with no entry are passed through unchanged and, if unrecognized by * the resolver, treated as ungated (custom actions were never gated by * `apiMethods`). + * + * [#6259] The `batch: 'bulk'` row was removed, and the line above no longer + * calls `batch` a runtime `callData` action. It was the one entry with no + * producer on either side: `callData` branches on a closed set that has not + * contained `batch` since that arm was retired (#5856), and every REST caller + * of `apiAccessDenialFromEnable` passes a canonical literal — including the + * cross-object `POST /batch` route, which spells `batch` in the URL and gates + * on `'bulk'`. A row nobody can reach still taught the reader (and any AI + * author) that `batch` is a live runtime spelling, inviting the consumer-side + * tolerance for a producer-less alias that Prime Directive #12 forbids. The + * spelling is `bulk`; a `batch` lookup is now `undefined` and falls to the + * `?? action` pass-through, exactly like any other unrecognized action. */ export const DATA_ACTION_TO_API_OPERATION: Record = { - // runtime callData actions + // runtime `callData` actions and REST primitives (identity where canonical) get: 'get', query: 'list', find: 'list', @@ -163,7 +177,6 @@ export const DATA_ACTION_TO_API_OPERATION: Record = { create: 'create', update: 'update', delete: 'delete', - batch: 'bulk', bulk: 'bulk', // derived operation literals (identity) upsert: 'upsert',