Skip to content

Commit fa48973

Browse files
hotlongclaude
andauthored
fix(runtime): enforce ListRunsRequestSchema's declared limit range (#8054) (#8204)
`parseIntegerParam` gains optional (min, max) bounds, threaded through from `ListRunsRequestSchema.shape.limit`'s own `.min()`/`.max()` at the one call site that declares a range (GET /automation/:name/runs), rather than re-listing (1, 100) as literals. `?limit=0`/`-5` no longer silently answer "no runs", and `?limit=101` is no longer served uncapped -- both now refused as 400 VALIDATION_FAILED with the ADR-0114 min_value/max_value field code. Callers that pass no bounds (e.g. notifications.ts) are unaffected. Claude-Session: https://claude.ai/code/session_01B3Kurx8qufrDzNjk4rag7V Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6df5135 commit fa48973

4 files changed

Lines changed: 194 additions & 37 deletions

File tree

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
---
2+
"@objectstack/runtime": patch
3+
---
4+
5+
fix(runtime): `GET /automation/:name/runs?limit=` now enforces its own declared 1..100 range (#8054)
6+
7+
`ListRunsRequestSchema.limit` has always declared `.min(1).max(100)`, but the
8+
boundary that reads it (`parseIntegerParam`) only checked that the value was a
9+
whole number, never that it fell inside the declared range. Two measured
10+
symptoms, both a `200` with the wrong answer:
11+
12+
- `?limit=0` (and any negative value) reached the engine as-is, and
13+
`store.listHistory(flowName, 0).slice(0, 0)` is `[]` — a confidently wrong
14+
"this flow has never run", the same shape #7300 fixed for `?limit=abc`, but
15+
produced by a value that *was* a valid integer.
16+
- `?limit=101` reached the engine uncapped, so the declared upper bound was
17+
decorative.
18+
19+
`parseIntegerParam` gains an optional third `bounds` argument
20+
(`{ min?, max? }`); every existing caller that omits it is byte-for-byte
21+
unaffected — range enforcement is opt-in, per call site. The one call site with
22+
a declared range (`GET /automation/:name/runs`) now threads
23+
`ListRunsRequestSchema.shape.limit`'s own `.min()`/`.max()` through, rather than
24+
re-listing `(1, 100)` as literals — the #7359 discipline
25+
(`ExecutionStatus.options`) applied to a bounded number instead of a closed set,
26+
so the wire's declared range and the boundary's enforced range cannot drift
27+
apart the next time the schema's bounds change.
28+
29+
A value outside the range is refused in the same house shape as everything else
30+
in this module: `400` `VALIDATION_FAILED` (ADR-0112) with a `details.fields[]`
31+
entry carrying the ADR-0114 field code the property names already mirror —
32+
`min_value` below 1, `max_value` above 100. Both declared boundary values
33+
(`?limit=1`, `?limit=100`) and every ordinary in-range value stay exactly as
34+
they were.

‎packages/runtime/src/domains/automation-runs-query-validation.test.ts‎

Lines changed: 73 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,24 @@
11
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
22

33
/**
4-
* #7300 / #7359 — `GET /api/v1/automation/:name/runs`'s query parameters, at
5-
* the boundary that reads them.
4+
* #7300 / #7359 / #8054 — `GET /api/v1/automation/:name/runs`'s query
5+
* parameters, at the boundary that reads them.
66
*
77
* #7300 (below) closed the two parameters this handler already forwarded but
8-
* COERCED. #7359 closed the third, which is the same 200-with-the-wrong-answer
9-
* arrived at from the opposite direction: `status` was declared by
8+
* COERCED. #7359 closed a third shape: `status` was declared by
109
* `ListRunsRequestSchema`, had no slot on `IAutomationService.listRuns`, and
1110
* was never built into the handler's option object — so `?status=failed` was
1211
* dropped here in silence and the caller was answered with EVERY run of the
1312
* flow. #7300 deliberately pinned that ignore-the-key behaviour rather than
14-
* decide it; #7359 took the enforce route, so that one pin is superseded here
15-
* by cases asserting the opposite on the same input.
13+
* decide it; #7359 took the enforce route, so that one pin was superseded by
14+
* cases asserting the opposite on the same input. #8054 is the sibling of
15+
* #7359 on the SAME route's OTHER declared constraint: `limit` was already
16+
* type-checked (#7300) but its declared RANGE (`.min(1).max(100)`) was never
17+
* read, so `?limit=0` answered 200 with zero rows — "this flow has never
18+
* run", confidently, about a flow with runs — and `?limit=101` reached the
19+
* engine with its cap simply not applied. The `?limit=1000`/`?limit=-5`/
20+
* `?limit=0` preservation rows #7300 pinned are superseded here the same way
21+
* #7359 superseded the `status`-ignored case: same input, opposite behaviour.
1622
*
1723
* The filed defect is character-for-character #6928's, one file over:
1824
* `{ limit: query.limit ? Number(query.limit) : undefined, cursor: query.cursor }`.
@@ -35,10 +41,14 @@
3541
* absence of a throw, which is not the defect. The defect is the missing
3642
* envelope.
3743
* 2. PRESERVATION — every value that had a defensible answer before keeps it,
38-
* byte for byte, at the exact `listRuns(name, options)` call. That includes
39-
* out-of-RANGE numbers (`?limit=1000`), which `ListRunsRequestSchema` bounds
40-
* and the engine slices by: range is the service's declared business and
41-
* stays reachable, unrefused.
44+
* byte for byte, at the exact `listRuns(name, options)` call. As of #8054
45+
* that no longer includes out-of-RANGE numbers (`?limit=1000`, `?limit=0`):
46+
* `ListRunsRequestSchema` bounds `limit` to 1..100 and the boundary now
47+
* enforces that declared range instead of only the value's type, so those
48+
* inputs moved from PRESERVATION to REFUSAL. An ORDINARY in-range value
49+
* (`?limit=25`) and both declared boundary values (`?limit=1`,
50+
* `?limit=100`) still keep their defensible answer — the over-block guard
51+
* for the new range check.
4252
*
4353
* The wire mapping of the thrown shape to `400` + `details.fields[]` is not
4454
* re-proved here — it is one mapping for every domain handler, pinned at both
@@ -197,6 +207,42 @@ describe('#7359 — a `?status=` outside the declared set is refused, not silent
197207
});
198208
});
199209

210+
describe('#8054 — a `?limit=` outside the declared 1..100 range is refused, not silently answered', () => {
211+
// Measured, twice, identical both passes: `?limit=0` answered 200 with
212+
// ZERO rows (a confidently wrong "this flow has never run" — the store
213+
// sliced `.slice(0, 0)`), and `?limit=101` answered 200 with the cap
214+
// simply not applied. `ListRunsRequestSchema` had declared `.min(1).max(100)`
215+
// the whole time; this boundary just never read it. Once the range is
216+
// enforced there is no safe reading for a value outside it — same
217+
// reasoning #7359 already applied to `status`, on a bounded number instead
218+
// of a closed set.
219+
it.each([
220+
['0 (the "no runs" trap)', '0', 'min_value'],
221+
['-5 (negative)', '-5', 'min_value'],
222+
['101 (one past the declared cap)', '101', 'max_value'],
223+
['1000 (far past the declared cap — the old preserved case, inverted)', '1000', 'max_value'],
224+
])('refuses ?limit=%s with 400 VALIDATION_FAILED (%s)', async (_label, raw, expectedCode) => {
225+
const { details, status, listRuns } = await refusalFor({ limit: raw });
226+
227+
// ADR-0112: the envelope, not merely the throw — `code` AND `status`.
228+
expect(details?.code).toBe('VALIDATION_FAILED');
229+
expect(status).toBe(400);
230+
// ADR-0114: `min_value`/`max_value` are the field codes the property
231+
// names already mirror — no new vocabulary minted for this.
232+
expect(details?.fields).toEqual([
233+
{ field: 'limit', code: expectedCode, message: expect.stringContaining('`limit`') },
234+
]);
235+
// The whole point: the service is never reached with a limit outside
236+
// its own declared contract, so no caller reads a wrong-but-confident
237+
// "no runs" and no caller gets an uncapped result set.
238+
expect(listRuns).not.toHaveBeenCalled();
239+
});
240+
241+
// The boundary values themselves — `?limit=1` and `?limit=100` — are
242+
// pinned as VALID in the `#7300` preservation block below (they were
243+
// always in range and stay unaffected), so they are not repeated here.
244+
});
245+
200246
describe('#7300 — every value that had a defensible answer keeps it', () => {
201247
async function listWith(query: Record<string, unknown> | undefined) {
202248
const { dispatcher, listRuns } = makeDispatcher();
@@ -207,19 +253,26 @@ describe('#7300 — every value that had a defensible answer keeps it', () => {
207253
it.each([
208254
// [label, query, the exact options object `listRuns` must receive]
209255
['?limit=20', { limit: '20' }, { limit: 20, cursor: undefined, status: undefined }],
256+
// An ordinary in-range value is the over-block guard for #8054: bounds
257+
// threading must not start refusing numbers that were always fine.
258+
['?limit=25 (ordinary, mid-range)', { limit: '25' }, { limit: 25, cursor: undefined, status: undefined }],
210259
['?limit=1 (the low boundary)', { limit: '1' }, { limit: 1, cursor: undefined, status: undefined }],
211260
['?limit=100 (the declared high boundary)', { limit: '100' }, { limit: 100, cursor: undefined, status: undefined }],
212-
// Out of RANGE is not out of DOMAIN. `ListRunsRequestSchema` bounds
213-
// `limit` to 1..100 and the engine slices by whatever it is handed;
214-
// neither answer is this boundary's to change, so both still arrive.
215-
['?limit=1000 (over the declared range)', { limit: '1000' }, { limit: 1000, cursor: undefined, status: undefined }],
216-
['?limit=-5 (under it)', { limit: '-5' }, { limit: -5, cursor: undefined, status: undefined }],
217-
// Falsy spellings meant "no limit here" before this gate existed and
218-
// still do — they must not become a new 400. `'0'` is NOT one of them:
219-
// the string is truthy, so `query.limit ? Number(query.limit) : …` read
220-
// it as the number `0` and passed it on, and that is preserved too.
261+
// Out-of-RANGE numbers used to be preserved here (`?limit=1000`,
262+
// `?limit=-5`, `?limit=0`) on the theory that range was the engine's
263+
// declared business, not this boundary's. #8054 found the one place
264+
// that reasoning was wrong: `ListRunsRequestSchema` had ALWAYS
265+
// declared `limit`'s range, and nothing enforced it, so `?limit=0`
266+
// answered "no runs" and `?limit=101` reached the engine uncapped.
267+
// Those three rows are superseded by the `#8054` refusal block below
268+
// rather than deleted outright — same input, opposite behaviour now.
269+
//
270+
// Falsy spellings still mean "no limit here", unaffected by bounds
271+
// because the falsy gate runs BEFORE the bounds check: absent, `null`,
272+
// `''`, and an in-process (non-string) `0` never reach it. `'0'` as a
273+
// QUERY-STRING value is different — the string is truthy, so it always
274+
// reached `Number()` — and is exercised in the `#8054` block instead.
221275
['?limit= (empty)', { limit: '' }, { limit: undefined, cursor: undefined, status: undefined }],
222-
['?limit=0', { limit: '0' }, { limit: 0, cursor: undefined, status: undefined }],
223276
['limit: 0 (in-process number)', { limit: 0 }, { limit: undefined, cursor: undefined, status: undefined }],
224277
['limit: null', { limit: null }, { limit: undefined, cursor: undefined, status: undefined }],
225278
['no parameters at all', {}, { limit: undefined, cursor: undefined, status: undefined }],

‎packages/runtime/src/domains/automation.ts‎

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
validationFailure, validationFailureDetails, fieldsFromZodIssues, VALIDATION_FAILED_STATUS,
2020
} from '../validation-failure.js';
2121
import { ExecutionStatus } from '@objectstack/spec/automation';
22+
import { ListRunsRequestSchema } from '@objectstack/spec/api';
2223
import { parseEnumParam, parseIntegerParam, parseStringParam } from '../query-param.js';
2324
import { capabilityUnavailable } from './unavailable.js';
2425
import type { HttpProtocolContext, HttpDispatcherResult } from '../http-dispatcher.js';
@@ -817,11 +818,29 @@ export async function handleAutomationRequest(deps: DomainHandlerDeps, path: str
817818
// first implementation that starts honouring cursors must not
818819
// be the one that discovers the type was never enforced.
819820
//
820-
// Out-of-range numbers are NOT refused — `?limit=1000` still
821-
// reaches the engine as 1000 and is sliced there. Range is the
822-
// service's declared business (`ListRunsRequestSchema` bounds it
823-
// 1..100); this gate only refuses values that were never whole
824-
// numbers.
821+
// [#8054] `limit`'s RANGE — `ListRunsRequestSchema` has always
822+
// declared `.min(1).max(100)`, and until now this gate only
823+
// checked that the value was a whole number at all, never that
824+
// it fell inside that declared range. `?limit=0` reached the
825+
// engine as 0, and `store.listHistory(flowName, 0).slice(0, 0)`
826+
// is `[]` — a confidently wrong "this flow has never run",
827+
// exactly #7300's shape but from a value that WAS a valid
828+
// integer. `?limit=101` reached the engine uncapped, so the
829+
// declared upper bound was decorative.
830+
//
831+
// The bounds are READ off `ListRunsRequestSchema.shape.limit`
832+
// rather than re-listed as `(1, 100)` here — the same
833+
// discipline `status` already applies via
834+
// `ExecutionStatus.options` two lines down. Re-listing the
835+
// literals would make the boundary correct today and silently
836+
// wrong again the moment the schema's own `.min()`/`.max()`
837+
// changes; reading them makes declared == enforced true by
838+
// construction, not by two call sites happening to agree.
839+
//
840+
// A value outside the range is refused in the same house shape
841+
// as everything else in this module — `VALIDATION_FAILED` with
842+
// an ADR-0114 field code, here `min_value` / `max_value`, the
843+
// ones the property names already mirror.
825844
//
826845
// [#7359] `status` is the THIRD declared parameter, and until
827846
// now the only one this handler never read. `ListRunsRequestSchema`
@@ -840,9 +859,13 @@ export async function handleAutomationRequest(deps: DomainHandlerDeps, path: str
840859
// rather than a list copied into this file: the wire schema is
841860
// built from that same enum, so a future member cannot be
842861
// accepted by one and refused by the other.
862+
const limitBounds = ListRunsRequestSchema.shape.limit.unwrap();
843863
const options = query
844864
? {
845-
limit: parseIntegerParam('limit', query.limit),
865+
limit: parseIntegerParam('limit', query.limit, {
866+
min: limitBounds.minValue ?? undefined,
867+
max: limitBounds.maxValue ?? undefined,
868+
}),
846869
cursor: parseStringParam('cursor', query.cursor),
847870
status: parseEnumParam('status', query.status, ExecutionStatus.options),
848871
}

‎packages/runtime/src/query-param.ts‎

Lines changed: 58 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,23 @@
3939
* spelling either: ADR-0112's registered `VALIDATION_FAILED` and ADR-0114's
4040
* closed field-level catalog (`FieldErrorCode`) already say all of this.
4141
*
42-
* What these parsers deliberately do NOT do is police RANGE. A value that is
42+
* What these parsers do NOT do, by default, is police RANGE. A value that is
4343
* out of range but in domain (`?limit=1000`) is the service's declared business
4444
* — the notifications inbox clamps it, the automation engine slices by it — and
4545
* a boundary that started refusing those would be changing an answer that was
46-
* already defensible. These gates add a refusal for values that were never of
47-
* the declared type at all.
46+
* already defensible. So range enforcement is opt-in, per call site
47+
* ({@link parseIntegerParam}'s `bounds`), read off the call site's own
48+
* declared schema rather than re-listed — never switched on module-wide.
49+
*
50+
* #8054 is that opt-in's origin case, and the same shape as #7359 one field
51+
* over: `ListRunsRequestSchema` had always declared `limit`'s range
52+
* (`.min(1).max(100)`), and the boundary was not reading it —
53+
*
54+
* ?limit=0 → 200, zero rows → "this flow has never run", confidently
55+
* ?limit=101 → 200, cap ignored → a result-set size nothing had asked for
56+
*
57+
* — so that one call site now threads its own bounds through; every other
58+
* caller of `parseIntegerParam` is unaffected, because it passes none.
4859
*/
4960

5061
import type { FieldErrorCode } from '@objectstack/spec/api';
@@ -94,9 +105,26 @@ export function parseBooleanParam(param: string, raw: unknown): boolean | undefi
94105
throw invalidQueryParam(param, 'invalid_boolean', '`true` or `false`', raw);
95106
}
96107

