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 }));