fix(rest)!: rollback accepts only a positive integer toVersion

.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
+
+<!-- adr-0087: not-required (no-migration-prescription) No metadata moves: no spec key, authorable spelling, object definition or stored shape is added, removed, renamed or re-shaped, so there is nothing for `objectstack migrate meta` to rewrite and no tombstone. No export, type, flag or error code is added, removed or renamed: the refusal is the door's existing `INVALID_REQUEST` with its existing message. What narrows is one REST door's accept set for a request value, and the remedy is the caller sending a JSON number. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id covers this door (not registered or already-registered); and nothing exported is removed or narrowed (not runtime-interface-only, type-surface-only or runtime-export-retired). -->
+
+**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}`.

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',

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<string, unknown>) => {
+describe('#25482 rollback `toVersion` — only a positive integer names a version', () => {
+    const rollback = async (req: { query?: Record<string, unknown>; 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);
+    });
 });