fix(runtime): enforce ListRunsRequestSchema's declared limit range (#8054)

.changeset/list-runs-limit-range.md

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

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

@@ -1,18 +1,24 @@
 // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
 
 /**
- * #7300 / #7359 — `GET /api/v1/automation/:name/runs`'s query parameters, at
- * the boundary that reads them.
+ * #7300 / #7359 / #8054 — `GET /api/v1/automation/:name/runs`'s query
+ * parameters, at the boundary that reads them.
  *
  * #7300 (below) closed the two parameters this handler already forwarded but
- * COERCED. #7359 closed the third, which is the same 200-with-the-wrong-answer
- * arrived at from the opposite direction: `status` was declared by
+ * COERCED. #7359 closed a third shape: `status` was declared by
  * `ListRunsRequestSchema`, had no slot on `IAutomationService.listRuns`, and
  * was never built into the handler's option object — so `?status=failed` was
  * dropped here in silence and the caller was answered with EVERY run of the
  * flow. #7300 deliberately pinned that ignore-the-key behaviour rather than
- * decide it; #7359 took the enforce route, so that one pin is superseded here
- * by cases asserting the opposite on the same input.
+ * decide it; #7359 took the enforce route, so that one pin was superseded by
+ * cases asserting the opposite on the same input. #8054 is the sibling of
+ * #7359 on the SAME route's OTHER declared constraint: `limit` was already
+ * type-checked (#7300) but its declared RANGE (`.min(1).max(100)`) was never
+ * read, so `?limit=0` answered 200 with zero rows — "this flow has never
+ * run", confidently, about a flow with runs — and `?limit=101` reached the
+ * engine with its cap simply not applied. The `?limit=1000`/`?limit=-5`/
+ * `?limit=0` preservation rows #7300 pinned are superseded here the same way
+ * #7359 superseded the `status`-ignored case: same input, opposite behaviour.
  *
  * The filed defect is character-for-character #6928's, one file over:
  * `{ limit: query.limit ? Number(query.limit) : undefined, cursor: query.cursor }`.
@@ -35,10 +41,14 @@
  * absence of a throw, which is not the defect. The defect is the missing
  * envelope.
  * 2. PRESERVATION — every value that had a defensible answer before keeps it,
- * byte for byte, at the exact `listRuns(name, options)` call. That includes
- * out-of-RANGE numbers (`?limit=1000`), which `ListRunsRequestSchema` bounds
- * and the engine slices by: range is the service's declared business and
- * stays reachable, unrefused.
+ * byte for byte, at the exact `listRuns(name, options)` call. As of #8054
+ * that no longer includes out-of-RANGE numbers (`?limit=1000`, `?limit=0`):
+ * `ListRunsRequestSchema` bounds `limit` to 1..100 and the boundary now
+ * enforces that declared range instead of only the value's type, so those
+ * inputs moved from PRESERVATION to REFUSAL. An ORDINARY in-range value
+ * (`?limit=25`) and both declared boundary values (`?limit=1`,
+ * `?limit=100`) still keep their defensible answer — the over-block guard
+ * for the new range check.
  *
  * The wire mapping of the thrown shape to `400` + `details.fields[]` is not
  * 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
  });
 });
 
+describe('#8054 — a `?limit=` outside the declared 1..100 range is refused, not silently answered', () => {
+ // Measured, twice, identical both passes: `?limit=0` answered 200 with
+ // ZERO rows (a confidently wrong "this flow has never run" — the store
+ // sliced `.slice(0, 0)`), and `?limit=101` answered 200 with the cap
+ // simply not applied. `ListRunsRequestSchema` had declared `.min(1).max(100)`
+ // the whole time; this boundary just never read it. Once the range is
+ // enforced there is no safe reading for a value outside it — same
+ // reasoning #7359 already applied to `status`, on a bounded number instead
+ // of a closed set.
+ it.each([
+ ['0 (the "no runs" trap)', '0', 'min_value'],
+ ['-5 (negative)', '-5', 'min_value'],
+ ['101 (one past the declared cap)', '101', 'max_value'],
+ ['1000 (far past the declared cap — the old preserved case, inverted)', '1000', 'max_value'],
+ ])('refuses ?limit=%s with 400 VALIDATION_FAILED (%s)', async (_label, raw, expectedCode) => {
+ const { details, status, listRuns } = await refusalFor({ limit: raw });
+
+ // ADR-0112: the envelope, not merely the throw — `code` AND `status`.
+ expect(details?.code).toBe('VALIDATION_FAILED');
+ expect(status).toBe(400);
+ // ADR-0114: `min_value`/`max_value` are the field codes the property
+ // names already mirror — no new vocabulary minted for this.
+ expect(details?.fields).toEqual([
+ { field: 'limit', code: expectedCode, message: expect.stringContaining('`limit`') },
+ ]);
+ // The whole point: the service is never reached with a limit outside
+ // its own declared contract, so no caller reads a wrong-but-confident
+ // "no runs" and no caller gets an uncapped result set.
+ expect(listRuns).not.toHaveBeenCalled();
+ });
+
+ // The boundary values themselves — `?limit=1` and `?limit=100` — are
+ // pinned as VALID in the `#7300` preservation block below (they were
+ // always in range and stay unaffected), so they are not repeated here.
+});
+
 describe('#7300 — every value that had a defensible answer keeps it', () => {
  async function listWith(query: Record<string, unknown> | undefined) {
  const { dispatcher, listRuns } = makeDispatcher();
@@ -207,19 +253,26 @@ describe('#7300 — every value that had a defensible answer keeps it', () => {
  it.each([
  // [label, query, the exact options object `listRuns` must receive]
  ['?limit=20', { limit: '20' }, { limit: 20, cursor: undefined, status: undefined }],
+ // An ordinary in-range value is the over-block guard for #8054: bounds
+ // threading must not start refusing numbers that were always fine.
+ ['?limit=25 (ordinary, mid-range)', { limit: '25' }, { limit: 25, cursor: undefined, status: undefined }],
  ['?limit=1 (the low boundary)', { limit: '1' }, { limit: 1, cursor: undefined, status: undefined }],
  ['?limit=100 (the declared high boundary)', { limit: '100' }, { limit: 100, cursor: undefined, status: undefined }],
- // Out of RANGE is not out of DOMAIN. `ListRunsRequestSchema` bounds
- // `limit` to 1..100 and the engine slices by whatever it is handed;
- // neither answer is this boundary's to change, so both still arrive.
- ['?limit=1000 (over the declared range)', { limit: '1000' }, { limit: 1000, cursor: undefined, status: undefined }],
- ['?limit=-5 (under it)', { limit: '-5' }, { limit: -5, cursor: undefined, status: undefined }],
- // Falsy spellings meant "no limit here" before this gate existed and
- // still do — they must not become a new 400. `'0'` is NOT one of them:
- // the string is truthy, so `query.limit ? Number(query.limit) : …` read
- // it as the number `0` and passed it on, and that is preserved too.
+ // Out-of-RANGE numbers used to be preserved here (`?limit=1000`,
+ // `?limit=-5`, `?limit=0`) on the theory that range was the engine's
+ // declared business, not this boundary's. #8054 found the one place
+ // that reasoning was wrong: `ListRunsRequestSchema` had ALWAYS
+ // declared `limit`'s range, and nothing enforced it, so `?limit=0`
+ // answered "no runs" and `?limit=101` reached the engine uncapped.
+ // Those three rows are superseded by the `#8054` refusal block below
+ // rather than deleted outright — same input, opposite behaviour now.
+ //
+ // Falsy spellings still mean "no limit here", unaffected by bounds
+ // because the falsy gate runs BEFORE the bounds check: absent, `null`,
+ // `''`, and an in-process (non-string) `0` never reach it. `'0'` as a
+ // QUERY-STRING value is different — the string is truthy, so it always
+ // reached `Number()` — and is exercised in the `#8054` block instead.
  ['?limit= (empty)', { limit: '' }, { limit: undefined, cursor: undefined, status: undefined }],
- ['?limit=0', { limit: '0' }, { limit: 0, cursor: undefined, status: undefined }],
  ['limit: 0 (in-process number)', { limit: 0 }, { limit: undefined, cursor: undefined, status: undefined }],
  ['limit: null', { limit: null }, { limit: undefined, cursor: undefined, status: undefined }],
  ['no parameters at all', {}, { limit: undefined, cursor: undefined, status: undefined }],

packages/runtime/src/domains/automation.ts

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

packages/runtime/src/query-param.ts

@@ -39,12 +39,23 @@
  * spelling either: ADR-0112's registered `VALIDATION_FAILED` and ADR-0114's
  * closed field-level catalog (`FieldErrorCode`) already say all of this.
  *
- * What these parsers deliberately do NOT do is police RANGE. A value that is
+ * What these parsers do NOT do, by default, is police RANGE. A value that is
  * out of range but in domain (`?limit=1000`) is the service's declared business
  * — the notifications inbox clamps it, the automation engine slices by it — and
  * a boundary that started refusing those would be changing an answer that was
- * already defensible. These gates add a refusal for values that were never of
- * the declared type at all.
+ * already defensible. So range enforcement is opt-in, per call site
+ * ({@link parseIntegerParam}'s `bounds`), read off the call site's own
+ * declared schema rather than re-listed — never switched on module-wide.
+ *
+ * #8054 is that opt-in's origin case, and the same shape as #7359 one field
+ * over: `ListRunsRequestSchema` had always declared `limit`'s range
+ * (`.min(1).max(100)`), and the boundary was not reading it —
+ *
+ * ?limit=0 → 200, zero rows → "this flow has never run", confidently
+ * ?limit=101 → 200, cap ignored → a result-set size nothing had asked for
+ *
+ * — so that one call site now threads its own bounds through; every other
+ * caller of `parseIntegerParam` is unaffected, because it passes none.
  */
 
 import type { FieldErrorCode } from '@objectstack/spec/api';
