From 6ca583a4182f4365c596b4fe232aa768e811d20b Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 7 Sep 2026 05:38:03 +0000 Subject: [PATCH 1/2] fix(rest): name the conjunct that actually failed in OBJECT_API_METHOD_NOT_ALLOWED MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An object declaring `apiMethods: ['get','list','update','bulk']` refused `deleteMany`, `createMany` and each op of a cross-object `POST /batch` with one indistinguishable envelope whose message named `bulk` while its own `allowed` array listed `bulk`. Every refusal was correct in outcome — `deleteMany` is `bulk AND delete`, `createMany` is `bulk AND create` — but the message named the conjunct that PASSED. The writeMode-refined `import` had the same shape: `import` derives from create OR update, so `update` alone puts `import` in the effective set while an `insert` import still needs `create`. The refusal now names a conjunct that is genuinely absent from the same envelope's `allowed` array, and still names `bulk` when the `bulk` primitive is the half the object withholds. The failing conjunct is found by asking the spec's own `isApiOperationAllowed` over a widened whitelist rather than by re-spelling its derivation table in the REST layer, so a future refinement is tracked with no second spelling to drift from it. `allowed` is unchanged in contents and in meaning: it is still the object's declared effective set, not the set the gate evaluated against. That field is read as a discriminator, so redefining it would silently retarget its consumer. No gate decision moved. The stdio MCP surface carries the same message template and needs no repair: `enforceApiExposure` passes no OperationCheckOptions, so its verdict reduces to membership in the very set it serializes, and its gated action set contains neither of the two conjunction-bearing operations. A structural pin now holds that, so re-opening the class from that door reddens instead of shipping. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YFY46JydE1gMxQG1TqBcMZ --- ...thod-not-allowed-names-failing-conjunct.md | 19 ++ content/docs/permissions/system-context.mdx | 8 +- .../src/stdio-data-bridge.exposure.test.ts | 95 +++++++- .../rest-method-not-allowed-conjunct.test.ts | 226 ++++++++++++++++++ packages/rest/src/rest-server.ts | 74 +++++- 5 files changed, 414 insertions(+), 8 deletions(-) create mode 100644 .changeset/rest-method-not-allowed-names-failing-conjunct.md create mode 100644 packages/rest/src/rest-method-not-allowed-conjunct.test.ts 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..bdd8d63557 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,83 @@ 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', () => { + // The structural statement of the paragraph above: with no options passed, + // the gate verdict IS membership in the serialized set. `bulk ∧ child` and + // the writeMode-refined `import` are the two shapes that break that + // identity, and neither is in the gated 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), }, }; } From a6e0736c1ae3d2dd9e1e902dbd951a566971ba7f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 7 Sep 2026 05:40:22 +0000 Subject: [PATCH 2/2] test(mcp): make the stdio single-conjunct pin falsifiable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first spelling asserted that the gate verdict equals membership in the serialized set. That identity holds for EVERY operation word when no options are passed, so no drift could ever break it — a phantom check dressed as the guard that keeps the refusal honest on this surface. It now derives, from the spec itself, the set of operations whose verdict MOVES under OperationCheckOptions — measured as exactly `bulk` and `import` — and asserts the gated action set contains neither. Adding a conjunction-bearing verb to the bridge, or threading options into the check, now reddens here. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YFY46JydE1gMxQG1TqBcMZ --- .../src/stdio-data-bridge.exposure.test.ts | 44 +++++++++++++++++-- 1 file changed, 40 insertions(+), 4 deletions(-) diff --git a/packages/mcp/src/stdio-data-bridge.exposure.test.ts b/packages/mcp/src/stdio-data-bridge.exposure.test.ts index bdd8d63557..4d0df700ec 100644 --- a/packages/mcp/src/stdio-data-bridge.exposure.test.ts +++ b/packages/mcp/src/stdio-data-bridge.exposure.test.ts @@ -441,10 +441,46 @@ describe('#15416 the stdio refusal never names an operation it also allows', () }); it('gates only single-conjunct operations, which is WHY the class is unreachable', () => { - // The structural statement of the paragraph above: with no options passed, - // the gate verdict IS membership in the serialized set. `bulk ∧ child` and - // the writeMode-refined `import` are the two shapes that break that - // identity, and neither is in the gated set. + // ⭐ 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[];