fix(agent-bff): reject unknown keys on list and count bodies

packages/agent-bff/src/data/agent-query.ts

@@ -1,105 +1,124 @@
-import type { ZodType } from 'zod';
-
-import { CountFlatInputs, ListFlatInputs } from './request-schemas';
+import type { PageInput, SortClauseInput } from './request-schemas';
+import type { BffHttpError } from '../http/bff-http-error';
+import type { Logger } from '../ports/logger-port';
+import type { ZodType, z } from 'zod';
+
+import {
+  CountFlatInputs,
+  ListFlatInputs,
+  RelationCountFlatInputs,
+  RelationListFlatInputs,
+} from './request-schemas';
 import { invalidRequest } from '../http/bff-local-errors';
 import { MAX_FILTER_DEPTH, isBranch, isLeaf } from '../validation/capabilities-validator';
 import { filterTooDeep } from '../validation/validation-errors';
 
 export { MAX_FILTER_DEPTH as MAX_PARSED_FILTER_DEPTH };
 
-export interface BffSortClause {
-  field: string;
-  direction?: 'asc' | 'desc';
-}
+export type BffSortClause = z.infer<typeof SortClauseInput>;
 
-export interface BffPage {
-  limit: number;
-  offset: number;
-}
+export type BffPage = z.infer<typeof PageInput>;
 
-export interface ListRequestBody {
-  filter?: unknown;
-  projection?: string[];
-  sort?: BffSortClause[];
-  page?: BffPage;
-  search?: string;
-  searchExtended?: boolean;
-}
+export type ListRequestBody = z.infer<typeof ListFlatInputs>;
 
-export interface CountRequestBody {
-  filter?: unknown;
-  search?: string;
-  searchExtended?: boolean;
-}
+export type CountRequestBody = z.infer<typeof CountFlatInputs>;
 
-export type RelationListRequestBody = ListRequestBody & { parentId: string };
+export type RelationListRequestBody = z.infer<typeof RelationListFlatInputs> & { parentId: string };
 
-export type RelationCountRequestBody = CountRequestBody & { parentId: string };
+export type RelationCountRequestBody = z.infer<typeof RelationCountFlatInputs> & {
+  parentId: string;
+};
 
 export type AgentQuery = Record<string, unknown> & { timezone: string };
 
 function isPlainObject(value: unknown): value is Record<string, unknown> {
   return typeof value === 'object' && value !== null && !Array.isArray(value);
 }
 
