From 6e23c7515ebd92a3625588dbdc2b633c2ca26bd9 Mon Sep 17 00:00:00 2001 From: Nicolas Bouliol Date: Mon, 13 Jul 2026 17:21:10 +0200 Subject: [PATCH 1/4] feat(agent-bff): validate top-level list and count against capabilities Wire the capabilities validator into the top-level list and count handlers so filter/sort/projection are checked before the agent call. List passes filter+sort+projection; count passes only the filter. Adds a Zendesk regression pinning that an unsupported operator on stripeEmail is rejected early with 400 invalid_filter_operator and the field's normalized operators, with no agent call. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/data/data-routes-middleware.ts | 28 +++ .../src/read-model/read-model-store.ts | 13 +- .../test/data/data-routes-middleware.test.ts | 224 +++++++++++++++++- 3 files changed, 259 insertions(+), 6 deletions(-) diff --git a/packages/agent-bff/src/data/data-routes-middleware.ts b/packages/agent-bff/src/data/data-routes-middleware.ts index 974928faca..0268ed176c 100644 --- a/packages/agent-bff/src/data/data-routes-middleware.ts +++ b/packages/agent-bff/src/data/data-routes-middleware.ts @@ -6,6 +6,7 @@ import type { RelationListRequestBody, } from './agent-query'; import type { Logger } from '../ports/logger-port'; +import type { CapabilitiesResult } from '../read-model/capabilities-cache'; import type ReadModel from '../read-model/read-model'; import type { PrimaryKeyField, RelationTarget } from '../read-model/read-model'; import type ReadModelStore from '../read-model/read-model-store'; @@ -30,6 +31,8 @@ import { resolveReadModel, } from '../http/agent-route-helpers'; import { unknownCollection, unknownRelation } from '../http/bff-local-errors'; +import createAgentCapabilitiesFetcher from '../read-model/agent-capabilities-fetcher'; +import { assertValidAgainstCapabilities } from '../validation/capabilities-validator'; import assertNoRelationFieldPaths from '../validation/relation-field-guard'; const DATA_ROUTE = /^\/agent\/v1\/([^/]+)\/(list|count)$/; @@ -54,15 +57,34 @@ export interface DataRoutesMiddlewareOptions { interface RequestHandlerDeps { collection: string; client: AgentDataClient; + store: ReadModelStore; + agentUrl: string; + token: string; timezone: string; logger: Logger; } type ListHandlerDeps = RequestHandlerDeps & { primaryKeys: PrimaryKeyField[] }; +function resolveCapabilities(deps: RequestHandlerDeps): Promise { + return deps.store.getCapabilities( + deps.collection, + createAgentCapabilitiesFetcher({ agentUrl: deps.agentUrl, token: deps.token }), + ); +} + async function handleList(ctx: Context, body: ListRequestBody, deps: ListHandlerDeps) { assertNoRelationFieldPaths(collectListFieldPaths(body)); + assertValidAgainstCapabilities( + { + filter: body.filter, + sortFields: body.sort?.map(clause => clause.field), + projectionFields: body.projection, + }, + await resolveCapabilities(deps), + ); + const query = buildListAgentQuery(deps.collection, deps.timezone, body); const records = await callAgent(() => deps.client.list(deps.collection, query), deps.logger); @@ -73,6 +95,9 @@ async function handleList(ctx: Context, body: ListRequestBody, deps: ListHandler async function handleCount(ctx: Context, body: CountRequestBody, deps: RequestHandlerDeps) { assertNoRelationFieldPaths(collectCountFieldPaths(body)); + // Count carries only a filter (no sort/projection), so that is all there is to validate. + assertValidAgainstCapabilities({ filter: body.filter }, await resolveCapabilities(deps)); + const query = buildCountAgentQuery(deps.timezone, body); const raw = await callAgent(() => deps.client.countRaw(deps.collection, query), deps.logger); @@ -198,6 +223,9 @@ export default function createDataRoutesMiddleware({ const deps: RequestHandlerDeps = { collection, client: createClient({ agentUrl, token }), + store, + agentUrl, + token, timezone: ctx.state.timezone as string, logger, }; diff --git a/packages/agent-bff/src/read-model/read-model-store.ts b/packages/agent-bff/src/read-model/read-model-store.ts index d2e1dd346e..d59222f9d9 100644 --- a/packages/agent-bff/src/read-model/read-model-store.ts +++ b/packages/agent-bff/src/read-model/read-model-store.ts @@ -40,11 +40,14 @@ export default class ReadModelStore { // Ensure any pending schema refresh (and its capabilities invalidation) runs first. await this.getReadModel(); - // TODO(wiring): possible TOCTOU once this is called from request handling. A concurrent schema - // refresh can clear capabilities while this fetch is in flight, so the caller could receive - // capabilities from the previous schema generation alongside the new allow-list. When wiring - // the data endpoints, re-check `schemaCache.revision` after the fetch resolves and retry on a - // mismatch so capabilities and schema stay atomically coupled. + // TODO(wiring): known TOCTOU, deferred. Two snapshots can straddle a schema generation: + // 1. the data middleware captures the read-model (allow-list) once, then calls this later; + // 2. this capabilities fetch can be in flight when a concurrent schema refresh clear()s it. + // Either gap lets the caller validate against capabilities from one generation while the + // allow-list came from another. A full fix couples both reads to a single generation (return + // read-model + capabilities together, or re-check `schemaCache.revision` across both and retry); + // a retry here alone only closes gap 2. Low risk: the trigger is a 24h-TTL refresh landing exactly + // during a request, and the agent stays the final validator. return this.capabilitiesCache.get(collection, fetcher); } diff --git a/packages/agent-bff/test/data/data-routes-middleware.test.ts b/packages/agent-bff/test/data/data-routes-middleware.test.ts index 0afe2f6e4f..ec5f30a8ec 100644 --- a/packages/agent-bff/test/data/data-routes-middleware.test.ts +++ b/packages/agent-bff/test/data/data-routes-middleware.test.ts @@ -1,5 +1,6 @@ import type { AgentDataClient } from '../../src/data/agent-data-client'; import type { Logger } from '../../src/ports/logger-port'; +import type { CapabilitiesResult } from '../../src/read-model/capabilities-cache'; import type ReadModelStore from '../../src/read-model/read-model-store'; import { AgentHttpError } from '@forestadmin/agent-client'; @@ -17,13 +18,33 @@ const TIMEZONE = 'Europe/Paris'; const noopLogger: Logger = () => {}; -function storeOf(readModel: ReadModel | Error): ReadModelStore { +type CapabilitiesStub = (collection: string) => Promise; + +const BROAD_SNAKE_OPERATORS = ['present', 'blank', 'equal', 'not_equal', 'in', 'like']; + +const defaultCapabilities: CapabilitiesStub = async () => ({ + fields: ['id', 'email', 'title', 'name', 'value', 'slug', 'label'].map(name => ({ + name, + type: 'String', + operators: BROAD_SNAKE_OPERATORS, + })), +}); + +function capabilitiesOf(fields: CapabilitiesResult['fields']): CapabilitiesStub { + return async () => ({ fields }); +} + +function storeOf( + readModel: ReadModel | Error, + getCapabilities: CapabilitiesStub = defaultCapabilities, +): ReadModelStore { return { getReadModel: async () => { if (readModel instanceof Error) throw readModel; return readModel; }, + getCapabilities: (collectionName: string) => getCapabilities(collectionName), } as unknown as ReadModelStore; } @@ -275,6 +296,207 @@ describe('data routes middleware', () => { }); }); + describe('capabilities validation', () => { + it('should reject a list filter operator unsupported by capabilities before calling the agent', async () => { + const list = jest.fn(async () => []); + const store = storeOf( + usersReadModel, + capabilitiesOf([{ name: 'email', type: 'String', operators: ['present'] }]), + ); + const app = buildApp(store, { list }); + + const response = await request(app.callback()) + .post('/agent/v1/users/list') + .send({ filter: { field: 'email', operator: 'Equal' } }); + + expect(response.status).toBe(400); + expect(response.body.error).toMatchObject({ + type: 'invalid_filter_operator', + status: 400, + details: { field: 'email', validOperators: ['Present'] }, + }); + expect(list).not.toHaveBeenCalled(); + }); + + it('should reject a list projection field absent from capabilities with 422 unknown_field', async () => { + const list = jest.fn(async () => []); + const store = storeOf( + usersReadModel, + capabilitiesOf([{ name: 'id', type: 'String', operators: ['equal'] }]), + ); + const app = buildApp(store, { list }); + + const response = await request(app.callback()) + .post('/agent/v1/users/list') + .send({ projection: ['id', 'ghost'] }); + + expect(response.status).toBe(422); + expect(response.body.error).toMatchObject({ + type: 'unknown_field', + status: 422, + details: { field: 'ghost' }, + }); + expect(list).not.toHaveBeenCalled(); + }); + + it('should reject a list sort field absent from capabilities with 422 unknown_field', async () => { + const list = jest.fn(async () => []); + const store = storeOf( + usersReadModel, + capabilitiesOf([{ name: 'id', type: 'String', operators: ['equal'] }]), + ); + const app = buildApp(store, { list }); + + const response = await request(app.callback()) + .post('/agent/v1/users/list') + .send({ sort: [{ field: 'ghost', direction: 'asc' }] }); + + expect(response.status).toBe(422); + expect(response.body.error).toMatchObject({ + type: 'unknown_field', + status: 422, + details: { field: 'ghost' }, + }); + expect(list).not.toHaveBeenCalled(); + }); + + it('should read capabilities before the agent call and skip the agent on a validation failure', async () => { + const calls: string[] = []; + const list = jest.fn(async () => { + calls.push('agent'); + + return []; + }); + const getCapabilities = jest.fn(async () => { + calls.push('capabilities'); + + return { + fields: [{ name: 'id', type: 'String', operators: ['equal'] }], + } as CapabilitiesResult; + }); + const app = buildApp(storeOf(usersReadModel, getCapabilities), { list }); + + await request(app.callback()) + .post('/agent/v1/users/list') + .send({ projection: ['ghost'] }); + + expect(getCapabilities).toHaveBeenCalledWith('users'); + expect(calls).toEqual(['capabilities']); + expect(list).not.toHaveBeenCalled(); + }); + + it('should proceed to the agent when the list filter, sort, and projection are all valid', async () => { + const list = jest.fn(async () => []); + const getCapabilities = jest.fn(defaultCapabilities); + const app = buildApp(storeOf(usersReadModel, getCapabilities), { list }); + + const response = await request(app.callback()) + .post('/agent/v1/users/list') + .send({ + projection: ['id', 'email'], + filter: { field: 'email', operator: 'Present' }, + sort: [{ field: 'id', direction: 'asc' }], + }); + + expect(response.status).toBe(200); + expect(getCapabilities).toHaveBeenCalledWith('users'); + expect(list).toHaveBeenCalledTimes(1); + }); + + it('should reject a count filter operator unsupported by capabilities before calling the agent', async () => { + const countRaw = jest.fn(async () => ({ count: 0 })); + const store = storeOf( + usersReadModel, + capabilitiesOf([{ name: 'email', type: 'String', operators: ['present'] }]), + ); + const app = buildApp(store, { countRaw }); + + const response = await request(app.callback()) + .post('/agent/v1/users/count') + .send({ filter: { field: 'email', operator: 'Equal' } }); + + expect(response.status).toBe(400); + expect(response.body.error).toMatchObject({ + type: 'invalid_filter_operator', + status: 400, + details: { field: 'email', validOperators: ['Present'] }, + }); + expect(countRaw).not.toHaveBeenCalled(); + }); + + it('should reject a count filter field absent from capabilities with 422 unknown_field', async () => { + const countRaw = jest.fn(async () => ({ count: 0 })); + const store = storeOf( + usersReadModel, + capabilitiesOf([{ name: 'id', type: 'String', operators: ['equal'] }]), + ); + const app = buildApp(store, { countRaw }); + + const response = await request(app.callback()) + .post('/agent/v1/users/count') + .send({ filter: { field: 'ghost', operator: 'Equal' } }); + + expect(response.status).toBe(422); + expect(response.body.error).toMatchObject({ + type: 'unknown_field', + status: 422, + details: { field: 'ghost' }, + }); + expect(countRaw).not.toHaveBeenCalled(); + }); + + it('should read capabilities before the agent call on the count path', async () => { + const calls: string[] = []; + const countRaw = jest.fn(async () => { + calls.push('agent'); + + return { count: 0 }; + }); + const getCapabilities = jest.fn(async () => { + calls.push('capabilities'); + + return { + fields: [{ name: 'id', type: 'String', operators: ['equal'] }], + } as CapabilitiesResult; + }); + const app = buildApp(storeOf(usersReadModel, getCapabilities), { countRaw }); + + await request(app.callback()) + .post('/agent/v1/users/count') + .send({ filter: { field: 'ghost', operator: 'Equal' } }); + + expect(getCapabilities).toHaveBeenCalledWith('users'); + expect(calls).toEqual(['capabilities']); + expect(countRaw).not.toHaveBeenCalled(); + }); + }); + + describe('Zendesk stripeEmail operator regression', () => { + it('should reject Equal on stripeEmail with 400 and its normalized supported operators, no agent call', async () => { + const list = jest.fn(async () => []); + const store = storeOf( + new ReadModel([collection('tickets', [column('id'), column('stripeEmail')])]), + capabilitiesOf([ + { name: 'id', type: 'String', operators: ['equal'] }, + { name: 'stripeEmail', type: 'String', operators: ['present', 'blank'] }, + ]), + ); + const app = buildApp(store, { list }); + + const response = await request(app.callback()) + .post('/agent/v1/tickets/list') + .send({ filter: { field: 'stripeEmail', operator: 'Equal' } }); + + expect(response.status).toBe(400); + expect(response.body.error).toMatchObject({ + type: 'invalid_filter_operator', + status: 400, + details: { field: 'stripeEmail', validOperators: ['Present', 'Blank'] }, + }); + expect(list).not.toHaveBeenCalled(); + }); + }); + describe('count', () => { it('should return available with the numeric count', async () => { const countRaw = jest.fn(async () => ({ count: 7 })); From ba6b2946abac5f5f9630a767fbd1b51e338ae0d9 Mon Sep 17 00:00:00 2001 From: Anthony Guimard Date: Mon, 3 Aug 2026 20:44:21 +0200 Subject: [PATCH 2/4] fix(agent-bff): bound filter depth, harden operator lookup and capabilities errors - normalizeOperator: Map instead of a plain object, so inherited keys (constructor, toString, __proto__) no longer resolve to truthy non-Operator values and skip the intended mapping error - collectLeaves: cap nesting at MAX_FILTER_DEPTH and return a 400 filter_too_deep, a deeply nested client filter previously raised RangeError from ~5000 levels (reachable with a 170KB payload) - resolveCapabilities: wrap in callAgent so an agent failure maps to 503 agent_unavailable like every other agent call, instead of falling through to 500 internal_error --- .../src/data/data-routes-middleware.ts | 10 +++-- .../src/validation/capabilities-validator.ts | 15 ++++++-- .../src/validation/operator-normalizer.ts | 4 +- .../src/validation/validation-errors.ts | 9 +++++ .../test/data/data-routes-middleware.test.ts | 18 +++++++++ .../validation/capabilities-validator.test.ts | 37 +++++++++++++++++++ .../validation/operator-normalizer.test.ts | 7 ++++ 7 files changed, 92 insertions(+), 8 deletions(-) diff --git a/packages/agent-bff/src/data/data-routes-middleware.ts b/packages/agent-bff/src/data/data-routes-middleware.ts index 0268ed176c..6631243cd7 100644 --- a/packages/agent-bff/src/data/data-routes-middleware.ts +++ b/packages/agent-bff/src/data/data-routes-middleware.ts @@ -67,9 +67,13 @@ interface RequestHandlerDeps { type ListHandlerDeps = RequestHandlerDeps & { primaryKeys: PrimaryKeyField[] }; function resolveCapabilities(deps: RequestHandlerDeps): Promise { - return deps.store.getCapabilities( - deps.collection, - createAgentCapabilitiesFetcher({ agentUrl: deps.agentUrl, token: deps.token }), + return callAgent( + () => + deps.store.getCapabilities( + deps.collection, + createAgentCapabilitiesFetcher({ agentUrl: deps.agentUrl, token: deps.token }), + ), + deps.logger, ); } diff --git a/packages/agent-bff/src/validation/capabilities-validator.ts b/packages/agent-bff/src/validation/capabilities-validator.ts index 125db36522..485bf85591 100644 --- a/packages/agent-bff/src/validation/capabilities-validator.ts +++ b/packages/agent-bff/src/validation/capabilities-validator.ts @@ -2,7 +2,12 @@ import type { BffHttpError } from '../http/bff-http-error'; import type { CapabilitiesResult } from '../read-model/capabilities-cache'; import { normalizeOperator } from './operator-normalizer'; -import { fieldNotFilterable, invalidFilterOperator, unknownField } from './validation-errors'; +import { + fieldNotFilterable, + filterTooDeep, + invalidFilterOperator, + unknownField, +} from './validation-errors'; import { mappingError } from '../http/bff-local-errors'; export interface ValidateParams { @@ -32,9 +37,13 @@ function isLeaf(node: unknown): node is FilterLeaf { ); } -function collectLeaves(node: unknown, acc: FilterLeaf[]): void { +export const MAX_FILTER_DEPTH = 100; + +function collectLeaves(node: unknown, acc: FilterLeaf[], depth = 0): void { + if (depth > MAX_FILTER_DEPTH) throw filterTooDeep(MAX_FILTER_DEPTH); + if (isBranch(node)) { - node.conditions.forEach(condition => collectLeaves(condition, acc)); + node.conditions.forEach(condition => collectLeaves(condition, acc, depth + 1)); } else if (isLeaf(node)) { const { operator } = node as { operator?: unknown }; acc.push({ field: node.field, operator: typeof operator === 'string' ? operator : undefined }); diff --git a/packages/agent-bff/src/validation/operator-normalizer.ts b/packages/agent-bff/src/validation/operator-normalizer.ts index fe0319a661..745ac736f2 100644 --- a/packages/agent-bff/src/validation/operator-normalizer.ts +++ b/packages/agent-bff/src/validation/operator-normalizer.ts @@ -14,7 +14,7 @@ export function toSnakeCaseOperator(operator: string): string { .toLowerCase(); } -const SNAKE_TO_PASCAL: Record = Object.fromEntries( +const SNAKE_TO_PASCAL = new Map( allOperators.map(operator => [toSnakeCaseOperator(operator), operator]), ); @@ -24,5 +24,5 @@ const SNAKE_TO_PASCAL: Record = Object.fromEntries( * only happens when the agent runs a newer operator set than this package (a version skew). */ export function normalizeOperator(snakeCaseOperator: string): Operator | undefined { - return SNAKE_TO_PASCAL[snakeCaseOperator]; + return SNAKE_TO_PASCAL.get(snakeCaseOperator); } diff --git a/packages/agent-bff/src/validation/validation-errors.ts b/packages/agent-bff/src/validation/validation-errors.ts index b8ac8889a6..8069965e28 100644 --- a/packages/agent-bff/src/validation/validation-errors.ts +++ b/packages/agent-bff/src/validation/validation-errors.ts @@ -10,6 +10,15 @@ export function fieldNotFilterable(field: string): BffHttpError { }); } +export function filterTooDeep(maxDepth: number): BffHttpError { + return new BffHttpError( + 400, + 'filter_too_deep', + `Filter nesting exceeds the maximum depth of ${maxDepth}`, + { maxDepth }, + ); +} + export function invalidFilterOperator(field: string, validOperators: string[]): BffHttpError { return new BffHttpError( 400, diff --git a/packages/agent-bff/test/data/data-routes-middleware.test.ts b/packages/agent-bff/test/data/data-routes-middleware.test.ts index ec5f30a8ec..0371198116 100644 --- a/packages/agent-bff/test/data/data-routes-middleware.test.ts +++ b/packages/agent-bff/test/data/data-routes-middleware.test.ts @@ -385,6 +385,24 @@ describe('data routes middleware', () => { expect(list).not.toHaveBeenCalled(); }); + it('should map a capabilities fetch failure to agent_unavailable instead of internal_error', async () => { + const list = jest.fn(async () => []); + const getCapabilities = jest.fn(async () => { + throw new AgentHttpError(503, {}, 'Service Unavailable'); + }); + const app = buildApp(storeOf(usersReadModel, getCapabilities), { list }); + + const response = await request(app.callback()) + .post('/agent/v1/users/list') + .send({ projection: ['id'] }); + + expect(response.status).toBe(503); + expect(response.body.error).toEqual( + expect.objectContaining({ type: 'agent_unavailable', status: 503 }), + ); + expect(list).not.toHaveBeenCalled(); + }); + it('should proceed to the agent when the list filter, sort, and projection are all valid', async () => { const list = jest.fn(async () => []); const getCapabilities = jest.fn(defaultCapabilities); diff --git a/packages/agent-bff/test/validation/capabilities-validator.test.ts b/packages/agent-bff/test/validation/capabilities-validator.test.ts index 68c9acb29d..b990ff9f19 100644 --- a/packages/agent-bff/test/validation/capabilities-validator.test.ts +++ b/packages/agent-bff/test/validation/capabilities-validator.test.ts @@ -2,6 +2,7 @@ import type { CapabilitiesResult } from '../../src/read-model/capabilities-cache import { toErrorBody } from '../../src/http/bff-http-error'; import { + MAX_FILTER_DEPTH, assertValidAgainstCapabilities, validateAgainstCapabilities, } from '../../src/validation/capabilities-validator'; @@ -15,6 +16,14 @@ const capabilities: CapabilitiesResult = { ], }; +function nestFilter(depth: number): unknown { + let node: unknown = { field: 'title', operator: 'Equal', value: 'x' }; + + for (let i = 0; i < depth; i += 1) node = { aggregator: 'And', conditions: [node] }; + + return node; +} + function captureError(fn: () => void): unknown { try { fn(); @@ -148,6 +157,34 @@ describe('validateAgainstCapabilities', () => { expect(error).toEqual(expect.objectContaining({ type: 'mapping_error', status: 500 })); }); + + it('accepts a filter nested up to the maximum depth', () => { + expect( + validateAgainstCapabilities({ filter: nestFilter(MAX_FILTER_DEPTH) }, capabilities), + ).toEqual([]); + }); + + it('rejects a filter nested beyond the maximum depth with a 400 instead of overflowing', () => { + const error = captureError(() => + validateAgainstCapabilities({ filter: nestFilter(MAX_FILTER_DEPTH + 1) }, capabilities), + ); + + expect(error).toEqual( + expect.objectContaining({ + type: 'filter_too_deep', + status: 400, + details: { maxDepth: MAX_FILTER_DEPTH }, + }), + ); + }); + + it('rejects a filter deep enough to blow the call stack without the guard', () => { + const error = captureError(() => + validateAgainstCapabilities({ filter: nestFilter(20000) }, capabilities), + ); + + expect(error).toEqual(expect.objectContaining({ type: 'filter_too_deep', status: 400 })); + }); }); describe('sort and projection', () => { diff --git a/packages/agent-bff/test/validation/operator-normalizer.test.ts b/packages/agent-bff/test/validation/operator-normalizer.test.ts index 734e8c50f1..1be2d489b8 100644 --- a/packages/agent-bff/test/validation/operator-normalizer.test.ts +++ b/packages/agent-bff/test/validation/operator-normalizer.test.ts @@ -32,5 +32,12 @@ describe('operator-normalizer', () => { it('returns undefined for an operator absent from the canonical set', () => { expect(normalizeOperator('made_up_operator')).toBeUndefined(); }); + + it.each(['constructor', 'toString', '__proto__', 'valueOf', 'hasOwnProperty'])( + 'returns undefined for the inherited object key %s', + key => { + expect(normalizeOperator(key)).toBeUndefined(); + }, + ); }); }); From 0bc69615b8ac18f3fbfa770de0d42e75b3af769d Mon Sep 17 00:00:00 2001 From: Anthony Guimard Date: Mon, 3 Aug 2026 20:44:29 +0200 Subject: [PATCH 3/4] fix(agent-bff): validate webhook and invalidated shapes in the execute mapper - a webhook payload without a string url/method (or an array) now falls through to the structured 501 instead of returning 200 with undefined fields - invalidated drops non-string entries rather than casting them, so the string[] contract holds for a malformed refresh.relationships --- .../src/action/action-execute-mapper.ts | 37 ++++++++++++------- .../test/action/action-execute-mapper.test.ts | 22 +++++++++++ 2 files changed, 46 insertions(+), 13 deletions(-) diff --git a/packages/agent-bff/src/action/action-execute-mapper.ts b/packages/agent-bff/src/action/action-execute-mapper.ts index 35979c7f4a..4b27d6a7c5 100644 --- a/packages/agent-bff/src/action/action-execute-mapper.ts +++ b/packages/agent-bff/src/action/action-execute-mapper.ts @@ -33,6 +33,24 @@ export interface ActionExecuteMapped { body: ActionExecuteMappedBody; } +interface AgentWebhookPayload { + url: string; + method: string; + headers: unknown; + body: unknown; +} + +// The agent always serializes the four webhook fields together +// (`agent/src/routes/modification/action/action.ts`), so a payload missing url/method is malformed +// and must reach the 501 path rather than surface as a 200 the client cannot act on. +function isWebhook(value: unknown): value is AgentWebhookPayload { + if (typeof value !== 'object' || value === null || Array.isArray(value)) return false; + + const { url, method } = value as Record; + + return typeof url === 'string' && typeof method === 'string'; +} + // Normalizes the agent's 200 execute payload into the flat BFF wrapper. The execute result is // untyped at the BFF boundary (`Action.execute(): Promise`), so we discriminate on the // agent HTTP payload shape. A File result streams a binary with no JSON marker, so any unrecognized @@ -43,19 +61,10 @@ export function mapActionExecuteResult(raw: unknown): ActionExecuteMapped { // Each branch validates the value shape, not just key presence: a malformed payload // (`{ webhook: null }`, `{ redirectTo: {} }`, `{ success: {} }`) must fall through to the 501 // path rather than be surfaced as a 200 with null fields the client cannot tell from a real one. - if (typeof body.webhook === 'object' && body.webhook !== null) { - const hook = body.webhook as Record; + if (isWebhook(body.webhook)) { + const { url, method, headers, body: hookBody } = body.webhook; - return { - status: 200, - body: { - type: 'webhook', - url: hook.url, - method: hook.method, - headers: hook.headers, - body: hook.body, - }, - }; + return { status: 200, body: { type: 'webhook', url, method, headers, body: hookBody } }; } if (typeof body.redirectTo === 'string') { @@ -74,7 +83,9 @@ export function mapActionExecuteResult(raw: unknown): ActionExecuteMapped { body: { type: 'success', message: typeof body.success === 'string' ? body.success : null, - invalidated: Array.isArray(relationships) ? (relationships as string[]) : [], + invalidated: Array.isArray(relationships) + ? relationships.filter((name): name is string => typeof name === 'string') + : [], html: typeof body.html === 'string' ? body.html : null, }, }; diff --git a/packages/agent-bff/test/action/action-execute-mapper.test.ts b/packages/agent-bff/test/action/action-execute-mapper.test.ts index 016aefe5d3..c55d95f2ae 100644 --- a/packages/agent-bff/test/action/action-execute-mapper.test.ts +++ b/packages/agent-bff/test/action/action-execute-mapper.test.ts @@ -60,6 +60,28 @@ describe('mapActionExecuteResult', () => { }); }); + it.each([ + ['an empty object', { webhook: {} }], + ['an array', { webhook: [] }], + ['url missing', { webhook: { method: 'POST' } }], + ['method missing', { webhook: { url: 'https://x.test' } }], + ['url not a string', { webhook: { url: 42, method: 'POST' } }], + ])('falls through to 501 when the webhook payload is %s', (_label, payload) => { + expect(mapActionExecuteResult(payload)).toEqual({ + status: 501, + body: { error: { type: 'unsupported_action_result', status: 501 } }, + }); + }); + + it('drops non-string entries from invalidated', () => { + expect( + mapActionExecuteResult({ success: 'ok', refresh: { relationships: ['orders', 42, null] } }), + ).toEqual({ + status: 200, + body: { type: 'success', message: 'ok', invalidated: ['orders'], html: null }, + }); + }); + it('maps a Redirect payload to the path', () => { expect(mapActionExecuteResult({ redirectTo: '/orders/1' })).toEqual({ status: 200, From 8b59bbc4c64002e9bb0f85282bef9e64ffdbda37 Mon Sep 17 00:00:00 2001 From: Anthony Guimard Date: Mon, 3 Aug 2026 20:47:15 +0200 Subject: [PATCH 4/4] perf(agent-bff): skip the capabilities fetch when there is nothing to validate A plain list/count carries no filter, sort, or projection, so it no longer pays a capabilities fetch and still succeeds while that fetch is unavailable. --- .../src/data/data-routes-middleware.ts | 26 ++++++++++------- .../src/validation/capabilities-validator.ts | 12 ++++++++ .../test/data/data-routes-middleware.test.ts | 28 +++++++++++++++++++ 3 files changed, 56 insertions(+), 10 deletions(-) diff --git a/packages/agent-bff/src/data/data-routes-middleware.ts b/packages/agent-bff/src/data/data-routes-middleware.ts index 6631243cd7..7b4681560f 100644 --- a/packages/agent-bff/src/data/data-routes-middleware.ts +++ b/packages/agent-bff/src/data/data-routes-middleware.ts @@ -32,7 +32,10 @@ import { } from '../http/agent-route-helpers'; import { unknownCollection, unknownRelation } from '../http/bff-local-errors'; import createAgentCapabilitiesFetcher from '../read-model/agent-capabilities-fetcher'; -import { assertValidAgainstCapabilities } from '../validation/capabilities-validator'; +import { + assertValidAgainstCapabilities, + hasCapabilityConstrainedInput, +} from '../validation/capabilities-validator'; import assertNoRelationFieldPaths from '../validation/relation-field-guard'; const DATA_ROUTE = /^\/agent\/v1\/([^/]+)\/(list|count)$/; @@ -80,14 +83,15 @@ function resolveCapabilities(deps: RequestHandlerDeps): Promise clause.field), - projectionFields: body.projection, - }, - await resolveCapabilities(deps), - ); + const validationInput = { + filter: body.filter, + sortFields: body.sort?.map(clause => clause.field), + projectionFields: body.projection, + }; + + if (hasCapabilityConstrainedInput(validationInput)) { + assertValidAgainstCapabilities(validationInput, await resolveCapabilities(deps)); + } const query = buildListAgentQuery(deps.collection, deps.timezone, body); const records = await callAgent(() => deps.client.list(deps.collection, query), deps.logger); @@ -100,7 +104,9 @@ async function handleCount(ctx: Context, body: CountRequestBody, deps: RequestHa assertNoRelationFieldPaths(collectCountFieldPaths(body)); // Count carries only a filter (no sort/projection), so that is all there is to validate. - assertValidAgainstCapabilities({ filter: body.filter }, await resolveCapabilities(deps)); + if (body.filter !== undefined) { + assertValidAgainstCapabilities({ filter: body.filter }, await resolveCapabilities(deps)); + } const query = buildCountAgentQuery(deps.timezone, body); const raw = await callAgent(() => deps.client.countRaw(deps.collection, query), deps.logger); diff --git a/packages/agent-bff/src/validation/capabilities-validator.ts b/packages/agent-bff/src/validation/capabilities-validator.ts index 485bf85591..be142dd53f 100644 --- a/packages/agent-bff/src/validation/capabilities-validator.ts +++ b/packages/agent-bff/src/validation/capabilities-validator.ts @@ -116,6 +116,18 @@ function dedupe(errors: BffHttpError[]): BffHttpError[] { return result; } +/** + * True when the request carries something capabilities can invalidate. Callers use it to skip the + * capabilities fetch entirely, so a plain list/count still succeeds while that fetch is unavailable. + */ +export function hasCapabilityConstrainedInput(params: ValidateParams): boolean { + return ( + params.filter !== undefined || + (params.sortFields?.length ?? 0) > 0 || + (params.projectionFields?.length ?? 0) > 0 + ); +} + /** * Validates a request's filter, sort, and projection fields against the target collection's * capabilities. Returns every offending field as a structured error (empty = valid); the caller diff --git a/packages/agent-bff/test/data/data-routes-middleware.test.ts b/packages/agent-bff/test/data/data-routes-middleware.test.ts index 0371198116..4b03d672ac 100644 --- a/packages/agent-bff/test/data/data-routes-middleware.test.ts +++ b/packages/agent-bff/test/data/data-routes-middleware.test.ts @@ -403,6 +403,34 @@ describe('data routes middleware', () => { expect(list).not.toHaveBeenCalled(); }); + it('should skip the capabilities fetch when the list carries nothing to validate', async () => { + const list = jest.fn(async () => []); + const getCapabilities = jest.fn(async () => { + throw new AgentHttpError(503, {}, 'Service Unavailable'); + }); + const app = buildApp(storeOf(usersReadModel, getCapabilities), { list }); + + const response = await request(app.callback()).post('/agent/v1/users/list').send({}); + + expect(response.status).toBe(200); + expect(getCapabilities).not.toHaveBeenCalled(); + expect(list).toHaveBeenCalled(); + }); + + it('should skip the capabilities fetch when the count carries no filter', async () => { + const countRaw = jest.fn(async () => ({ count: 3 })); + const getCapabilities = jest.fn(async () => { + throw new AgentHttpError(503, {}, 'Service Unavailable'); + }); + const app = buildApp(storeOf(usersReadModel, getCapabilities), { countRaw }); + + const response = await request(app.callback()).post('/agent/v1/users/count').send({}); + + expect(response.status).toBe(200); + expect(getCapabilities).not.toHaveBeenCalled(); + expect(countRaw).toHaveBeenCalled(); + }); + it('should proceed to the agent when the list filter, sort, and projection are all valid', async () => { const list = jest.fn(async () => []); const getCapabilities = jest.fn(defaultCapabilities);