|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | + |
| 3 | +/** |
| 4 | + * [#6603] `PUT /api/v1/meta/:type/:name` demands the `manage_metadata` |
| 5 | + * authoring capability (ADR-0066 D1) — the same gate, by the same mechanism, |
| 6 | + * that `POST /meta/_migrate-stored` already demands next door. |
| 7 | + * |
| 8 | + * ## What this suite exists to stop |
| 9 | + * |
| 10 | + * ADR-0106 D1 removes an unreadable field **whole** from a served object |
| 11 | + * schema, and this route persists the body it is handed. Until this gate, a |
| 12 | + * non-exempt caller's most ordinary sequence — |
| 13 | + * |
| 14 | + * 1. `GET /meta/object/account` → a schema with `salary_grade` and |
| 15 | + * `bonus_formula` absent (correct: that is the whole point of D1); |
| 16 | + * 2. edit something unrelated — a label; |
| 17 | + * 3. `PUT /meta/object/account` with that body, |
| 18 | + * |
| 19 | + * — stored the schema back MINUS the two fields, i.e. the caller deleted |
| 20 | + * exactly the fields they were never allowed to see, and nothing in the |
| 21 | + * exchange said so. The headline case below drives that real sequence against |
| 22 | + * a real store, so what is pinned is the DATA LOSS, not just a status code: a |
| 23 | + * gate that answers 403 after `saveMetaItem` has already run would still be |
| 24 | + * the bug, and would still pass a status-only assertion. |
| 25 | + * |
| 26 | + * The gate also closes a hole that has nothing to do with masking: before it, |
| 27 | + * any authenticated session could clobber any metadata item. |
| 28 | + * |
| 29 | + * ## Scope of what is pinned here |
| 30 | + * |
| 31 | + * THIS ROUTE ONLY. The same round trip is still reachable through the |
| 32 | + * compound-name save `PUT /meta/:type/:section/:name` (measured) and the |
| 33 | + * runtime dispatcher's own `/meta` PUT — filed as #7019, out of this change's |
| 34 | + * region. A reader who takes this suite as proof that the defect is closed |
| 35 | + * platform-wide has read more into it than it asserts. |
| 36 | + * |
| 37 | + * ## Rejection cases assert the ENVELOPE (ADR-0112) |
| 38 | + * |
| 39 | + * Every refusal here asserts `code` AND `status`, never a bare "it threw" — |
| 40 | + * this route answers by *sending* rather than throwing, so a throw-shaped |
| 41 | + * assertion could not tell "refused with the wrong envelope" from "did not |
| 42 | + * refuse at all". |
| 43 | + */ |
| 44 | + |
| 45 | +import { describe, it, expect, vi } from 'vitest'; |
| 46 | +import { FLS_CONTRACT_OBJECT } from '@objectstack/metadata-core/testing'; |
| 47 | +import { RestServer } from './rest-server'; |
| 48 | + |
| 49 | +const copy = <T>(value: T): T => JSON.parse(JSON.stringify(value)); |
| 50 | + |
| 51 | +/** The four fields `FLS_CONTRACT_OBJECT` declares, sorted. */ |
| 52 | +const ALL_FIELDS = ['bonus_formula', 'id', 'name', 'salary_grade']; |
| 53 | +/** What the security double lets a restricted caller read. */ |
| 54 | +const READABLE_TO_RESTRICTED = ['id', 'name']; |
| 55 | + |
| 56 | +const SINGLE_PATH = '/api/v1/meta/:type/:name'; |
| 57 | + |
| 58 | +function mockServer() { |
| 59 | + return { |
| 60 | + get: vi.fn(), post: vi.fn(), put: vi.fn(), delete: vi.fn(), patch: vi.fn(), |
| 61 | + use: vi.fn(), listen: vi.fn().mockResolvedValue(undefined), close: vi.fn().mockResolvedValue(undefined), |
| 62 | + }; |
| 63 | +} |
| 64 | + |
| 65 | +function mockRes() { |
| 66 | + const res: any = { |
| 67 | + statusCode: 200, |
| 68 | + json: vi.fn(function (this: any, body: any) { this._body = body; return this; }), |
| 69 | + send: vi.fn(), |
| 70 | + status: vi.fn(function (this: any, code: number) { this.statusCode = code; return this; }), |
| 71 | + header: vi.fn(), |
| 72 | + }; |
| 73 | + return res; |
| 74 | +} |
| 75 | + |
| 76 | +interface BootOptions { |
| 77 | + /** The caller, as `resolveExecCtx` resolves it. `undefined` = anonymous. */ |
| 78 | + context: Record<string, unknown> | undefined; |
| 79 | + /** What `security.getMetadataReadableFields` answers; omit for no security service. */ |
| 80 | + readable?: readonly string[]; |
| 81 | + /** Drop `saveMetaItem` from the protocol (the 501 kernel). */ |
| 82 | + withoutSave?: boolean; |
| 83 | +} |
| 84 | + |
| 85 | +/** |
| 86 | + * Boot the route over a protocol backed by a REAL in-memory store, so a GET → |
| 87 | + * edit → PUT sequence actually round-trips and the stored document can be |
| 88 | + * inspected after the write is refused. |
| 89 | + */ |
| 90 | +function boot(opts: BootOptions) { |
| 91 | + const stored: Record<string, any> = { account: copy(FLS_CONTRACT_OBJECT as unknown as Record<string, unknown>) }; |
| 92 | + |
| 93 | + const saveMetaItem = vi.fn(async ({ name, item }: any) => { |
| 94 | + stored[name] = copy(item); |
| 95 | + return { success: true, type: 'object', name }; |
| 96 | + }); |
| 97 | + |
| 98 | + const protocol: any = { |
| 99 | + getDiscovery: vi.fn().mockResolvedValue({ version: 'v0', routes: { data: '', metadata: '', ui: '', auth: '/auth' } }), |
| 100 | + getMetaTypes: vi.fn().mockResolvedValue([]), |
| 101 | + getMetaItems: vi.fn(async () => Object.values(stored).map(copy)), |
| 102 | + // No `getMetaItemCached` — the uncached branch, so the read always |
| 103 | + // reflects the store rather than a fixture snapshot. |
| 104 | + getMetaItem: vi.fn(async ({ type, name }: any) => ({ type, name, item: copy(stored[name]), lock: 'none' })), |
| 105 | + findData: vi.fn().mockResolvedValue([]), |
| 106 | + getData: vi.fn().mockResolvedValue({}), |
| 107 | + createData: vi.fn().mockResolvedValue({ id: '1' }), |
| 108 | + updateData: vi.fn().mockResolvedValue({}), |
| 109 | + deleteData: vi.fn().mockResolvedValue({ success: true }), |
| 110 | + }; |
| 111 | + if (!opts.withoutSave) protocol.saveMetaItem = saveMetaItem; |
| 112 | + |
| 113 | + const security = opts.readable === undefined ? undefined : { |
| 114 | + getReadableFields: async () => [...opts.readable!], |
| 115 | + getMetadataReadableFields: async () => [...opts.readable!], |
| 116 | + }; |
| 117 | + |
| 118 | + const rest = new RestServer( |
| 119 | + mockServer() as any, |
| 120 | + protocol as any, |
| 121 | + { api: { requireAuth: false } } as any, |
| 122 | + undefined, undefined, undefined, undefined, undefined, undefined, undefined, |
| 123 | + undefined, undefined, undefined, undefined, undefined, undefined, undefined, |
| 124 | + security ? (async () => security as any) : undefined, |
| 125 | + ); |
| 126 | + (rest as any).resolveExecCtx = async () => opts.context; |
| 127 | + rest.registerRoutes(); |
| 128 | + |
| 129 | + const route = (method: string) => (rest as any).getRoutes().find( |
| 130 | + (r: any) => r.method === method && r.path === SINGLE_PATH, |
| 131 | + ); |
| 132 | + |
| 133 | + return { |
| 134 | + rest, |
| 135 | + saveMetaItem, |
| 136 | + /** Field names currently in the STORE (not in any response). */ |
| 137 | + storedFields: () => Object.keys(stored.account.fields ?? {}).sort(), |
| 138 | + storedLabel: () => stored.account.label, |
| 139 | + get: async () => { |
| 140 | + const res = mockRes(); |
| 141 | + await route('GET')!.handler({ params: { type: 'object', name: 'account' }, query: {}, headers: {} }, res); |
| 142 | + return { res, body: res.json.mock.calls.at(-1)?.[0] }; |
| 143 | + }, |
| 144 | + put: async (item: unknown) => { |
| 145 | + const res = mockRes(); |
| 146 | + await route('PUT')!.handler( |
| 147 | + { params: { type: 'object', name: 'account' }, query: {}, headers: {}, body: item }, |
| 148 | + res, |
| 149 | + ); |
| 150 | + return { res, body: res.json.mock.calls.at(-1)?.[0] }; |
| 151 | + }, |
| 152 | + }; |
| 153 | +} |
| 154 | + |
| 155 | +describe('#6603 — PUT /meta/:type/:name: the ADR-0106 GET → edit → PUT round trip', () => { |
| 156 | + it('refuses a restricted caller\'s round-trip write, and the masked fields SURVIVE in the store', async () => { |
| 157 | + const stack = boot({ |
| 158 | + context: { userId: 'u_portal', systemPermissions: [] }, |
| 159 | + readable: READABLE_TO_RESTRICTED, |
| 160 | + }); |
| 161 | + |
| 162 | + // 1. The read is masked — the premise. Asserted rather than assumed so |
| 163 | + // this case cannot go quietly green by the masking disappearing. |
| 164 | + const read = await stack.get(); |
| 165 | + expect(Object.keys(read.body.item.fields).sort()).toEqual(READABLE_TO_RESTRICTED); |
| 166 | + expect(read.body.item.fields).not.toHaveProperty('salary_grade'); |
| 167 | + expect(read.body.item.fields).not.toHaveProperty('bonus_formula'); |
| 168 | + |
| 169 | + // 2. The caller edits something unrelated and sends the body back. |
| 170 | + const edited = { ...copy(read.body.item), label: 'Account (renamed)' }; |
| 171 | + |
| 172 | + // 3. The write is refused — envelope, not just "it failed". |
| 173 | + const write = await stack.put(edited); |
| 174 | + expect(write.res.statusCode).toBe(403); |
| 175 | + expect(write.body).toMatchObject({ error: { code: 'FORBIDDEN' } }); |
| 176 | + |
| 177 | + // 4. THE POINT: nothing was written. A gate that 403s *after* the |
| 178 | + // store has already been overwritten is the failure mode worth |
| 179 | + // guarding, and a status-only assertion cannot see it. |
| 180 | + expect(stack.saveMetaItem).not.toHaveBeenCalled(); |
| 181 | + expect(stack.storedFields()).toEqual(ALL_FIELDS); |
| 182 | + expect(stack.storedLabel()).toBe('Account'); |
| 183 | + }); |
| 184 | + |
| 185 | + it('the refusal is the gate, not the masking: an UNRESTRICTED but uncapable caller is refused too', async () => { |
| 186 | + // Everything readable ⇒ no field would have been lost. The write is |
| 187 | + // still refused, because reason (2) — any authenticated session could |
| 188 | + // clobber any metadata item — is independent of ADR-0106. |
| 189 | + const stack = boot({ |
| 190 | + context: { userId: 'u_staff', systemPermissions: [] }, |
| 191 | + readable: ALL_FIELDS, |
| 192 | + }); |
| 193 | + const read = await stack.get(); |
| 194 | + expect(Object.keys(read.body.item.fields).sort()).toEqual(ALL_FIELDS); |
| 195 | + |
| 196 | + const write = await stack.put({ ...copy(read.body.item), label: 'clobbered' }); |
| 197 | + expect(write.res.statusCode).toBe(403); |
| 198 | + expect(write.body).toMatchObject({ error: { code: 'FORBIDDEN' } }); |
| 199 | + expect(stack.storedLabel()).toBe('Account'); |
| 200 | + }); |
| 201 | +}); |
| 202 | + |
| 203 | +describe('#6603 — the gate itself', () => { |
| 204 | + it('fires BEFORE the protocol is probed, so 403-vs-501 leaks no kernel capability', async () => { |
| 205 | + const stack = boot({ context: { userId: 'u1', systemPermissions: [] }, withoutSave: true }); |
| 206 | + const write = await stack.put({ name: 'account' }); |
| 207 | + // An authorized caller would get 501 here. An unauthorized one must |
| 208 | + // not be able to tell the two kernels apart. |
| 209 | + expect(write.res.statusCode).toBe(403); |
| 210 | + expect(write.body).toMatchObject({ error: { code: 'FORBIDDEN' } }); |
| 211 | + }); |
| 212 | + |
| 213 | + it('an anonymous caller never reaches the capability gate — 401 from the /meta umbrella', async () => { |
| 214 | + // Every `/meta` route inherits the anonymous-deny wrapper, so this gate |
| 215 | + // is the second layer rather than the only one. |
| 216 | + const stack = boot({ context: undefined }); |
| 217 | + const write = await stack.put({ name: 'account' }); |
| 218 | + expect(write.res.statusCode).toBe(401); |
| 219 | + expect(stack.saveMetaItem).not.toHaveBeenCalled(); |
| 220 | + }); |
| 221 | + |
| 222 | + it('allows a caller holding `manage_metadata`', async () => { |
| 223 | + const stack = boot({ context: { userId: 'u_author', systemPermissions: ['manage_metadata'] } }); |
| 224 | + const write = await stack.put({ name: 'account', label: 'Account', fields: {} }); |
| 225 | + expect(write.res.statusCode).toBe(200); |
| 226 | + expect(stack.saveMetaItem).toHaveBeenCalledTimes(1); |
| 227 | + }); |
| 228 | + |
| 229 | + it('`isSystem` bypasses, matching every other capability gate on the platform', async () => { |
| 230 | + const stack = boot({ context: { isSystem: true } }); |
| 231 | + const write = await stack.put({ name: 'account', label: 'Account', fields: {} }); |
| 232 | + expect(write.res.statusCode).toBe(200); |
| 233 | + expect(stack.saveMetaItem).toHaveBeenCalledTimes(1); |
| 234 | + }); |
| 235 | + |
| 236 | + /** |
| 237 | + * MEASURED, and deliberately pinned as-is: the capability this gate demands |
| 238 | + * (`manage_metadata`) and the ADR-0106 D4 mask-exemption set |
| 239 | + * (`OBJECT_SCHEMA_MASK_EXEMPT_CAPABILITIES` = `studio.access`, |
| 240 | + * `setup.access`) are DIFFERENT SETS. So holding a D4 exemption is not by |
| 241 | + * itself permission to write — and, in the other direction, passing this |
| 242 | + * gate is not by itself an exemption from the mask. What this route now |
| 243 | + * enforces is "a writer holds `manage_metadata`", which is NOT the same |
| 244 | + * sentence as the ruling's rationale, "a writer sees the whole schema"; |
| 245 | + * the two coincide only because `admin_full_access` happens to carry both. |
| 246 | + * Recorded as #7020 — do not "fix" this matrix to match the rationale |
| 247 | + * without a ruling. |
| 248 | + * |
| 249 | + * In the permission sets the platform ships this never separates on the |
| 250 | + * write side: `admin_full_access` carries `manage_metadata` AND |
| 251 | + * `studio.access` AND `setup.access`, and it is the only shipped set with |
| 252 | + * `studio.access`. `organization_admin` carries `setup.access` without |
| 253 | + * `manage_metadata` — D4-exempt (so it never had the round-trip hazard) but |
| 254 | + * refused here, which is consistent with its own declaration that a tenant |
| 255 | + * does not mutate shared metadata. |
| 256 | + */ |
| 257 | + it.each([ |
| 258 | + { held: 'no capabilities at all', systemPermissions: [] as string[], status: 403 }, |
| 259 | + { held: '`studio.access` alone — D4-exempt, but not an authoring capability', systemPermissions: ['studio.access'], status: 403 }, |
| 260 | + { held: '`setup.access` alone — likewise; this is `organization_admin`', systemPermissions: ['setup.access'], status: 403 }, |
| 261 | + { held: '`manage_metadata` alone', systemPermissions: ['manage_metadata'], status: 200 }, |
| 262 | + { held: 'the shipped `admin_full_access` shape', systemPermissions: ['manage_metadata', 'studio.access', 'setup.access'], status: 200 }, |
| 263 | + ])('$held → $status', async ({ systemPermissions, status }) => { |
| 264 | + const stack = boot({ context: { userId: 'u1', systemPermissions } }); |
| 265 | + const write = await stack.put({ name: 'account', label: 'Account', fields: {} }); |
| 266 | + expect(write.res.statusCode).toBe(status); |
| 267 | + }); |
| 268 | +}); |
| 269 | + |
| 270 | +describe('#6603 — the exempt authoring caller is unaffected', () => { |
| 271 | + /** |
| 272 | + * A GUARD, not evidence: this case is green both before and after the gate |
| 273 | + * (a platform admin could always write, and being D4-exempt their read was |
| 274 | + * never masked, so their round trip was never lossy). It is here so a |
| 275 | + * future tightening of the gate cannot silently lock the platform |
| 276 | + * administrator out of the console's own schema designer. |
| 277 | + */ |
| 278 | + it('an `admin_full_access`-shaped caller round-trips losslessly', async () => { |
| 279 | + const stack = boot({ |
| 280 | + context: { userId: 'u_admin', systemPermissions: ['manage_metadata', 'studio.access', 'setup.access'] }, |
| 281 | + readable: READABLE_TO_RESTRICTED, // the service would restrict — D4 exemption outranks it |
| 282 | + }); |
| 283 | + |
| 284 | + const read = await stack.get(); |
| 285 | + // D4 — exempt callers are served the UNMASKED schema. |
| 286 | + expect(Object.keys(read.body.item.fields).sort()).toEqual(ALL_FIELDS); |
| 287 | + |
| 288 | + const write = await stack.put({ ...copy(read.body.item), label: 'Account (renamed)' }); |
| 289 | + expect(write.res.statusCode).toBe(200); |
| 290 | + expect(stack.storedFields()).toEqual(ALL_FIELDS); |
| 291 | + expect(stack.storedLabel()).toBe('Account (renamed)'); |
| 292 | + }); |
| 293 | +}); |
0 commit comments