-function assertNoNodeReadableAsBothLeafAndBranch(node: unknown, depth = 0): void {
-  if (depth > MAX_FILTER_DEPTH) throw filterTooDeep(MAX_FILTER_DEPTH);
-  if (typeof node !== 'object' || node === null) return;
+const LEAF_KEYS = ['field', 'operator', 'value'];
+const BRANCH_KEYS = ['aggregator', 'conditions'];
+
+function rejectBody(logger: Logger, error: BffHttpError): never {
+  logger('Warn', 'Request body rejected', { reason: error.message });
+
+  throw error;
+}
+
+// The tree is closed like the flat body is: a leaf carrying `valu` instead of `value` builds a
+// condition with `value: undefined`, which the agent reads as null and runs — a typo must not
+// silently change the returned rows.
+function assertNoStrayKey(node: Record<string, unknown>, allowed: string[], logger: Logger): void {
+  const stray = Object.keys(node).find(key => !allowed.includes(key));
+
+  if (stray !== undefined) {
+    rejectBody(logger, invalidRequest(`A filter node cannot carry "${stray}"`));
+  }
+}
+
+function assertFilterNode(node: unknown, logger: Logger, depth = 0): void {
+  if (depth > MAX_FILTER_DEPTH) rejectBody(logger, filterTooDeep(MAX_FILTER_DEPTH));
+  if (!isPlainObject(node)) return;
 
   const readableAsBranch = isBranch(node);
 
   if (isLeaf(node) && readableAsBranch) {
-    throw invalidRequest('A filter node cannot carry both "field" and "conditions"');
+    rejectBody(logger, invalidRequest('A filter node cannot carry both "field" and "conditions"'));
   }
 
   if (readableAsBranch) {
-    node.conditions.forEach(condition =>
-      assertNoNodeReadableAsBothLeafAndBranch(condition, depth + 1),
-    );
+    assertNoStrayKey(node, BRANCH_KEYS, logger);
+    node.conditions.forEach(condition => assertFilterNode(condition, logger, depth + 1));
+
+    return;
+  }
+
+  if (isLeaf(node)) {
+    assertNoStrayKey(node, LEAF_KEYS, logger);
+
+    return;
   }
+
+  // Neither readable as a leaf nor as a branch: `feild` instead of `field` reaches the agent as a
+  // node it cannot act on. An empty object stays allowed — it is how an absent filter is spelled.
+  assertNoStrayKey(node, [], logger);
 }
 
-/**
- * Checks the flat inputs against the shared schema and reports the first failure as
- * 400 invalid_request, so a malformed shape (`projection` as a string, a fractional `page.limit`)
- * surfaces as a client error rather than a 500 from an array method blowing up downstream.
- *
- * Unknown keys are left alone: the object schemas ignore them, so `filter`, `timezone` and
- * `parentId` travel through untouched and the body is returned by reference, not rebuilt.
- */
-function assertFlatInputs(schema: ZodType, body: Record<string, unknown>): void {
+function assertFlatInputs(schema: ZodType, body: Record<string, unknown>, logger: Logger): void {
   const result = schema.safeParse(body);
   if (result.success) return;
 
-  const [issue] = result.error.issues;
+  const { issues } = result.error;
+  const issue = issues.find(candidate => candidate.code === 'unrecognized_keys') ?? issues[0];
   const path = issue.path.join('.');
+  const reason = path ? `${path}: ${issue.message}` : issue.message;
 
-  throw invalidRequest(path ? `${path}: ${issue.message}` : issue.message);
+  rejectBody(logger, invalidRequest(reason));
 }
 
-function assertFilter(filter: unknown): void {
+function assertFilter(filter: unknown, logger: Logger): void {
   if (filter === undefined) return;
-  if (!isPlainObject(filter)) throw invalidRequest('filter must be an object');
+  if (!isPlainObject(filter)) rejectBody(logger, invalidRequest('filter must be an object'));
 
-  assertNoNodeReadableAsBothLeafAndBranch(filter);
+  assertFilterNode(filter, logger);
 }
 
-export function parseListRequest(body: unknown): ListRequestBody {
-  if (!isPlainObject(body)) throw invalidRequest('Request body must be an object');
+function parseRequest<S extends ZodType>(schema: S, body: unknown, logger: Logger): z.output<S> {
+  if (!isPlainObject(body)) {
+    rejectBody(logger, invalidRequest('Request body must be an object'));
+  }
 
-  assertFlatInputs(ListFlatInputs, body);
-  assertFilter(body.filter);
+  assertFlatInputs(schema, body, logger);
+  assertFilter(body.filter, logger);
 
-  return body as ListRequestBody;
+  return body as z.output<S>;
 }
 
-export function parseCountRequest(body: unknown): CountRequestBody {
-  if (!isPlainObject(body)) throw invalidRequest('Request body must be an object');
-
-  assertFlatInputs(CountFlatInputs, body);
-  assertFilter(body.filter);
+export function parseListRequest(body: unknown, logger: Logger): ListRequestBody {
+  return parseRequest(ListFlatInputs, body, logger);
+}
 
-  return body as CountRequestBody;
+export function parseCountRequest(body: unknown, logger: Logger): CountRequestBody {
+  return parseRequest(CountFlatInputs, body, logger);
 }
 
 function collectFilterFields(filter: unknown, acc: string[]): void {
@@ -188,9 +207,11 @@ export function collectListFieldPaths(body: ListRequestBody): string[] {
 // The parent record id is opaque: a packed/composite id must survive unchanged, so its content is
 // never inspected — only presence and primitive type. A finite number (single numeric pk) is
 // coerced to string; anything else is a BFF-local 400 with no agent call.
-export function parseParentId(parentId: unknown): string {
+export function parseParentId(parentId: unknown, logger: Logger): string {
   if (typeof parentId === 'string') {
-    if (parentId.trim() === '') throw invalidRequest('parentId must not be empty');
+    if (parentId.trim() === '') {
+      rejectBody(logger, invalidRequest('parentId must not be empty'));
+    }
 
     return parentId;
   }
@@ -199,19 +220,22 @@ export function parseParentId(parentId: unknown): string {
     return String(parentId);
   }
 
-  throw invalidRequest('parentId is required and must be a non-empty string or a number');
+  rejectBody(
+    logger,
+    invalidRequest('parentId is required and must be a non-empty string or a number'),
+  );
 }
 
-export function parseRelationListRequest(body: unknown): RelationListRequestBody {
-  const parentId = parseParentId((body as { parentId?: unknown } | null)?.parentId);
+export function parseRelationListRequest(body: unknown, logger: Logger): RelationListRequestBody {
+  const parentId = parseParentId((body as { parentId?: unknown } | null)?.parentId, logger);
 
-  return { ...parseListRequest(body), parentId };
+  return { ...parseRequest(RelationListFlatInputs, body, logger), parentId };
 }
 
-export function parseRelationCountRequest(body: unknown): RelationCountRequestBody {
-  const parentId = parseParentId((body as { parentId?: unknown } | null)?.parentId);
+export function parseRelationCountRequest(body: unknown, logger: Logger): RelationCountRequestBody {
+  const parentId = parseParentId((body as { parentId?: unknown } | null)?.parentId, logger);
 
-  return { ...parseCountRequest(body), parentId };
+  return { ...parseRequest(RelationCountFlatInputs, body, logger), parentId };
 }
 
 export function collectCountFieldPaths(body: CountRequestBody): string[] {

packages/agent-bff/src/data/data-routes-middleware.ts

@@ -295,12 +295,12 @@ async function handleRelation(
   const relationDeps: RelationHandlerDeps = { ...deps, relation, foreignCollection };
 
   if (operation === 'list') {
-    await handleRelationList(ctx, parseRelationListRequest(rawBody), {
+    await handleRelationList(ctx, parseRelationListRequest(rawBody, deps.logger), {
       ...relationDeps,
       primaryKeys: readModel.getPrimaryKeys(foreignCollection),
     });
   } else {
-    await handleRelationCount(ctx, parseRelationCountRequest(rawBody), relationDeps);
+    await handleRelationCount(ctx, parseRelationCountRequest(rawBody, deps.logger), relationDeps);
   }
 }
 
@@ -355,10 +355,10 @@ export default function createDataRoutesMiddleware({
     const operation = match[2] as 'list' | 'count';
 
     if (operation === 'list') {
-      const body = parseListRequest(rawBody);
+      const body = parseListRequest(rawBody, logger);
       await handleList(ctx, body, { ...deps, primaryKeys: readModel.getPrimaryKeys(collection) });
     } else {
-      const body = parseCountRequest(rawBody);
+      const body = parseCountRequest(rawBody, logger);
       await handleCount(ctx, body, deps);
     }
   };

packages/agent-bff/src/data/request-schemas.ts

@@ -1,20 +1,11 @@
 import { z } from '../zod';
 
-/**
- * The validation rules of a request body, declared once. `src/openapi/schemas.ts` decorates these
- * with names and descriptions to publish them, and the parsers below check bodies against them, so
- * the document and the runtime cannot drift apart.
- *
- * `filter` is deliberately absent: its operators are checked against the collection capabilities
- * (`capabilities-validator`), and a node readable as both leaf and branch has no schema equivalent,
- * so it keeps its own path in `agent-query.ts`.
- */
-export const SortClauseInput = z.object({
+export const SortClauseInput = z.strictObject({
   field: z.string(),
   direction: z.enum(['asc', 'desc']).optional(),
 });
 
-export const PageInput = z.object({
+export const PageInput = z.strictObject({
   limit: z.number().int().positive(),
   offset: z.number().int().nonnegative(),
 });
@@ -25,21 +16,30 @@ export const SearchInput = z.string();
 
 export const SearchExtendedInput = z.boolean();
 
-export const TimezoneInput = z.string();
+export const TimezoneInput = z.string().regex(/\S/);
 
 export const ParentIdInput = z.union([z.string().regex(/\S/), z.number()]);
 
-/** Everything a list body carries except `filter`, which is validated separately. */
-export const ListFlatInputs = z.object({
+const validatedElsewhere = z.unknown().optional();
+
+export const ListFlatInputs = z.strictObject({
+  filter: validatedElsewhere,
   projection: ProjectionInput.optional(),
   sort: z.array(SortClauseInput).optional(),
   page: PageInput.optional(),
   search: SearchInput.optional(),
   searchExtended: SearchExtendedInput.optional(),
+  timezone: TimezoneInput.optional(),
 });
 
 /** A count body carries no projection, sort or page. */
-export const CountFlatInputs = z.object({
+export const CountFlatInputs = z.strictObject({
+  filter: validatedElsewhere,
   search: SearchInput.optional(),
   searchExtended: SearchExtendedInput.optional(),
+  timezone: TimezoneInput.optional(),
 });
+
+export const RelationListFlatInputs = ListFlatInputs.extend({ parentId: validatedElsewhere });
+
+export const RelationCountFlatInputs = CountFlatInputs.extend({ parentId: validatedElsewhere });