From d7a56c1ceb34590424c6ce2d1a4a9a054dd9a3c1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 11:16:26 +0000 Subject: [PATCH] =?UTF-8?q?feat(rest,runtime):=20=E5=85=83=E6=95=B0?= =?UTF-8?q?=E6=8D=AE=E5=86=99=E5=85=A5=E7=9A=84=E5=85=B6=E4=BD=99=E4=B8=89?= =?UTF-8?q?=E6=89=87=E9=97=A8=E5=90=8C=E6=A0=B7=E8=A6=81=E6=B1=82=20manage?= =?UTF-8?q?=5Fmetadata=20(#7019)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #6603 只给 PUT /api/v1/meta/:type/:name 落了 manage_metadata 门,同一个写 操作还有三扇门没有门。本次补齐,各自照抄所在文件的既有先例,不引入新写法: - PUT /api/v1/meta/:type/:section/:name(复合名保存) - DELETE /api/v1/meta/:type/:name(重置为构件默认值) - 运行时 dispatcher 自己的 /meta PUT(同一操作的第二条传输) 三处门都落在解析 protocol 之前,使未授权调用方无法用 501-vs-200 指纹探测 内核能力,且拒绝时什么都没写、什么都没删。isSystem 照例旁路。 复合名那扇门的前后实测(同一往返、真实内存库): 加门前 PUT 200 / saveMetaItem 1 次 / 库中只剩 id,name 加门后 PUT 403 / saveMetaItem 0 次 / 库中四个字段完好 DELETE 那扇门是另一条理由:不往返、不掩码,只是把定制覆盖层整个丢掉。 连带修改 3 个既有测试文件的 boot 桩(补 manage_metadata):它们的桩把 「只要有 session 就能写元数据」当作既成前提,而这正是本次要消灭的东西。 被测机制、断言、期望值一字未动。 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017uFVNMmTxLpmfQYiuKM1Yx --- .../meta-write-doors-manage-metadata-gate.md | 51 +++ ...und-save-and-reset-capability-gate.test.ts | 331 ++++++++++++++++++ packages/rest/src/rest-route-ledger.ts | 4 +- packages/rest/src/rest-server.ts | 70 ++++ .../src/domains/error-passthrough.test.ts | 13 +- .../domains/meta-save-capability-gate.test.ts | 240 +++++++++++++ packages/runtime/src/domains/meta.ts | 34 ++ .../src/http-dispatcher.error-leak.test.ts | 15 +- packages/runtime/src/http-dispatcher.test.ts | 33 +- 9 files changed, 779 insertions(+), 12 deletions(-) create mode 100644 .changeset/meta-write-doors-manage-metadata-gate.md create mode 100644 packages/rest/src/meta-compound-save-and-reset-capability-gate.test.ts create mode 100644 packages/runtime/src/domains/meta-save-capability-gate.test.ts diff --git a/.changeset/meta-write-doors-manage-metadata-gate.md b/.changeset/meta-write-doors-manage-metadata-gate.md new file mode 100644 index 0000000000..f1f77e1aa8 --- /dev/null +++ b/.changeset/meta-write-doors-manage-metadata-gate.md @@ -0,0 +1,51 @@ +--- +"@objectstack/rest": minor +"@objectstack/runtime": minor +--- + +feat(rest,runtime): 元数据写入的其余三扇门同样要求 `manage_metadata` 能力 (#7019) + +**这是一次访问面收紧,线上可见。** #6603 只给 `PUT /api/v1/meta/:type/:name` +一条路由落了 `manage_metadata` 门,而同一个写操作还有另外三扇门没有门。本次 +把它们补齐,用的是**同一道门、同一套机制**(各自照抄所在文件的既有先例): + +- `PUT /api/v1/meta/:type/:section/:name` —— 复合名保存(`@objectstack/rest`); +- `DELETE /api/v1/meta/:type/:name` —— 重置为构件默认值(`@objectstack/rest`); +- 运行时 dispatcher 自己的 `/meta` PUT —— 同一操作的**第二条传输**(`@objectstack/runtime`)。 + +## 谁开始吃 403,需要什么 + +**任何不持 `manage_metadata` 的已认证调用方**,对上述三条路径的写入一律 403 +(匿名调用方仍先吃 `/meta` 伞下的 401,能力门是第二层)。`isSystem`(引擎自调) +照例放行。平台自带的 `admin_full_access` 权限集本就带 `manage_metadata`,所以 +Studio / Setup 里的管理员与 CLI 的 dev admin **不受影响**;受影响的是自建集成、 +自建权限集,以及只持 `setup.access` 的 `organization_admin`。 + +**要恢复写入:给该调用方的权限集加上 `manage_metadata`**(Setup → +Permission Sets → `systemPermissions`),而不是绕过这些路由。 + +## 为什么必须收紧 + +两条**各自独立成立**的理由: + +1. **ADR-0106 的读写不对称。** D1 会把调用方不可读的字段**整个**从服务出的对象 + schema 里摘掉,而这些路由原样持久化收到的 body。#6603 落地后**实测**:同一次 + GET → 改个 label → PUT 的字段丢失,经复合名这扇门可原样复现 —— 缺陷没有被修复, + 只是换了一扇门。本次复测的前后对照: + + ``` + 加门前: compound PUT status : 200 | saveMetaItem calls : 1 | STORE after PUT : id, name + 加门后: compound PUT status : 403 | saveMetaItem calls : 0 | STORE after PUT : bonus_formula, id, name, salary_grade + ``` + +2. **一个与掩码无关、更早就存在的洞:** 任何已认证会话都能覆写(或重置)任意 + 元数据项。`DELETE` 这条尤其是这个理由而**不是**掩码理由 —— 它不往返、不掩码, + 只是把定制覆盖层整个丢掉,`?dropStorage=true` 还会连对象的物理表一起拆掉。 + +三处门都落在解析 protocol **之前**,所以未授权调用方无法用 501-vs-200 指纹探测 +内核能力,且拒绝时**什么都没写、什么都没删**。 + +## 不在本次范围 + +只收紧写入面;读路径的姿态(ADR-0106 掩码)不变。#7020 记录的「门要求的能力集 +与 D4 掩码豁免集不是同一个集合」仍然成立,本次不替维护者选对齐方向。 diff --git a/packages/rest/src/meta-compound-save-and-reset-capability-gate.test.ts b/packages/rest/src/meta-compound-save-and-reset-capability-gate.test.ts new file mode 100644 index 0000000000..6629cea712 --- /dev/null +++ b/packages/rest/src/meta-compound-save-and-reset-capability-gate.test.ts @@ -0,0 +1,331 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#7019] The two REST metadata-write doors #6603 did not gate: + * + * 1. `PUT /api/v1/meta/:type/:section/:name` — the compound-name save; + * 2. `DELETE /api/v1/meta/:type/:name` — the reset-to-artifact-default. + * + * Both now demand the `manage_metadata` authoring capability (ADR-0066 D1), by + * the same mechanism as the single-name `PUT` next door + * (`meta-item-save-capability-gate.test.ts`) and `POST /meta/_migrate-stored`. + * + * ## Why these two are one file but not one argument + * + * The compound `PUT` is the same defect as #6603's, one route over: it was + * MEASURED that with #6603's gate in place, the identical ADR-0106 + * GET → edit → PUT still round-tripped a masked object schema back into the + * store through this door, deleting the fields the caller was never allowed to + * see. The headline case below drives exactly that sequence. + * + * The `DELETE` is a different argument reaching the same fix. Nothing is masked + * and nothing is round-tripped — it discards a customization overlay outright. + * What was wrong there is simply that an authenticated session holding no + * authoring capability could reset any customized metadata item in the + * deployment (and, with `?dropStorage=true`, drop the object's table with it). + * Keeping the two arguments distinct matters: a reader who takes the masking + * story as the reason for the DELETE gate would conclude, wrongly, that a + * caller with unmasked reads needs no gate. + * + * ## What the refusal cases assert + * + * `status` AND `code` (the ADR-0112 envelope), and — the load-bearing part — + * that the target function was **never entered**, checked against the STORE. + * "Wrote/deleted first, refused second" is precisely the failure worth + * guarding, and it passes any status-only assertion. This route answers by + * *sending* rather than throwing, so a `toThrow`-shaped assertion could not + * separate "refused with the wrong envelope" from "did not refuse at all". + */ + +import { describe, it, expect, vi } from 'vitest'; +import { FLS_CONTRACT_OBJECT } from '@objectstack/metadata-core/testing'; +import { RestServer } from './rest-server'; + +const copy = (value: T): T => JSON.parse(JSON.stringify(value)); + +/** The four fields `FLS_CONTRACT_OBJECT` declares, sorted. */ +const ALL_FIELDS = ['bonus_formula', 'id', 'name', 'salary_grade']; +/** What the security double lets a restricted caller read. */ +const READABLE_TO_RESTRICTED = ['id', 'name']; + +const COMPOUND_PATH = '/api/v1/meta/:type/:section/:name'; +const SINGLE_PATH = '/api/v1/meta/:type/:name'; +/** The compound name the section + name params spell. */ +const COMPOUND_NAME = 'crm/account'; + +function mockServer() { + return { + get: vi.fn(), post: vi.fn(), put: vi.fn(), delete: vi.fn(), patch: vi.fn(), + use: vi.fn(), listen: vi.fn().mockResolvedValue(undefined), close: vi.fn().mockResolvedValue(undefined), + }; +} + +function mockRes() { + const res: any = { + statusCode: 200, + json: vi.fn(function (this: any, body: any) { this._body = body; return this; }), + send: vi.fn(), + status: vi.fn(function (this: any, code: number) { this.statusCode = code; return this; }), + header: vi.fn(), + }; + return res; +} + +interface BootOptions { + /** The caller, as `resolveExecCtx` resolves it. `undefined` = anonymous. */ + context: Record | undefined; + /** What `security.getMetadataReadableFields` answers; omit for no security service. */ + readable?: readonly string[]; + /** Drop `saveMetaItem` / `deleteMetaItem` from the protocol (the 501 kernel). */ + withoutWriters?: boolean; +} + +/** + * Boot both routes over a protocol backed by a REAL in-memory store, so the + * compound GET → edit → PUT sequence actually round-trips, and so a refused + * DELETE can be checked against the store rather than only against the answer. + */ +function boot(opts: BootOptions) { + const stored: Record = { + [COMPOUND_NAME]: copy(FLS_CONTRACT_OBJECT as unknown as Record), + account: copy(FLS_CONTRACT_OBJECT as unknown as Record), + }; + /** The customization overlay rows `DELETE` exists to remove (ADR-0005). */ + const overlays = new Set(['account']); + + const saveMetaItem = vi.fn(async ({ name, item }: any) => { + stored[name] = copy(item); + return { success: true, type: 'object', name }; + }); + + const deleteMetaItem = vi.fn(async ({ name }: any) => { + overlays.delete(name); + delete stored[name]; + return { success: true, reset: true }; + }); + + const protocol: any = { + getDiscovery: vi.fn().mockResolvedValue({ version: 'v0', routes: { data: '', metadata: '', ui: '', auth: '/auth' } }), + getMetaTypes: vi.fn().mockResolvedValue([]), + getMetaItems: vi.fn(async () => Object.values(stored).map(copy)), + getMetaItem: vi.fn(async ({ type, name }: any) => ({ type, name, item: copy(stored[name]), lock: 'none' })), + findData: vi.fn().mockResolvedValue([]), + getData: vi.fn().mockResolvedValue({}), + createData: vi.fn().mockResolvedValue({ id: '1' }), + updateData: vi.fn().mockResolvedValue({}), + deleteData: vi.fn().mockResolvedValue({ success: true }), + }; + if (!opts.withoutWriters) { + protocol.saveMetaItem = saveMetaItem; + protocol.deleteMetaItem = deleteMetaItem; + } + + const security = opts.readable === undefined ? undefined : { + getReadableFields: async () => [...opts.readable!], + getMetadataReadableFields: async () => [...opts.readable!], + }; + + const rest = new RestServer( + mockServer() as any, + protocol as any, + { api: { requireAuth: false } } as any, + undefined, undefined, undefined, undefined, undefined, undefined, undefined, + undefined, undefined, undefined, undefined, undefined, undefined, undefined, + security ? (async () => security as any) : undefined, + ); + (rest as any).resolveExecCtx = async () => opts.context; + rest.registerRoutes(); + + const route = (method: string, path: string) => (rest as any).getRoutes().find( + (r: any) => r.method === method && r.path === path, + ); + + return { + saveMetaItem, + deleteMetaItem, + /** Field names of the COMPOUND-named document in the store. */ + compoundFields: () => Object.keys(stored[COMPOUND_NAME].fields ?? {}).sort(), + compoundLabel: () => stored[COMPOUND_NAME].label, + /** Whether the single-name item's customization overlay still exists. */ + hasOverlay: () => overlays.has('account'), + + compoundGet: async () => { + const res = mockRes(); + await route('GET', COMPOUND_PATH)!.handler( + { params: { type: 'object', section: 'crm', name: 'account' }, query: {}, headers: {} }, res, + ); + return { res, body: res.json.mock.calls.at(-1)?.[0] }; + }, + compoundPut: async (item: unknown) => { + const res = mockRes(); + await route('PUT', COMPOUND_PATH)!.handler( + { params: { type: 'object', section: 'crm', name: 'account' }, query: {}, headers: {}, body: item }, res, + ); + return { res, body: res.json.mock.calls.at(-1)?.[0] }; + }, + del: async (query: Record = {}) => { + const res = mockRes(); + await route('DELETE', SINGLE_PATH)!.handler( + { params: { type: 'object', name: 'account' }, query, headers: {} }, res, + ); + return { res, body: res.json.mock.calls.at(-1)?.[0] }; + }, + }; +} + +describe('#7019 — compound-name PUT: the ADR-0106 round trip, one route over', () => { + it('refuses the restricted round-trip write, and the masked fields SURVIVE in the store', async () => { + const stack = boot({ + context: { userId: 'u_portal', systemPermissions: [] }, + readable: READABLE_TO_RESTRICTED, + }); + + // 1. The compound read is masked — the premise, asserted rather than + // assumed so this case cannot go quietly green if masking stops. + const read = await stack.compoundGet(); + expect(Object.keys(read.body.item.fields).sort()).toEqual(READABLE_TO_RESTRICTED); + expect(read.body.item.fields).not.toHaveProperty('salary_grade'); + expect(read.body.item.fields).not.toHaveProperty('bonus_formula'); + + // 2. The caller edits something unrelated and sends the body back — + // the exact sequence that was MEASURED to still lose fields here + // after #6603 gated the single-name door. + const write = await stack.compoundPut({ ...copy(read.body.item), label: 'Account (renamed)' }); + + // 3. Refused, with the envelope. + expect(write.res.statusCode).toBe(403); + expect(write.body).toMatchObject({ error: { code: 'FORBIDDEN' } }); + + // 4. THE POINT: nothing was written. + expect(stack.saveMetaItem).not.toHaveBeenCalled(); + expect(stack.compoundFields()).toEqual(ALL_FIELDS); + expect(stack.compoundLabel()).toBe('Account'); + }); + + it('the refusal is the gate, not the masking: an UNRESTRICTED but uncapable caller is refused too', async () => { + // Everything readable ⇒ no field would have been lost. Still refused, + // because the second reason — any authenticated session could clobber + // any metadata item — is independent of ADR-0106. + const stack = boot({ context: { userId: 'u_staff', systemPermissions: [] }, readable: ALL_FIELDS }); + + const read = await stack.compoundGet(); + expect(Object.keys(read.body.item.fields).sort()).toEqual(ALL_FIELDS); + + const write = await stack.compoundPut({ ...copy(read.body.item), label: 'clobbered' }); + expect(write.res.statusCode).toBe(403); + expect(write.body).toMatchObject({ error: { code: 'FORBIDDEN' } }); + expect(stack.compoundLabel()).toBe('Account'); + }); + + it('fires BEFORE the protocol is probed, so 403-vs-501 leaks no kernel capability', async () => { + const stack = boot({ context: { userId: 'u1', systemPermissions: [] }, withoutWriters: true }); + const write = await stack.compoundPut({ name: 'account' }); + // An authorized caller would get 501 here. + expect(write.res.statusCode).toBe(403); + expect(write.body).toMatchObject({ error: { code: 'FORBIDDEN' } }); + }); + + it('an anonymous caller never reaches the capability gate — 401 from the /meta umbrella', async () => { + const stack = boot({ context: undefined }); + const write = await stack.compoundPut({ name: 'account' }); + expect(write.res.statusCode).toBe(401); + expect(stack.saveMetaItem).not.toHaveBeenCalled(); + }); + + it.each([ + { held: 'no capabilities at all', systemPermissions: [] as string[], status: 403 }, + { held: '`studio.access` alone — ADR-0106 D4-exempt, but not an authoring capability', systemPermissions: ['studio.access'], status: 403 }, + { held: '`setup.access` alone — this is `organization_admin`', systemPermissions: ['setup.access'], status: 403 }, + { held: '`manage_metadata` alone', systemPermissions: ['manage_metadata'], status: 200 }, + { held: 'the shipped `admin_full_access` shape', systemPermissions: ['manage_metadata', 'studio.access', 'setup.access'], status: 200 }, + ])('$held → $status', async ({ systemPermissions, status }) => { + const stack = boot({ context: { userId: 'u1', systemPermissions } }); + const write = await stack.compoundPut({ name: 'account', label: 'Account', fields: {} }); + expect(write.res.statusCode).toBe(status); + }); + + it('`isSystem` bypasses, matching every other capability gate on the platform', async () => { + const stack = boot({ context: { isSystem: true } }); + const write = await stack.compoundPut({ name: 'account', label: 'Account', fields: {} }); + expect(write.res.statusCode).toBe(200); + expect(stack.saveMetaItem).toHaveBeenCalledTimes(1); + }); + + it('leaves the compound READ alone — this card gates writes', async () => { + // The read side has its own posture (the ADR-0106 projection asserted + // in the headline case). Turning this into a blanket gate on the + // compound route pair would be a different, unruled change. + const stack = boot({ context: { userId: 'u_portal', systemPermissions: [] }, readable: READABLE_TO_RESTRICTED }); + const read = await stack.compoundGet(); + expect(read.res.statusCode).not.toBe(403); + expect(Object.keys(read.body.item.fields).sort()).toEqual(READABLE_TO_RESTRICTED); + }); +}); + +describe('#7019 — DELETE /meta/:type/:name: an ungated reset, not a round trip', () => { + it('refuses an uncapable caller, and the overlay is STILL THERE', async () => { + const stack = boot({ context: { userId: 'u_portal', systemPermissions: [] } }); + + const res = await stack.del(); + + expect(res.res.statusCode).toBe(403); + expect(res.body).toMatchObject({ error: { code: 'FORBIDDEN' } }); + // THE POINT, and the reason this is not a status-only assertion: + // "deleted first, refused second" is the failure mode worth guarding, + // and it answers 403 too. + expect(stack.deleteMetaItem).not.toHaveBeenCalled(); + expect(stack.hasOverlay()).toBe(true); + }); + + it('refuses the DESTRUCTIVE `?dropStorage=true` form with the same gate', async () => { + // This variant also tears down the object's physical table, so an + // ungated door here discards data, not just customization. + const stack = boot({ context: { userId: 'u_portal', systemPermissions: [] } }); + + const res = await stack.del({ dropStorage: 'true' }); + + expect(res.res.statusCode).toBe(403); + expect(res.body).toMatchObject({ error: { code: 'FORBIDDEN' } }); + expect(stack.deleteMetaItem).not.toHaveBeenCalled(); + expect(stack.hasOverlay()).toBe(true); + }); + + it('fires BEFORE the protocol is probed, so 403-vs-501 leaks no kernel capability', async () => { + const stack = boot({ context: { userId: 'u1', systemPermissions: [] }, withoutWriters: true }); + const res = await stack.del(); + expect(res.res.statusCode).toBe(403); + expect(res.body).toMatchObject({ error: { code: 'FORBIDDEN' } }); + }); + + it('an anonymous caller never reaches the capability gate — 401 from the /meta umbrella', async () => { + const stack = boot({ context: undefined }); + const res = await stack.del(); + expect(res.res.statusCode).toBe(401); + expect(stack.deleteMetaItem).not.toHaveBeenCalled(); + }); + + it('allows a caller holding `manage_metadata` — the overlay is removed', async () => { + const stack = boot({ context: { userId: 'u_author', systemPermissions: ['manage_metadata'] } }); + + const res = await stack.del(); + + expect(res.res.statusCode).toBe(200); + expect(stack.deleteMetaItem).toHaveBeenCalledTimes(1); + expect(stack.hasOverlay()).toBe(false); + }); + + it('`isSystem` bypasses, matching every other capability gate on the platform', async () => { + const stack = boot({ context: { isSystem: true } }); + const res = await stack.del(); + expect(res.res.statusCode).toBe(200); + expect(stack.deleteMetaItem).toHaveBeenCalledTimes(1); + }); + + it('holding an unrelated capability is not enough', async () => { + const stack = boot({ context: { userId: 'u_admin', systemPermissions: ['setup.access', 'studio.access'] } }); + const res = await stack.del(); + expect(res.res.statusCode).toBe(403); + expect(stack.deleteMetaItem).not.toHaveBeenCalled(); + expect(stack.hasOverlay()).toBe(true); + }); +}); diff --git a/packages/rest/src/rest-route-ledger.ts b/packages/rest/src/rest-route-ledger.ts index d5c40d9819..e1bbb1d1c6 100644 --- a/packages/rest/src/rest-route-ledger.ts +++ b/packages/rest/src/rest-route-ledger.ts @@ -160,7 +160,7 @@ export const REST_ROUTE_LEDGER: readonly RestRouteLedgerEntry[] = [ { route: 'PUT /api/v1/meta/:type/:name', family: 'metadata', source: 'route-manager', disposition: 'sdk', client: 'meta.saveItem', note: '[#6603] gated on `manage_metadata` (ADR-0066 D1), same mechanism as POST /meta/_migrate-stored — a session alone is no longer enough. The write-side answer to ADR-0106 D1: a masked read PUT back verbatim used to delete the fields the caller could not see' }, { route: 'DELETE /api/v1/meta/:type/:name', family: 'metadata', source: 'route-manager', disposition: 'sdk', client: 'meta.deleteItem', - note: 'REST-only: the dispatcher /meta branch has no DELETE handling — it falls into the read path' }, + note: 'REST-only: the dispatcher /meta branch has no DELETE handling — it falls into the read path. [#7019] gated on `manage_metadata` (ADR-0066 D1), same mechanism as the PUT twins — but NOT for the ADR-0106 reason: nothing is masked or round-tripped here, this discards a customization overlay outright, and `?dropStorage=true` takes the object table with it' }, { route: 'GET /api/v1/meta/:type/:name/history', family: 'metadata', source: 'route-manager', disposition: 'sdk', client: 'meta.getHistory', note: 'REST-only: the dispatcher /meta branch swallows /history as a compound name and 404s' }, { route: 'GET /api/v1/meta/:type/:name/audit', family: 'metadata', source: 'route-manager', disposition: 'sdk', client: 'meta.getAudit' }, @@ -171,7 +171,7 @@ export const REST_ROUTE_LEDGER: readonly RestRouteLedgerEntry[] = [ { route: 'GET /api/v1/meta/:type/:section/:name', family: 'metadata', source: 'route-manager', disposition: 'sdk', client: 'meta.getItem', note: 'compound names pass through getItem unencoded (URL-pinned in client.test.ts); only deleteItem encodes' }, { route: 'PUT /api/v1/meta/:type/:section/:name', family: 'metadata', source: 'route-manager', disposition: 'sdk', client: 'meta.saveItem', - note: 'compound names pass through saveItem unencoded (URL-pinned in client.test.ts)' }, + note: 'compound names pass through saveItem unencoded (URL-pinned in client.test.ts). [#7019] gated on `manage_metadata` (ADR-0066 D1), identical to the single-name PUT — it was MEASURED that with #6603 in place the same ADR-0106 masked round trip still deleted fields through this door' }, // ── ui ──────────────────────────────────────────────────────────────────── { route: 'GET /api/v1/ui/view/:object/:type', family: 'ui', source: 'route-manager', disposition: 'sdk', client: 'meta.getView', diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index dd3f27f6ed..fc0d71b2c9 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -5252,6 +5252,40 @@ export class RestServer { handler: async (req: any, res: any) => { try { const environmentId = isScoped ? req.params?.environmentId : undefined; + // [#7019] Same gate, same mechanism as the `PUT` twins — + // but the argument for it is NOT the ADR-0106 round trip, + // and saying so matters. Nothing is masked here and nothing + // is round-tripped: this route discards a customization + // overlay outright, so before this gate an authenticated + // session holding no authoring capability at all could + // reset any customized metadata item in the deployment to + // its artifact default — and with `?dropStorage=true`, drop + // the object's physical table with it. + // + // It belongs with the two PUTs because deleting a + // customization is authoring it (ADR-0066 D1), and because + // the fix is the same four lines — not because it is the + // same argument. + // + // Gate FIRST — before the protocol is resolved — so the + // 501-vs-200 answer leaks no kernel capability, and, the + // point here, so the refusal happens with the overlay row + // still intact. A gate that answers 403 after + // `deleteMetaItem` has run would still be the bug. + // `isSystem` bypasses, as everywhere else. + const ctx = await this.resolveExecCtx(environmentId, req).catch(() => undefined); + const held = new Set( + Array.isArray(ctx?.systemPermissions) ? ctx!.systemPermissions : [], + ); + if (!ctx?.isSystem && !held.has('manage_metadata')) { + res.status(403).json({ + error: { + code: 'FORBIDDEN', + message: 'Resetting a metadata item requires the `manage_metadata` capability.', + }, + }); + return; + } const p = await this.resolveProtocol(environmentId, req); if (!(p as any).deleteMetaItem) { res.status(501).json({ @@ -5578,6 +5612,42 @@ export class RestServer { handler: async (req: any, res: any) => { try { const environmentId = isScoped ? req.params?.environmentId : undefined; + // [#7019] The compound-name twin of the gate #6603 put on + // `PUT /meta/:type/:name` — WORD FOR WORD the same + // mechanism, because it is word for word the same + // operation: one generic `saveMetaItem`, reached by a name + // spelled in two segments instead of one. + // + // Gating only the single-segment door left this one as a + // bypass of it, and that was measured rather than reasoned: + // with #6603's gate in place, the identical ADR-0106 + // GET → edit a label → PUT still round-tripped a MASKED + // object schema back into the store through here, deleting + // the fields the caller was never allowed to see. Same + // caller, same object, same loss, one route over. + // + // Independently of masking, this door also served the older + // hole for EVERY metadata type: any authenticated session + // could clobber any metadata item. + // + // Gate FIRST — before the protocol is resolved — so an + // unauthorized caller cannot use the 501-vs-200 answer to + // probe which kernels implement saving, and so nothing is + // written before the refusal. `isSystem` bypasses, matching + // every other capability gate on the platform. + const ctx = await this.resolveExecCtx(environmentId, req).catch(() => undefined); + const held = new Set( + Array.isArray(ctx?.systemPermissions) ? ctx!.systemPermissions : [], + ); + if (!ctx?.isSystem && !held.has('manage_metadata')) { + res.status(403).json({ + error: { + code: 'FORBIDDEN', + message: 'Saving a metadata item requires the `manage_metadata` capability.', + }, + }); + return; + } const p = await this.resolveProtocol(environmentId, req); if (!p.saveMetaItem) { res.status(501).json({ error: 'Save operation not supported by protocol implementation', code: 'NOT_IMPLEMENTED' }); diff --git a/packages/runtime/src/domains/error-passthrough.test.ts b/packages/runtime/src/domains/error-passthrough.test.ts index 07d67f706b..cc26bc46fb 100644 --- a/packages/runtime/src/domains/error-passthrough.test.ts +++ b/packages/runtime/src/domains/error-passthrough.test.ts @@ -141,7 +141,18 @@ describe('#3918 follow-up — deliberate per-route fallbacks are preserved', () getService: resolve, getServiceAsync: async (name: string) => resolve(name), }; - const result: any = await new HttpDispatcher(kernel).dispatch( + const dispatcher = new HttpDispatcher(kernel); + // [#7019] The `/meta` PUT now demands the `manage_metadata` capability — + // an authoring capability, not just a session. `dispatch()` re-resolves + // the execution context from the auth / objectql services, and this stub + // has no objectql, so the resolved caller would hold no capabilities and + // be refused with a 403 before ever reaching the 501/400 fallback branch + // this test pins. Only the caller's capability is stubbed; the fallback + // mechanism and its expected statuses are unchanged. + (dispatcher as any).timedResolveExecutionContext = async () => ({ + userId: 'u1', systemPermissions: ['manage_metadata'], + }); + const result: any = await dispatcher.dispatch( 'PUT', '/meta/object/widget', { name: 'widget' }, {}, {} as any, ); return result.response; diff --git a/packages/runtime/src/domains/meta-save-capability-gate.test.ts b/packages/runtime/src/domains/meta-save-capability-gate.test.ts new file mode 100644 index 0000000000..360f9c014d --- /dev/null +++ b/packages/runtime/src/domains/meta-save-capability-gate.test.ts @@ -0,0 +1,240 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#7019] The dispatcher's `/meta` PUT demands the `manage_metadata` authoring + * capability (ADR-0066 D1) — the SECOND TRANSPORT for the operation #6603 + * gated on the REST side. + * + * ## Why a second file for "the same" gate + * + * #6603 gated `PUT /api/v1/meta/:type/:name` in `packages/rest`. This is a + * different door into the same `protocol.saveMetaItem`: the runtime + * dispatcher's own `/meta` branch, which serves the cloud runtime. Until this + * gate, closing the REST one moved the hole rather than shutting it — a caller + * refused there was accepted here, for every metadata type. + * + * ## The two independent reasons, both pinned below + * + * 1. **The ADR-0106 read/write asymmetry.** This dispatcher masks object + * schemas on the way OUT (`resolveObjectSchemaMaskPosture` / + * `applyObjectSchemaMask` in `meta.ts`) — a caller is served a schema with + * the fields they may not read removed WHOLE. Sending that document back + * used to persist it verbatim, deleting exactly the fields the caller was + * never allowed to see, with nothing in the exchange saying so. + * 2. **The older, mask-independent hole:** any authenticated session could + * clobber any metadata item through this transport. + * + * ## What the refusal cases assert + * + * `status` AND `code` (the ADR-0112 envelope), plus — the point — that + * `saveMetaItem` was **never entered**. A gate that refuses *after* the write + * has landed would still be the bug and would still pass a status-only + * assertion, so the spy is the load-bearing assertion here, not decoration. + * + * Note the code differs from the REST twin's `FORBIDDEN` on purpose: this + * transport builds its body through `deps.error(msg, 403)`, whose code is + * derived from the status by `standardErrorCodeForHttpStatus` — the same + * `PERMISSION_DENIED` the `_migrate-stored` gate next door in `meta.ts` + * answers. Copying that file's own precedent is deliberate; unifying the two + * spellings would be a contract change, and is not this card's business. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { HttpDispatcher } from '../http-dispatcher.js'; + +/** An object schema carrying a field a restricted caller would never be served. */ +const STORED_SCHEMA = { + name: 'account', + label: 'Account', + fields: { + id: { type: 'text' }, + name: { type: 'text' }, + salary_grade: { type: 'text' }, + }, +}; + +const copy = (v: T): T => JSON.parse(JSON.stringify(v)); + +/** + * A dispatcher over a protocol backed by a REAL in-memory store, so a refused + * write can be checked against the STORE rather than only against the answer. + */ +function boot() { + const stored: Record = { account: copy(STORED_SCHEMA) }; + + const saveMetaItem = vi.fn(async ({ name, item }: any) => { + stored[name] = copy(item); + return { success: true, name }; + }); + + const protocol = { saveMetaItem }; + const kernel = { + context: { getService: (n: string) => (n === 'protocol' ? protocol : null) }, + } as any; + + return { + dispatcher: new HttpDispatcher(kernel), + saveMetaItem, + /** Field names currently in the STORE — not in any response. */ + storedFields: () => Object.keys(stored.account.fields ?? {}).sort(), + storedLabel: () => stored.account.label, + }; +} + +const ctx = (executionContext: any): any => ({ + request: {}, environmentId: 'platform', executionContext, +}); + +const ALL_FIELDS = ['id', 'name', 'salary_grade']; + +describe('#7019 — dispatcher PUT /meta/:type/:name: the capability gate', () => { + it('refuses a caller holding no capabilities, and NOTHING is written', async () => { + const stack = boot(); + + const res = await stack.dispatcher.handleMetadata( + '/object/account', + ctx({ userId: 'u_portal', systemPermissions: [] }), + 'PUT', + { name: 'account', label: 'clobbered', fields: { id: { type: 'text' } } }, + ); + + // The envelope, not just "it failed" — ADR-0112 wants both halves. + expect(res.response?.status).toBe(403); + expect(res.response?.body?.error?.code).toBe('PERMISSION_DENIED'); + + // THE POINT: the write never happened. A 403 issued after the store was + // already overwritten would satisfy the two assertions above. + expect(stack.saveMetaItem).not.toHaveBeenCalled(); + expect(stack.storedFields()).toEqual(ALL_FIELDS); + expect(stack.storedLabel()).toBe('Account'); + }); + + it('refuses the compound-name form too — a name in two segments is the same operation', async () => { + // `/metadata/lead/views/all_leads` → type `lead`, name `views/all_leads`. + // The dispatcher reaches ONE `saveMetaItem` for both name shapes, so a + // gate that only covered single-segment names would be no gate at all. + const stack = boot(); + + const res = await stack.dispatcher.handleMetadata( + '/lead/views/all_leads', + ctx({ userId: 'u_portal', systemPermissions: [] }), + 'PUT', + { density: 'compact' }, + ); + + expect(res.response?.status).toBe(403); + expect(res.response?.body?.error?.code).toBe('PERMISSION_DENIED'); + expect(stack.saveMetaItem).not.toHaveBeenCalled(); + }); + + it('fires BEFORE the protocol is resolved, so 403-vs-501 leaks no kernel capability', async () => { + // A kernel with NO protocol service at all: an authorized caller would + // get the 501 "Save not supported" fallback here. An unauthorized one + // must not be able to tell the two kernels apart. + const kernel = { context: { getService: () => null } } as any; + + const res = await new HttpDispatcher(kernel).handleMetadata( + '/object/account', + ctx({ userId: 'u_portal', systemPermissions: [] }), + 'PUT', + { label: 'x' }, + ); + + expect(res.response?.status).toBe(403); + expect(res.response?.body?.error?.code).toBe('PERMISSION_DENIED'); + }); + + it('holding an unrelated capability is not enough', async () => { + const stack = boot(); + + const res = await stack.dispatcher.handleMetadata( + '/object/account', + ctx({ userId: 'u_admin', systemPermissions: ['setup.access', 'studio.access'] }), + 'PUT', + { label: 'x' }, + ); + + expect(res.response?.status).toBe(403); + expect(stack.saveMetaItem).not.toHaveBeenCalled(); + }); + + it('allows a caller holding `manage_metadata`', async () => { + const stack = boot(); + + const res = await stack.dispatcher.handleMetadata( + '/object/account', + ctx({ userId: 'u_author', systemPermissions: ['manage_metadata'] }), + '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)'); + }); + + it('engine self-invocation (`isSystem`) bypasses, matching every other capability gate', async () => { + const stack = boot(); + + const res = await stack.dispatcher.handleMetadata( + '/object/account', + ctx({ isSystem: true }), + 'PUT', + { name: 'account', label: 'Account', fields: copy(STORED_SCHEMA.fields) }, + ); + + expect(res.response?.status).toBe(200); + expect(stack.saveMetaItem).toHaveBeenCalledTimes(1); + }); + + it('an anonymous caller never reaches this gate — 401 from the domain gate', async () => { + // The anonymous-deny gate (#3963) runs first, so this gate is the second + // layer rather than the only one. + const stack = boot(); + + const res = await stack.dispatcher.handleMetadata('/object/account', ctx({}), 'PUT', { label: 'x' }); + + expect(res.response?.status).toBe(401); + expect(stack.saveMetaItem).not.toHaveBeenCalled(); + }); + + it('the MetadataService fallback is gated too — the refusal precedes both writers', async () => { + // When the protocol has no `saveMetaItem` the handler falls back to + // `metadata.saveItem`. That is a second writer behind the same door, so + // the gate must sit in front of the branch, not inside one arm of it. + const saveItem = vi.fn().mockResolvedValue({ success: true }); + const kernel = { + context: { + getService: (n: string) => (n === 'metadata' ? { saveItem } : n === 'protocol' ? {} : null), + }, + } as any; + + const res = await new HttpDispatcher(kernel).handleMetadata( + '/object/account', + ctx({ userId: 'u_portal', systemPermissions: [] }), + 'PUT', + { label: 'x' }, + ); + + expect(res.response?.status).toBe(403); + expect(saveItem).not.toHaveBeenCalled(); + }); + + it('leaves the READ path alone — a capability-less caller can still GET', async () => { + // This card gates WRITES. The read side has its own posture (ADR-0106 + // masking); turning this into a blanket `/meta` gate would be a + // different, unruled change. + const getMetaItem = vi.fn().mockResolvedValue({ type: 'object', name: 'account', item: copy(STORED_SCHEMA) }); + const kernel = { + context: { getService: (n: string) => (n === 'protocol' ? { getMetaItem } : null) }, + } as any; + + const res = await new HttpDispatcher(kernel).handleMetadata( + '/object/account', + ctx({ userId: 'u_portal', systemPermissions: [] }), + 'GET', + ); + + expect(res.response?.status).not.toBe(403); + }); +}); diff --git a/packages/runtime/src/domains/meta.ts b/packages/runtime/src/domains/meta.ts index 3c9a9a5de5..9927eba50a 100644 --- a/packages/runtime/src/domains/meta.ts +++ b/packages/runtime/src/domains/meta.ts @@ -262,6 +262,40 @@ export async function handleMetadataRequest(deps: DomainHandlerDeps, path: strin // PUT /metadata/:type/:name (Save) if (method === 'PUT' && body) { + // [#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 + // detour sign. Mechanism copied from `POST /_migrate-stored` below + // in this very file rather than reinvented. + // + // The read side of this dispatcher already runs the ADR-0106 mask + // (`resolveObjectMasker` / `maskObjectSchema` above), which is + // exactly the read/write asymmetry #6603 describes: a caller is + // served an object schema with the fields they may not read removed + // WHOLE, and sending that document straight back used to persist it + // — deleting those fields. Refusing the write is the write-side + // answer, here as there. + // + // Independently of masking: before this, any authenticated session + // could clobber any metadata item through this transport. + // + // Gate FIRST — before the protocol service is resolved — so an + // unauthorized caller cannot use the 501-vs-200 answer to probe + // which kernels can save, and so nothing is written before the + // refusal. `manage_metadata` is ADR-0066 D1's authoring capability; + // engine self-invocation (`isSystem`) bypasses, matching + // `actionPermissionError` and the migrate-stored gate below. + const ec: any = _context.executionContext; + if (!ec?.isSystem && !new Set(ec?.systemPermissions ?? []).has('manage_metadata')) { + return { + handled: true, + response: deps.error( + 'Saving a metadata item requires the `manage_metadata` capability.', + 403, + ), + }; + } + // Try to get the protocol service directly const protocol = await deps.resolveService(_context, 'protocol'); diff --git a/packages/runtime/src/http-dispatcher.error-leak.test.ts b/packages/runtime/src/http-dispatcher.error-leak.test.ts index 0b85d2ceeb..9db2b9df15 100644 --- a/packages/runtime/src/http-dispatcher.error-leak.test.ts +++ b/packages/runtime/src/http-dispatcher.error-leak.test.ts @@ -44,7 +44,20 @@ function makeDispatcher(saveError: unknown) { getService: svc, getServiceAsync: async (name: string) => svc(name), }; - return new HttpDispatcher(kernel); + const dispatcher = new HttpDispatcher(kernel); + // [#7019] The `/meta` PUT this guard uses as its vehicle now demands the + // `manage_metadata` capability — an authoring capability, not just a + // session. `dispatch()` re-resolves the execution context from the auth / + // objectql services (overwriting whatever the caller passes in), and these + // stubs have no objectql, so the resolved caller would hold NO capabilities + // and be refused before reaching the error path under test. Stubbing the + // resolution is the dispatcher-side spelling of the `resolveExecCtx` + // override the REST suites use. What is under test — the sanitisation of a + // RETURNED error — is untouched. + (dispatcher as any).timedResolveExecutionContext = async () => ({ + userId: 'u1', systemPermissions: ['manage_metadata'], + }); + return dispatcher; } async function putMeta(saveError: unknown) { diff --git a/packages/runtime/src/http-dispatcher.test.ts b/packages/runtime/src/http-dispatcher.test.ts index 3a01681fef..137edf7d87 100644 --- a/packages/runtime/src/http-dispatcher.test.ts +++ b/packages/runtime/src/http-dispatcher.test.ts @@ -31,6 +31,23 @@ type ContractMock = Partial>; */ const AUTHED_CALLER = () => ({ request: {}, executionContext: { userId: 'u_test', isSystem: false, positions: [], permissions: [], systemPermissions: [] } }) as any; +/** + * [#7019] The same move as `AUTHED_CALLER` above, one rung up: the dispatcher's + * `/meta` PUT now demands the `manage_metadata` authoring capability (ADR-0066 + * D1), matching the gate #6603 put on the REST twin and the one + * `POST /_migrate-stored` already had next door. + * + * The PUT cases below are about ROUTING and ERROR MAPPING — which service + * method a path reaches, whether a 422's issues survive, whether the 501 + * fallback still means "no kernel support". They were written when a session + * alone could write metadata, i.e. their `{ userId: 'u1' }` stub encoded + * exactly the premise the gate destroys, so without a capability they now stop + * at the 403 before reaching the behaviour each one is named after. Only the + * caller changes here; every mechanism, assertion and expected value is + * untouched. The gate itself is pinned in `domains/meta-save-capability-gate.test.ts`. + */ +const METADATA_AUTHOR = () => ({ request: {}, executionContext: { userId: 'u1', systemPermissions: ['manage_metadata'] } }) as any; + describe('HttpDispatcher', () => { let kernel: ObjectKernel; let dispatcher: HttpDispatcher; @@ -74,7 +91,7 @@ describe('HttpDispatcher', () => { describe('handleMetadata', () => { it('should handle PUT /metadata/:type/:name by calling protocol.saveMetaItem', async () => { - const context = { request: {}, executionContext: { userId: 'u1' } }; + const context = METADATA_AUTHOR(); const body = { label: 'New Label' }; const path = '/objects/my_obj'; @@ -95,7 +112,7 @@ describe('HttpDispatcher', () => { }); it('should handle PUT with compound name (3+ path segments)', async () => { - const context = { request: {}, executionContext: { userId: 'u1' } }; + const context = METADATA_AUTHOR(); const body = { density: 'compact' }; // /metadata/lead/views/all_leads → type='lead', name='views/all_leads' const path = '/lead/views/all_leads'; @@ -123,7 +140,7 @@ describe('HttpDispatcher', () => { return null; }; - const context = { request: {}, executionContext: { userId: 'u1' } }; + const context = METADATA_AUTHOR(); const body = { label: 'Fallback' }; const path = '/objects/my_obj'; @@ -137,7 +154,7 @@ describe('HttpDispatcher', () => { it('should return error if save fails', async () => { mockProtocol.saveMetaItem.mockRejectedValue(new Error('Save failed')); - const context = { request: {}, executionContext: { userId: 'u1' } }; + const context = METADATA_AUTHOR(); const body = {}; const path = '/objects/bad_obj'; @@ -161,7 +178,7 @@ describe('HttpDispatcher', () => { ]; mockProtocol.saveMetaItem.mockRejectedValue(err); - const result = await dispatcher.handleMetadata('/objects/bad', { request: {}, executionContext: { userId: 'u1' } } as any, 'PUT', {}); + const result = await dispatcher.handleMetadata('/objects/bad', METADATA_AUTHOR(), 'PUT', {}); expect(result.handled).toBe(true); expect(result.response?.status).toBe(422); // NOT the old hardcoded 400 @@ -1145,7 +1162,7 @@ describe('HttpDispatcher', () => { return null; }); - const result = await dispatcher.handleMetadata('/objects/my_obj', { request: {}, executionContext: { userId: 'u1' } } as any, 'PUT', { label: 'Test' }); + const result = await dispatcher.handleMetadata('/objects/my_obj', METADATA_AUTHOR(), 'PUT', { label: 'Test' }); expect(result.handled).toBe(true); expect(result.response?.status).toBe(200); expect(asyncProtocol.saveMetaItem).toHaveBeenCalled(); @@ -1261,7 +1278,7 @@ describe('HttpDispatcher', () => { // Remove context.getService to ensure getServiceAsync is used (kernel as any).context = {}; - const result = await dispatcher.handleMetadata('/objects/my_obj', { request: {}, executionContext: { userId: 'u1' } } as any, 'PUT', { label: 'Test' }); + const result = await dispatcher.handleMetadata('/objects/my_obj', METADATA_AUTHOR(), 'PUT', { label: 'Test' }); expect(result.handled).toBe(true); expect(result.response?.status).toBe(200); expect(asyncProtocol.saveMetaItem).toHaveBeenCalled(); @@ -3377,7 +3394,7 @@ describe('HttpDispatcher', () => { }); it('PUT /meta/:type/:name should return 501 when protocol is unavailable', async () => { - const context = { request: {}, executionContext: { userId: 'u1' } }; + const context = METADATA_AUTHOR(); const body = { label: 'Test' }; const result = await minimalDispatcher.handleMetadata('/objects/my_obj', context, 'PUT', body); expect(result.handled).toBe(true);