diff --git a/.changeset/27669-clone-overrides-stray-keys.md b/.changeset/27669-clone-overrides-stray-keys.md new file mode 100644 index 00000000000..9d46ad098a3 --- /dev/null +++ b/.changeset/27669-clone-overrides-stray-keys.md @@ -0,0 +1,19 @@ +--- +'@objectstack/rest': patch +--- + +fix(rest)!: the clone door refuses a body that carries `overrides` beside other top-level keys, instead of cloning with `overrides` alone (#27669) + +Clause-②: no (narrowing) — `POST /data/:object/:id/clone` refuses a body that mixes the nested `{ "overrides": {...} }` shape with other top-level keys; it used to answer `201` with those keys ignored. Nothing is widened. +Metadata-protocol: none + + + +**BREAKING** accept-set narrowing at one REST data door (`@objectstack/rest`). + +- **Before.** `{ "overrides": { "name": "Copy" }, "status": "draft" }` read `overrides` alone: the clone got the new name, `status` was dropped, and the door answered `201`. +- **Now.** A body that carries `overrides` beside any other top-level key answers `400` with `error.code: 'VALIDATION_ERROR'` before the clone runs, naming the stray keys and the two accepted shapes. +- **Unchanged.** `{ "overrides": {...} }` alone, a bare field map without an `overrides` key, `{}`, and no body at all clone exactly as before. The SDK's `data.clone` sends only the nested shape. +- **Who sees it.** An API caller or script that mixes the two body shapes. Such a request used to succeed while applying only part of what was sent. + +**The remedy.** Send either `{ "overrides": {...} }` or a bare field map, not both. A record with a real field named `overrides` sets it through the nested shape: `{ "overrides": { "overrides": ... } }`. diff --git a/packages/rest/src/clone-overrides-refusal.test.ts b/packages/rest/src/clone-overrides-refusal.test.ts index 64aeb27ec06..86111a15123 100644 --- a/packages/rest/src/clone-overrides-refusal.test.ts +++ b/packages/rest/src/clone-overrides-refusal.test.ts @@ -98,3 +98,35 @@ describe('[#27489] clone — object overrides and absent bodies keep their answe expect(out.cloneData.mock.calls[0][0]).toMatchObject({ object: 'account', id: 'a1' }); }); }); + +/** + * [#27669] A body that carries `overrides` beside other top-level keys is + * refused, instead of reading `overrides` alone and dropping the rest with a + * `201`. The refusal names the stray keys and both accepted shapes, and points + * a record with a real field named `overrides` at the nested shape. + */ +describe('[#27669] clone — overrides beside other top-level keys is refused before cloneData', () => { + it.each([ + ['one stray key', '{"overrides":{"name":"Copy"},"status":"draft"}', ['"status"']], + ['two stray keys', '{"overrides":{"name":"Copy"},"status":"draft","owner":"u2"}', ['"status"', '"owner"']], + ['an empty overrides with a stray key', '{"overrides":{},"status":"draft"}', ['"status"']], + ])('%s → 400 VALIDATION_ERROR naming the stray keys and both shapes, cloneData never called', async (_label, body, strays) => { + const out = await clone(body); + + expect(out.status).toBe(400); + expect(out.body?.error?.code).toBe('VALIDATION_ERROR'); + const message: string = out.body?.error?.message ?? ''; + for (const key of strays) expect(message).toContain(key); + expect(message).toContain('{ "overrides": {...} }'); + expect(message).toContain('a bare field map'); + expect(message).toContain('{ "overrides": { "overrides": ... } }'); + expect(out.cloneData).not.toHaveBeenCalled(); + }); + + it('a field named overrides set through the nested shape → 201, cloneData receives it', async () => { + const out = await clone('{"overrides":{"overrides":"x","name":"Copy"}}'); + + expect(out.status).toBe(201); + expect(out.cloneData.mock.calls[0][0]).toMatchObject({ overrides: { overrides: 'x', name: 'Copy' } }); + }); +}); diff --git a/packages/rest/src/data-action-routes.ts b/packages/rest/src/data-action-routes.ts index 916792149eb..d427b7da0a7 100644 --- a/packages/rest/src/data-action-routes.ts +++ b/packages/rest/src/data-action-routes.ts @@ -369,7 +369,8 @@ export function registerDataActionEndpoints(this: DataActionRouteHost, basePath: // object's `enable.clone` capability, default on). Optional JSON body // `{ overrides?: {...} }` (or a bare field map) is applied on top of // the copied values, e.g. to set a new name or clear a unique field. - // Any other `overrides` (or bare body) is a 400. + // Any other `overrides` (or bare body), or `overrides` beside other + // top-level keys, is a 400. // Distinct path segment (`/clone`) keeps it clear of the greedy // `/data/:object/:id` matchers. this.routeManager.register({ @@ -392,6 +393,21 @@ export function registerDataActionEndpoints(this: DataActionRouteHost, basePath: // callers can POST overrides directly without nesting. const nested = isFieldMap(body) && 'overrides' in body; const overrides = nested ? body.overrides : body; + // [#27669] Refused, not dropped: `overrides` beside other + // top-level keys used to clone with `overrides` alone and + // answer `201`, the other keys silently ignored. A record + // with a real field named `overrides` sets it through the + // nested shape. + if (nested) { + const strays = Object.keys(body).filter((key) => key !== 'overrides'); + if (strays.length > 0) { + sendEnvelopeError(res, 400, 'VALIDATION_ERROR', + `The clone body carries "overrides" beside other keys (${strays.map((key) => `"${key}"`).join(', ')}). ` + + 'Send either { "overrides": {...} } or a bare field map, not both. ' + + 'To set a field named "overrides", nest it: { "overrides": { "overrides": ... } }.'); + return; + } + } // [#27489] Refused, not dropped: a non-object `overrides` // (`"x"`, `1`, `true`, `null`) used to be skipped and answer // `201` with a plain copy, and an array passed the `typeof`