From 6ebb03ad9f90b2400b90c9522cd8e39fd81ecfc4 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Aug 2026 09:30:27 +0000 Subject: [PATCH] fix(runtime): refuse a falsy-body PUT /meta/:type/:name instead of serving it as a read (#8842) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The metadata save branch opened `if (method === 'PUT' && body)`. The `&& body` conjunct was not a guard but a hole: every path inside the block returns (including the terminal 501), so a falsy body fell through to the read `try` below and was answered with the ordinary metadata read. A write verb came back looking like a successful read, and the `manage_metadata` gate — the first thing the save branch does — was skipped entirely for such a request. Reachable from an ordinary client, measured rather than read: the Hono adapter's catch-all builds the body as `await c.req.json().catch(() => ({}))`, whose catch covers a parse failure but not a successful parse of a falsy JSON value. Driven against a real Hono app, payloads of `null`, `false`, `0` and `""` all arrive falsy; only unparseable input lands on the `{}` fallback. The branch now keys off the method alone and folds a nullish body to `{}`, matching what packages/rest's `PUT /meta/:type/:name` already does (`req.body ?? {}`), so the per-type schema refuses downstream with 422 INVALID_METADATA. Two doors onto one saveMetaItem disagreeing about what a bodyless metadata write means was the defect; the fix gives them one answer. Pinned in both directions: the refusal (422 for an authorized caller, 403 PERMISSION_DENIED for one lacking the capability, and the read spy proving no read was served) and the over-refusal guard (a PUT carrying a body still saves, body reaching the writer verbatim; a GET is still a read). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NaS1PAHJcPfAA2acnV53Tn --- .../dispatcher-meta-put-falsy-body-refused.md | 46 ++++ .../src/domains/meta-put-falsy-body.test.ts | 238 ++++++++++++++++++ packages/runtime/src/domains/meta.ts | 35 ++- 3 files changed, 316 insertions(+), 3 deletions(-) create mode 100644 .changeset/dispatcher-meta-put-falsy-body-refused.md create mode 100644 packages/runtime/src/domains/meta-put-falsy-body.test.ts diff --git a/.changeset/dispatcher-meta-put-falsy-body-refused.md b/.changeset/dispatcher-meta-put-falsy-body-refused.md new file mode 100644 index 0000000000..70923bc548 --- /dev/null +++ b/.changeset/dispatcher-meta-put-falsy-body-refused.md @@ -0,0 +1,46 @@ +--- +"@objectstack/runtime": patch +--- + +fix(runtime): a `PUT /meta/:type/:name` with a falsy body is refused instead of being answered as a READ (#8842) + +The http-dispatcher's metadata save branch opened `if (method === 'PUT' && body)`. +The `&& body` conjunct was not a guard — it was a hole. Every path inside that +block returns (including the terminal `501`), so a falsy body did not merely skip +the write: execution continued past the whole save block into the read `try` +below, which resolved the type and answered the ordinary metadata **read**. + +A caller who asked to write received what looks like a successful read. No +status, header or field distinguished it from a real write acknowledgement — +the shape "Absence must be loud" exists to prevent. The `manage_metadata` +capability gate, which is the first thing the save branch does, was skipped +entirely for such a request as well. (Not an escalation: the request was answered +by the read path, which runs the same ADR-0106 mask a plain `GET` runs, and +nothing was written. Skipping a write gate on a request that performs no write +grants nothing — the defect is the lie, not a privilege.) + +**Reachable from an ordinary client, measured rather than read.** The host that +mounts this dispatcher path is the Hono adapter's catch-all, which builds the +body as `await c.req.json().catch(() => ({}))`. That `.catch` covers a parse +*failure* — an empty body or garbage lands on `{}` — but not a *successful* +parse of a falsy JSON value. Driven against a real Hono app, a `PUT` with +`content-type: application/json` and a payload of `null`, `false`, `0` or `""` +each arrive at the dispatcher falsy. + +**The fix matches the sibling transport rather than inventing a second answer.** +`packages/rest`'s `PUT /meta/:type/:name` already folds `req.body ?? {}` and +proceeds into the save unconditionally, so its bodyless writes are refused +downstream by the per-type schema with `422 INVALID_METADATA`. The dispatcher now +does the same: the branch keys off the method alone, and a nullish body folds to +`{}`. Two doors onto one `saveMetaItem` disagreeing about what a bodyless +metadata write means was the actual defect. + +What callers see instead of a spurious read: + +- holding `manage_metadata` → `422 INVALID_METADATA` from the per-type schema, + with the structured `issues` the Studio form reads; +- not holding it → `403 PERMISSION_DENIED` from the capability gate, which now + runs on this request at all. + +A `PUT` carrying a real body is untouched — it saves exactly as before, and the +body still reaches the writer verbatim. diff --git a/packages/runtime/src/domains/meta-put-falsy-body.test.ts b/packages/runtime/src/domains/meta-put-falsy-body.test.ts new file mode 100644 index 0000000000..e335160bf6 --- /dev/null +++ b/packages/runtime/src/domains/meta-put-falsy-body.test.ts @@ -0,0 +1,238 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#8842] A `PUT /meta/:type/:name` carrying a FALSY body must be REFUSED — + * never silently answered as a read. + * + * ## The defect this pins + * + * The save branch used to open `if (method === 'PUT' && body)`. Every path + * inside it returns (including the terminal `501`), so a falsy `body` did not + * merely skip the write — it fell through to the read `try` below and was + * answered with the ordinary metadata READ. A write verb came back looking + * like a successful read, with no status, header or field distinguishing it + * from a real write acknowledgement. That is the shape "Absence must be loud" + * exists to prevent (AGENTS.md, Route & surface ownership §3). + * + * ## Reachability — measured, not assumed + * + * The host that mounts this dispatcher path is the Hono adapter's catch-all + * (`packages/adapters/hono/src/index.ts`), which builds the body as: + * + * body = await c.req.json().catch(() => ({})) + * + * The `.catch` covers a parse FAILURE (empty body, garbage) — it does not + * cover a SUCCESSFUL parse of a falsy JSON value. Driven against a real Hono + * app, `PUT` with `content-type: application/json` and a payload of `null`, + * `false`, `0` or `""` each resolve to a falsy `body`; only an unparseable + * payload lands on the `{}` fallback. So the falsy body is reachable from an + * ordinary client, which is what makes this a defect rather than dead code. + * + * ## Why refusing is spelled as "fold to `{}`" + * + * The sibling transport already answers this question: `packages/rest`'s + * `PUT /meta/:type/:name` folds `req.body ?? {}` and proceeds into the save + * unconditionally, so its bodyless writes are refused DOWNSTREAM by the + * per-type schema with `422 INVALID_METADATA`. Matching it makes the two + * transports agree about what a bodyless metadata write means, instead of + * minting a second, bespoke refusal here. + * + * ## What the fake protocol does and does not prove + * + * `saveMetaItem` below is a double that raises the real ADR-0112 envelope + * (422 / `INVALID_METADATA` / `issues`) for a body that is not a usable + * metadata document. It pins THIS DOOR's behaviour: that the write path is + * entered at all, what `item` it is handed, and that the refusal reaches the + * caller intact. It deliberately does not re-prove the protocol's own + * validation — that is `packages/metadata-protocol`'s, measured end to end on + * the REST door. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { HttpDispatcher } from '../http-dispatcher.js'; + +const STORED_SCHEMA = { + name: 'account', + label: 'Account', + fields: { + id: { type: 'text' }, + name: { type: 'text' }, + }, +}; + +const copy = (v: T): T => JSON.parse(JSON.stringify(v)); + +/** The per-type schema's verdict on a body that carries no metadata document. */ +function invalidMetadata(type: string, name: string): Error { + const err = new Error(`[invalid_metadata] ${type}/${name} failed spec validation: : Required`); + (err as any).code = 'INVALID_METADATA'; + (err as any).status = 422; + (err as any).issues = [{ path: '', message: 'Required' }]; + return err; +} + +function boot() { + const stored: Record = { account: copy(STORED_SCHEMA) }; + + const saveMetaItem = vi.fn(async ({ type, name, item }: any) => { + // What the real per-type schema does with an empty / non-document body. + if (!item || typeof item !== 'object' || Object.keys(item).length === 0) { + throw invalidMetadata(type, name); + } + stored[name] = copy(item); + return { success: true, name }; + }); + + // The READ the fall-through used to answer. Spying on it is the + // load-bearing half: "was this write served as a read?" is exactly the + // question `getMetaItem` having been called answers. + const getMetaItem = vi.fn(async ({ name }: any) => ({ + type: 'object', + name, + item: copy(stored[name] ?? STORED_SCHEMA), + })); + + const protocol = { saveMetaItem, getMetaItem }; + const kernel = { + context: { getService: (n: string) => (n === 'protocol' ? protocol : null) }, + } as any; + + return { + dispatcher: new HttpDispatcher(kernel), + saveMetaItem, + getMetaItem, + storedLabel: () => stored.account.label, + }; +} + +const ctx = (executionContext: any): any => ({ + request: {}, environmentId: 'platform', executionContext, +}); + +const AUTHOR = { userId: 'u_author', systemPermissions: ['manage_metadata'] }; +const NO_CAPS = { userId: 'u_portal', systemPermissions: [] }; + +/** + * Every payload a client can send that `JSON.parse` accepts and JS calls + * falsy. Each one reached the read branch before this fix. + */ +const FALSY_BODIES: ReadonlyArray = [ + ['null', null], + ['false', false], + ['0', 0], + ['empty string', ''], + ['undefined (no body parsed at all)', undefined], +]; + +describe('#8842 — dispatcher PUT /meta/:type/:name with a falsy body', () => { + describe('is refused, not answered as a read', () => { + it.each(FALSY_BODIES)('%s → 422 INVALID_METADATA, and no read is served', async (_label, falsy) => { + const stack = boot(); + + const res = await stack.dispatcher.handleMetadata( + '/object/account', + ctx(AUTHOR), + 'PUT', + falsy, + ); + + // The ADR-0112 envelope — both halves, not just "it failed". + expect(res.response?.status).toBe(422); + expect(res.response?.body?.error?.code).toBe('INVALID_METADATA'); + + // THE POINT: the request was judged as a WRITE. The read branch + // never ran, so nothing about this answer can be mistaken for the + // successful GET the caller never asked for. + expect(stack.getMetaItem).not.toHaveBeenCalled(); + + // It reached the writer, and the falsy body was folded to `{}` — + // the same normalization the REST door performs. + expect(stack.saveMetaItem).toHaveBeenCalledTimes(1); + expect(stack.saveMetaItem.mock.calls[0][0]).toMatchObject({ + type: 'object', name: 'account', item: {}, + }); + + // Nothing was written. + expect(stack.storedLabel()).toBe('Account'); + }); + + it('the capability gate now runs on a falsy body — it used to be skipped entirely', async () => { + // The gate is the first thing the save branch does. A falsy body + // that never enters the branch is judged only by whatever the read + // path enforces, which is a different posture from the write door's. + const stack = boot(); + + const res = await stack.dispatcher.handleMetadata( + '/object/account', + ctx(NO_CAPS), + 'PUT', + null, + ); + + expect(res.response?.status).toBe(403); + expect(res.response?.body?.error?.code).toBe('PERMISSION_DENIED'); + expect(stack.saveMetaItem).not.toHaveBeenCalled(); + expect(stack.getMetaItem).not.toHaveBeenCalled(); + }); + + it('the compound-name form too — a name in two segments is the same operation', async () => { + const stack = boot(); + + const res = await stack.dispatcher.handleMetadata( + '/lead/views/all_leads', + ctx(AUTHOR), + 'PUT', + null, + ); + + expect(res.response?.status).toBe(422); + expect(res.response?.body?.error?.code).toBe('INVALID_METADATA'); + expect(stack.saveMetaItem.mock.calls[0][0]).toMatchObject({ + type: 'lead', name: 'views/all_leads', item: {}, + }); + }); + }); + + /** + * The over-refusal guard. A pin that only asserted the new refusal would + * be satisfied by a change that broke EVERY metadata write, so these two + * are not optional company for the cases above — they are the half that + * says the fix is narrow. + */ + describe('leaves every real write exactly as it was', () => { + it('a PUT carrying a body still saves', async () => { + const stack = boot(); + + const res = await stack.dispatcher.handleMetadata( + '/object/account', + ctx(AUTHOR), + 'PUT', + { name: 'account', label: 'Account (renamed)', fields: copy(STORED_SCHEMA.fields) }, + ); + + expect(res.response?.status).toBe(200); + expect(stack.saveMetaItem).toHaveBeenCalledTimes(1); + expect(stack.storedLabel()).toBe('Account (renamed)'); + expect(stack.getMetaItem).not.toHaveBeenCalled(); + }); + + it('the body is handed to the writer VERBATIM — the fold touches only falsy bodies', async () => { + const stack = boot(); + const item = { name: 'account', label: 'Kept', fields: copy(STORED_SCHEMA.fields) }; + + await stack.dispatcher.handleMetadata('/object/account', ctx(AUTHOR), 'PUT', item); + + expect(stack.saveMetaItem.mock.calls[0][0].item).toEqual(item); + }); + + it('a GET is still served as a read', async () => { + const stack = boot(); + + const res = await stack.dispatcher.handleMetadata('/object/account', ctx(AUTHOR), 'GET'); + + expect(res.response?.status).toBe(200); + expect(stack.getMetaItem).toHaveBeenCalledTimes(1); + expect(stack.saveMetaItem).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/packages/runtime/src/domains/meta.ts b/packages/runtime/src/domains/meta.ts index 60a53800cd..3345deffef 100644 --- a/packages/runtime/src/domains/meta.ts +++ b/packages/runtime/src/domains/meta.ts @@ -312,7 +312,26 @@ export async function handleMetadataRequest(deps: DomainHandlerDeps, path: strin const packageId = query?.package || undefined; // PUT /metadata/:type/:name (Save) - if (method === 'PUT' && body) { + // + // [#8842] The condition is the METHOD alone. It used to be + // `method === 'PUT' && body`, and the `&& body` was not a guard — it + // was a hole. Every path inside this block returns (including the + // terminal `501`), so a falsy body did not merely skip the write: it + // fell through to the read `try` below and was answered with the + // ordinary metadata READ. A write verb came back looking like a + // successful read, with no status, header or field telling the caller + // their write never happened — the shape "Absence must be loud" exists + // to prevent (AGENTS.md, Route & surface ownership §3). It also meant + // the `manage_metadata` gate below, the first thing this block does, + // was skipped entirely for such a request. + // + // Reachable from an ordinary client: the host mounting this path is the + // Hono adapter's catch-all, which builds `body` as + // `await c.req.json().catch(() => ({}))`. The `.catch` covers a parse + // FAILURE (empty body, garbage) — not a SUCCESSFUL parse of a falsy + // JSON value, so a payload of `null`, `false`, `0` or `""` arrives here + // falsy. Driven, not read: see `meta-put-falsy-body.test.ts`. + if (method === 'PUT') { // [#7019] The SECOND TRANSPORT for the operation #6603 gated on the // REST side. Same `protocol.saveMetaItem`, same metadata, different // door — so a gate on only one of them is not a gate, it is a @@ -347,6 +366,16 @@ export async function handleMetadataRequest(deps: DomainHandlerDeps, path: strin }; } + // [#8842] Fold a nullish body to `{}` and let the per-type schema + // refuse it downstream with `422 INVALID_METADATA`, rather than + // minting a second, bespoke refusal here. This is byte-for-byte + // what the sibling transport already does — `packages/rest`'s + // `PUT /meta/:type/:name` opens `const body = req.body ?? {}` and + // proceeds into the save unconditionally. Two doors onto one + // `saveMetaItem` disagreeing about what a bodyless metadata write + // means was the actual defect; one answer, from one authority. + const item = body ?? {}; + // Try to get the protocol service directly const protocol = await deps.resolveService(_context, 'protocol'); @@ -366,7 +395,7 @@ export async function handleMetadataRequest(deps: DomainHandlerDeps, path: strin // static registry flag and not `isOverlayAllowed`. const activeOrganizationId = await deps.resolveActiveOrganizationId(_context); const organizationId = organizationIdForMetaWrite(type, activeOrganizationId); - const result = await protocol.saveMetaItem({ type, name, item: body, organizationId, ...(packageId ? { packageId } : {}) }); + const result = await protocol.saveMetaItem({ type, name, item, organizationId, ...(packageId ? { packageId } : {}) }); return { handled: true, response: deps.success(result) }; } catch (e: any) { // Preserve the 422 + structured spec-validation `issues` so @@ -380,7 +409,7 @@ export async function handleMetadataRequest(deps: DomainHandlerDeps, path: strin const metaSvc = await deps.resolveService(_context, 'metadata', _context.environmentId); if (metaSvc && typeof (metaSvc as any).saveItem === 'function') { try { - const data = await (metaSvc as any).saveItem(type, name, body); + const data = await (metaSvc as any).saveItem(type, name, item); return { handled: true, response: deps.success(data) }; } catch (e: any) { // 501 stays the FALLBACK (this branch is reached only when