Skip to content

Commit 0fab7ba

Browse files
fix(rest)!: rollback accepts only a positive integer toVersion (#25532)
<!-- claude-projects-attribution: {"github_login":"os-sales"} --> _Requested by **Sales** · [project thread](https://claude.ai/code/project/chan_01D5bWFY3rFzgJcytD5gD4Uu?thread=cmsg_01D5bWFY3rFzgJcytD5gD4UuDWJsBGQpadCH8JR2xZQ5re)_ Fixes #25482 Clause-②: no (narrowing) Metadata-protocol: none ## What changed `POST /api/v1/meta/:type/:name/rollback` (`packages/rest/src/meta-item-history-routes.ts`) read its version with a bare `Number(...)`, so `{"toVersion": true}` restored v1, `[2]` and `"2"` restored v2, and `1.5` reached the protocol and answered `404 VERSION_NOT_FOUND`. - **JSON body** (`toVersion`, then the `version` alias): the value is judged as it came against `UNDECLARED_WHOLE_NUMBER_PARAM`, the same whole-number declaration `/diff` reads its versions against. A boolean, an array, a string, an object or a fraction does not parse. Then the door's existing `< 1` check applies. - **Query string** (`?toVersion=`): read through `readDeclaredQueryNumber` with that declaration, the reader `/diff` uses. No second number parser. Its thrown `VALIDATION_FAILED` is caught at the read and answered as this door's own refusal. - Every refusal is still the door's existing `400 INVALID_REQUEST` with the message `'toVersion' (positive integer) is required`. The precedence (`toVersion`, `version`, `?toVersion=`) and the gates in front of the read are unchanged. ### Why a numeric string in the body is now refused (the narrowing arm) The door declares no request schema. The only in-repo client, `@objectstack/client`'s `rollbackItem(type, name, toVersion: number)`, sends `JSON.stringify({ toVersion })` with a number. The console's caller, objectui `packages/data-objectstack/src/metadata-client.ts` `rollback(type, name, toVersion: number)` read at the pinned `.objectui-sha` `6fb31dcc99`, sends a JSON number too. No real client sends a string, so `"2"` in a JSON body is refused like the other non-numbers. A `?toVersion=3` query string is still read. That refusal of an input that used to be accepted is the `(narrowing)` arm. The changeset carries the BREAKING banner, the FROM/TO and the ADR-0087 disposition (`not-required (no-migration-prescription)`). ### The census ledger `rest-server-query-number-census.test.ts` ledgered the rollback's `Number(toVersionRaw)` as an `exempt` coercion. That site is gone, so its row is deleted and the pinned site count goes from 9 to 8. The row's own reason said that moving the door onto `VALIDATION_FAILED` would be a wire change, so the refusal stays `INVALID_REQUEST`. The exemption's three query pins (`abc`, blank, `Infinity`) and its lit control are kept in the new block. ## Tests Through the real `RestServer` route (`registerRoutes()` then the registered handler), in `packages/rest/src/rest-server-query-number-census.test.ts`, describe `#25482 rollback toVersion`: - Body `toVersion` set to `true`, `[2]`, `"2"`, `1.5`, `0`, `-1`, `"abc"` or `{}` answers `400`, `code: INVALID_REQUEST`, and `rollbackMetaItem` is never called. - The `version` alias set to `true`, `[2]`, `1.5` or `0` gets the same answer. - `?toVersion=` set to `abc`, blank, `Infinity`, `1.5`, `0` or `-1` gets the same answer, and so does an absent version. - Lit controls: body `{toVersion: 2}` reaches `rollbackMetaItem` as `2` with `200`, the alias `{version: 4}` reaches it as `4`, `?toVersion=3` reaches it as `3`, and a body version wins over `?toVersion=`. All run at head `e03bd96c3a`: - `pnpm --filter @objectstack/rest exec vitest run --maxWorkers=2 src/rest-server-query-number-census.test.ts src/rest-server-query-multiplicity.test.ts` passed 2/2 files and 67/67 tests (lock `VERDICT command-exit 0`). - `pnpm --filter @objectstack/rest typecheck && pnpm --filter @objectstack/rest test`: typecheck clean, then 316 files passed, 6347 tests passed and 326 skipped (lock `VERDICT command-exit 0`). - `pnpm lint` exited 0, and `pnpm check:type-check-debt` exited 0 (re-measured, none above its recorded number). Both ran under the lock. - Ablation, run once with the fix committed. `scripts/ablation-replace.mjs` swapped the body judgement for `Number.isFinite(Number(bodyVersion)) ? Number(bodyVersion) : undefined`. The anchor count went 1 to 0, the replacement count 0 to 1, and the blob `cd8508485ed8` became `a1c834b191a0`. The `#25482` block then failed 7 tests and passed 16: body `true`, `[2]`, `"2"` and `1.5`, and the alias `true`, `[2]` and `1.5`. That is the expected red. A trap restored the file: its blob equals HEAD (`cd8508485ed8`) and `git diff HEAD` is empty. The test imports `./rest-server.js` from `src`, so no `dist` was involved. - Derived gates: `node scripts/pm/dispatch-gates.mjs --commands` named 67, and `--ran` reconciled them as 67 accounted for, 66 run and 1 NOT MEASURED. The one is `pnpm check:dual-build-cjs-loads` (exit 3, PREREQUISITE NOT MET: it reads built `dist/` for every package and this worktree built only the `@objectstack/rest` closure). CI's `Lint & Repo Gates` runs it. All other 66 exited 0. - Dogfood battery (the `(narrowing)` arm requires it): `pnpm --filter '@objectstack/dogfood^...' build && pnpm --filter @objectstack/dogfood test`, run under the lock at `e03bd96c3a` with no shard and no path. Lock `VERDICT command-exit 0`. 290 test files passed and 3 skipped (293). 2494 tests passed and 18 skipped (2512). No failures. ## Acceptance notes - `rest-server-query-number-census.test.ts`'s module header still says "ten sites in the whole file today", which was already stale before this change. Not touched here. --- 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01CxBKycaiUpA1yGx1XyoGqX Co-authored-by: Claude <noreply@anthropic.com>
1 parent c15fd9a commit 0fab7ba

3 files changed

Lines changed: 115 additions & 22 deletions

File tree

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
'@objectstack/rest': patch
3+
---
4+
5+
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
6+
7+
Clause-②: no (narrowing)
8+
Metadata-protocol: none
9+
10+
<!-- 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). -->
11+
12+
**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.
13+
14+
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`.
15+
16+
- **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`.
17+
- **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.
18+
- **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.
19+
- **Fix:** send `toVersion` as a JSON number, for example `{"toVersion": 2}`.

‎packages/rest/src/meta-item-history-routes.ts‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -797,7 +797,7 @@ export function registerMetaItemHistoryRoutes(this: MetaItemHistoryRouteHost, sc
797797
});
798798
return;
799799
}
800-
// [#6877] The `Number(...)` below already refuses an array
800+
// [#6877] The version read below already refuses an array
801801
// — but as `INVALID_REQUEST` "'toVersion' (positive integer)
802802
// is required", which tells a caller who supplied two valid
803803
// integers that they supplied none. Refusing the multiplicity
@@ -806,9 +806,33 @@ export function registerMetaItemHistoryRoutes(this: MetaItemHistoryRouteHost, sc
806806
if (refuseUnknownQueryParams(req, res, META_ITEM_ROLLBACK_PARAMS)) return;
807807
if (refuseRepeatedQueryParams(req, res, ['toVersion', META_ITEM_MEMBER_PARAM])) return;
808808
const body = (req.body && typeof req.body === 'object') ? req.body : {};
809-
const toVersionRaw = body.toVersion ?? body.version ?? req.query?.toVersion;
810-
const toVersion = Number(toVersionRaw);
811-
if (!Number.isFinite(toVersion) || toVersion < 1) {
809+
// [#25482] `toVersion` names a history row, so only a positive
810+
// integer is a version. A bare `Number(...)` here coerced
811+
// whatever came — `true` restored v1, `[2]` and `"2"` restored
812+
// v2, and `1.5` reached the protocol and answered `404` — on the
813+
// one door that restores a caller-supplied version as live. The
814+
// JSON body is typed, so its value is judged AS IT CAME against
815+
// the same whole-number declaration `/diff` reads (the client
816+
// SDK's `rollbackItem` and the console's metadata client both
817+
// send a JSON number); only the untyped query string goes
818+
// through `readDeclaredQueryNumber`'s faithful coercion.
819+
// Precedence is unchanged — `toVersion`, then the `version`
820+
// alias, then `?toVersion=` — and every refusal stays this
821+
// door's own `INVALID_REQUEST`.
822+
const bodyVersion = body.toVersion ?? body.version;
823+
let toVersion: number | undefined;
824+
if (bodyVersion !== undefined && bodyVersion !== null) {
825+
const parsed = UNDECLARED_WHOLE_NUMBER_PARAM.safeParse(bodyVersion);
826+
toVersion = parsed.success ? parsed.data : undefined;
827+
} else {
828+
try {
829+
toVersion = readDeclaredQueryNumber(req.query, 'toVersion',
830+
UNDECLARED_WHOLE_NUMBER_PARAM, { emptyIsAbsent: false });
831+
} catch {
832+
toVersion = undefined;
833+
}
834+
}
835+
if (toVersion === undefined || toVersion < 1) {
812836
res.status(400).json({
813837
error: `'toVersion' (positive integer) is required`,
814838
code: 'INVALID_REQUEST',

‎packages/rest/src/rest-server-query-number-census.test.ts‎

Lines changed: 68 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -183,16 +183,6 @@ const LEDGER: readonly LedgerRow[] = [
183183
reason: 'A counter column of a persisted `sys_import_job` row, read back from the store and '
184184
+ 'mapped to the ImportJobProgress DTO — no request value reaches it.',
185185
})),
186-
{
187-
site: 'POST ${metaPath}/:type/:name/rollback » Number(toVersionRaw)',
188-
disposition: 'exempt',
189-
reason: 'Read body-first (`body.toVersion ?? body.version ?? req.query.toVersion`) and then '
190-
+ 'CHECKED, not served: a non-finite or below-1 result is already refused `400` before '
191-
+ '`rollbackMetaItem` runs, so an unreadable value is never dropped or substituted — '
192-
+ 'outside this family by its own definition. Its refusal is the door\'s own '
193-
+ '`INVALID_REQUEST`; moving it onto `VALIDATION_FAILED` would be a wire change to a door '
194-
+ 'that already refuses, which no card here decides. Pinned below.',
195-
},
196186
{
197187
site: 'GET ${dataPath}/:object/export » Number(q.page)',
198188
disposition: 'exempt',
@@ -306,7 +296,7 @@ describe('#20139 census — every numeric coercion in rest-server.ts is classifi
306296
}
307297
});
308298

309-
it('positive control: the census sees the reader and both exemptions in the real file', () => {
299+
it('positive control: the census sees the reader and every exemption in the real file', () => {
310300
const keys = new Set(sites.map((s) => s.key));
311301
for (const row of LEDGER.filter((r) => r.disposition !== 'not-a-query-value')) {
312302
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
361351
// `meta-item-history-routes.ts`): both read their query as strings and coerce
362352
// no number of it:
363353
// 9 = 7 + 1 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 1 + 0, length 17 → 18.
354+
// [#25482] The rollback's `Number(toVersionRaw)` is gone: its body value is
355+
// judged as it came and its query value goes through the reader, so its
356+
// exemption row left the ledger with it:
357+
// 8 = 7 + 1 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0, length 18.
364358
expect(REST_SERVER_REGISTRAR_FILES).toHaveLength(18);
365359
expect(SOURCES.map(([file]) => file)).toEqual([...REST_SERVER_REGISTRAR_FILES]);
366-
expect(sites).toHaveLength(9);
360+
expect(sites).toHaveLength(8);
367361
});
368362
});
369363

@@ -493,31 +487,87 @@ describe('#20139 census — exemption: export `?page=` changes chunking, never t
493487
});
494488
});
495489