108+
/**
109+
* Inclusive numeric bounds a call site may pass to {@link parseIntegerParam}
110+
* so it can police RANGE on top of type — read off the caller's own declared
111+
* schema (`ExistingSchema.shape.limit.unwrap().minValue` / `.maxValue`),
112+
* never re-listed as literals. That is the #7359 discipline
113+
* ({@link parseEnumParam} reading `ExecutionStatus.options`) applied to a
114+
* bounded number instead of a closed set: the wire's declared range and the
115+
* boundary's enforced range cannot drift, because they are the same read.
116+
*/
117+
export interface IntegerParamBounds {
118+
/** Inclusive lower bound — a value below it is refused as `min_value`. */
119+
readonly min?: number;
120+
/** Inclusive upper bound — a value above it is refused as `max_value`. */
121+
readonly max?: number;
122+
}
123+
97124
/**
98125
* A whole-number parameter — the window sizes both `?limit=` defects were filed
99-
* against (#6928, #7300).
126+
* against (#6928, #7300), and, once `bounds` is supplied, the RANGE defect
127+
* #8054 filed against the same parameter.
100128
*
101129
* `Number(query.limit)` answers `NaN` for `?limit=abc`, and NaN then survives
102130
* the guards downstream, because the two idioms services use to default a
@@ -108,22 +136,41 @@ export function parseBooleanParam(param: string, raw: unknown): boolean | undefi
108136
* REFUSED: values that are not a whole number at all — `abc`, `10abc`, `1.5`,
109137
* `Infinity`, a repeated `?limit=1&limit=2`, a structured value.
110138
*
111-
* NOT refused, deliberately: an out-of-RANGE number. Range is the consuming
112-
* service's declared contract (clamp, slice, or reject with its own message),
113-
* and this gate must not start answering 400 for a value that already had a
114-
* defensible answer.
139+
* RANGE (`bounds`) is opt-in, per call site, and OFF unless a bounds object is
140+
* passed — a caller that omits the third argument is byte-for-byte the
141+
* pre-#8054 gate: an out-of-range number (`?limit=1000`) reaches the service
142+
* unrefused, exactly as before, because range used to be nobody's job at this
143+
* boundary. #8054 found the one call site (`ListRunsRequestSchema`'s `limit`)
144+
* that HAD declared a range and was not enforcing it — `?limit=0` answered
145+
* "no runs" with a 200, and `?limit=101` was served uncapped — so that call
146+
* site now threads its own `.min()`/`.max()` through as `bounds`, and a value
147+
* outside them is refused with the ADR-0114 field code the property name
148+
* already mirrors (`min_value` / `max_value`), the same shape
149+
* {@link parseEnumParam}'s `invalid_option` refusal takes for `status`.
115150
*
116151
* The falsy gate is the one both call sites already had
117152
* (`query.limit ? Number(query.limit) : undefined`): absent, `null`, `''` and
118-
* `0` have always meant "no limit here", and they keep meaning that instead of
119-
* becoming a new 400.
153+
* an in-process (non-string) `0` have always meant "no limit here", and they
154+
* keep meaning that — checked BEFORE `bounds`, so they never become a new 400
155+
* even when a `min` above 0 is supplied. Only a numeric STRING (`?limit=0`,
156+
* truthy as a string) reaches the bounds check.
120157
*/
121-
export function parseIntegerParam(param: string, raw: unknown): number | undefined {
158+
export function parseIntegerParam(
159+
param: string,
160+
raw: unknown,
161+
bounds?: IntegerParamBounds,
162+
): number | undefined {
122163
if (!raw) return undefined;
123164
const parsed = Number(raw);
124165
if (!Number.isInteger(parsed)) {
125166
throw invalidQueryParam(param, 'invalid_number', 'a whole number', raw);
126167
}
168+
if (bounds?.min !== undefined && parsed < bounds.min) {
169+
throw invalidQueryParam(param, 'min_value', `a whole number >= ${bounds.min}`, raw);
170+
}
171+
if (bounds?.max !== undefined && parsed > bounds.max) {
172+
throw invalidQueryParam(param, 'max_value', `a whole number <= ${bounds.max}`, raw);
173+
}
127174
return parsed;
128175
}
129176

0 commit comments

Comments
 (0)