diff --git a/.changeset/rest-method-not-allowed-names-failing-conjunct.md b/.changeset/rest-method-not-allowed-names-failing-conjunct.md new file mode 100644 index 0000000000..854e8bb470 --- /dev/null +++ b/.changeset/rest-method-not-allowed-names-failing-conjunct.md @@ -0,0 +1,19 @@ +--- +"@objectstack/rest": patch +--- + +`OBJECT_API_METHOD_NOT_ALLOWED` now names the conjunct that actually failed, instead of one its own `allowed` array lists. + +An object declaring `apiMethods: ['get','list','update','bulk']` refused `deleteMany`, `createMany` and each op of a cross-object `POST /batch` with an identical body: + +```json +{ "error": "API operation 'bulk' is not allowed on object 'sys_user'", + "code": "OBJECT_API_METHOD_NOT_ALLOWED", + "allowed": ["get","list","update","bulk","aggregate","history","search","import","export"] } +``` + +Every one of those refusals was correct in outcome — `deleteMany` is `bulk ∧ delete`, `createMany` is `bulk ∧ create`, and `updateMany` / `batch`, which need only `bulk`, are still admitted — but the message named the half that PASSED, and the same envelope listed it as allowed. The writeMode-refined `import` had the identical shape: `import` derives from create ∨ update, so `update` alone puts `import` in the effective set while an `insert` import still needs `create`. + +The message now names a conjunct that is genuinely missing: `delete`, `create` or `update` for the cases above, and still `bulk` when the `bulk` primitive itself is what the object withholds. Three requests that previously produced one indistinguishable envelope are now told apart. + +**`allowed` is unchanged, in contents and in meaning** — it is still the object's declared effective operation set, not the set the gate evaluated against. That matters because the array is read as a discriminator: a declaration re-widened to create/update can still 405 for an unrelated reason, so only the set proves which gate answered. Nothing about which requests are admitted or refused moved; the HTTP status, the `code` and the `object` field are all as before. A client matching on the `error` string for these bulk and import refusals sees the new name. diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 996d2c325f..518c2cc1b1 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -64,7 +64,7 @@ not on any flag. ## How the flag is set `isSystem` is **server-constructed and never client-supplied**. Inbound HTTP -cannot set it (`packages/rest/src/rest-server.ts:1788`, `:1817`), and neither +cannot set it (`packages/rest/src/rest-server.ts:1858`, `:1887`), and neither can an action body (`packages/runtime/src/domains/actions.ts:414`). It is written by internal callers only, as an option on the engine call: @@ -103,7 +103,7 @@ that silently does not happen. | 14 | MCP stdio bridge skips the object API-exposure gate | mcp | Get: the bridge reaches objects whose `apiEnabled` / `apiMethods` would refuse an external caller | `stdio-data-bridge.ts:250` | | 15 | **Read-audit rows are not written** | plugin-audit | Lose: the "a person opened this record" trail. `sudo()` keeps the caller's `userId`, so this flag is the only thing separating a human read from a platform one | `read-audit.ts:556` | | 16 | Approval snapshot payload redaction skipped | plugin-approvals | Get: the whole snapshot on `find` / `findOne` — the audit/replay channel. Lose: field-visibility redaction over approval payloads | `payload-redaction-middleware.ts:115` | -| 17 | REST anonymous-deny seam satisfied | rest | Get: `enforceAuth` passes with no `userId`. Not reachable from the wire — `isSystem` is never set on an inbound request | `rest-server.ts:1820` | +| 17 | REST anonymous-deny seam satisfied | rest | Get: `enforceAuth` passes with no `userId`. Not reachable from the wire — `isSystem` is never set on an inbound request | `rest-server.ts:1890` | ### 2. Write pipeline and data integrity @@ -158,7 +158,7 @@ The largest single consumer — **17 of the 105 sites**. |:--|:---|:---|:---|:---| | 48 | Object API-exposure gate bypassed (`apiEnabled` / `apiMethods`) | runtime | Get: internal self-writes ignore exposure declarations — these govern **external** exposure, not engine self-writes | `action-execution.ts:138` | | 49 | Action `requiredPermissions` bypassed | runtime | Get: engine self-invocation runs any action | `action-execution.ts:401` | -| 50 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability | `domains/meta.ts:471`, `:874`, `rest-server.ts:5450`, `:6907`, `:7155`, `:7586`, `:7779` | +| 50 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability | `domains/meta.ts:471`, `:874`, `rest-server.ts:5520`, `:6977`, `:7225`, `:7656`, `:7849` | | 51 | The shared metadata-write verdict itself returns `allowed` | metadata-core | Get: the one function all of row 50's doors consult answers yes before any capability is examined | `meta-write-capability.ts:134` | | 52 | Anonymous-deny seam satisfied on the domain dispatchers and the package/federation routes | runtime, rest | Get: passes with no `userId` | `domains/actions.ts:421`, `domains/ai.ts:60`, `domains/automation.ts:989`, `domains/meta.ts:232`, `domains/security.ts:78`, `domains/packages.ts:552`, `external-datasource-routes.ts:302`, `package-routes.ts:97` | | 53 | MCP principal check satisfied | runtime | Get: MCP surface reachable with no user | `domains/mcp.ts:61` | @@ -199,7 +199,7 @@ assuming `isSystem` covers it is a documented source of bugs. | "It preserves a supplied `updated_at` / `updated_by`" | **No.** That is `preserveAudit`, a separate opt-in — and an UPDATE-path exemption only | `field.zod.ts:1590` (#3493 / #6640) | | "It stamps `created_by`" | **No.** Audit stamping reads `userId` from the context. A user-less system write stamps nothing — that is today's behaviour, not an error | `runtime-identity.ts:280`–`281` | | "It bypasses every guard" | **No.** The last-admin guard applies to **every** context, `isSystem` included — the deprovision path that actually locks an org out is the system one | `last-admin-guard.ts:299` | -| "A client can request it" | **No.** Never settable from inbound HTTP or from an action body | `rest-server.ts:1788`, `:1817`; `domains/actions.ts:414` | +| "A client can request it" | **No.** Never settable from inbound HTTP or from an action body | `rest-server.ts:1858`, `:1887`; `domains/actions.ts:414` | --- diff --git a/packages/mcp/src/stdio-data-bridge.exposure.test.ts b/packages/mcp/src/stdio-data-bridge.exposure.test.ts index 75f2fcc02a..4d0df700ec 100644 --- a/packages/mcp/src/stdio-data-bridge.exposure.test.ts +++ b/packages/mcp/src/stdio-data-bridge.exposure.test.ts @@ -35,10 +35,21 @@ */ import { describe, it, expect, vi } from 'vitest'; -import { DATA_ACTION_TO_API_OPERATION } from '@objectstack/spec/data'; +import { + API_PRIMITIVES, + DATA_ACTION_TO_API_OPERATION, + effectiveOperationsArray, + isApiOperationAllowed, + resolveEffectiveApiMethods, +} from '@objectstack/spec/data'; import type { ExecutionContext } from '@objectstack/spec/kernel'; import type { IDataEngine, IMetadataService } from '@objectstack/spec/contracts'; -import { createStdioDataBridge, GATED_ACTIONS, type McpExposureError } from './stdio-data-bridge.js'; +import { + createStdioDataBridge, + enforceApiExposure, + GATED_ACTIONS, + type McpExposureError, +} from './stdio-data-bridge.js'; import { assertEngineFindOnePredicate, type EngineFindOneQueryInput } from '@objectstack/metadata-core'; // --------------------------------------------------------------------------- @@ -367,3 +378,119 @@ describe('#8083 the gate runs before the existence probe', () => { }, ); }); + +// --------------------------------------------------------------------------- +// #15416 — the refusal must not name an operation its own set contains +// --------------------------------------------------------------------------- + +/** + * #15416 was filed from the REST door, where `deleteMany` (`bulk ∧ delete`) was + * refused with `API operation 'bulk' is not allowed` beside an `allowed` array + * containing `bulk` — the message named the conjunct that PASSED. The template + * it used lives in exactly two places, and this file guards the second one. + * + * ⭐ Measured, not assumed: stdio CANNOT currently reach that class, and the + * reason is structural rather than lucky. `enforceApiExposure` passes no + * `OperationCheckOptions` — no `bulkChild`, no `writeMode` — and without them + * `isApiOperationAllowed` reduces to membership in the very set that becomes + * `allowedOperations`. Message and set are then two reads of one set and cannot + * disagree. {@link GATED_ACTIONS} also contains neither `bulk` nor `import`, + * the only two operations the spec judges as a conjunction. + * + * So the REST repair was NOT copied here: there is nothing on this surface for + * it to repair. What is added instead is the pin that keeps that true — adding + * `bulk` or a writeMode-refined `import` to the gated set, or threading options + * into the check, reddens the sweep below instead of quietly re-opening #15416 + * from the stdio door. + */ +describe('#15416 the stdio refusal never names an operation it also allows', () => { + /** Every subset of the six primitives — the whole declaration space. */ + const EVERY_WHITELIST: string[][] = Array.from( + { length: 1 << API_PRIMITIVES.length }, + (_, mask) => API_PRIMITIVES.filter((_p, i) => mask & (1 << i)), + ); + + function metadataFor(apiMethods: string[]): IMetadataService { + return { getObject: async () => ({ name: 'task', enable: { apiMethods } }) } as unknown as IMetadataService; + } + + it('holds for every gated action across every whitelist', async () => { + let refusals = 0; + for (const apiMethods of EVERY_WHITELIST) { + for (const action of Object.values(GATED_ACTIONS)) { + let err: McpExposureError | undefined; + try { + await enforceApiExposure(metadataFor(apiMethods), 'task', action, {} as ExecutionContext); + } catch (e) { + err = e as McpExposureError; + } + if (!err) continue; + expect(err.status).toBe(405); + refusals += 1; + const named = /^API operation '([^']*)' is not allowed on object '([^']*)'$/.exec(err.message)?.[1]; + expect(named, `unparseable refusal for ${action} on [${apiMethods}]`).toBeTruthy(); + expect( + err.allowedOperations, + `[${apiMethods}] refused ${action} by naming "${named}", which it also lists as allowed`, + ).not.toContain(named); + } + } + // Vacuously-green guard: a matrix that admitted everything would satisfy + // the assertion above without measuring anything. + expect(refusals).toBeGreaterThan(100); + }); + + it('gates only single-conjunct operations, which is WHY the class is unreachable', () => { + // ⭐ Falsifiable, and deliberately not "verdict === membership": with no + // options passed that identity holds for EVERY operation word, so asserting + // it would be a phantom check that no drift could ever break. The property + // that actually protects this surface is narrower — the gated set must + // contain no operation whose verdict MOVES when options are supplied — and + // it is DERIVED from the spec here rather than hand-copied, so a new + // conjunction-bearing verb is caught the day it is added. + const PROBES = [ + { bulkChild: 'create' }, { bulkChild: 'update' }, { bulkChild: 'delete' }, + { bulkChild: 'upsert' }, { writeMode: 'insert' }, { writeMode: 'update' }, + { writeMode: 'upsert' }, + ]; + const conjunctionRefined = new Set(); + for (const apiMethods of EVERY_WHITELIST) { + const eff = resolveEffectiveApiMethods({ apiMethods }); + for (const operation of Object.values(DATA_ACTION_TO_API_OPERATION)) { + const bare = isApiOperationAllowed(eff, operation); + for (const opts of PROBES) { + if (isApiOperationAllowed(eff, operation, opts) !== bare) conjunctionRefined.add(operation); + } + } + } + // The derivation has to have FOUND the shapes #15416 was filed about, or the + // assertion below is measuring an empty set. + expect([...conjunctionRefined].sort()).toEqual(['bulk', 'import']); + + for (const action of Object.values(GATED_ACTIONS)) { + const operation = DATA_ACTION_TO_API_OPERATION[action] ?? action; + expect( + conjunctionRefined.has(operation), + `bridge gates on "${action}" → "${operation}", whose verdict moves under OperationCheckOptions; ` + + 'this surface passes none, so its refusal can now name a conjunct its own allowed set contains', + ).toBe(false); + } + }); + + it('the serialized set is the same one the verdict is read from', () => { + // The half that makes the paragraph above load-bearing rather than lucky: + // for the operations this surface DOES gate, the verdict and the array the + // refusal ships are two reads of one set. + for (const apiMethods of EVERY_WHITELIST) { + const eff = resolveEffectiveApiMethods({ apiMethods }); + const serialized = effectiveOperationsArray(eff) as string[]; + for (const action of Object.values(GATED_ACTIONS)) { + const operation = DATA_ACTION_TO_API_OPERATION[action] ?? action; + expect( + isApiOperationAllowed(eff, operation), + `[${apiMethods}] ${action} → ${operation}: verdict diverged from the serialized set`, + ).toBe(serialized.includes(operation)); + } + } + }); +}); diff --git a/packages/rest/src/rest-method-not-allowed-conjunct.test.ts b/packages/rest/src/rest-method-not-allowed-conjunct.test.ts new file mode 100644 index 0000000000..b2326663a0 --- /dev/null +++ b/packages/rest/src/rest-method-not-allowed-conjunct.test.ts @@ -0,0 +1,226 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #15416 — `OBJECT_API_METHOD_NOT_ALLOWED` named the conjunct that PASSED. + * + * Measured on a walled boot: `sys_user` declares + * `apiMethods: ['get','list','update','bulk']`, and `deleteMany`, `createMany` + * and the cross-object `POST /batch` each answered + * + * {"error":"API operation 'bulk' is not allowed on object 'sys_user'", + * "allowed":[... ,"bulk", ...]} + * + * — a message contradicted by its own envelope. The refusals were CORRECT in + * outcome (`deleteMany` is `bulk ∧ delete`, `createMany` is `bulk ∧ create`, + * and `updateMany` / `batch`, which need only `bulk`, are admitted); it was the + * NAME that pointed at the half that passed. + * + * ## Why a "some 405 happened" test would not do + * + * The `allowed` array is load-bearing as a DISCRIMINATOR, not decoration: a + * declaration re-widened to create/update can still 405 for an unrelated + * reason, so only the set proves WHICH gate answered. Every assertion below + * therefore reads the NAME out of the message and grades it against the same + * envelope's `allowed` array — a test that only counted refusals would have + * been green throughout the defect. + * + * The last block is the one that generalises: it sweeps every whitelist against + * every conjunction-bearing call shape and asserts the envelope's two halves + * can never disagree again, whatever the derivation grows into. + */ + +import { describe, it, expect } from 'vitest'; +import { API_PRIMITIVES, resolveEffectiveApiMethods, effectiveOperationsArray } from '@objectstack/spec/data'; +import { apiAccessDenialFromEnable } from './rest-server'; + +/** The operation the refusal NAMES, read back out of the wire message. */ +function namedOperation(body: Record): string | undefined { + return /^API operation '([^']*)' is not allowed on object '([^']*)'$/.exec(String(body.error))?.[1]; +} + +/** A 405 body, asserted to be one — a 404/null here is a different gate answering. */ +function refusal(enable: any, op: string, opts?: any): Record { + const d = apiAccessDenialFromEnable(enable, 'sys_user', op, opts); + expect(d, `expected ${op} to be refused`).not.toBeNull(); + expect(d!.status).toBe(405); + return d!.body; +} + +/** The card's own declaration, verbatim. */ +const SYS_USER = { apiMethods: ['get', 'list', 'update', 'bulk'] }; + +describe('#15416 the refusal names the conjunct that FAILED (bulk ∧ child)', () => { + it('deleteMany names `delete`, not the `bulk` its own set contains', () => { + const body = refusal(SYS_USER, 'bulk', { bulkChild: 'delete' }); + expect(namedOperation(body)).toBe('delete'); + // Both halves of the repair, stated separately: the name is now absent from + // the set, and the set still means "what the object declares" (option 2 — + // redefining `allowed` — was explicitly not taken, so `bulk` stays in it). + expect(body.allowed).not.toContain('delete'); + expect(body.allowed).toContain('bulk'); + expect(body.code).toBe('OBJECT_API_METHOD_NOT_ALLOWED'); + expect(body.object).toBe('sys_user'); + }); + + it('createMany names `create`', () => { + const body = refusal(SYS_USER, 'bulk', { bulkChild: 'create' }); + expect(namedOperation(body)).toBe('create'); + expect(body.allowed).not.toContain('create'); + expect(body.allowed).toContain('bulk'); + }); + + it('the cross-object batch route names the child op it was refused for', () => { + // `POST /api/v1/batch` gates each op as `bulk ∧ child(op.action)`; the card + // measured all three shapes collapsing onto one indistinguishable envelope. + expect(namedOperation(refusal(SYS_USER, 'bulk', { bulkChild: 'delete' }))).toBe('delete'); + expect(namedOperation(refusal(SYS_USER, 'bulk', { bulkChild: 'create' }))).toBe('create'); + }); + + it('still names `bulk` when `bulk` is the half that actually failed', () => { + // The mirror case, and the reason this is not "always name the child": + // here the child is granted and the bulk primitive is not. + const body = refusal({ apiMethods: ['create'] }, 'bulk', { bulkChild: 'create' }); + expect(namedOperation(body)).toBe('bulk'); + expect(body.allowed).not.toContain('bulk'); + }); + + it('names a missing write when the batched child is itself a conjunction', () => { + // `upsert` is create ∧ update; granting bulk + update leaves `create`. + const body = refusal({ apiMethods: ['bulk', 'update'] }, 'bulk', { bulkChild: 'upsert' }); + expect(namedOperation(body)).toBe('create'); + expect(body.allowed).not.toContain('create'); + }); + + it('leaves the admitted bulk shapes admitted — no decision moved', () => { + expect(apiAccessDenialFromEnable(SYS_USER, 'sys_user', 'bulk', { bulkChild: 'update' })).toBeNull(); + expect(apiAccessDenialFromEnable(SYS_USER, 'sys_user', 'bulk')).toBeNull(); + }); +}); + +describe('#15416 the same repair on the writeMode-refined import', () => { + it('an `insert` import names `create`, not the `import` its set contains', () => { + // The card's fourth measurement: `import` derives from create ∨ update, so + // `update` alone puts `import` IN the effective set while an insert-mode + // import still needs `create`. + const body = refusal(SYS_USER, 'import', { writeMode: 'insert' }); + expect(namedOperation(body)).toBe('create'); + expect(body.allowed).toContain('import'); + expect(body.allowed).not.toContain('create'); + }); + + it('an `update` import names `update`', () => { + const body = refusal({ apiMethods: ['get', 'list', 'create'] }, 'import', { writeMode: 'update' }); + expect(namedOperation(body)).toBe('update'); + expect(body.allowed).toContain('import'); + expect(body.allowed).not.toContain('update'); + }); + + it('an `upsert` import names the write it is missing', () => { + expect(namedOperation(refusal(SYS_USER, 'import', { writeMode: 'upsert' }))).toBe('create'); + expect(namedOperation(refusal({ apiMethods: ['create'] }, 'import', { writeMode: 'upsert' }))).toBe('update'); + }); +}); + +describe('#15416 refusals that were never self-contradicting are left alone', () => { + it('a plain primitive miss still names the primitive asked for', () => { + const body = refusal({ apiMethods: ['get', 'list'] }, 'delete'); + expect(namedOperation(body)).toBe('delete'); + }); + + it('deny-all names the operation requested', () => { + const body = refusal({ apiMethods: [] }, 'bulk', { bulkChild: 'create' }); + expect(namedOperation(body)).toBe('bulk'); + expect(body.allowed).toEqual([]); + }); + + it('a flag-gated derived verb still names itself', () => { + // `search` needs `list` AND `searchable`; the flag is not a primitive, so + // there is no conjunct to name and `search` is already absent from the set. + const body = refusal({ apiMethods: ['get', 'list'], searchable: false }, 'search'); + expect(namedOperation(body)).toBe('search'); + expect(body.allowed).not.toContain('search'); + }); + + it('the 404 arm is untouched', () => { + const d = apiAccessDenialFromEnable({ apiEnabled: false }, 'sys_user', 'bulk', { bulkChild: 'delete' }); + expect(d?.status).toBe(404); + expect(d?.body.code).toBe('OBJECT_API_DISABLED'); + }); +}); + +describe('#15416 the envelope halves cannot disagree, over every whitelist', () => { + /** Every subset of the six primitives — the whole declaration space. */ + function everyWhitelist(): string[][] { + const out: string[][] = []; + for (let mask = 0; mask < 1 << API_PRIMITIVES.length; mask += 1) { + out.push(API_PRIMITIVES.filter((_, i) => mask & (1 << i))); + } + return out; + } + + const CALLS: Array<[string, any]> = [ + ['bulk', { bulkChild: 'create' }], + ['bulk', { bulkChild: 'update' }], + ['bulk', { bulkChild: 'delete' }], + ['bulk', { bulkChild: 'upsert' }], + ['bulk', undefined], + ['import', { writeMode: 'insert' }], + ['import', { writeMode: 'update' }], + ['import', { writeMode: 'upsert' }], + ['import', undefined], + ['upsert', undefined], + ['export', undefined], + ['aggregate', undefined], + ['get', undefined], + ['create', undefined], + ['delete', undefined], + ]; + + it('never names an operation the same envelope lists as allowed', () => { + let refusals = 0; + for (const apiMethods of everyWhitelist()) { + for (const [op, opts] of CALLS) { + const d = apiAccessDenialFromEnable({ apiMethods }, 'sys_user', op, opts); + if (!d || d.status !== 405) continue; + refusals += 1; + const named = namedOperation(d.body); + expect(named, `unparseable refusal for ${op} on [${apiMethods}]`).toBeTruthy(); + expect( + d.body.allowed, + `[${apiMethods}] refused ${op} ${JSON.stringify(opts ?? {})} by naming "${named}", which it also lists as allowed`, + ).not.toContain(named); + } + } + // The sweep is only evidence if it actually refused things: a matrix that + // admitted everything would pass the assertion above vacuously. + expect(refusals).toBeGreaterThan(100); + }); + + it('names something the declaration could have granted — never a fiction', () => { + // The name must be a real operation word, not an invented one: it is either + // the operation asked for or a primitive the object did not declare. + for (const apiMethods of everyWhitelist()) { + for (const [op, opts] of CALLS) { + const d = apiAccessDenialFromEnable({ apiMethods }, 'sys_user', op, opts); + if (!d || d.status !== 405) continue; + const named = namedOperation(d.body)!; + const isPrimitive = (API_PRIMITIVES as readonly string[]).includes(named); + expect( + isPrimitive || named === op, + `[${apiMethods}] refused ${op} by naming "${named}", which is neither the operation asked for nor a primitive`, + ).toBe(true); + } + } + }); + + it('the `allowed` array itself is unchanged by this card', () => { + // Option 2 (redefining `allowed` to be the set the gate evaluated against) + // was NOT taken: the set is still the object's declared effective closure, + // computed the way every other consumer computes it. + for (const apiMethods of everyWhitelist()) { + const d = apiAccessDenialFromEnable({ apiMethods }, 'sys_user', 'bulk', { bulkChild: 'delete' }); + if (!d || d.status !== 405) continue; + expect(d.body.allowed).toEqual(effectiveOperationsArray(resolveEffectiveApiMethods({ apiMethods }))); + } + }); +}); diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index 65afcc4c90..d2957e407c 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -153,6 +153,8 @@ import { resolveEffectiveApiMethods, effectiveOperationsArray, apiExposureDenialReason, + isApiOperationAllowed, + API_PRIMITIVES, DATA_ACTION_TO_API_OPERATION, } from '@objectstack/spec/data'; // [#8013] The SHARED envelope writer (#3973), aliased. [#9098] The alias no @@ -402,6 +404,68 @@ interface ApiAccessOpts { */ type NavServabilityGate = (objectName: string, entry: any, appName: string) => boolean; +/** + * [#15416] The operation to NAME in a `method-not-allowed` refusal. + * + * The message and the `allowed` array are ONE envelope and have to agree. They + * stopped agreeing wherever the gate judged a CONJUNCTION: `deleteMany` is + * `bulk ∧ delete`, and an `insert` import is `import` refined to `create`. An + * object granting `bulk` and `update` but not `delete` was therefore refused + * with `API operation 'bulk' is not allowed` beside an `allowed` array that + * CONTAINS `bulk` — the refusal named the conjunct that PASSED. + * + * That is worse than a vague message, because the envelope is read as a + * DISCRIMINATOR rather than as decoration: a declaration re-widened to + * create/update can still 405 for an unrelated reason, so only the set proves + * WHICH gate answered. A set contradicting its own message is specific and + * wrong, and a later reader concludes either that the verb is still open or + * that the gate closed the composite — both false. + * + * ## Why this probes instead of re-spelling the derivation + * + * The failing conjunct is found by asking the spec's OWN decision function, + * never by copying its table here: widen the declared whitelist by the missing + * primitives — smallest combination first — and the widening that clears the + * refusal names what the refusal was about. `isApiOperationAllowed` stays the + * single source of truth, so a future refinement (another `writeMode`, another + * derived verb) is tracked with no second spelling to drift away from it. The + * search is monotone and runs only on the 405 path, over six primitives. + * + * Returns `undefined` when the name is already truthful — the operation as + * named is absent from the effective set, so there is no contradiction and the + * existing message is left exactly as it was. + */ +function deniedConjunctName( + enable: any, + eff: ReturnType, + canonical: string, + opts?: ApiAccessOpts, +): string | undefined { + // The contradiction IS the trigger: only rewrite a name the envelope's own + // `allowed` array contradicts. + if (!eff.operations.has(canonical as any)) return undefined; + + const granted = API_PRIMITIVES.filter((p) => eff.primitives.has(p)); + const missing = API_PRIMITIVES.filter((p) => !eff.primitives.has(p)); + const clears = (extra: readonly string[]): boolean => + isApiOperationAllowed( + resolveEffectiveApiMethods({ ...(enable ?? {}), apiMethods: [...granted, ...extra] }), + canonical, + opts, + ); + + for (const p of missing) if (clears([p])) return p; + // Two conjuncts can be missing at once (`upsert` needs create AND update). + // Naming either one is truthful and neither is in `allowed`; name the first + // in the enum's own order so the message is deterministic. + for (let i = 0; i < missing.length; i += 1) { + for (let j = i + 1; j < missing.length; j += 1) { + if (clears([missing[i]!, missing[j]!])) return missing[i]; + } + } + return undefined; +} + /** * Pure per-object API-exposure check: given an object's `enable` block, decide * whether `operation` is denied on the *external* REST surface (ADR-0049 / @@ -447,13 +511,19 @@ export function apiAccessDenialFromEnable( }, }; } + // [#15416] Name the conjunct that actually FAILED, not the composite the + // route gated under; `deniedConjunctName` returns `undefined` when the + // requested name is already absent from `allowed` and nothing needs saying + // differently. + const eff = resolveEffectiveApiMethods(enable); + const named = deniedConjunctName(enable, eff, canonical, opts) ?? operation; return { status: 405, body: { - error: `API operation '${operation}' is not allowed on object '${objectName}'`, + error: `API operation '${named}' is not allowed on object '${objectName}'`, code: 'OBJECT_API_METHOD_NOT_ALLOWED', object: objectName, - allowed: effectiveOperationsArray(resolveEffectiveApiMethods(enable)), + allowed: effectiveOperationsArray(eff), }, }; }