496-
describe('#20139 census — exemption: rollback `toVersion` is already refused, never substituted', () => {
497-
const rollback = async (query: Record<string, unknown>) => {
490+
describe('#25482 rollback `toVersion` — only a positive integer names a version', () => {
491+
const rollback = async (req: { query?: Record<string, unknown>; body?: unknown }) => {
498492
const { drive, protocol } = boot();
499493
const answer = await drive('POST', '/api/v1/meta/:type/:name/rollback', {
500494
params: { type: 'view', name: 'all_accounts' },
501-
query,
495+
query: req.query ?? {},
496+
body: req.body ?? {},
502497
});
503498
return { answer, protocol };
504499
};
505500

501+
it.each([
502+
[true], // was `Number(true)` = 1 → restored v1
503+
[[2]], // was `Number([2])` = 2 → restored v2
504+
['2'], // a JSON string is not a JSON number
505+
[1.5], // was forwarded → `404 VERSION_NOT_FOUND`
506+
[0],
507+
[-1],
508+
['abc'],
509+
[{}],
510+
])('body { toVersion: %j } answers 400 INVALID_REQUEST and nothing is restored', async (toVersion) => {
511+
const { answer, protocol } = await rollback({ body: { toVersion } });
512+
expect(answer.status).toBe(400);
513+
expect(answer.body?.code).toBe('INVALID_REQUEST');
514+
expect(protocol.rollbackMetaItem).not.toHaveBeenCalled();
515+
});
516+
517+
it.each([[true], [[2]], [1.5], [0]])(
518+
'the `version` alias is judged the same way: body { version: %j } answers 400',
519+
async (version) => {
520+
const { answer, protocol } = await rollback({ body: { version } });
521+
expect(answer.status).toBe(400);
522+
expect(answer.body?.code).toBe('INVALID_REQUEST');
523+
expect(protocol.rollbackMetaItem).not.toHaveBeenCalled();
524+
},
525+
);
526+
506527
it.each([
507528
['abc'], // `NaN` → refused
508-
[''], // `Number('')` = 0 → below 1 → refused
529+
[''], // blank → refused, never read as 0 or absent-then-defaulted
509530
['Infinity'], // non-finite → refused
531+
['1.5'],
532+
['0'],
533+
['-1'],
510534
])('?toVersion=%j answers 400 INVALID_REQUEST and nothing is restored', async (toVersion) => {
511-
const { answer, protocol } = await rollback({ toVersion });
535+
const { answer, protocol } = await rollback({ query: { toVersion } });
536+
expect(answer.status).toBe(400);
537+
expect(answer.body?.code).toBe('INVALID_REQUEST');
538+
expect(protocol.rollbackMetaItem).not.toHaveBeenCalled();
539+
});
540+
541+
it('an absent version answers 400 INVALID_REQUEST', async () => {
542+
const { answer, protocol } = await rollback({});
512543
expect(answer.status).toBe(400);
513544
expect(answer.body?.code).toBe('INVALID_REQUEST');
514545
expect(protocol.rollbackMetaItem).not.toHaveBeenCalled();
515546
});
516547

548+
it('lit control: body { toVersion: 2 } reaches rollbackMetaItem as 2', async () => {
549+
const { answer, protocol } = await rollback({ body: { toVersion: 2 } });
550+
expect(answer.status).toBe(200);
551+
expect(protocol.rollbackMetaItem).toHaveBeenCalledTimes(1);
552+
expect(protocol.rollbackMetaItem.mock.calls[0][0].toVersion).toBe(2);
553+
});
554+
555+
it('lit control: body { version: 4 } (the alias) reaches rollbackMetaItem as 4', async () => {
556+
const { answer, protocol } = await rollback({ body: { version: 4 } });
557+
expect(answer.status).toBe(200);
558+
expect(protocol.rollbackMetaItem.mock.calls[0][0].toVersion).toBe(4);
559+
});
560+
517561
it('lit control: ?toVersion=3 reaches rollbackMetaItem as 3', async () => {
518-
const { answer, protocol } = await rollback({ toVersion: '3' });
562+
const { answer, protocol } = await rollback({ query: { toVersion: '3' } });
519563
expect(answer.status).toBe(200);
520564
expect(protocol.rollbackMetaItem).toHaveBeenCalledTimes(1);
521565
expect(protocol.rollbackMetaItem.mock.calls[0][0].toVersion).toBe(3);
522566
});
567+
568+
it('precedence is unchanged: a body version wins over ?toVersion=', async () => {
569+
const { answer, protocol } = await rollback({ body: { toVersion: 2 }, query: { toVersion: '5' } });
570+
expect(answer.status).toBe(200);
571+
expect(protocol.rollbackMetaItem.mock.calls[0][0].toVersion).toBe(2);
572+
});
523573
});

0 commit comments

Comments
 (0)