feat(agent-bff): validate relation filters against the foreign collection

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

@@ -1,4 +1,8 @@
 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;
@@ -31,6 +35,23 @@ 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 readableAsBranch = isBranch(node);
+
+  if (isLeaf(node) && readableAsBranch) {
+    throw invalidRequest('A filter node cannot carry both "field" and "conditions"');
+  }
+
+  if (readableAsBranch) {
+    node.conditions.forEach(condition =>
+      assertNoNodeReadableAsBothLeafAndBranch(condition, depth + 1),
+    );
+  }
+}
+
 // Validate the untyped request body before it reaches the query builders, so malformed shapes
 // (e.g. `projection` or `sort` as a string) surface as 400 invalid_request rather than a 500 from
 // an array method blowing up downstream.
@@ -59,8 +80,9 @@ export function parseListRequest(body: unknown): ListRequestBody {
     if (!valid) throw invalidRequest('sort must be an array of { field, direction? }');
   }
 
-  if (filter !== undefined && !isPlainObject(filter)) {
-    throw invalidRequest('filter must be an object');
+  if (filter !== undefined) {
+    if (!isPlainObject(filter)) throw invalidRequest('filter must be an object');
+    assertNoNodeReadableAsBothLeafAndBranch(filter);
   }
 
   if (page !== undefined) {
@@ -83,37 +105,14 @@ export function parseListRequest(body: unknown): ListRequestBody {
 export function parseCountRequest(body: unknown): CountRequestBody {
   if (!isPlainObject(body)) throw invalidRequest('Request body must be an object');
 
-  if (body.filter !== undefined && !isPlainObject(body.filter)) {
-    throw invalidRequest('filter must be an object');
+  if (body.filter !== undefined) {
+    if (!isPlainObject(body.filter)) throw invalidRequest('filter must be an object');
+    assertNoNodeReadableAsBothLeafAndBranch(body.filter);
   }
 
   return body as CountRequestBody;
 }
 
-interface ConditionTreeBranch {
-  conditions: unknown[];
-}
-
-interface ConditionTreeLeaf {
-  field: string;
-}
-
-function isBranch(node: unknown): node is ConditionTreeBranch {
-  return (
-    typeof node === 'object' &&
-    node !== null &&
-    Array.isArray((node as { conditions?: unknown }).conditions)
-  );
-}
-
-function isLeaf(node: unknown): node is ConditionTreeLeaf {
-  return (
-    typeof node === 'object' &&
-    node !== null &&
-    typeof (node as { field?: unknown }).field === 'string'
-  );
-}
-
 function collectFilterFields(filter: unknown, acc: string[]): void {
   if (isBranch(filter)) {
     filter.conditions.forEach(condition => collectFilterFields(condition, acc));

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

@@ -81,11 +81,12 @@ function assertCollectionStillAllowed(readModel: ReadModel, collection: string):
 
 function resolveCapabilities(
   deps: RequestHandlerDeps,
+  collection: string,
 ): Promise<{ capabilities: CapabilitiesResult; readModel: ReadModel }> {
   return callAgent(
     () =>
       deps.store.getCapabilities(
-        deps.collection,
+        collection,
         createAgentCapabilitiesFetcher({
           agentUrl: deps.agentUrl,
           token: deps.token,
@@ -111,7 +112,7 @@ async function handleList(ctx: Context, body: ListRequestBody, deps: ListHandler
   let { primaryKeys } = deps;
 
   if (hasCapabilityConstrainedInput(validationInput)) {
-    const { capabilities, readModel } = await resolveCapabilities(deps);
+    const { capabilities, readModel } = await resolveCapabilities(deps, deps.collection);
     assertCollectionStillAllowed(readModel, deps.collection);
     assertValidAgainstCapabilities(validationInput, capabilities);
     primaryKeys = readModel.getPrimaryKeys(deps.collection);
@@ -129,7 +130,7 @@ async function handleCount(ctx: Context, body: CountRequestBody, deps: RequestHa
 
   // Count carries only a filter (no sort/projection), so that is all there is to validate.
   if (body.filter !== undefined) {
-    const { capabilities, readModel } = await resolveCapabilities(deps);
+    const { capabilities, readModel } = await resolveCapabilities(deps, deps.collection);
     assertCollectionStillAllowed(readModel, deps.collection);
     assertValidAgainstCapabilities({ filter: body.filter }, capabilities);
   }
@@ -150,24 +151,76 @@ interface RelationHandlerDeps extends RequestHandlerDeps {
 
 type RelationListHandlerDeps = RelationHandlerDeps & { primaryKeys: PrimaryKeyField[] };
 
+function assertRelationStillExposed(readModel: ReadModel, deps: RelationHandlerDeps): void {
+  assertCollectionStillAllowed(readModel, deps.collection);
+
+  const stillTargets = resolveForeignCollection(
+    readModel.getRelationTarget(deps.collection, deps.relation),
+  );
+
+  if (stillTargets !== deps.foreignCollection) {
+    throw unknownRelation(`Unknown relation: ${deps.collection}.${deps.relation}`);
+  }
+
+  assertCollectionStillAllowed(readModel, deps.foreignCollection);
+}
+
+async function resolveForeignCapabilitiesAndReassertRelationIsStillExposed(
+  deps: RelationHandlerDeps,
+): Promise<{ capabilities: CapabilitiesResult; readModel: ReadModel }> {
+  assertRelationStillExposed(await resolveReadModel(deps.store), deps);
+
+  let result: { capabilities: CapabilitiesResult; readModel: ReadModel };
+
+  try {
+    result = await resolveCapabilities(deps, deps.foreignCollection);
+  } catch (error) {
+    deps.logger('Warn', 'Foreign capabilities lookup failed; re-checking relation exposure', {
+      collection: deps.collection,
+      relation: deps.relation,
+      foreignCollection: deps.foreignCollection,
+      cause: error instanceof Error ? `${error.name}: ${error.message}` : String(error),
+    });
+
+    assertRelationStillExposed(await resolveReadModel(deps.store), deps);
+    throw error;
+  }
+
+  assertRelationStillExposed(result.readModel, deps);
+
+  return result;
+}
+
 async function handleRelationList(
   ctx: Context,
   body: RelationListRequestBody,
   deps: RelationListHandlerDeps,
 ) {
-  // The nested-relation guard IS wired here: the agent's list-related asserts browse only on the
-  // immediate foreign collection, so a nested `:`-path would traverse to a third collection whose
-  // browse is never checked. Plain foreign fields (no `:`) are unaffected.
   assertNoRelationFieldPaths(collectListFieldPaths(body));
 
+  const validationInput = {
+    filter: body.filter,
+    sortFields: body.sort?.map(clause => clause.field),
+    projectionFields: body.projection,
+  };
+
+  let { primaryKeys } = deps;
+
+  if (hasCapabilityConstrainedInput(validationInput)) {
+    const { capabilities, readModel } =
+      await resolveForeignCapabilitiesAndReassertRelationIsStillExposed(deps);
+    assertValidAgainstCapabilities(validationInput, capabilities);
+    primaryKeys = readModel.getPrimaryKeys(deps.foreignCollection);
+  }
+
   const query = buildListAgentQuery(deps.foreignCollection, deps.timezone, body);
   const records = await callAgent(
     () => deps.client.listRelation(deps.collection, body.parentId, deps.relation, query),
     deps.logger,
   );
 
   ctx.status = 200;
-  ctx.body = mapListResponse(deps.foreignCollection, records, deps.primaryKeys);
+  ctx.body = mapListResponse(deps.foreignCollection, records, primaryKeys);
 }
 
 async function handleRelationCount(
@@ -177,6 +230,13 @@ async function handleRelationCount(
 ) {
   assertNoRelationFieldPaths(collectCountFieldPaths(body));
 
+  if (body.filter !== undefined) {
+    const { capabilities } = await resolveForeignCapabilitiesAndReassertRelationIsStillExposed(
+      deps,
+    );
+    assertValidAgainstCapabilities({ filter: body.filter }, capabilities);
+  }
+
   const query = buildCountAgentQuery(deps.timezone, body);
   const raw = await callAgent(
     () => deps.client.countRelationRaw(deps.collection, body.parentId, deps.relation, query),

packages/agent-bff/src/validation/capabilities-validator.ts

@@ -21,15 +21,15 @@ interface FilterLeaf {
   operator?: string;
 }
 
-function isBranch(node: unknown): node is { conditions: unknown[] } {
+export function isBranch(node: unknown): node is { conditions: unknown[] } {
   return (
     typeof node === 'object' &&
     node !== null &&
     Array.isArray((node as { conditions?: unknown }).conditions)
   );
 }
 
-function isLeaf(node: unknown): node is FilterLeaf {
+export function isLeaf(node: unknown): node is FilterLeaf {
   return (
     typeof node === 'object' &&
     node !== null &&

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

@@ -1,4 +1,5 @@
 import {
+  MAX_PARSED_FILTER_DEPTH,
   buildCountAgentQuery,
   buildListAgentQuery,
   collectCountFieldPaths,
@@ -132,6 +133,59 @@ describe('parseCountRequest', () => {
   });
 });
 
+describe('a filter node readable as both a leaf and a branch', () => {
+  const READABLE_AS_BOTH = {
+    field: 'publisher:secretRevenue',
+    operator: 'Equal',
+    value: 1,
+    conditions: [],
+  };
+
+  it.each([
+    ['parseListRequest', parseListRequest],
+    ['parseCountRequest', parseCountRequest],
+  ])('should reject it in %s with 400 invalid_request', (_label, parse) => {
+    expect(() => parse({ filter: READABLE_AS_BOTH })).toThrow(
+      expect.objectContaining({ type: 'invalid_request', status: 400 }),
+    );
+  });
+
+  it('should reject it nested inside a legitimate branch', () => {
+    expect(() =>
+      parseListRequest({ filter: { aggregator: 'And', conditions: [READABLE_AS_BOTH] } }),
+    ).toThrow(expect.objectContaining({ type: 'invalid_request', status: 400 }));
+  });
+
+  it('should be invisible to the field-path collector', () => {
+    expect(collectCountFieldPaths({ filter: READABLE_AS_BOTH })).toEqual([]);
+  });
+
+  it('should still accept a plain leaf and a plain branch', () => {
+    const leaf = { field: 'title', operator: 'Present' };
+
+    expect(() => parseCountRequest({ filter: leaf })).not.toThrow();
+    expect(() =>
+      parseListRequest({ filter: { aggregator: 'And', conditions: [leaf] } }),
+    ).not.toThrow();
+  });
+
+  it('should reject a filter nested past the depth cap with 400 rather than blowing the stack', () => {
+    let filter: unknown = { field: 'title', operator: 'Present' };
+
+    for (let i = 0; i <= MAX_PARSED_FILTER_DEPTH; i += 1) {
+      filter = { aggregator: 'And', conditions: [filter] };
+    }
+
+    expect(() => parseCountRequest({ filter })).toThrow(
+      expect.objectContaining({
+        type: 'filter_too_deep',
+        status: 400,
+        details: { maxDepth: MAX_PARSED_FILTER_DEPTH },
+      }),
+    );
+  });
+});
+
 describe('parseParentId', () => {
   it('should return a non-empty string unchanged, including a composite packed id', () => {
     expect(parseParentId('a|b')).toBe('a|b');