fix(rest): refuse a repeated ?filter= as a repetition, not as a malformed filter (#7390)

.changeset/repeated-filter-param-refusal.md

@@ -0,0 +1,60 @@
+---
+"@objectstack/rest": patch
+---
+
+fix(rest): a repeated `?filter=` on `GET /data/:object` is refused as a repetition, not misdiagnosed as a malformed filter (#7390)
+
+**This is a behaviour change on a live surface.** A request that previously
+answered `200` now answers `400`, and a request that already answered `400` now
+carries a different message. Both changes make the response describe what the
+caller actually did.
+
+Repeating a query parameter used to be invisible: the production Hono adapter
+collapsed repeats to the first value before any handler ran. Since #6878 route 2
+(PR #7396) it surfaces them as arrays, so a repeated `?filter=` now reaches the
+shared list-query normalizer — and that normalizer structurally cannot tell what
+it is looking at. A filter AST **is** an array (`["status","=","open"]`), so the
+arity gate #7386 added to every other query slot had to leave this one alone: on
+the filter slot, an array is the ordinary shape of a legitimate body-form filter
+sent to `POST /data/:object/query`.
+
+Two shapes came out of that, both live:
+
+| request | before | now |
+| :--- | :--- | :--- |
+| `?filter={"a":1}&filter={"b":2}` | `400 INVALID_FILTER`, diagnosed as a **malformed** filter | `400 INVALID_FILTER`, diagnosed as a **repetition** |
+| `?filter=status&filter=%3D&filter=open` | **`200`**, applying `{status:"open"}` | `400 INVALID_FILTER` |
+
+The first was the common one, and its message was actively misleading: both
+filters the caller sent were well-formed, the response told them to check their
+AST syntax, and the operator vocabulary it listed could not help. The second is
+contrived to write by hand but is the sharper defect — three occurrences of one
+parameter happened to spell a valid AST, so the request succeeded while applying
+a filter nobody expressed.
+
+The refusal now names the condition: `Repeated "filter" query parameter — send
+exactly one.` A repeated filter is **not** merged and **not** resolved by
+precedence — either would silently serve one of two intents the caller actually
+expressed, which is the authoring trap this refusal exists to close.
+
+The judgement is made at the REST querystring parse rather than in the shared
+normalizer, because the querystring layer is the only one that knows it is
+looking at a querystring: there, an array on the filter slot is a repeated
+parameter and can be nothing else. All four wire spellings of the one slot
+(`filter`, `where`, `filters`, `$filter`) are covered.
+
+**Unaffected:**
+
+- A **single** `?filter=` in either accepted form — the JSON object
+ (`?filter={"status":"open"}`) and the bare AST
+ (`?filter=["status","=","open"]`).
+- `POST /data/:object/query` — the body face legitimately sends an array, and is
+ untouched.
+- Genuinely multi-valued query parameters (`$select`, `$expand`,
+ `$searchFields`), which keep their array arm.
+- A one-element array from a repeat-preserving adapter, which is one occurrence
+ and is unwrapped rather than refused — this also stops it being read as a
+ malformed AST.
+
+No spec change: `INVALID_FILTER` is already a standard-catalog code, and the
+accepted wire forms of `filter` are unchanged.

packages/rest/src/query-multiplicity.ts

@@ -1,5 +1,7 @@
 // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
 
+import { RPC_QUERY_ALIAS_SLOTS } from '@objectstack/spec/data';
+
 /**
  * Query-parameter MULTIPLICITY, for every REST handler in this package (#6877).
  *
@@ -145,3 +147,110 @@ export function refuseRepeatedQueryParams(
  }
  return false;
 }
+
+/**
+ * Every WIRE spelling of the ONE filter slot, as `GET /data/:object` receives
+ * it (#7390).
+ *
+ * The canonical key and its schema-declared alias are read off the spec's own
+ * table — `RPC_QUERY_ALIAS_SLOTS`, "the ONE place the alias to canonical
+ * mapping is declared" — so a spelling added there reaches this gate without
+ * anybody remembering to copy it.
+ *
+ * `filters` and `$filter` are named here instead, because they are wire-only:
+ * no schema declares them, and `metadata-protocol` extends that same spec
+ * table with them for exactly that reason. Deriving them was not an option —
+ * `@objectstack/metadata-protocol` is a dev-only dependency of this package,
+ * so no runtime import of its table exists. Gating fewer than all four would
+ * leave three quarters of one slot misdiagnosed, which is the defect, not a
+ * narrower version of the fix. `filterSlotSpellingsAreComplete` in
+ * `rest-server-repeated-filter-param.test.ts` pins the composition so a spec
+ * table that loses `where` goes red here rather than silently ungating a
+ * spelling.
+ */
+export const FILTER_SLOT_QUERY_PARAMS: readonly string[] = (() => {
+ const slot = RPC_QUERY_ALIAS_SLOTS.find((s) => s.canonical === 'where');
+ return [...(slot ? [slot.canonical, ...slot.aliases] : []), 'filters', '$filter'];
+})();
+
+/**
+ * The one refusal message for a repeated filter parameter.
+ *
+ * It names REPETITION, and that is the whole point of #7390 rather than a
+ * wording preference. Until this gate existed the same request was answered by
+ * `malformedFilterArrayError` in the normalizer — a 400 whose text told the
+ * caller their filter was *malformed*, listing the AST operator vocabulary,
+ * when every filter they sent was well-formed and the mistake was sending two.
+ * A caller reading that message re-checks their syntax, which is the one thing
+ * that cannot help them.
+ *
+ * It also does not offer a resolution, because there is none to offer
+ * (maintainer ruling, 2026-08-11): last-wins and AND-merge were both rejected
+ * as silent selection among duplicates.
+ */
+export function repeatedFilterParamMessage(name: string, count: number): string {
+ return `Repeated "${name}" query parameter — send exactly one. It was supplied ${count} times. `
+ + 'A repeated filter is neither merged nor resolved by precedence: either would apply a '
+ + 'filter you did not express.';
+}
+
+/**
+ * Refuse a repeated filter parameter on a QUERYSTRING ingress (#7390).
+ *
+ * ## Why this rule cannot live in the shared normalizer
+ *
+ * `metadata-protocol`'s list-query normalizer serves two ingresses through one
+ * door — `GET /data/:object`, where a repeat arrives as `string[]`, and
+ * `POST /data/:object/query`, whose body is arbitrary JSON — and a filter AST
+ * *is* an array (`['status','=','open']`). So the two are byte-identical
+ * there, which is precisely why #7386's arity gate had to leave this slot
+ * alone ({@link https://github.com/objectstack-ai/objectstack/issues/7390}).
+ * On a querystring the ambiguity does not exist: an array on the filter slot
+ * is a repeated parameter and can be nothing else. This layer is the only one
+ * that knows it is looking at a querystring, so the judgement is made here and
+ * the normalizer stays free of the heuristic (`an array of strings each
+ * parseable as JSON is probably a repetition`) that #4181 and #4121 spent
+ * effort removing.
+ *
+ * ## Why it THROWS instead of responding
+ *
+ * Its sibling {@link refuseRepeatedQueryParams} writes the ADR-0112 NESTED
+ * body itself, which is right for the `/meta` family it guards. The data
+ * routes speak the FLAT `mapDataError` envelope (`{ error, code, object }`),
+ * and that is the envelope this route's OTHER filter refusals already arrive
+ * in — `unusableFilterError` and `malformedFilterArrayError` both throw
+ * `400` / `INVALID_FILTER` and are shaped by the handler's own catch. So this
+ * gate throws the same shape from inside the same `try`: one slot, one wire
+ * code, one body shape, whether the filter was unreadable or sent twice.
+ * Responding here instead would author a second dialect for one condition.
+ *
+ * `INVALID_FILTER` is a STANDARD-catalog code (`spec/src/api/errors.zod.ts`),
+ * not a new one — nothing in `packages/spec` moves for this.
+ *
+ * A one-element array is one occurrence encoded by an adapter, and is unwrapped
+ * rather than refused — the same count-not-shape rule this module's header
+ * states, and the reason a `['{"a":1}']` from a repeat-preserving adapter stops
+ * being read as a malformed AST too.
+ *
+ * @param query the handler's `req.query` (`any`-shaped; `rest-server.ts` types
+ * its handlers that way). Non-object values are left alone.
+ * @throws a `400` / `INVALID_FILTER` error when a filter spelling was supplied
+ * more than once.
+ */
+export function assertFilterParamSuppliedOnce(query: unknown): void {
+ if (!query || typeof query !== 'object') return;
+ const bag = query as Record<string, unknown>;
+ for (const name of FILTER_SLOT_QUERY_PARAMS) {
+ const raw = bag[name];
+ if (!Array.isArray(raw)) continue;
+ const read = readSingleQueryValue(raw as string[]);
+ if (!read.ok) {
+ const err: any = new Error(repeatedFilterParamMessage(name, read.count));
+ err.status = 400;
+ err.code = 'INVALID_FILTER';
+ err.param = name;
+ throw err;
+ }
+ bag[name] = read.value as string;
+ }
+}