From e03bd96c3a30d1b4efa2a4a81d196bf116f202a7 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 18:30:41 +0000 Subject: [PATCH] fix(rest)!: rollback accepts only a positive integer toVersion POST /meta/:type/:name/rollback coerced toVersion with a bare Number(), so true, [2] and "2" in a JSON body restored a version and 1.5 answered 404 VERSION_NOT_FOUND. The body value is now judged as it came against the whole-number declaration /diff reads, the query string goes through readDeclaredQueryNumber, and everything else answers the door's existing 400 INVALID_REQUEST. The census ledger loses the rollback exemption row. Claude-Session: https://claude.ai/code/session_01CxBKycaiUpA1yGx1XyoGqX Co-authored-by: Claude --- .../25482-rollback-to-version-integer.md | 19 ++++ packages/rest/src/meta-item-history-routes.ts | 32 ++++++- .../rest-server-query-number-census.test.ts | 86 +++++++++++++++---- 3 files changed, 115 insertions(+), 22 deletions(-) create mode 100644 .changeset/25482-rollback-to-version-integer.md diff --git a/.changeset/25482-rollback-to-version-integer.md b/.changeset/25482-rollback-to-version-integer.md new file mode 100644 index 00000000000..780befe4b94 --- /dev/null +++ b/.changeset/25482-rollback-to-version-integer.md @@ -0,0 +1,19 @@ +--- +'@objectstack/rest': patch +--- + +fix(rest)!: `POST /api/v1/meta/:type/:name/rollback` accepts only a positive integer as `toVersion`, and refuses everything else with its own `400 INVALID_REQUEST` instead of coercing it into a version + +Clause-②: no (narrowing) +Metadata-protocol: none + + + +**BREAKING** (the rollback door's accept set narrows), shipped as `patch`: it changes which requests one REST door serves, not what a metadata author can write. + +The rollback door read its version with a bare `Number(...)` and accepted any result that was finite and at least 1. So `{"toVersion": true}` restored version 1, `{"toVersion": [2]}` and `{"toVersion": "2"}` restored version 2, and `{"toVersion": 1.5}` reached the protocol and answered `404 VERSION_NOT_FOUND`. This is the door that makes a caller-supplied version the live row, and its own refusal already said `'toVersion' (positive integer) is required`. + +- **FROM:** a JSON body `toVersion` (or its `version` alias) that `Number()` turned into a number of at least 1 restored that version: a boolean, a one-element array, a numeric string. A fractional value answered `404`. +- **TO:** the body value must be a JSON number that is a whole number of at least 1. A `?toVersion=` query string is read the way `/diff` reads its versions (`readDeclaredQueryNumber`), so `?toVersion=3` still restores version 3. Everything else, `true`, `[2]`, `"2"`, `1.5`, `0`, `-1`, `"abc"`, a blank or non-finite query value, answers `400 INVALID_REQUEST` with the same message as before, and nothing is restored. +- **Unchanged:** the precedence (`toVersion`, then `version`, then `?toVersion=`), the capability and walled-catalog gates in front of it, and every answer for a valid version. The client SDK's `rollbackItem` and the console's metadata client already send a JSON number. +- **Fix:** send `toVersion` as a JSON number, for example `{"toVersion": 2}`. diff --git a/packages/rest/src/meta-item-history-routes.ts b/packages/rest/src/meta-item-history-routes.ts index 5e850bcc6a6..cd8508485ed 100644 --- a/packages/rest/src/meta-item-history-routes.ts +++ b/packages/rest/src/meta-item-history-routes.ts @@ -797,7 +797,7 @@ export function registerMetaItemHistoryRoutes(this: MetaItemHistoryRouteHost, sc }); return; } - // [#6877] The `Number(...)` below already refuses an array + // [#6877] The version read below already refuses an array // — but as `INVALID_REQUEST` "'toVersion' (positive integer) // is required", which tells a caller who supplied two valid // integers that they supplied none. Refusing the multiplicity @@ -806,9 +806,33 @@ export function registerMetaItemHistoryRoutes(this: MetaItemHistoryRouteHost, sc if (refuseUnknownQueryParams(req, res, META_ITEM_ROLLBACK_PARAMS)) return; if (refuseRepeatedQueryParams(req, res, ['toVersion', META_ITEM_MEMBER_PARAM])) return; const body = (req.body && typeof req.body === 'object') ? req.body : {}; - const toVersionRaw = body.toVersion ?? body.version ?? req.query?.toVersion; - const toVersion = Number(toVersionRaw); - if (!Number.isFinite(toVersion) || toVersion < 1) { + // [#25482] `toVersion` names a history row, so only a positive + // integer is a version. A bare `Number(...)` here coerced + // whatever came — `true` restored v1, `[2]` and `"2"` restored + // v2, and `1.5` reached the protocol and answered `404` — on the + // one door that restores a caller-supplied version as live. The + // JSON body is typed, so its value is judged AS IT CAME against + // the same whole-number declaration `/diff` reads (the client + // SDK's `rollbackItem` and the console's metadata client both + // send a JSON number); only the untyped query string goes + // through `readDeclaredQueryNumber`'s faithful coercion. + // Precedence is unchanged — `toVersion`, then the `version` + // alias, then `?toVersion=` — and every refusal stays this + // door's own `INVALID_REQUEST`. + const bodyVersion = body.toVersion ?? body.version; + let toVersion: number | undefined; + if (bodyVersion !== undefined && bodyVersion !== null) { + const parsed = UNDECLARED_WHOLE_NUMBER_PARAM.safeParse(bodyVersion); + toVersion = parsed.success ? parsed.data : undefined; + } else { + try { + toVersion = readDeclaredQueryNumber(req.query, 'toVersion', + UNDECLARED_WHOLE_NUMBER_PARAM, { emptyIsAbsent: false }); + } catch { + toVersion = undefined; + } + } + if (toVersion === undefined || toVersion < 1) { res.status(400).json({ error: `'toVersion' (positive integer) is required`, code: 'INVALID_REQUEST', diff --git a/packages/rest/src/rest-server-query-number-census.test.ts b/packages/rest/src/rest-server-query-number-census.test.ts index 7dc6d06c254..2e949caad89 100644 --- a/packages/rest/src/rest-server-query-number-census.test.ts +++ b/packages/rest/src/rest-server-query-number-census.test.ts @@ -183,16 +183,6 @@ const LEDGER: readonly LedgerRow[] = [ reason: 'A counter column of a persisted `sys_import_job` row, read back from the store and ' + 'mapped to the ImportJobProgress DTO — no request value reaches it.', })), - { - site: 'POST ${metaPath}/:type/:name/rollback » Number(toVersionRaw)', - disposition: 'exempt', - reason: 'Read body-first (`body.toVersion ?? body.version ?? req.query.toVersion`) and then ' - + 'CHECKED, not served: a non-finite or below-1 result is already refused `400` before ' - + '`rollbackMetaItem` runs, so an unreadable value is never dropped or substituted — ' - + 'outside this family by its own definition. Its refusal is the door\'s own ' - + '`INVALID_REQUEST`; moving it onto `VALIDATION_FAILED` would be a wire change to a door ' - + 'that already refuses, which no card here decides. Pinned below.', - }, { site: 'GET ${dataPath}/:object/export » Number(q.page)', disposition: 'exempt', @@ -306,7 +296,7 @@ describe('#20139 census — every numeric coercion in rest-server.ts is classifi } }); - it('positive control: the census sees the reader and both exemptions in the real file', () => { + it('positive control: the census sees the reader and every exemption in the real file', () => { const keys = new Set(sites.map((s) => s.key)); for (const row of LEDGER.filter((r) => r.disposition !== 'not-a-query-value')) { expect(keys.has(row.site), row.site).toBe(true); @@ -361,9 +351,13 @@ describe('#20139 census — every numeric coercion in rest-server.ts is classifi // `meta-item-history-routes.ts`): both read their query as strings and coerce // no number of it: // 9 = 7 + 1 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 1 + 0, length 17 → 18. + // [#25482] The rollback's `Number(toVersionRaw)` is gone: its body value is + // judged as it came and its query value goes through the reader, so its + // exemption row left the ledger with it: + // 8 = 7 + 1 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0, length 18. expect(REST_SERVER_REGISTRAR_FILES).toHaveLength(18); expect(SOURCES.map(([file]) => file)).toEqual([...REST_SERVER_REGISTRAR_FILES]); - expect(sites).toHaveLength(9); + expect(sites).toHaveLength(8); }); }); @@ -493,31 +487,87 @@ describe('#20139 census — exemption: export `?page=` changes chunking, never t }); }); -describe('#20139 census — exemption: rollback `toVersion` is already refused, never substituted', () => { - const rollback = async (query: Record) => { +describe('#25482 rollback `toVersion` — only a positive integer names a version', () => { + const rollback = async (req: { query?: Record; body?: unknown }) => { const { drive, protocol } = boot(); const answer = await drive('POST', '/api/v1/meta/:type/:name/rollback', { params: { type: 'view', name: 'all_accounts' }, - query, + query: req.query ?? {}, + body: req.body ?? {}, }); return { answer, protocol }; }; + it.each([ + [true], // was `Number(true)` = 1 → restored v1 + [[2]], // was `Number([2])` = 2 → restored v2 + ['2'], // a JSON string is not a JSON number + [1.5], // was forwarded → `404 VERSION_NOT_FOUND` + [0], + [-1], + ['abc'], + [{}], + ])('body { toVersion: %j } answers 400 INVALID_REQUEST and nothing is restored', async (toVersion) => { + const { answer, protocol } = await rollback({ body: { toVersion } }); + expect(answer.status).toBe(400); + expect(answer.body?.code).toBe('INVALID_REQUEST'); + expect(protocol.rollbackMetaItem).not.toHaveBeenCalled(); + }); + + it.each([[true], [[2]], [1.5], [0]])( + 'the `version` alias is judged the same way: body { version: %j } answers 400', + async (version) => { + const { answer, protocol } = await rollback({ body: { version } }); + expect(answer.status).toBe(400); + expect(answer.body?.code).toBe('INVALID_REQUEST'); + expect(protocol.rollbackMetaItem).not.toHaveBeenCalled(); + }, + ); + it.each([ ['abc'], // `NaN` → refused - [''], // `Number('')` = 0 → below 1 → refused + [''], // blank → refused, never read as 0 or absent-then-defaulted ['Infinity'], // non-finite → refused + ['1.5'], + ['0'], + ['-1'], ])('?toVersion=%j answers 400 INVALID_REQUEST and nothing is restored', async (toVersion) => { - const { answer, protocol } = await rollback({ toVersion }); + const { answer, protocol } = await rollback({ query: { toVersion } }); + expect(answer.status).toBe(400); + expect(answer.body?.code).toBe('INVALID_REQUEST'); + expect(protocol.rollbackMetaItem).not.toHaveBeenCalled(); + }); + + it('an absent version answers 400 INVALID_REQUEST', async () => { + const { answer, protocol } = await rollback({}); expect(answer.status).toBe(400); expect(answer.body?.code).toBe('INVALID_REQUEST'); expect(protocol.rollbackMetaItem).not.toHaveBeenCalled(); }); + it('lit control: body { toVersion: 2 } reaches rollbackMetaItem as 2', async () => { + const { answer, protocol } = await rollback({ body: { toVersion: 2 } }); + expect(answer.status).toBe(200); + expect(protocol.rollbackMetaItem).toHaveBeenCalledTimes(1); + expect(protocol.rollbackMetaItem.mock.calls[0][0].toVersion).toBe(2); + }); + + it('lit control: body { version: 4 } (the alias) reaches rollbackMetaItem as 4', async () => { + const { answer, protocol } = await rollback({ body: { version: 4 } }); + expect(answer.status).toBe(200); + expect(protocol.rollbackMetaItem.mock.calls[0][0].toVersion).toBe(4); + }); + it('lit control: ?toVersion=3 reaches rollbackMetaItem as 3', async () => { - const { answer, protocol } = await rollback({ toVersion: '3' }); + const { answer, protocol } = await rollback({ query: { toVersion: '3' } }); expect(answer.status).toBe(200); expect(protocol.rollbackMetaItem).toHaveBeenCalledTimes(1); expect(protocol.rollbackMetaItem.mock.calls[0][0].toVersion).toBe(3); }); + + it('precedence is unchanged: a body version wins over ?toVersion=', async () => { + const { answer, protocol } = await rollback({ body: { toVersion: 2 }, query: { toVersion: '5' } }); + expect(answer.status).toBe(200); + expect(protocol.rollbackMetaItem.mock.calls[0][0].toVersion).toBe(2); + }); });