@@ -94,9 +105,26 @@ export function parseBooleanParam(param: string, raw: unknown): boolean | undefi
  throw invalidQueryParam(param, 'invalid_boolean', '`true` or `false`', raw);
 }
 
+/**
+ * Inclusive numeric bounds a call site may pass to {@link parseIntegerParam}
+ * so it can police RANGE on top of type — read off the caller's own declared
+ * schema (`ExistingSchema.shape.limit.unwrap().minValue` / `.maxValue`),
+ * never re-listed as literals. That is the #7359 discipline
+ * ({@link parseEnumParam} reading `ExecutionStatus.options`) applied to a
+ * bounded number instead of a closed set: the wire's declared range and the
+ * boundary's enforced range cannot drift, because they are the same read.
+ */
+export interface IntegerParamBounds {
+ /** Inclusive lower bound — a value below it is refused as `min_value`. */
+ readonly min?: number;
+ /** Inclusive upper bound — a value above it is refused as `max_value`. */
+ readonly max?: number;
+}
+
 /**
  * A whole-number parameter — the window sizes both `?limit=` defects were filed
- * against (#6928, #7300).
+ * against (#6928, #7300), and, once `bounds` is supplied, the RANGE defect
+ * #8054 filed against the same parameter.
  *
  * `Number(query.limit)` answers `NaN` for `?limit=abc`, and NaN then survives
  * 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
  * REFUSED: values that are not a whole number at all — `abc`, `10abc`, `1.5`,
  * `Infinity`, a repeated `?limit=1&limit=2`, a structured value.
  *
- * NOT refused, deliberately: an out-of-RANGE number. Range is the consuming
- * service's declared contract (clamp, slice, or reject with its own message),
- * and this gate must not start answering 400 for a value that already had a
- * defensible answer.
+ * RANGE (`bounds`) is opt-in, per call site, and OFF unless a bounds object is
+ * passed — a caller that omits the third argument is byte-for-byte the
+ * pre-#8054 gate: an out-of-range number (`?limit=1000`) reaches the service
+ * unrefused, exactly as before, because range used to be nobody's job at this
+ * boundary. #8054 found the one call site (`ListRunsRequestSchema`'s `limit`)
+ * that HAD declared a range and was not enforcing it — `?limit=0` answered
+ * "no runs" with a 200, and `?limit=101` was served uncapped — so that call
+ * site now threads its own `.min()`/`.max()` through as `bounds`, and a value
+ * outside them is refused with the ADR-0114 field code the property name
+ * already mirrors (`min_value` / `max_value`), the same shape
+ * {@link parseEnumParam}'s `invalid_option` refusal takes for `status`.
  *
  * The falsy gate is the one both call sites already had
  * (`query.limit ? Number(query.limit) : undefined`): absent, `null`, `''` and
- * `0` have always meant "no limit here", and they keep meaning that instead of
- * becoming a new 400.
+ * an in-process (non-string) `0` have always meant "no limit here", and they
+ * keep meaning that — checked BEFORE `bounds`, so they never become a new 400
+ * even when a `min` above 0 is supplied. Only a numeric STRING (`?limit=0`,
+ * truthy as a string) reaches the bounds check.
  */
-export function parseIntegerParam(param: string, raw: unknown): number | undefined {
+export function parseIntegerParam(
+ param: string,
+ raw: unknown,
+ bounds?: IntegerParamBounds,
+): number | undefined {
  if (!raw) return undefined;
  const parsed = Number(raw);
  if (!Number.isInteger(parsed)) {
  throw invalidQueryParam(param, 'invalid_number', 'a whole number', raw);
  }
+ if (bounds?.min !== undefined && parsed < bounds.min) {
+ throw invalidQueryParam(param, 'min_value', `a whole number >= ${bounds.min}`, raw);
+ }
+ if (bounds?.max !== undefined && parsed > bounds.max) {
+ throw invalidQueryParam(param, 'max_value', `a whole number <= ${bounds.max}`, raw);
+ }
  return parsed;
 }