From c6fd576c15928f633d6c8685595f6b9506f27845 Mon Sep 17 00:00:00 2001 From: Nicolas Bouliol Date: Wed, 2 Sep 2026 00:11:38 +0200 Subject: [PATCH 01/12] feat(agent): serve a BFF in-process with addBff() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Running a BFF meant a second deployment: another process, another port, another set of secrets to keep in sync with the agent's. `addBff()` serves it at /bff on the agent's own port instead, on every mount target. The BFF reaches the agent through the in-process dispatcher the embedded MCP server already uses, so there is no socket, no agent url to guess per host framework, and no second listener. Everything it shares with the agent — the secrets, the Forest urls, the logger — is inherited rather than repeated. The dispatcher is registered in addBff() rather than at start(): its hook is pushed on first use and mount() only runs the hooks registered before it, so asking later would leave every BFF call throwing until the first restart. `/bff` answers 503 while the agent is starting and stops answering entirely once it stopped, since a host application keeps the middleware it registered. The search integration suite moves here from agent-bff, which loses its dev dependency on the agent and with it the build cycle that dependency would have created. It now covers the embedded path end to end. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/build.yml | 8 +- packages/agent-bff/package.json | 2 - .../src/agent/in-process-transport.ts | 6 +- packages/agent-bff/src/build-bff.ts | 87 +++-- .../agent-bff/src/http/bff-http-server.ts | 13 +- packages/agent-bff/src/http/health-route.ts | 23 +- packages/agent-bff/src/index.ts | 9 + packages/agent-bff/test/build-bff.test.ts | 12 +- .../test/data/fixtures/live-agent-harness.ts | 93 ----- .../data/record-contract.integration.test.ts | 74 ---- .../data/search-agent.integration.test.ts | 206 ----------- .../test/http/bff-http-server.test.ts | 4 +- packages/agent/package.json | 14 +- packages/agent/src/agent.ts | 70 +++- packages/agent/src/bff-routes.ts | 28 ++ packages/agent/src/embedded-bff.ts | 125 +++++++ packages/agent/src/framework-mounter.ts | 6 + packages/agent/src/types.ts | 31 ++ packages/agent/test/agent-bff.test.ts | 48 +++ packages/agent/test/bff-routes.test.ts | 34 ++ .../agent/test/bff/embedded-bff.e2e.test.ts | 338 ++++++++++++++++++ .../bff}/fixtures/in-memory-collection.ts | 0 .../fixtures/record-contract-datasource.ts | 0 .../test/bff}/fixtures/search-datasource.ts | 0 .../test/bff/record-contract.e2e.test.ts | 126 +++++++ 25 files changed, 942 insertions(+), 415 deletions(-) delete mode 100644 packages/agent-bff/test/data/fixtures/live-agent-harness.ts delete mode 100644 packages/agent-bff/test/data/record-contract.integration.test.ts delete mode 100644 packages/agent-bff/test/data/search-agent.integration.test.ts create mode 100644 packages/agent/src/bff-routes.ts create mode 100644 packages/agent/src/embedded-bff.ts create mode 100644 packages/agent/test/agent-bff.test.ts create mode 100644 packages/agent/test/bff-routes.test.ts create mode 100644 packages/agent/test/bff/embedded-bff.e2e.test.ts rename packages/{agent-bff/test/data => agent/test/bff}/fixtures/in-memory-collection.ts (100%) rename packages/{agent-bff/test/data => agent/test/bff}/fixtures/record-contract-datasource.ts (100%) rename packages/{agent-bff/test/data => agent/test/bff}/fixtures/search-datasource.ts (100%) create mode 100644 packages/agent/test/bff/record-contract.e2e.test.ts diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 500159817a..573eaedaee 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -150,10 +150,10 @@ jobs: OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} - # Boots a real agent on a local HTTP port, so it stays out of the unit job. Unlike the LLM suite - # it reaches no third party and is deterministic, hence no continue-on-error: it must gate. + # Boots a real agent, so it stays out of the unit job. Unlike the LLM suite it reaches no third + # party and is deterministic, hence no continue-on-error: it must gate. bff-integration-tests: - name: BFF Integration Tests (agent-bff) + name: BFF Integration Tests (embedded BFF) runs-on: ubuntu-latest timeout-minutes: 15 needs: [build] @@ -177,7 +177,7 @@ jobs: key: ${{ runner.os }}-build-${{ github.sha }} fail-on-cache-miss: true - name: Run BFF integration tests - run: yarn workspace @forestadmin/agent-bff test --testPathPattern='search-agent.integration' + run: yarn workspace @forestadmin/agent test --testPathPattern='bff/.*\.e2e' send-coverage: name: Send Coverage diff --git a/packages/agent-bff/package.json b/packages/agent-bff/package.json index b281df36a0..870d48d123 100644 --- a/packages/agent-bff/package.json +++ b/packages/agent-bff/package.json @@ -44,8 +44,6 @@ "zod": "4.3.6" }, "devDependencies": { - "@forestadmin/agent": "1.99.2", - "@forestadmin/agent-testing": "1.2.16", "@hey-api/openapi-ts": "0.99.0", "@redocly/cli": "2.35.1", "@types/inflected": "^1.1.29", diff --git a/packages/agent-bff/src/agent/in-process-transport.ts b/packages/agent-bff/src/agent/in-process-transport.ts index a75bff2cb8..efd58b7b40 100644 --- a/packages/agent-bff/src/agent/in-process-transport.ts +++ b/packages/agent-bff/src/agent/in-process-transport.ts @@ -5,7 +5,7 @@ import { HttpRequester } from '@forestadmin/agent-client'; import { streamingUnsupported } from '../http/bff-local-errors'; /** A sentinel that never reaches the network: `query` answers before any socket is opened. */ -const IN_PROCESS_URL = 'http://in-process.agent'; +export const IN_PROCESS_AGENT_URL = 'http://in-process.agent'; export interface AgentDispatchRequest { method: 'get' | 'post' | 'put' | 'delete'; @@ -46,7 +46,7 @@ class InProcessRequester extends HttpRequester { private readonly dispatcher: AgentDispatcher, private readonly defaultTimeoutMs?: number, ) { - super(bearerToken, { url: IN_PROCESS_URL }); + super(bearerToken, { url: IN_PROCESS_AGENT_URL }); } // No socket to stream from, and nothing in the BFF streams today. A typed 501 rather than a raw @@ -141,7 +141,7 @@ export default function createInProcessTransport({ timeoutMs, }: InProcessTransportOptions): AgentTransport { return { - url: IN_PROCESS_URL, + url: IN_PROCESS_AGENT_URL, createRequester: token => new InProcessRequester(token, dispatcher, timeoutMs), }; } diff --git a/packages/agent-bff/src/build-bff.ts b/packages/agent-bff/src/build-bff.ts index a0650f290a..86132fbd41 100644 --- a/packages/agent-bff/src/build-bff.ts +++ b/packages/agent-bff/src/build-bff.ts @@ -1,3 +1,5 @@ +import type { AgentTransport } from './agent/agent-transport'; +import type { AgentDispatcher } from './agent/in-process-transport'; import type { BFFConfig } from './config/env-config'; import type { EnvironmentIdResolver } from './oauth/environment-id'; import type { SessionStore } from './oauth/session-store'; @@ -15,6 +17,7 @@ import createActionRoutesMiddleware from './action/action-routes-middleware'; import createConsoleLogger from './adapters/console-logger'; import createAgentStubMiddleware from './agent/agent-stub'; import { createHttpTransport } from './agent/agent-transport'; +import createInProcessTransport from './agent/in-process-transport'; import AiProxyClient from './ai/ai-proxy-client'; import createAiRoutesMiddleware, { AI_QUERY_ROUTE } from './ai/ai-routes-middleware'; import createApiKeyAuthenticator from './api-key/api-key-authenticator'; @@ -64,6 +67,11 @@ export interface BuildBffOptions { * origin root. Normalized by `normalizeBasePath`, which throws on anything else. */ basePath?: string; + /** + * Reaches an agent living in the same process, without a socket. When set, it replaces the HTTP + * transport entirely: `AGENT_URL` then only names where the agent answers, never how it is called. + */ + dispatcher?: AgentDispatcher; } export interface Bff { @@ -283,6 +291,23 @@ function resolveReadModelBundle( return { store, apiKeyConfig }; } +/** + * How this deployment reaches the agent: in-process when a dispatcher was handed over, over HTTP + * when an AGENT_URL was configured, and not at all otherwise — which is what makes the data and + * action routes fall back to their stub. + */ +function resolveTransport( + config: BFFConfig, + dispatcher: AgentDispatcher | undefined, +): AgentTransport | undefined { + const timeoutMs = config.agentTimeoutMs; + + if (dispatcher) return createInProcessTransport({ dispatcher, timeoutMs }); + if (config.agentUrl) return createHttpTransport({ agentUrl: config.agentUrl, timeoutMs }); + + return undefined; +} + /** * When unfolding is possible, in ONE place: the document needs the AGENT_URL the store does not, * because a collection's field set comes from the agent capabilities. The server passes the bundle it @@ -291,19 +316,12 @@ function resolveReadModelBundle( */ function toUnfoldSource( bundle: ReadModelBundle | undefined, - config: BFFConfig, + transport: AgentTransport | undefined, logger: Logger, ): UnfoldSource | undefined { - if (!bundle || !config.agentUrl) return undefined; + if (!bundle || !transport) return undefined; - return { - store: bundle.store, - transport: createHttpTransport({ - agentUrl: config.agentUrl, - timeoutMs: config.agentTimeoutMs, - }), - logger, - }; + return { store: bundle.store, transport, logger }; } /** @@ -315,13 +333,17 @@ function toUnfoldSource( const UNMEASURED: Metrics = { increment: () => undefined, gauge: () => undefined }; export function resolveUnfoldSource(config: BFFConfig, logger: Logger): UnfoldSource | undefined { - return toUnfoldSource(resolveReadModelBundle(config, logger, UNMEASURED), config, logger); + return toUnfoldSource( + resolveReadModelBundle(config, logger, UNMEASURED), + resolveTransport(config, undefined), + logger, + ); } // The data middleware falls through to the action middleware on a non-data path. function buildAgentRouteMiddlewares( bundle: ReadModelBundle | undefined, - config: BFFConfig, + transport: AgentTransport | undefined, logger: Logger, permissionsCache: PermissionsCache, ): Middleware[] { @@ -335,7 +357,6 @@ function buildAgentRouteMiddlewares( } const { store, apiKeyConfig } = bundle; - const { agentUrl, agentTimeoutMs: timeoutMs } = config; const permissionsMiddleware = createPermissionsRoutesMiddleware({ store, @@ -347,14 +368,12 @@ function buildAgentRouteMiddlewares( logger, }); - if (!agentUrl) { + if (!transport) { logger('Warn', 'Data and action endpoints disabled: AGENT_URL is missing'); return [permissionsMiddleware, createAgentStubMiddleware()]; } - const transport = createHttpTransport({ agentUrl, timeoutMs }); - return [ permissionsMiddleware, createDataRoutesMiddleware({ store, transport, logger }), @@ -398,6 +417,7 @@ function buildAgentMiddlewares( oauth: OAuthEdge, aiMiddlewares: Middleware[], basePath: string, + transport: AgentTransport | undefined, ): AgentEdge { const { forestAuthSecret, defaultTimezone } = config; @@ -411,7 +431,7 @@ function buildAgentMiddlewares( // One store for the whole edge. The document only unfolds when the agent is reachable too: with no // AGENT_URL every data path answers 501, so concrete paths would advertise a dead surface. const bundle = resolveReadModelBundle(config, logger); - const source = toUnfoldSource(bundle, config, logger); + const source = toUnfoldSource(bundle, transport, logger); const permissionsCache = new PermissionsCache(); const chain: Middleware[] = [ @@ -442,7 +462,7 @@ function buildAgentMiddlewares( : []), ...aiMiddlewares, createTimezoneMiddleware({ defaultTimezone }), - ...buildAgentRouteMiddlewares(bundle, config, logger, permissionsCache), + ...buildAgentRouteMiddlewares(bundle, transport, logger, permissionsCache), ]; return { @@ -468,6 +488,7 @@ export default async function buildBff({ config, logger = createConsoleLogger(), basePath, + dispatcher, }: BuildBffOptions): Promise { // Before anything is assembled: a mount the host does not serve must fail at boot, not surface as // a docs page that cannot load itself. @@ -481,9 +502,24 @@ export default async function buildBff({ warnMissingConfig(config, logger); + if (config.allowedOrigins.length === 0) { + logger( + 'Warn', + 'No allowed origin: no browser can call this BFF. Set BFF_ALLOWED_ORIGINS, or `allowedOrigins`.', + ); + } + + const transport = resolveTransport(config, dispatcher); const oauth = buildOAuthMiddlewares(config, logger); const aiMiddlewares = buildAiMiddlewares(config, oauth, logger); - const agentEdge = buildAgentMiddlewares(config, logger, oauth, aiMiddlewares, mountPath); + const agentEdge = buildAgentMiddlewares( + config, + logger, + oauth, + aiMiddlewares, + mountPath, + transport, + ); const agentMiddlewares = agentEdge.middlewares; const hasAgentEdge = agentMiddlewares.length > 0; const agentErrorMiddleware = hasAgentEdge ? [agentScoped(createErrorMiddleware({ logger }))] : []; @@ -491,7 +527,18 @@ export default async function buildBff({ const middlewares = [ createVersionHeaderMiddleware(version), - createHealthRoute({ config, version }), + createHealthRoute({ + version, + // Embedded, everything required is inherited from the agent, so there is no gap to report: a + // 503 here would let a load balancer restart a process that serves api-key traffic fine. + healthy: dispatcher !== undefined || config.hasAllRequired, + features: { + oauth: oauth.middlewares.length > 0, + ai: aiMiddlewares.length > 0, + cors: config.allowedOrigins.length > 0, + openapi: config.openapiEnabled && agentMiddlewares.length > 0, + }, + }), createCorsMiddleware({ allowedOrigins: config.allowedOrigins, logger }), ...agentErrorMiddleware, ...agentJsonOnlyGuard, diff --git a/packages/agent-bff/src/http/bff-http-server.ts b/packages/agent-bff/src/http/bff-http-server.ts index 276387d5e4..17f6d87f63 100644 --- a/packages/agent-bff/src/http/bff-http-server.ts +++ b/packages/agent-bff/src/http/bff-http-server.ts @@ -67,7 +67,18 @@ export default class BFFHttpServer { const app = new Koa(); app.use(createVersionHeaderMiddleware(version)); - app.use(createHealthRoute({ config, version })); + app.use( + createHealthRoute({ + version, + healthy: config.hasAllRequired, + features: { + oauth: Boolean(config.tokenEncryptionKey), + ai: Boolean(config.tokenEncryptionKey), + cors: config.allowedOrigins.length > 0, + openapi: config.openapiEnabled, + }, + }), + ); for (const middleware of options.middlewares ?? []) { app.use(middleware); diff --git a/packages/agent-bff/src/http/health-route.ts b/packages/agent-bff/src/http/health-route.ts index a1165952af..fc42ef1386 100644 --- a/packages/agent-bff/src/http/health-route.ts +++ b/packages/agent-bff/src/http/health-route.ts @@ -1,14 +1,27 @@ -import type { BFFConfig } from '../config/env-config'; import type { Middleware } from 'koa'; export const HEALTH_PATH = '/health'; +/** What the deployment actually serves, and therefore what its configuration switched on. */ +export interface HealthFeatures { + oauth: boolean; + ai: boolean; + cors: boolean; + openapi: boolean; +} + export interface HealthRouteOptions { - config: BFFConfig; version: string; + /** Whether everything this deployment needs is configured. Embedded, it always is. */ + healthy: boolean; + features: HealthFeatures; } -export default function createHealthRoute({ config, version }: HealthRouteOptions): Middleware { +export default function createHealthRoute({ + version, + healthy, + features, +}: HealthRouteOptions): Middleware { return async function health(ctx, next) { const isHealthRequest = (ctx.method === 'GET' || ctx.method === 'HEAD') && ctx.path === HEALTH_PATH; @@ -19,7 +32,7 @@ export default function createHealthRoute({ config, version }: HealthRouteOption return; } - ctx.status = config.hasAllRequired ? 200 : 503; - ctx.body = { status: config.hasAllRequired ? 'ok' : 'degraded', version }; + ctx.status = healthy ? 200 : 503; + ctx.body = { status: healthy ? 'ok' : 'degraded', version, features }; }; } diff --git a/packages/agent-bff/src/index.ts b/packages/agent-bff/src/index.ts index 72b9fe34bf..1d35a720c0 100644 --- a/packages/agent-bff/src/index.ts +++ b/packages/agent-bff/src/index.ts @@ -3,6 +3,15 @@ export { parseConfig, REQUIRED_KEYS } from './config/env-config'; export type { BFFConfig, PresenceMap, RequiredKey } from './config/env-config'; export { default as runCli } from './cli-core'; export { default as buildBff } from './build-bff'; +export { + default as createInProcessTransport, + IN_PROCESS_AGENT_URL, +} from './agent/in-process-transport'; +export type { + AgentDispatcher, + AgentDispatchRequest, + AgentDispatchResponse, +} from './agent/in-process-transport'; export type { Bff, BuildBffOptions, BffCallback } from './build-bff'; export { ConfigurationError } from './errors'; export { default as DEFAULT_BFF_PORT } from './defaults'; diff --git a/packages/agent-bff/test/build-bff.test.ts b/packages/agent-bff/test/build-bff.test.ts index 5d34b1ca7b..917df3caad 100644 --- a/packages/agent-bff/test/build-bff.test.ts +++ b/packages/agent-bff/test/build-bff.test.ts @@ -59,7 +59,11 @@ describe('buildBff', () => { const response = await request(callback).get('/health'); expect(response.status).toBe(200); - expect(response.body).toEqual({ status: 'ok', version }); + expect(response.body).toEqual({ + status: 'ok', + version, + features: { oauth: true, ai: true, cors: false, openapi: true }, + }); }); it('should set the version header on /health, which only holds if it is mounted first', async () => { @@ -78,7 +82,11 @@ describe('buildBff', () => { const response = await request(callback).get('/health'); expect(response.status).toBe(503); - expect(response.body).toEqual({ status: 'degraded', version }); + expect(response.body).toEqual({ + status: 'degraded', + version, + features: { oauth: false, ai: false, cors: false, openapi: true }, + }); }); it('should warn naming the missing keys', async () => { diff --git a/packages/agent-bff/test/data/fixtures/live-agent-harness.ts b/packages/agent-bff/test/data/fixtures/live-agent-harness.ts deleted file mode 100644 index fb848a78b5..0000000000 --- a/packages/agent-bff/test/data/fixtures/live-agent-harness.ts +++ /dev/null @@ -1,93 +0,0 @@ -import type { Logger } from '../../../src/ports/logger-port'; -import type { SchemaFetcher } from '../../../src/read-model/forest-schema-client'; -import type { ForestSchemaCollection } from '@forestadmin/forestadmin-client'; - -import { bodyParser } from '@koa/bodyparser'; -import fs from 'fs/promises'; -import jsonwebtoken from 'jsonwebtoken'; -import Koa from 'koa'; -import net from 'net'; - -import { createHttpTransport } from '../../../src/agent/agent-transport'; -import createDataRoutesMiddleware from '../../../src/data/data-routes-middleware'; -import createErrorMiddleware from '../../../src/http/error-middleware'; -import CapabilitiesCache from '../../../src/read-model/capabilities-cache'; -import ReadModelStore from '../../../src/read-model/read-model-store'; -import SchemaCache from '../../../src/read-model/schema-cache'; - -const TIMEZONE = 'Europe/Paris'; -export const AUTH_SECRET = 'b0bdf0a639c16bae8851dd24ee3d79ef0a352e957c5b86cb'; -export const ENV_SECRET = 'ceba742f5bc73946b34da192816a4d7177b3233fee7769955c29c0e90fd584f2'; -export const BOOT_TIMEOUT_MS = 60_000; - -// agent-testing only deletes a schema file whose name carries this prefix, so reusing it keeps the -// temporary schema cleaned up by `agent.stop()` even though the path is chosen here. -export const RESERVED_SCHEMA_PREFIX = 'reserved-forestadmin-schema-test-'; - -const noopLogger: Logger = () => {}; - -export async function findFreePort(): Promise { - return new Promise((resolve, reject) => { - const server = net.createServer(); - - server.on('error', reject); - server.listen(0, () => { - const { port } = server.address() as net.AddressInfo; - - server.close(() => resolve(port)); - }); - }); -} - -function agentToken(): string { - return jsonwebtoken.sign( - { id: 1, email: 'forest@forest.com', renderingId: 1, team: 'admin' }, - AUTH_SECRET, - { expiresIn: '1 hour' }, - ); -} - -function schemaFetcherFromFile(schemaPath: string): SchemaFetcher { - return { - fetchSchema: async () => { - const { collections } = JSON.parse(await fs.readFile(schemaPath, 'utf8')) as { - collections: ForestSchemaCollection[]; - }; - - return collections; - }, - }; -} - -/** - * The BFF in front of a real agent: the middleware production mounts, the real data client, and a - * read-model built from the schema the agent just wrote. Only the schema transport is swapped — - * production fetches it from the Forest server, which plays no part in what these suites assert. - */ -export function buildApp(agentUrl: string, schemaPath: string): Koa { - const token = agentToken(); - const schemaCache = new SchemaCache({ - fetcher: schemaFetcherFromFile(schemaPath), - metrics: { increment: () => {}, gauge: () => {} }, - }); - const store = new ReadModelStore(schemaCache, new CapabilitiesCache()); - const app = new Koa(); - - app.silent = true; - app.use(createErrorMiddleware({ logger: noopLogger })); - app.use(bodyParser()); - app.use(async (ctx, next) => { - ctx.state.timezone = TIMEZONE; - ctx.state.agentToken = token; - await next(); - }); - app.use( - createDataRoutesMiddleware({ - store, - transport: createHttpTransport({ agentUrl }), - logger: noopLogger, - }), - ); - - return app; -} diff --git a/packages/agent-bff/test/data/record-contract.integration.test.ts b/packages/agent-bff/test/data/record-contract.integration.test.ts deleted file mode 100644 index 69c92faaf9..0000000000 --- a/packages/agent-bff/test/data/record-contract.integration.test.ts +++ /dev/null @@ -1,74 +0,0 @@ -import type { TestableAgent } from '@forestadmin/agent-testing'; -import type Koa from 'koa'; - -import { createTestableAgent } from '@forestadmin/agent-testing'; -import os from 'os'; -import path from 'path'; -import request from 'supertest'; - -import { - AUTH_SECRET, - BOOT_TIMEOUT_MS, - ENV_SECRET, - RESERVED_SCHEMA_PREFIX, - buildApp, - findFreePort, -} from './fixtures/live-agent-harness'; -import RecordContractDataSource from './fixtures/record-contract-datasource'; - -describe('the record contract against a real agent', () => { - let agent: TestableAgent; - let app: Koa; - - beforeAll(async () => { - const port = await findFreePort(); - const schemaPath = path.join( - os.tmpdir(), - `${RESERVED_SCHEMA_PREFIX}-bff-record-contract-${port}.json`, - ); - - agent = await createTestableAgent( - forestAgent => { - forestAgent.addDataSource(async () => new RecordContractDataSource()); - }, - { authSecret: AUTH_SECRET, envSecret: ENV_SECRET, isProduction: false, port, schemaPath }, - ); - - await agent.start(); - - app = buildApp(`http://localhost:${port}`, schemaPath); - }, BOOT_TIMEOUT_MS); - - afterAll(async () => { - await agent?.stop(); - }); - - it('should carry the flat id as a string while __forest.primaryKey holds it typed', async () => { - const response = await request(app.callback()) - .post('/agent/v1/people/list') - .send({ projection: ['id'] }); - - expect(response.status).toBe(200); - expect(response.body.data).toEqual([ - { - id: '8', - __forest: { collection: 'people', primaryKey: { id: 8 } }, - }, - ]); - }); - - it('should return a snake_case column under its camelCase key, while projecting its schema name', async () => { - const response = await request(app.callback()) - .post('/agent/v1/people/list') - .send({ projection: ['id', 'first_name'] }); - - expect(response.status).toBe(200); - expect(response.body.data).toEqual([ - { - id: '8', - firstName: 'Ada', - __forest: { collection: 'people', primaryKey: { id: 8 } }, - }, - ]); - }); -}); diff --git a/packages/agent-bff/test/data/search-agent.integration.test.ts b/packages/agent-bff/test/data/search-agent.integration.test.ts deleted file mode 100644 index aebd268dae..0000000000 --- a/packages/agent-bff/test/data/search-agent.integration.test.ts +++ /dev/null @@ -1,206 +0,0 @@ -import type { TestableAgent } from '@forestadmin/agent-testing'; -import type Koa from 'koa'; - -import { createTestableAgent } from '@forestadmin/agent-testing'; -import os from 'os'; -import path from 'path'; -import request from 'supertest'; - -import { - AUTH_SECRET, - BOOT_TIMEOUT_MS, - ENV_SECRET, - RESERVED_SCHEMA_PREFIX, - buildApp, - findFreePort, -} from './fixtures/live-agent-harness'; -import SearchDataSource from './fixtures/search-datasource'; - -describe('search against a real agent', () => { - let agent: TestableAgent; - let app: Koa; - - beforeAll(async () => { - const port = await findFreePort(); - const schemaPath = path.join(os.tmpdir(), `${RESERVED_SCHEMA_PREFIX}-bff-search-${port}.json`); - - agent = await createTestableAgent( - forestAgent => { - forestAgent.addDataSource(async () => new SearchDataSource()); - forestAgent.customizeCollection('books', collection => - collection.addManyToOneRelation('author', 'authors', { foreignKey: 'authorId' }), - ); - forestAgent.customizeCollection('ledgers', collection => collection.disableSearch()); - }, - { authSecret: AUTH_SECRET, envSecret: ENV_SECRET, isProduction: false, port, schemaPath }, - ); - - await agent.start(); - - app = buildApp(`http://localhost:${port}`, schemaPath); - }, BOOT_TIMEOUT_MS); - - afterAll(async () => { - await agent?.stop(); - }); - - function titlesOf(body: { data: Array<{ title: string }> }): string[] { - return body.data.map(record => record.title).sort(); - } - - describe('list', () => { - it('should return only the records the search matches', async () => { - const response = await request(app.callback()) - .post('/agent/v1/books/list') - .send({ projection: ['id', 'title'], search: 'foundation' }); - - expect(response.status).toBe(200); - expect(titlesOf(response.body)).toEqual(['Foundation']); - }); - - it('should return every record when no search is sent', async () => { - const response = await request(app.callback()) - .post('/agent/v1/books/list') - .send({ projection: ['id', 'title'] }); - - expect(response.status).toBe(200); - expect(titlesOf(response.body)).toEqual(['Foundation', 'I, Robot', 'The Dispossessed']); - }); - - it('should not match a related record without searchExtended', async () => { - const response = await request(app.callback()) - .post('/agent/v1/books/list') - .send({ projection: ['id', 'title'], search: 'asimov' }); - - expect(response.status).toBe(200); - expect(response.body.data).toEqual([]); - }); - - it('should match through a relation when searchExtended is true', async () => { - const response = await request(app.callback()) - .post('/agent/v1/books/list') - .send({ projection: ['id', 'title'], search: 'asimov', searchExtended: true }); - - expect(response.status).toBe(200); - expect(titlesOf(response.body)).toEqual(['Foundation', 'I, Robot']); - }); - - // Accepted behaviour, pinned here so it cannot change unnoticed: the search value is a query, - // and `relation.column:value` crosses a relation with no `searchExtended`. The same path in a - // filter draws 422 relation_field_not_supported, so this is the one way a request reaches a - // relation column through the top-level routes. - it('should cross a relation through the query syntax without searchExtended', async () => { - const response = await request(app.callback()) - .post('/agent/v1/books/list') - .send({ projection: ['id', 'title'], search: 'author.name:asimov' }); - - expect(response.status).toBe(200); - expect(titlesOf(response.body)).toEqual(['Foundation', 'I, Robot']); - }); - - it('should reject the same relation path in a filter, unlike in a search', async () => { - const response = await request(app.callback()) - .post('/agent/v1/books/list') - .send({ - projection: ['id', 'title'], - filter: { field: 'author:name', operator: 'IContains', value: 'asimov' }, - }); - - expect(response.status).toBe(422); - expect(response.body.error).toMatchObject({ type: 'relation_field_not_supported' }); - }); - - it('should intersect the search with the filter rather than replace it', async () => { - const response = await request(app.callback()) - .post('/agent/v1/books/list') - .send({ - projection: ['id', 'title'], - search: 'asimov', - searchExtended: true, - filter: { field: 'title', operator: 'IContains', value: 'robot' }, - }); - - expect(response.status).toBe(200); - expect(titlesOf(response.body)).toEqual(['I, Robot']); - }); - - it('should list everything when the search holds only whitespace', async () => { - const response = await request(app.callback()) - .post('/agent/v1/books/list') - .send({ projection: ['id', 'title'], search: ' ' }); - - expect(response.status).toBe(200); - expect(titlesOf(response.body)).toEqual(['Foundation', 'I, Robot', 'The Dispossessed']); - }); - }); - - describe('count', () => { - it('should count the searched rows, not the whole collection', async () => { - const searched = await request(app.callback()) - .post('/agent/v1/books/count') - .send({ search: 'foundation' }); - const all = await request(app.callback()).post('/agent/v1/books/count').send({}); - - expect(all.body).toEqual({ count: 3, countStatus: 'available' }); - expect(searched.body).toEqual({ count: 1, countStatus: 'available' }); - }); - - it('should count the rows a relation-extended search returns', async () => { - const response = await request(app.callback()) - .post('/agent/v1/books/count') - .send({ search: 'asimov', searchExtended: true }); - - expect(response.status).toBe(200); - expect(response.body).toEqual({ count: 2, countStatus: 'available' }); - }); - }); - - describe('a collection whose search is disabled', () => { - // Outside production the agent dumps every handled error to stderr, so the two rejections - // asserted here would drown the suite output in expected stack traces. - let consoleError: jest.SpyInstance; - - beforeAll(() => { - consoleError = jest.spyOn(console, 'error').mockImplementation(() => {}); - }); - - afterAll(() => { - consoleError.mockRestore(); - }); - - it('should reject a list search rather than return an unfiltered listing', async () => { - const response = await request(app.callback()) - .post('/agent/v1/ledgers/list') - .send({ projection: ['id', 'label'], search: 'foundation' }); - - expect(response.status).toBe(400); - expect(response.body.error).toMatchObject({ - type: 'validation_error', - status: 400, - message: 'Collection is not searchable', - }); - }); - - it('should reject a count search the same way list does', async () => { - const response = await request(app.callback()) - .post('/agent/v1/ledgers/count') - .send({ search: 'foundation' }); - - expect(response.status).toBe(400); - expect(response.body.error).toMatchObject({ - type: 'validation_error', - status: 400, - message: 'Collection is not searchable', - }); - }); - - it('should still serve it when no search is sent', async () => { - const response = await request(app.callback()) - .post('/agent/v1/ledgers/list') - .send({ projection: ['id', 'label'] }); - - expect(response.status).toBe(200); - expect(response.body.data).toHaveLength(1); - }); - }); -}); diff --git a/packages/agent-bff/test/http/bff-http-server.test.ts b/packages/agent-bff/test/http/bff-http-server.test.ts index 91b9b73055..a64d319129 100644 --- a/packages/agent-bff/test/http/bff-http-server.test.ts +++ b/packages/agent-bff/test/http/bff-http-server.test.ts @@ -85,7 +85,7 @@ describe('BFFHttpServer', () => { const response = await request(server.callback).get('/health'); expect(response.status).toBe(200); - expect(response.body).toEqual({ status: 'ok', version: VERSION }); + expect(response.body).toMatchObject({ status: 'ok', version: VERSION }); }); it('should never expose config presence or secret values in the response body', async () => { @@ -116,7 +116,7 @@ describe('BFFHttpServer', () => { const response = await request(server.callback).get('/health'); expect(response.status).toBe(503); - expect(response.body).toEqual({ status: 'degraded', version: VERSION }); + expect(response.body).toMatchObject({ status: 'degraded', version: VERSION }); }); it('should answer HEAD /health with 503', async () => { diff --git a/packages/agent/package.json b/packages/agent/package.json index e263420f83..b8e8b9de85 100644 --- a/packages/agent/package.json +++ b/packages/agent/package.json @@ -49,6 +49,7 @@ }, "devDependencies": { "@fastify/express": "^1.1.0", + "@forestadmin/agent-bff": "1.25.4", "@forestadmin/datasource-sql": "1.17.14", "@forestadmin/workflow-executor": "1.28.0", "@nestjs/common": "^11.1.24", @@ -56,30 +57,39 @@ "@nestjs/platform-express": "^11.1.24", "@nestjs/platform-fastify": "^11.1.24", "@shopify/jest-koa-mocks": "^3.1.0", - "sqlite3": "^5.1.7", "@types/json-api-serializer": "^2.6.3", "@types/jsonwebtoken": "^9.0.1", "@types/koa": "^2.13.5", "@types/koa__cors": "^3.3.0", "@types/superagent": "^8.1.9", + "@types/supertest": "^6.0.2", "express": "^4.18.2", "fastify": "^3.29.0", "fastify2": "npm:fastify@^2.15.3", "fastify4": "npm:fastify@^4.28.0", "openid-client": "^5.7.1", "reflect-metadata": "^0.1.13", - "rxjs": "^7.8.0" + "rxjs": "^7.8.0", + "sqlite3": "^5.1.7", + "supertest": "^7.1.3" }, "peerDependencies": { "@fastify/express": "^1.1.0 || ^2.0.0 || ^3.0.0 || ^4.0.0", + "@forestadmin/agent-bff": "1.25.4", "@forestadmin/workflow-executor": "1.28.0" }, "peerDependenciesMeta": { "@fastify/express": { "optional": true }, + "@forestadmin/agent-bff": { + "optional": true + }, "@forestadmin/workflow-executor": { "optional": true } + }, + "engines": { + "node": ">=22.12.0" } } diff --git a/packages/agent/src/agent.ts b/packages/agent/src/agent.ts index f46aa15828..a14c22176f 100644 --- a/packages/agent/src/agent.ts +++ b/packages/agent/src/agent.ts @@ -3,6 +3,7 @@ import type { ForestAdminHttpDriverServices } from './services'; import type { AgentOptions, AgentOptionsWithDefaults, + BffEmbedOptions, RootHandler, WorkflowExecutorEmbedOptions, } from './types'; @@ -26,6 +27,8 @@ import { readFile, writeFile } from 'fs/promises'; import stringify from 'json-stringify-pretty-compact'; import { installAuditTrailHooks } from './audit-trail'; +import { BFF_PREFIX } from './bff-routes'; +import EmbeddedBff from './embedded-bff'; import EmbeddedWorkflowExecutor from './embedded-workflow-executor'; import FrameworkMounter from './framework-mounter'; import makeRoutes from './routes'; @@ -35,6 +38,12 @@ import { CORRELATION_ID_HEADER, correlationIdMiddleware } from './utils/correlat import SchemaGenerator from './utils/forest-schema/generator'; import OptionsValidator from './utils/options-validator'; +// Whichever is registered second raises it: the MCP server claims `/oauth` and +// `/mcp`, and it is consulted first, so it would shadow two of the BFF's own routes. +const BFF_MCP_COLLISION = + `Cannot use addBff together with mountAiMcpServer({ basePath: '${BFF_PREFIX}' }): the MCP ` + + `server would claim ${BFF_PREFIX}/oauth and ${BFF_PREFIX}/mcp. Mount the MCP server elsewhere.`; + /** * Allow to create a new Forest Admin agent from scratch. * Builds the application by composing and configuring all the collection decorators. @@ -63,6 +72,9 @@ export default class Agent extends FrameworkMounter /** In-process workflow executor, created only when addWorkflowExecutor() is called. */ private embeddedExecutor: EmbeddedWorkflowExecutor | null = null; + /** In-process BFF, created only when addBff() is called. */ + private embeddedBff: EmbeddedBff | null = null; + private isRestarting = false; /** @@ -112,6 +124,8 @@ export default class Agent extends FrameworkMounter // Boot after mount(): the embedded executor reaches the agent over HTTP, and the // standalone server's host/port (used to derive that URL) are only known once mounted. await this.embeddedExecutor?.start(this.standaloneServerHost, this.standaloneServerPort); + // Same reason, without the socket: the dispatcher injects into the stack mount() just built. + await this.embeddedBff?.start(this.getInProcessDispatcher()); } catch (error) { const { message } = error as Error; this.options.logger('Error', `Forest Admin agent startup failure: ${message}`); @@ -141,7 +155,10 @@ export default class Agent extends FrameworkMounter * Stop the agent. */ override async stop(): Promise { - // Drain the embedded executor first, while the agent it depends on is still serving. + // Stop answering before the stack it dispatches into goes away: the host application keeps + // whatever middleware it registered, so a stopped agent would otherwise still serve BFF data. + this.embeddedBff?.stop(); + // Drain the embedded executor next, while the agent it depends on is still serving. await this.embeddedExecutor?.stop(); // Close anything related to ForestAdmin client this.options.forestAdminClient.close(); @@ -183,6 +200,8 @@ export default class Agent extends FrameworkMounter this.setMcpCallback(mcp ?? null); await this.remount(router); + // A restart means the customizations changed, so the schema the BFF read is stale. + this.embeddedBff?.invalidate(); } finally { this.isRestarting = false; } @@ -314,6 +333,10 @@ export default class Agent extends FrameworkMounter allowedOAuthClients?: string[]; fileUploads?: false | FileUploadsOptions; }): this { + if (this.embeddedBff && options?.basePath === BFF_PREFIX) { + throw new Error(BFF_MCP_COLLISION); + } + this.mcpEnabled = true; this.mcpEnabledTools = options?.enabledTools; this.mcpBasePath = options?.basePath; @@ -362,6 +385,51 @@ export default class Agent extends FrameworkMounter return this; } + /** + * Serve a BFF in-process, alongside the agent, at `/bff` — no second deployment, no second port. + * The agent builds it on start(), stops it on stop(), and hands it a dispatcher that reaches its + * own stack without a socket, so this works the same on every mount target. + * + * Requires the `@forestadmin/agent-bff` package to be installed: + * ```bash + * npm install @forestadmin/agent-bff + * ``` + * + * The secrets, the Forest urls and the logger are inherited from the agent. Everything left is + * a feature the BFF switches on when configured: `tokenEncryptionKey` enables OAuth (and with it + * the AI relay), `allowedOrigins` enables browser access, `openapiEnabled` serves the docs. + * + * @param options embedded BFF options + * @returns the agent instance for chaining + * @throws Error if called more than once, or if the MCP server already claims `/bff` + * + * @example + * createAgent(options) + * .addDataSource(...) + * .addBff({ allowedOrigins: ['https://my-app.com'] }) + * .start(); + */ + addBff(options: BffEmbedOptions = {}): this { + if (this.embeddedBff) { + throw new Error('addBff can only be called once.'); + } + + if (this.mcpBasePath === BFF_PREFIX) { + throw new Error(BFF_MCP_COLLISION); + } + + const bff = new EmbeddedBff(this.options); + bff.configure(options); + this.embeddedBff = bff; + // Registered now rather than at start(): getInProcessDispatcher() pushes its hook the first + // time it is called, and mount() only runs the hooks registered before it — asked for later, + // the dispatcher would have no handler until the first restart. + this.getInProcessDispatcher(); + this.setBffCallback(bff.handle); + + return this; + } + protected getRoutes(dataSource: DataSource, services: ForestAdminHttpDriverServices) { return makeRoutes(dataSource, this.options, services); } diff --git a/packages/agent/src/bff-routes.ts b/packages/agent/src/bff-routes.ts new file mode 100644 index 0000000000..8dfe5c7f89 --- /dev/null +++ b/packages/agent/src/bff-routes.ts @@ -0,0 +1,28 @@ +/** + * Where an embedded BFF answers, at the root of the host application. Fixed: the BFF serves its own + * `/oauth/*` and `/docs`, which would otherwise collide with the MCP server's root paths and with + * whatever the host already serves. + */ +export const BFF_PREFIX = '/bff'; + +/** + * Matches on the pathname and on a segment boundary, so `/bff?x=1` is claimed and `/bffalo` is not. + */ +export function isBffRoute(url: string): boolean { + const [pathname] = url.split(/[?#]/, 1); + + return pathname === BFF_PREFIX || pathname.startsWith(`${BFF_PREFIX}/`); +} + +/** + * The url the BFF itself expects: it knows nothing of the prefix the host serves it under, and its + * routes are absolute (`/agent/v1/…`, `/health`). Never yields an empty string — Koa would read that + * as a malformed request rather than as the root. + */ +export function stripBffPrefix(url: string): string { + const remainder = url.slice(BFF_PREFIX.length); + + if (remainder === '') return '/'; + + return remainder.startsWith('/') ? remainder : `/${remainder}`; +} diff --git a/packages/agent/src/embedded-bff.ts b/packages/agent/src/embedded-bff.ts new file mode 100644 index 0000000000..a09da31795 --- /dev/null +++ b/packages/agent/src/embedded-bff.ts @@ -0,0 +1,125 @@ +import type { AgentOptionsWithDefaults, BffEmbedOptions, HttpCallback } from './types'; +import type { AgentDispatcher, Bff } from '@forestadmin/agent-bff'; + +import { BFF_PREFIX, stripBffPrefix } from './bff-routes'; + +/** + * Serialize the BFF's structured log context onto the message: the agent's logger only accepts an + * Error as its third argument, so the context would be dropped otherwise. Never throws — logging + * must not break a request. + */ +function formatLog(message: string, context?: Record): string { + if (!context || Object.keys(context).length === 0) return `[bff] ${message}`; + + try { + return `[bff] ${message} ${JSON.stringify(context)}`; + } catch { + return `[bff] ${message} [unserializable context]`; + } +} + +/** + * Owns the lifecycle of a BFF embedded in the agent process: configuration, dynamic loading of the + * optional package, build, and the request handler the agent mounts at `/bff`. The agent only wires + * the callback and delegates start/stop. + */ +export default class EmbeddedBff { + private embedOptions: BffEmbedOptions | null = null; + private bff: Bff | null = null; + + constructor(private readonly options: AgentOptionsWithDefaults) {} + + /** Register the embedded BFF. Nothing is built yet: the agent must be mounted first. */ + configure(embedOptions: BffEmbedOptions): void { + this.embedOptions = embedOptions; + } + + /** + * Build the BFF. Called from agent.start() after mount(), because the dispatcher only reaches the + * agent's own stack once that stack is mounted. + */ + async start(dispatcher: AgentDispatcher): Promise { + const { embedOptions } = this; + if (!embedOptions) return; + + const { buildBff, parseConfig, IN_PROCESS_AGENT_URL } = await this.importPackage(); + + const config = parseConfig({ + FOREST_AUTH_SECRET: this.options.authSecret, + FOREST_ENV_SECRET: this.options.envSecret, + FOREST_SERVER_URL: this.options.forestServerUrl, + FOREST_APP_URL: this.options.forestAppUrl, + // Names where the agent answers, never how it is reached: the dispatcher below is the transport. + AGENT_URL: IN_PROCESS_AGENT_URL, + BFF_TOKEN_ENCRYPTION_KEY: embedOptions.tokenEncryptionKey, + BFF_ALLOWED_ORIGINS: embedOptions.allowedOrigins?.join(','), + BFF_DEFAULT_TIMEZONE: embedOptions.defaultTimezone, + BFF_AGENT_TIMEOUT_MS: embedOptions.agentTimeoutMs?.toString(), + BFF_AI_TIMEOUT_MS: embedOptions.aiTimeoutMs?.toString(), + // Off unless asked for: the document is not filtered per caller, so mounting a BFF must not + // publish the name of every exposed collection and field on an already-open port. + BFF_OPENAPI_ENABLED: String(embedOptions.openapiEnabled ?? false), + }); + + this.bff = await buildBff({ + config, + dispatcher, + basePath: BFF_PREFIX, + logger: (level, message, context) => this.options.logger(level, formatLog(message, context)), + }); + + this.options.logger('Info', formatLog(`Embedded BFF mounted on ${BFF_PREFIX}`)); + } + + /** Drop what the BFF read from the SaaS. Called on a restart, which means the schema moved. */ + invalidate(): void { + this.bff?.invalidate(); + } + + /** + * Stop answering. The host application keeps whatever middleware it registered, so without this + * a stopped agent would go on serving BFF data through a dispatcher pointing at a dead stack. + */ + stop(): void { + this.bff = null; + } + + /** + * The callback the agent registers at `/bff`. Answers 503 rather than falling through while the + * BFF is configured but not built yet: a 404 from the host would read as "wrong url" instead of + * "not started". + */ + readonly handle: HttpCallback = (req, res, next) => { + if (!this.embedOptions) { + next?.(); + + return; + } + + if (!this.bff) { + res.statusCode = 503; + res.setHeader('Content-Type', 'application/json'); + res.end(JSON.stringify({ error: { type: 'bff_not_started', status: 503 } })); + + return; + } + + req.url = stripBffPrefix(req.url ?? BFF_PREFIX); + this.bff.callback(req, res); + }; + + /** + * Dynamically load the optional @forestadmin/agent-bff package. Deferred so agents that embed no + * BFF never load its code at startup. + */ + private async importPackage() { + try { + return await import('@forestadmin/agent-bff'); + } catch (error) { + throw new Error( + 'The embedded BFF requires the `@forestadmin/agent-bff` package. ' + + 'Install it with `npm install @forestadmin/agent-bff`.', + ); + } + } +} diff --git a/packages/agent/src/framework-mounter.ts b/packages/agent/src/framework-mounter.ts index 4fa10994fd..21dd637d90 100644 --- a/packages/agent/src/framework-mounter.ts +++ b/packages/agent/src/framework-mounter.ts @@ -8,6 +8,7 @@ import { createServer } from 'http'; import Koa from 'koa'; import path from 'path'; +import { isBffRoute } from './bff-routes'; import FastifyAdapter from './fastify-adapter'; import InProcessDispatcher from './mcp-in-process-dispatcher'; import RootMiddleware from './root-middleware'; @@ -50,6 +51,11 @@ export default class FrameworkMounter { this.rootMiddleware.set('mcp', handler); } + /** Serve an embedded BFF at `/bff`. Pass null to stop answering there. */ + protected setBffCallback(callback: HttpCallback | null): void { + this.rootMiddleware.set('bff', callback, isBffRoute); + } + /** * Dispatcher that runs agent-client requests against the agent's own `/forest` stack in-memory. * The handler is rebuilt on every (re)mount so a captured reference never goes stale. diff --git a/packages/agent/src/types.ts b/packages/agent/src/types.ts index c66feda2d9..d54e7337ac 100644 --- a/packages/agent/src/types.ts +++ b/packages/agent/src/types.ts @@ -148,6 +148,37 @@ export type WorkflowExecutorEmbedOptions = Omit { + it('should return the agent so it can be chained', () => { + const agent = buildAgent(); + + expect(agent.addBff()).toBe(agent); + }); + + it('should refuse a second call rather than silently keep the first', () => { + const agent = buildAgent().addBff(); + + expect(() => agent.addBff()).toThrow('addBff can only be called once.'); + }); + + describe('when the MCP server already claims /bff', () => { + it('should refuse, since the MCP server would shadow /bff/oauth and /bff/mcp', () => { + const agent = buildAgent(); + agent.mountAiMcpServer({ basePath: '/bff' }); + + expect(() => agent.addBff()).toThrow('Cannot use addBff together with mountAiMcpServer'); + }); + }); + + describe('when the MCP server claims /bff after the BFF was added', () => { + it('should refuse just the same, whichever order the two are called in', () => { + const agent = buildAgent().addBff(); + + expect(() => agent.mountAiMcpServer({ basePath: '/bff' })).toThrow( + 'Cannot use addBff together with mountAiMcpServer', + ); + }); + }); + + describe('when the MCP server is mounted elsewhere', () => { + it('should accept both', () => { + const agent = buildAgent(); + agent.mountAiMcpServer({ basePath: '/ai' }); + + expect(() => agent.addBff()).not.toThrow(); + }); + }); +}); diff --git a/packages/agent/test/bff-routes.test.ts b/packages/agent/test/bff-routes.test.ts new file mode 100644 index 0000000000..232446f398 --- /dev/null +++ b/packages/agent/test/bff-routes.test.ts @@ -0,0 +1,34 @@ +import { isBffRoute, stripBffPrefix } from '../src/bff-routes'; + +describe('isBffRoute', () => { + it.each(['/bff', '/bff/health', '/bff/agent/v1/books/list', '/bff?x=1', '/bff/health?x=1'])( + 'should claim %s', + url => { + expect(isBffRoute(url)).toBe(true); + }, + ); + + it.each(['/bffalo', '/bff-server/health', '/forest', '/', '/mcp'])( + 'should leave %s to the host', + url => { + expect(isBffRoute(url)).toBe(false); + }, + ); +}); + +describe('stripBffPrefix', () => { + it.each([ + ['/bff', '/'], + ['/bff/', '/'], + ['/bff/health', '/health'], + ['/bff/agent/v1/books/list', '/agent/v1/books/list'], + ['/bff?x=1', '/?x=1'], + ['/bff/health?x=1', '/health?x=1'], + ])('should turn %s into %s', (url, expected) => { + expect(stripBffPrefix(url)).toBe(expected); + }); + + it('should never yield an empty url, which Koa would read as malformed', () => { + expect(stripBffPrefix('/bff')).not.toBe(''); + }); +}); diff --git a/packages/agent/test/bff/embedded-bff.e2e.test.ts b/packages/agent/test/bff/embedded-bff.e2e.test.ts new file mode 100644 index 0000000000..40ea375c42 --- /dev/null +++ b/packages/agent/test/bff/embedded-bff.e2e.test.ts @@ -0,0 +1,338 @@ +import express from 'express'; +import jsonwebtoken from 'jsonwebtoken'; +import { tmpdir } from 'os'; +import path from 'path'; +import supertest from 'supertest'; + +import SearchDataSource from './fixtures/search-datasource'; +import Agent from '../../src/agent'; +import MockForestServer from '../__helper__/mock-forest-server'; + +const AUTH_SECRET = 'test-auth-secret-32-chars-min!!!'; +const ENV_SECRET = '0'.repeat(64); +const BOOT_TIMEOUT_MS = 30_000; + +const COLLECTION_PERMISSIONS = { + collection: { + browseEnabled: true, + readEnabled: true, + editEnabled: true, + addEnabled: true, + deleteEnabled: true, + exportEnabled: true, + }, + actions: {}, +}; + +const COLLECTIONS = [ + { + name: 'authors', + fields: [ + { field: 'id', type: 'Number', isPrimaryKey: true }, + { field: 'name', type: 'String' }, + ], + }, + { + name: 'books', + fields: [ + { field: 'id', type: 'Number', isPrimaryKey: true }, + { field: 'title', type: 'String' }, + { field: 'authorId', type: 'Number' }, + ], + }, + { + name: 'ledgers', + fields: [ + { field: 'id', type: 'Number', isPrimaryKey: true }, + { field: 'label', type: 'String' }, + ], + }, +]; + +/** What the third-party UI presents. Signed like the BFF's own OAuth mode signs it. */ +function sessionToken(): string { + return jsonwebtoken.sign( + { + type: 'bff_access', + sid: 'session-1', + id: 1, + email: 'test@example.com', + first_name: 'Test', + last_name: 'User', + team: 'admin', + rendering_id: '1', + permission_level: 'admin', + tags: {}, + }, + AUTH_SECRET, + { expiresIn: '1h' }, + ); +} + +describe('embedded BFF', () => { + let mockForestServer: MockForestServer; + let agent: Agent; + let app: express.Express; + + beforeAll(async () => { + mockForestServer = new MockForestServer(); + mockForestServer + // Registered first: the first matching route wins, and the default one only grants the + // collections of the sqlite fixture. + .get('/liana/v4/permissions/environment', { + collections: { + authors: COLLECTION_PERMISSIONS, + books: COLLECTION_PERMISSIONS, + ledgers: COLLECTION_PERMISSIONS, + }, + }) + .setupDefaultRoutes({ envSecret: ENV_SECRET, collections: COLLECTIONS }) + .setupSuperagentMock() + .setupFetchMock(); + + agent = new Agent({ + authSecret: AUTH_SECRET, + envSecret: ENV_SECRET, + forestServerUrl: 'https://api.forestadmin.com', + forestAppUrl: 'https://app.forestadmin.com', + isProduction: false, + schemaPath: path.join(tmpdir(), `.forestadmin-schema-bff-${Date.now()}.json`), + logger: () => undefined, + }) + .addDataSource(async () => new SearchDataSource()) + .addBff({ allowedOrigins: ['https://my-app.com'] }); + + app = express(); + agent.mountOnExpress(app); + + agent.customizeCollection('books', collection => + collection.addManyToOneRelation('author', 'authors', { foreignKey: 'authorId' }), + ); + agent.customizeCollection('ledgers', collection => collection.disableSearch()); + + await agent.start(); + }, BOOT_TIMEOUT_MS); + + afterAll(async () => { + await agent?.stop(); + mockForestServer?.restore(); + }); + + function post(url: string, body: unknown) { + return supertest(app) + .post(url) + .set('Authorization', `Bearer ${sessionToken()}`) + .set('X-Forest-Timezone', 'Europe/Paris') + .send(body as object); + } + + describe('/bff/health', () => { + it('should report ok with the features this deployment switched on', async () => { + const response = await supertest(app).get('/bff/health'); + + expect(response.status).toBe(200); + expect(response.body).toMatchObject({ + status: 'ok', + features: { oauth: false, ai: false, cors: true, openapi: false }, + }); + }); + }); + + function titlesOf(body: { data: Array<{ title: string }> }): string[] { + return body.data.map(record => record.title).sort(); + } + + describe('data routes', () => { + it('should list the records the agent serves, through the in-process transport', async () => { + const response = await post('/bff/agent/v1/books/list', { projection: ['id', 'title'] }); + + expect(response.status).toBe(200); + expect(titlesOf(response.body)).toEqual(['Foundation', 'I, Robot', 'The Dispossessed']); + }); + + it('should return only what the search matches', async () => { + const response = await post('/bff/agent/v1/books/list', { + projection: ['id', 'title'], + search: 'foundation', + }); + + expect(response.status).toBe(200); + expect(titlesOf(response.body)).toEqual(['Foundation']); + }); + + it('should not match a related record without searchExtended', async () => { + const response = await post('/bff/agent/v1/books/list', { + projection: ['id', 'title'], + search: 'asimov', + }); + + expect(response.status).toBe(200); + expect(response.body.data).toEqual([]); + }); + + it('should reach a relation when searchExtended is set', async () => { + const response = await post('/bff/agent/v1/books/list', { + projection: ['id', 'title'], + search: 'asimov', + searchExtended: true, + }); + + expect(response.status).toBe(200); + expect(titlesOf(response.body)).toEqual(['Foundation', 'I, Robot']); + }); + + // Accepted behaviour, pinned here so it cannot change unnoticed: the search value is a query, + // and `relation.column:value` crosses a relation with no `searchExtended`. The same path in a + // filter draws 422 relation_field_not_supported, so this is the one way a request reaches a + // relation column through the top-level routes. + it('should cross a relation through the query syntax without searchExtended', async () => { + const response = await post('/bff/agent/v1/books/list', { + projection: ['id', 'title'], + search: 'author.name:asimov', + }); + + expect(response.status).toBe(200); + expect(titlesOf(response.body)).toEqual(['Foundation', 'I, Robot']); + }); + + it('should reject the same relation path in a filter, unlike in a search', async () => { + const response = await post('/bff/agent/v1/books/list', { + projection: ['id', 'title'], + filter: { field: 'author:name', operator: 'IContains', value: 'asimov' }, + }); + + expect(response.status).toBe(422); + expect(response.body.error).toMatchObject({ type: 'relation_field_not_supported' }); + }); + + it('should intersect the search with the filter rather than replace it', async () => { + const response = await post('/bff/agent/v1/books/list', { + projection: ['id', 'title'], + search: 'asimov', + searchExtended: true, + filter: { field: 'title', operator: 'IContains', value: 'robot' }, + }); + + expect(response.status).toBe(200); + expect(titlesOf(response.body)).toEqual(['I, Robot']); + }); + + it('should list everything when the search holds only whitespace', async () => { + const response = await post('/bff/agent/v1/books/list', { + projection: ['id', 'title'], + search: ' ', + }); + + expect(response.status).toBe(200); + expect(titlesOf(response.body)).toEqual(['Foundation', 'I, Robot', 'The Dispossessed']); + }); + + it('should count the searched rows rather than the collection', async () => { + const searched = await post('/bff/agent/v1/books/count', { search: 'foundation' }); + const all = await post('/bff/agent/v1/books/count', {}); + + expect(all.body).toEqual({ count: 3, countStatus: 'available' }); + expect(searched.body).toEqual({ count: 1, countStatus: 'available' }); + }); + + it('should count the rows a relation-extended search returns', async () => { + const response = await post('/bff/agent/v1/books/count', { + search: 'asimov', + searchExtended: true, + }); + + expect(response.status).toBe(200); + expect(response.body).toEqual({ count: 2, countStatus: 'available' }); + }); + }); + + describe('a collection whose search is disabled', () => { + // Outside production the agent dumps every handled error to stderr, so the two rejections + // asserted here would drown the suite output in expected stack traces. + let consoleError: jest.SpyInstance; + + beforeAll(() => { + consoleError = jest.spyOn(console, 'error').mockImplementation(() => {}); + }); + + afterAll(() => { + consoleError.mockRestore(); + }); + + it('should surface an agent-side refusal as the BFF error contract', async () => { + const response = await post('/bff/agent/v1/ledgers/list', { + projection: ['id', 'label'], + search: 'foundation', + }); + + expect(response.status).toBe(400); + expect(response.body.error).toMatchObject({ + type: 'validation_error', + status: 400, + message: 'Collection is not searchable', + }); + }); + + it('should reject a count search the same way list does', async () => { + const response = await post('/bff/agent/v1/ledgers/count', { search: 'foundation' }); + + expect(response.status).toBe(400); + expect(response.body.error).toMatchObject({ + type: 'validation_error', + status: 400, + message: 'Collection is not searchable', + }); + }); + + it('should still serve it when no search is sent', async () => { + const response = await post('/bff/agent/v1/ledgers/list', { projection: ['id', 'label'] }); + + expect(response.status).toBe(200); + expect(response.body.data).toHaveLength(1); + }); + }); + + describe('the agent routes next to it', () => { + it('should keep answering on their own prefix', async () => { + const response = await supertest(app).get('/forest'); + + expect(response.status).toBe(200); + }); + + it('should not claim a url that merely starts with the same letters', async () => { + const response = await supertest(app).get('/bffalo'); + + expect(response.status).toBe(404); + }); + }); + + describe('once the agent is stopped', () => { + it( + 'should stop answering rather than dispatch into a dead stack', + async () => { + const stoppedAgent = new Agent({ + authSecret: AUTH_SECRET, + envSecret: ENV_SECRET, + forestServerUrl: 'https://api.forestadmin.com', + forestAppUrl: 'https://app.forestadmin.com', + isProduction: false, + schemaPath: path.join(tmpdir(), `.forestadmin-schema-bff-stop-${Date.now()}.json`), + logger: () => undefined, + }) + .addDataSource(async () => new SearchDataSource()) + .addBff({}); + + const stoppedApp = express(); + stoppedAgent.mountOnExpress(stoppedApp); + await stoppedAgent.start(); + await stoppedAgent.stop(); + + const response = await supertest(stoppedApp).get('/bff/health'); + + expect(response.status).toBe(503); + expect(response.body.error).toMatchObject({ type: 'bff_not_started' }); + }, + BOOT_TIMEOUT_MS, + ); + }); +}); diff --git a/packages/agent-bff/test/data/fixtures/in-memory-collection.ts b/packages/agent/test/bff/fixtures/in-memory-collection.ts similarity index 100% rename from packages/agent-bff/test/data/fixtures/in-memory-collection.ts rename to packages/agent/test/bff/fixtures/in-memory-collection.ts diff --git a/packages/agent-bff/test/data/fixtures/record-contract-datasource.ts b/packages/agent/test/bff/fixtures/record-contract-datasource.ts similarity index 100% rename from packages/agent-bff/test/data/fixtures/record-contract-datasource.ts rename to packages/agent/test/bff/fixtures/record-contract-datasource.ts diff --git a/packages/agent-bff/test/data/fixtures/search-datasource.ts b/packages/agent/test/bff/fixtures/search-datasource.ts similarity index 100% rename from packages/agent-bff/test/data/fixtures/search-datasource.ts rename to packages/agent/test/bff/fixtures/search-datasource.ts diff --git a/packages/agent/test/bff/record-contract.e2e.test.ts b/packages/agent/test/bff/record-contract.e2e.test.ts new file mode 100644 index 0000000000..e5db1817bc --- /dev/null +++ b/packages/agent/test/bff/record-contract.e2e.test.ts @@ -0,0 +1,126 @@ +import express from 'express'; +import jsonwebtoken from 'jsonwebtoken'; +import { tmpdir } from 'os'; +import path from 'path'; +import supertest from 'supertest'; + +import RecordContractDataSource from './fixtures/record-contract-datasource'; +import Agent from '../../src/agent'; +import MockForestServer from '../__helper__/mock-forest-server'; + +const AUTH_SECRET = 'test-auth-secret-32-chars-min!!!'; +const ENV_SECRET = '0'.repeat(64); +const BOOT_TIMEOUT_MS = 30_000; + +const COLLECTION_PERMISSIONS = { + collection: { + browseEnabled: true, + readEnabled: true, + editEnabled: true, + addEnabled: true, + deleteEnabled: true, + exportEnabled: true, + }, + actions: {}, +}; + +const COLLECTIONS = [ + { + name: 'people', + fields: [ + { field: 'id', type: 'Number', isPrimaryKey: true }, + { field: 'first_name', type: 'String' }, + ], + }, +]; + +function sessionToken(): string { + return jsonwebtoken.sign( + { + type: 'bff_access', + sid: 'session-1', + id: 1, + email: 'test@example.com', + first_name: 'Test', + last_name: 'User', + team: 'admin', + rendering_id: '1', + permission_level: 'admin', + tags: {}, + }, + AUTH_SECRET, + { expiresIn: '1h' }, + ); +} + +describe('the record contract of the embedded BFF', () => { + let mockForestServer: MockForestServer; + let agent: Agent; + let app: express.Express; + + beforeAll(async () => { + mockForestServer = new MockForestServer(); + mockForestServer + .get('/liana/v4/permissions/environment', { + collections: { people: COLLECTION_PERMISSIONS }, + }) + .setupDefaultRoutes({ envSecret: ENV_SECRET, collections: COLLECTIONS }) + .setupSuperagentMock() + .setupFetchMock(); + + agent = new Agent({ + authSecret: AUTH_SECRET, + envSecret: ENV_SECRET, + forestServerUrl: 'https://api.forestadmin.com', + forestAppUrl: 'https://app.forestadmin.com', + isProduction: false, + schemaPath: path.join(tmpdir(), `.forestadmin-schema-bff-record-${Date.now()}.json`), + logger: () => undefined, + }) + .addDataSource(async () => new RecordContractDataSource()) + .addBff({}); + + app = express(); + agent.mountOnExpress(app); + + await agent.start(); + }, BOOT_TIMEOUT_MS); + + afterAll(async () => { + await agent?.stop(); + mockForestServer?.restore(); + }); + + function post(url: string, body: unknown) { + return supertest(app) + .post(url) + .set('Authorization', `Bearer ${sessionToken()}`) + .set('X-Forest-Timezone', 'Europe/Paris') + .send(body as object); + } + + it('should carry the flat id as a string while __forest.primaryKey holds it typed', async () => { + const response = await post('/bff/agent/v1/people/list', { projection: ['id'] }); + + expect(response.status).toBe(200); + expect(response.body.data).toEqual([ + { + id: '8', + __forest: { collection: 'people', primaryKey: { id: 8 } }, + }, + ]); + }); + + it('should return a snake_case column under its camelCase key, while projecting its schema name', async () => { + const response = await post('/bff/agent/v1/people/list', { projection: ['id', 'first_name'] }); + + expect(response.status).toBe(200); + expect(response.body.data).toEqual([ + { + id: '8', + firstName: 'Ada', + __forest: { collection: 'people', primaryKey: { id: 8 } }, + }, + ]); + }); +}); From c861425475e335d7a328d0d6a4aaa754ff47e4de Mon Sep 17 00:00:00 2001 From: Nicolas Bouliol Date: Wed, 2 Sep 2026 00:22:18 +0200 Subject: [PATCH 02/12] fix(agent-bff): let the host choose where the read-model reports Without a sink `createReadModel` builds a console one, which reports its gauges at Info. That is what the standalone deployment wants; embedded it puts a schema-cache age line in the host's own logs on every read, for a number nobody reads there. `buildBff` now takes the sink, and the agent passes a no-op. Co-Authored-By: Claude Opus 5 (1M context) --- packages/agent-bff/src/build-bff.ts | 10 +++++++++- packages/agent/src/embedded-bff.ts | 3 +++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/packages/agent-bff/src/build-bff.ts b/packages/agent-bff/src/build-bff.ts index 86132fbd41..521eefe65a 100644 --- a/packages/agent-bff/src/build-bff.ts +++ b/packages/agent-bff/src/build-bff.ts @@ -72,6 +72,11 @@ export interface BuildBffOptions { * transport entirely: `AGENT_URL` then only names where the agent answers, never how it is called. */ dispatcher?: AgentDispatcher; + /** + * Where the read-model reports its gauges. Defaults to the console sink, which is what the + * standalone deployment wants; an embedding host passes its own, or a no-op. + */ + metrics?: Metrics; } export interface Bff { @@ -418,6 +423,7 @@ function buildAgentMiddlewares( aiMiddlewares: Middleware[], basePath: string, transport: AgentTransport | undefined, + metrics: Metrics | undefined, ): AgentEdge { const { forestAuthSecret, defaultTimezone } = config; @@ -430,7 +436,7 @@ function buildAgentMiddlewares( const apiKeyStep = buildApiKeyMiddleware(config, logger) ?? createApiKeyUnavailableGuard(logger); // One store for the whole edge. The document only unfolds when the agent is reachable too: with no // AGENT_URL every data path answers 501, so concrete paths would advertise a dead surface. - const bundle = resolveReadModelBundle(config, logger); + const bundle = resolveReadModelBundle(config, logger, metrics); const source = toUnfoldSource(bundle, transport, logger); const permissionsCache = new PermissionsCache(); @@ -489,6 +495,7 @@ export default async function buildBff({ logger = createConsoleLogger(), basePath, dispatcher, + metrics, }: BuildBffOptions): Promise { // Before anything is assembled: a mount the host does not serve must fail at boot, not surface as // a docs page that cannot load itself. @@ -519,6 +526,7 @@ export default async function buildBff({ aiMiddlewares, mountPath, transport, + metrics, ); const agentMiddlewares = agentEdge.middlewares; const hasAgentEdge = agentMiddlewares.length > 0; diff --git a/packages/agent/src/embedded-bff.ts b/packages/agent/src/embedded-bff.ts index a09da31795..95c75607e3 100644 --- a/packages/agent/src/embedded-bff.ts +++ b/packages/agent/src/embedded-bff.ts @@ -65,6 +65,9 @@ export default class EmbeddedBff { config, dispatcher, basePath: BFF_PREFIX, + // Dropped rather than logged: the default sink reports gauges at Info, which would put a + // schema-cache age line in the host's logs on every read, for a number nobody reads there. + metrics: { increment: () => undefined, gauge: () => undefined }, logger: (level, message, context) => this.options.logger(level, formatLog(message, context)), }); From b370c1649bb0b081e3ebf35a22f4e113ad685260 Mon Sep 17 00:00:00 2001 From: Nicolas Bouliol Date: Wed, 2 Sep 2026 17:13:18 +0200 Subject: [PATCH 03/12] fix(agent): close the review gaps on the embedded BFF MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The /bff collision guard compared the raw mountAiMcpServer basePath to the literal "/bff", but the MCP server normalizes its own: "bff", "/bff/" and "/bff/ai" all landed inside /bff and slipped past. The BFF is registered at builder time and wins the root middleware first-match, so in every one of those cases the MCP server booted, logged success and was never reachable. Normalize before comparing, and test containment rather than equality — the comment above the error also had the victim backwards. Configuration is now parsed in a prepare() step that runs before mount(), not after it: everything it validates is what the caller handed to addBff(), so a mistyped tokenEncryptionKey used to leave the host serving /forest with /bff permanently answering 503 and no way back short of a restart. It also moves that failure ahead of subscribeToServerEvents, so a retry cannot duplicate the subscription. Counters now reach the host logs. They are the schema cache and the action-endpoint resolver only channel — neither takes a logger — and every one reports a failure, so dropping them made a stale schema served to third-party UIs completely silent. Gauges stay dropped, which is what the original comment reasoned about. Also: an Error in a log context is unfolded instead of serializing to {}; a package that fails to load keeps its reason and cause, since it resolves from the host node_modules and "install it" is often the wrong advice; a stopped BFF answers bff_stopped with a message rather than bff_not_started, so a probe can tell shutdown from boot; originalUrl keeps the url the client asked for; /health no longer reports ok on a dispatcher without an auth secret, where the agent edge is a stub; the tokenEncryptionKey doc no longer claims it closes the data surface (it gates the login flow, authSecret guards the session bearer); the agentTimeoutMs doc says the timeout cancels nothing. The unit job ignored search-agent.integration, a file this stack deleted, so the e2e suite ran inside the fail-fast matrix as well as its own job. The suite also now runs its data contract over both transports: the HTTP one had no end-to-end coverage left anywhere in the repo, and it is the standalone deployment only route to the agent. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/build.yml | 10 +- .../src/agent/in-process-transport.ts | 4 +- packages/agent-bff/src/build-bff.ts | 9 +- .../test/build-bff-dispatcher.test.ts | 171 ++++++++++ .../test/http/bff-http-server.test.ts | 30 +- packages/agent/src/agent.ts | 30 +- packages/agent/src/bff-routes.ts | 21 ++ packages/agent/src/embedded-bff.ts | 108 ++++-- packages/agent/src/framework-mounter.ts | 2 +- packages/agent/src/index.ts | 2 +- packages/agent/src/types.ts | 17 +- .../agent/test/agent-bff-lifecycle.test.ts | 213 ++++++++++++ .../agent/test/agent-bff-missing-dep.test.ts | 58 ++++ packages/agent/test/agent-bff.test.ts | 31 ++ packages/agent/test/bff-routes.test.ts | 18 +- .../agent/test/bff/embedded-bff.e2e.test.ts | 311 +++++++++++------- .../test/bff/record-contract.e2e.test.ts | 126 ------- 17 files changed, 854 insertions(+), 307 deletions(-) create mode 100644 packages/agent-bff/test/build-bff-dispatcher.test.ts create mode 100644 packages/agent/test/agent-bff-lifecycle.test.ts create mode 100644 packages/agent/test/agent-bff-missing-dep.test.ts delete mode 100644 packages/agent/test/bff/record-contract.e2e.test.ts diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 573eaedaee..563e87c2a5 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -111,7 +111,7 @@ jobs: # broadly: most packages keep genuine unit tests under `test/integration*/`, and a wide # pattern would silently stop running hundreds of them. - name: Test code (excluding integration suites with their own job) - run: cd packages/${{ matrix.package }} && yarn test --coverage --testPathIgnorePatterns='llm.integration|search-agent.integration' && cd - + run: cd packages/${{ matrix.package }} && yarn test --coverage --testPathIgnorePatterns='llm.integration|embedded-bff.e2e' && cd - - name: Upload coverage uses: actions/upload-artifact@v4 with: @@ -150,10 +150,12 @@ jobs: OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} - # Boots a real agent, so it stays out of the unit job. Unlike the LLM suite it reaches no third - # party and is deterministic, hence no continue-on-error: it must gate. + # Boots a real agent on a local HTTP port, so it stays out of the unit job (which ignores this + # path). Gates both transports: the in-process dispatcher an embedded BFF uses and the socket a + # standalone one uses. Unlike the LLM suite it reaches no third party and is deterministic, hence + # no continue-on-error: it must gate. bff-integration-tests: - name: BFF Integration Tests (embedded BFF) + name: BFF Integration Tests (embedded and http transports) runs-on: ubuntu-latest timeout-minutes: 15 needs: [build] diff --git a/packages/agent-bff/src/agent/in-process-transport.ts b/packages/agent-bff/src/agent/in-process-transport.ts index efd58b7b40..4fcfe877bc 100644 --- a/packages/agent-bff/src/agent/in-process-transport.ts +++ b/packages/agent-bff/src/agent/in-process-transport.ts @@ -118,14 +118,14 @@ class InProcessRequester extends HttpRequester { * enough: `escapeUrlSlug` prefixes `+?*` with a backslash, which the WHATWG parser `buildUrl` * feeds reads as a path separator, and that parse also resolves `..` and splits a trailing query * off. Running it here is what keeps a given id addressing the same record over both transports — - * both remain wrong for those three characters, which is PRD-1124's to fix on both at once. + * both remain wrong for those three characters — TODO(PRD-1124), to fix on both at once. */ private static toDispatchTarget(path: string): { path: string; query: Record; } { const normalized = path.startsWith('/') ? path : `/${path}`; - const url = new URL(`${IN_PROCESS_URL}${HttpRequester.escapeUrlSlug(normalized)}`); + const url = new URL(`${IN_PROCESS_AGENT_URL}${HttpRequester.escapeUrlSlug(normalized)}`); return { path: url.pathname, query: Object.fromEntries(url.searchParams) }; } diff --git a/packages/agent-bff/src/build-bff.ts b/packages/agent-bff/src/build-bff.ts index 521eefe65a..0b88ca3937 100644 --- a/packages/agent-bff/src/build-bff.ts +++ b/packages/agent-bff/src/build-bff.ts @@ -537,9 +537,12 @@ export default async function buildBff({ createVersionHeaderMiddleware(version), createHealthRoute({ version, - // Embedded, everything required is inherited from the agent, so there is no gap to report: a - // 503 here would let a load balancer restart a process that serves api-key traffic fine. - healthy: dispatcher !== undefined || config.hasAllRequired, + // Embedded, the rest is inherited from the agent, so there is no gap to report: a 503 here + // would let a load balancer restart a process that serves api-key traffic fine. The auth + // secret is still required — without it the agent edge is a stub and nothing authenticated + // can be served, which is exactly what a probe must see. + healthy: + (dispatcher !== undefined && Boolean(config.forestAuthSecret)) || config.hasAllRequired, features: { oauth: oauth.middlewares.length > 0, ai: aiMiddlewares.length > 0, diff --git a/packages/agent-bff/test/build-bff-dispatcher.test.ts b/packages/agent-bff/test/build-bff-dispatcher.test.ts new file mode 100644 index 0000000000..24b31e6842 --- /dev/null +++ b/packages/agent-bff/test/build-bff-dispatcher.test.ts @@ -0,0 +1,171 @@ +import type { AgentDispatchRequest, AgentDispatcher } from '../src/agent/in-process-transport'; + +import request from 'supertest'; + +import buildBff from '../src/build-bff'; +import { restoreFetchAfterEach, stubEnvironmentIdFetch } from './helpers/fetch-stub'; +import { collection, column } from './read-model/fixtures'; +import { parseConfig } from '../src/config/env-config'; +import { issueBffAccessToken } from '../src/oauth/bff-token'; + +const fetchSchema = jest.fn(); + +jest.mock('../src/read-model/forest-schema-client', () => ({ + __esModule: true, + default: class { + fetchSchema = fetchSchema; + }, +})); + +const AUTH_SECRET = 'auth-secret'; + +// No AGENT_URL on purpose: that is what an embedded deployment's config looks like, and it is what +// tells the in-process transport apart from the HTTP one through the public surface. +const EMBEDDED_ENV = { + FOREST_AUTH_SECRET: AUTH_SECRET, + FOREST_ENV_SECRET: 'env-secret', + FOREST_SERVER_URL: 'https://api.forestadmin.com', + FOREST_APP_URL: 'https://app.forestadmin.com', +} satisfies NodeJS.ProcessEnv; + +function sessionToken(): string { + return issueBffAccessToken({ + sid: 'session-1', + user: { + id: 1, + email: 'test@example.com', + firstName: 'Test', + lastName: 'User', + team: 'admin', + permissionLevel: 'admin', + renderingId: 1, + role: 'admin', + tags: {}, + }, + renderingId: 1, + authSecret: AUTH_SECRET, + expiresInSeconds: 3600, + }); +} + +/** Answers the two calls a list goes through: the capabilities probe, then the records. */ +function agentDispatcher(): jest.Mocked { + return { + request: jest.fn(async ({ path }: AgentDispatchRequest) => { + if (path.endsWith('/_internal/capabilities')) { + return { + status: 200, + body: { + collections: [ + { + name: 'books', + fields: [ + { name: 'id', type: 'Number', operators: ['equal'] }, + { name: 'title', type: 'String', operators: ['equal', 'like'] }, + ], + }, + ], + }, + }; + } + + return { + status: 200, + body: { data: [{ type: 'books', id: '1', attributes: { title: 'Foundation' } }] }, + }; + }), + } as unknown as jest.Mocked; +} + +async function buildEmbedded(dispatcher?: AgentDispatcher, env: NodeJS.ProcessEnv = EMBEDDED_ENV) { + return buildBff({ config: parseConfig(env), logger: () => undefined, dispatcher }); +} + +function list(callback: Parameters[0]) { + return request(callback) + .post('/agent/v1/books/list') + .set('Authorization', `Bearer ${sessionToken()}`) + .set('X-Forest-Timezone', 'Europe/Paris') + .send({ projection: ['id', 'title'] }); +} + +describe('buildBff with an in-process dispatcher', () => { + restoreFetchAfterEach(); + + beforeEach(() => { + stubEnvironmentIdFetch(); + fetchSchema.mockReset(); + fetchSchema.mockResolvedValue([collection('books', [column('id'), column('title')])]); + }); + + it('should serve the records the dispatcher returns, without opening a socket', async () => { + const dispatcher = agentDispatcher(); + const { callback } = await buildEmbedded(dispatcher); + + const response = await list(callback); + + expect(response.status).toBe(200); + expect(response.body.data).toEqual([ + { + id: '1', + title: 'Foundation', + __forest: { collection: 'books', primaryKey: { id: '1' } }, + }, + ]); + expect(dispatcher.request).toHaveBeenCalledWith( + expect.objectContaining({ + method: 'get', + path: '/forest/books', + headers: expect.objectContaining({ Authorization: expect.stringMatching(/^Bearer .+/) }), + }), + ); + }); + + it('should hand the configured agent timeout to every dispatched call', async () => { + const dispatcher = agentDispatcher(); + const { callback } = await buildEmbedded(dispatcher, { + ...EMBEDDED_ENV, + BFF_AGENT_TIMEOUT_MS: '2500', + }); + + await list(callback); + + expect(dispatcher.request).toHaveBeenCalledWith( + expect.objectContaining({ path: '/forest/books', timeoutMs: 2500 }), + ); + }); + + describe('when neither a dispatcher nor an AGENT_URL is given', () => { + it('should fall back to the stub instead of mounting a data edge', async () => { + const { callback } = await buildEmbedded(undefined); + + const response = await list(callback); + + expect(response.status).toBe(501); + expect(response.body.error).toMatchObject({ type: 'not_implemented' }); + }); + }); + + describe('/health', () => { + it('should report ok on a config with no AGENT_URL, since the agent is in-process', async () => { + const { callback } = await buildEmbedded(agentDispatcher()); + + const response = await request(callback).get('/health'); + + expect(response.status).toBe(200); + expect(response.body).toMatchObject({ status: 'ok' }); + }); + + it('should report degraded without an auth secret, dispatcher or not', async () => { + const { callback } = await buildEmbedded(agentDispatcher(), { + ...EMBEDDED_ENV, + FOREST_AUTH_SECRET: undefined, + }); + + const response = await request(callback).get('/health'); + + expect(response.status).toBe(503); + expect(response.body).toMatchObject({ status: 'degraded' }); + }); + }); +}); diff --git a/packages/agent-bff/test/http/bff-http-server.test.ts b/packages/agent-bff/test/http/bff-http-server.test.ts index a64d319129..57302af0de 100644 --- a/packages/agent-bff/test/http/bff-http-server.test.ts +++ b/packages/agent-bff/test/http/bff-http-server.test.ts @@ -79,16 +79,40 @@ function closeServer(server: Server): Promise { describe('BFFHttpServer', () => { describe('when config is complete', () => { - it('should answer GET /health with 200 ok and the version only', async () => { + it('should answer GET /health with 200 ok, the version and the features it serves', async () => { const server = createServer({ ...VALID_ENV }); const response = await request(server.callback).get('/health'); expect(response.status).toBe(200); - expect(response.body).toMatchObject({ status: 'ok', version: VERSION }); + expect(response.body).toEqual({ + status: 'ok', + version: VERSION, + features: { oauth: true, ai: true, cors: false, openapi: true }, + }); + }); + + it('should report the features a partial configuration leaves off', async () => { + const server = createServer({ + ...VALID_ENV, + BFF_TOKEN_ENCRYPTION_KEY: undefined, + BFF_ALLOWED_ORIGINS: 'https://my-app.com', + BFF_OPENAPI_ENABLED: 'false', + }); + + const response = await request(server.callback).get('/health'); + + expect(response.body.features).toEqual({ + oauth: false, + ai: false, + cors: true, + openapi: false, + }); }); - it('should never expose config presence or secret values in the response body', async () => { + // The features block names capabilities, never values: an anonymous caller learns that OAuth is + // configured, not what any secret holds. + it('should never expose secret values or the config itself in the response body', async () => { const server = createServer({ ...VALID_ENV }); const response = await request(server.callback).get('/health'); diff --git a/packages/agent/src/agent.ts b/packages/agent/src/agent.ts index a14c22176f..a99ddc0c78 100644 --- a/packages/agent/src/agent.ts +++ b/packages/agent/src/agent.ts @@ -27,7 +27,7 @@ import { readFile, writeFile } from 'fs/promises'; import stringify from 'json-stringify-pretty-compact'; import { installAuditTrailHooks } from './audit-trail'; -import { BFF_PREFIX } from './bff-routes'; +import { BFF_PREFIX, collidesWithBff } from './bff-routes'; import EmbeddedBff from './embedded-bff'; import EmbeddedWorkflowExecutor from './embedded-workflow-executor'; import FrameworkMounter from './framework-mounter'; @@ -38,11 +38,13 @@ import { CORRELATION_ID_HEADER, correlationIdMiddleware } from './utils/correlat import SchemaGenerator from './utils/forest-schema/generator'; import OptionsValidator from './utils/options-validator'; -// Whichever is registered second raises it: the MCP server claims `/oauth` and -// `/mcp`, and it is consulted first, so it would shadow two of the BFF's own routes. -const BFF_MCP_COLLISION = - `Cannot use addBff together with mountAiMcpServer({ basePath: '${BFF_PREFIX}' }): the MCP ` + - `server would claim ${BFF_PREFIX}/oauth and ${BFF_PREFIX}/mcp. Mount the MCP server elsewhere.`; +// Whichever is registered second raises it. `addBff()` registers at builder time and the MCP +// server only at start(), and the root middleware answers with the first handler whose matcher +// claims the url — so the BFF wins and the whole MCP surface would go silently dark. +const bffMcpCollision = (mcpBasePath: string) => + `Cannot use addBff together with mountAiMcpServer({ basePath: '${mcpBasePath}' }): the MCP ` + + `server would claim ${BFF_PREFIX} paths the embedded BFF answers on (${BFF_PREFIX}/oauth, ` + + `${BFF_PREFIX}/mcp). Mount the MCP server elsewhere.`; /** * Allow to create a new Forest Admin agent from scratch. @@ -112,6 +114,11 @@ export default class Agent extends FrameworkMounter let mounted = false; try { + // First, before anything is mounted or subscribed: everything it validates is what the caller + // handed to addBff(), so a mistyped key must fail on that line rather than leave the host + // serving /forest with a permanently bricked /bff. + await this.embeddedBff?.prepare(); + const { router, mcp } = await this.buildRouterAndSendSchema(); await this.options.forestAdminClient.subscribeToServerEvents(); @@ -333,8 +340,8 @@ export default class Agent extends FrameworkMounter allowedOAuthClients?: string[]; fileUploads?: false | FileUploadsOptions; }): this { - if (this.embeddedBff && options?.basePath === BFF_PREFIX) { - throw new Error(BFF_MCP_COLLISION); + if (this.embeddedBff && collidesWithBff(options?.basePath)) { + throw new Error(bffMcpCollision(options?.basePath as string)); } this.mcpEnabled = true; @@ -414,12 +421,11 @@ export default class Agent extends FrameworkMounter throw new Error('addBff can only be called once.'); } - if (this.mcpBasePath === BFF_PREFIX) { - throw new Error(BFF_MCP_COLLISION); + if (collidesWithBff(this.mcpBasePath)) { + throw new Error(bffMcpCollision(this.mcpBasePath as string)); } - const bff = new EmbeddedBff(this.options); - bff.configure(options); + const bff = new EmbeddedBff(this.options, options); this.embeddedBff = bff; // Registered now rather than at start(): getInProcessDispatcher() pushes its hook the first // time it is called, and mount() only runs the hooks registered before it — asked for later, diff --git a/packages/agent/src/bff-routes.ts b/packages/agent/src/bff-routes.ts index 8dfe5c7f89..383816ebca 100644 --- a/packages/agent/src/bff-routes.ts +++ b/packages/agent/src/bff-routes.ts @@ -26,3 +26,24 @@ export function stripBffPrefix(url: string): string { return remainder.startsWith('/') ? remainder : `/${remainder}`; } + +/** + * Whether an MCP mount path would land inside `/bff`. The MCP server normalizes its `basePath` + * (trim, leading slash, collapsed and stripped trailing slashes) before deriving + * `/oauth/` and `/mcp`, so comparing the raw option to `/bff` would let `bff`, + * `/bff/` and `/bff/ai` through — each of which claims paths the BFF answers on. The normalization + * is mirrored rather than imported: mcp-server keeps it internal. + */ +export function collidesWithBff(mcpBasePath?: string): boolean { + if (!mcpBasePath) return false; + + const trimmed = mcpBasePath.trim(); + + if (trimmed === '' || trimmed === '/') return false; + + const normalized = (trimmed.startsWith('/') ? trimmed : `/${trimmed}`) + .replace(/\/+/g, '/') + .replace(/\/+$/, ''); + + return isBffRoute(`${normalized}/mcp`); +} diff --git a/packages/agent/src/embedded-bff.ts b/packages/agent/src/embedded-bff.ts index 95c75607e3..5a3b16b787 100644 --- a/packages/agent/src/embedded-bff.ts +++ b/packages/agent/src/embedded-bff.ts @@ -1,18 +1,25 @@ import type { AgentOptionsWithDefaults, BffEmbedOptions, HttpCallback } from './types'; -import type { AgentDispatcher, Bff } from '@forestadmin/agent-bff'; +import type { AgentDispatcher, BFFConfig, Bff } from '@forestadmin/agent-bff'; import { BFF_PREFIX, stripBffPrefix } from './bff-routes'; /** * Serialize the BFF's structured log context onto the message: the agent's logger only accepts an - * Error as its third argument, so the context would be dropped otherwise. Never throws — logging - * must not break a request. + * Error as its third argument, so the context would be dropped otherwise. Errors are unfolded by + * hand — their `message` and `stack` are not enumerable, so `JSON.stringify` alone turns the one + * value worth logging into `{}`. Never throws — logging must not break a request. */ function formatLog(message: string, context?: Record): string { if (!context || Object.keys(context).length === 0) return `[bff] ${message}`; try { - return `[bff] ${message} ${JSON.stringify(context)}`; + const serialized = JSON.stringify(context, (_key, value) => + value instanceof Error + ? { name: value.name, message: value.message, stack: value.stack } + : value, + ); + + return `[bff] ${message} ${serialized}`; } catch { return `[bff] ${message} [unserializable context]`; } @@ -24,32 +31,31 @@ function formatLog(message: string, context?: Record): string { * the callback and delegates start/stop. */ export default class EmbeddedBff { - private embedOptions: BffEmbedOptions | null = null; + private config: BFFConfig | null = null; private bff: Bff | null = null; + private stopped = false; - constructor(private readonly options: AgentOptionsWithDefaults) {} - - /** Register the embedded BFF. Nothing is built yet: the agent must be mounted first. */ - configure(embedOptions: BffEmbedOptions): void { - this.embedOptions = embedOptions; - } + constructor( + private readonly options: AgentOptionsWithDefaults, + private readonly embedOptions: BffEmbedOptions, + ) {} /** - * Build the BFF. Called from agent.start() after mount(), because the dispatcher only reaches the - * agent's own stack once that stack is mounted. + * Load the package and validate the options the caller handed to `addBff()`. Called before the + * agent mounts anything: everything checked here is caller input, and `parseConfig` throws on a + * mistyped `tokenEncryptionKey`, timezone, timeout or url. Failing here leaves nothing serving, + * where failing after mount() would leave the host with a live `/forest` and a bricked `/bff`. */ - async start(dispatcher: AgentDispatcher): Promise { + async prepare(): Promise { + const { parseConfig, IN_PROCESS_AGENT_URL } = await this.importPackage(); const { embedOptions } = this; - if (!embedOptions) return; - const { buildBff, parseConfig, IN_PROCESS_AGENT_URL } = await this.importPackage(); - - const config = parseConfig({ + this.config = parseConfig({ FOREST_AUTH_SECRET: this.options.authSecret, FOREST_ENV_SECRET: this.options.envSecret, FOREST_SERVER_URL: this.options.forestServerUrl, FOREST_APP_URL: this.options.forestAppUrl, - // Names where the agent answers, never how it is reached: the dispatcher below is the transport. + // Names where the agent answers, never how it is reached: the dispatcher is the transport. AGENT_URL: IN_PROCESS_AGENT_URL, BFF_TOKEN_ENCRYPTION_KEY: embedOptions.tokenEncryptionKey, BFF_ALLOWED_ORIGINS: embedOptions.allowedOrigins?.join(','), @@ -60,16 +66,32 @@ export default class EmbeddedBff { // publish the name of every exposed collection and field on an already-open port. BFF_OPENAPI_ENABLED: String(embedOptions.openapiEnabled ?? false), }); + } + + /** + * Build the BFF. Called from agent.start() after mount(), because the dispatcher only reaches the + * agent's own stack once that stack is mounted. + */ + async start(dispatcher: AgentDispatcher): Promise { + if (!this.config) await this.prepare(); + + const { buildBff } = await this.importPackage(); this.bff = await buildBff({ - config, + config: this.config as BFFConfig, dispatcher, basePath: BFF_PREFIX, - // Dropped rather than logged: the default sink reports gauges at Info, which would put a - // schema-cache age line in the host's logs on every read, for a number nobody reads there. - metrics: { increment: () => undefined, gauge: () => undefined }, + // Counters are the schema cache's and the action-endpoint resolver's only channel — they take + // no logger — and every one of them reports a failure, so they go to the host's logs. Gauges + // do not: they are periodic cache sizes, and the default console sink reports them at Info, + // which would flood a host that only asked for a BFF. + metrics: { + increment: name => this.options.logger('Warn', formatLog(`metric ${name}`)), + gauge: () => undefined, + }, logger: (level, message, context) => this.options.logger(level, formatLog(message, context)), }); + this.stopped = false; this.options.logger('Info', formatLog(`Embedded BFF mounted on ${BFF_PREFIX}`)); } @@ -85,28 +107,33 @@ export default class EmbeddedBff { */ stop(): void { this.bff = null; + this.stopped = true; } /** * The callback the agent registers at `/bff`. Answers 503 rather than falling through while the - * BFF is configured but not built yet: a 404 from the host would read as "wrong url" instead of - * "not started". + * BFF is not serving: a 404 from the host would read as "wrong url" instead of "not available". + * Boot and shutdown are told apart, because a probe should wait on the first and drain on the + * second. */ - readonly handle: HttpCallback = (req, res, next) => { - if (!this.embedOptions) { - next?.(); - - return; - } - + readonly handle: HttpCallback = (req, res) => { if (!this.bff) { + const type = this.stopped ? 'bff_stopped' : 'bff_not_started'; + const message = this.stopped + ? 'The embedded BFF was stopped with the agent.' + : 'The embedded BFF is not started yet.'; + res.statusCode = 503; res.setHeader('Content-Type', 'application/json'); - res.end(JSON.stringify({ error: { type: 'bff_not_started', status: 503 } })); + res.end(JSON.stringify({ error: { type, status: 503, message } })); return; } + // The BFF knows nothing of the prefix it is served under. `originalUrl` is claimed before the + // rewrite so a host logger reading it still reports the url the client asked for. + const bffReq = req as typeof req & { originalUrl?: string }; + bffReq.originalUrl ??= bffReq.url; req.url = stripBffPrefix(req.url ?? BFF_PREFIX); this.bff.callback(req, res); }; @@ -119,10 +146,19 @@ export default class EmbeddedBff { try { return await import('@forestadmin/agent-bff'); } catch (error) { - throw new Error( - 'The embedded BFF requires the `@forestadmin/agent-bff` package. ' + - 'Install it with `npm install @forestadmin/agent-bff`.', + // The original reason is kept: the package resolves from the host's own node_modules, so this + // also fires on a broken transitive dependency, a SyntaxError from a partial install, or a + // throw during the package's own evaluation — none of which "install it" would fix. + const { message } = error as Error; + const wrapped = new Error( + `The embedded BFF requires the \`@forestadmin/agent-bff\` package, which failed to ` + + `load: ${message}. Install it with \`npm install @forestadmin/agent-bff\`.`, ); + // Assigned rather than passed to the constructor: the repo targets ES2020, whose lib has no + // options bag on Error, though the declared engines guarantee a runtime that reads it. + (wrapped as Error & { cause?: unknown }).cause = error; + + throw wrapped; } } } diff --git a/packages/agent/src/framework-mounter.ts b/packages/agent/src/framework-mounter.ts index 21dd637d90..4285877f3f 100644 --- a/packages/agent/src/framework-mounter.ts +++ b/packages/agent/src/framework-mounter.ts @@ -53,7 +53,7 @@ export default class FrameworkMounter { /** Serve an embedded BFF at `/bff`. Pass null to stop answering there. */ protected setBffCallback(callback: HttpCallback | null): void { - this.rootMiddleware.set('bff', callback, isBffRoute); + this.rootMiddleware.set('bff', callback && { callback, matches: isBffRoute }); } /** diff --git a/packages/agent/src/index.ts b/packages/agent/src/index.ts index ff8c25fdc8..5fdbb9d197 100644 --- a/packages/agent/src/index.ts +++ b/packages/agent/src/index.ts @@ -8,7 +8,7 @@ export function createAgent(options: AgentOptions): } export { Agent }; -export { AgentOptions, WorkflowExecutorEmbedOptions } from './types'; +export { AgentOptions, BffEmbedOptions, WorkflowExecutorEmbedOptions } from './types'; export * from '@forestadmin/datasource-customizer'; // export is necessary for the agent-generator package diff --git a/packages/agent/src/types.ts b/packages/agent/src/types.ts index d54e7337ac..d8e096f0b1 100644 --- a/packages/agent/src/types.ts +++ b/packages/agent/src/types.ts @@ -155,8 +155,12 @@ export type WorkflowExecutorEmbedOptions = Omit ({ + __esModule: true, + default: (...args) => mockMakeRoutes(...args), +})); + +jest.mock('@forestadmin/datasource-customizer'); + +const mockBuildBff = jest.fn(); +const mockParseConfig = jest.fn(); +const mockInvalidate = jest.fn(); +const mockBffCallback = jest.fn(); + +jest.mock('@forestadmin/agent-bff', () => ({ + __esModule: true, + IN_PROCESS_AGENT_URL: 'http://in-process.agent', + parseConfig: (env: unknown) => mockParseConfig(env), + buildBff: (options: unknown) => mockBuildBff(options), +})); + +function responseSpy() { + const response = { + statusCode: 200, + setHeader: jest.fn(), + end: jest.fn(), + }; + + return response as unknown as ServerResponse & { end: jest.Mock; statusCode: number }; +} + +function requestFor(url: string) { + return { url } as unknown as IncomingMessage & { originalUrl?: string }; +} + +function bodyOf(response: { end: jest.Mock }) { + return JSON.parse(response.end.mock.calls[0][0] as string); +} + +function bffHandlerOf(agent: Agent) { + return (agent as any).rootMiddleware.handlers.get('bff').callback; +} + +beforeEach(() => { + jest.clearAllMocks(); + + mockMakeRoutes.mockReturnValue([{ setupRoutes: jest.fn(), bootstrap: jest.fn() }]); + mockParseConfig.mockReturnValue({ agentTimeoutMs: 10_000 }); + mockBuildBff.mockResolvedValue({ callback: mockBffCallback, invalidate: mockInvalidate }); + jest + .mocked(DataSourceCustomizer.prototype.getDataSource) + .mockResolvedValue(factories.dataSource.build()); +}); + +function buildAgent() { + return new Agent(factories.forestAdminHttpDriverOptions.build({ skipSchemaUpdate: true })); +} + +describe('the embedded BFF lifecycle', () => { + describe('on restart', () => { + it('should drop what the BFF read from the SaaS, since the customizations moved', async () => { + const agent = buildAgent().addBff(); + await agent.start(); + + await agent.restart(); + + expect(mockInvalidate).toHaveBeenCalledTimes(1); + }); + + it('should leave the BFF serving, rather than rebuild it', async () => { + const agent = buildAgent().addBff(); + await agent.start(); + + await agent.restart(); + + expect(mockBuildBff).toHaveBeenCalledTimes(1); + }); + }); + + describe('when the caller options are invalid', () => { + it('should reject start() before anything is mounted or subscribed', async () => { + mockParseConfig.mockImplementation(() => { + throw new Error('Invalid configuration: BFF_TOKEN_ENCRYPTION_KEY must be 32 bytes.'); + }); + const options = factories.forestAdminHttpDriverOptions.build({ skipSchemaUpdate: true }); + const agent = new Agent(options).addBff({ tokenEncryptionKey: 'nope' }); + + await expect(agent.start()).rejects.toThrow( + 'Invalid configuration: BFF_TOKEN_ENCRYPTION_KEY must be 32 bytes.', + ); + expect(options.forestAdminClient.subscribeToServerEvents).not.toHaveBeenCalled(); + expect(mockMakeRoutes).not.toHaveBeenCalled(); + }); + }); + + describe('before start()', () => { + it('should answer 503 bff_not_started rather than fall through to the host 404', () => { + const agent = buildAgent().addBff(); + const response = responseSpy(); + + bffHandlerOf(agent)(requestFor('/bff/health'), response); + + expect(response.statusCode).toBe(503); + expect(bodyOf(response).error).toEqual({ + type: 'bff_not_started', + status: 503, + message: 'The embedded BFF is not started yet.', + }); + }); + }); + + describe('after stop()', () => { + it('should answer 503 bff_stopped, so a probe drains instead of waiting on a boot', async () => { + const agent = buildAgent().addBff(); + await agent.start(); + await agent.stop(); + const response = responseSpy(); + + bffHandlerOf(agent)(requestFor('/bff/health'), response); + + expect(response.statusCode).toBe(503); + expect(bodyOf(response).error).toEqual({ + type: 'bff_stopped', + status: 503, + message: 'The embedded BFF was stopped with the agent.', + }); + }); + }); + + describe('while serving', () => { + it('should hand the BFF the url without the prefix it knows nothing about', async () => { + const agent = buildAgent().addBff(); + await agent.start(); + const request = requestFor('/bff/agent/v1/books/list'); + + bffHandlerOf(agent)(request, responseSpy()); + + expect(request.url).toBe('/agent/v1/books/list'); + }); + + it('should keep the url the client asked for on originalUrl, for the host own logs', async () => { + const agent = buildAgent().addBff(); + await agent.start(); + const request = requestFor('/bff/agent/v1/books/list'); + + bffHandlerOf(agent)(request, responseSpy()); + + expect(request.originalUrl).toBe('/bff/agent/v1/books/list'); + }); + + it('should not overwrite an originalUrl the host already set', async () => { + const agent = buildAgent().addBff(); + await agent.start(); + const request = requestFor('/bff/health'); + request.originalUrl = '/mounted/bff/health'; + + bffHandlerOf(agent)(request, responseSpy()); + + expect(request.originalUrl).toBe('/mounted/bff/health'); + }); + }); + + describe('the metrics sink handed to the BFF', () => { + it('should send counters to the host logs, since they only ever report a failure', async () => { + const logger = jest.fn(); + const agent = new Agent( + factories.forestAdminHttpDriverOptions.build({ skipSchemaUpdate: true, logger }), + ).addBff(); + await agent.start(); + + mockBuildBff.mock.calls[0][0].metrics.increment('schema_cache_refresh_error'); + + expect(logger).toHaveBeenCalledWith('Warn', '[bff] metric schema_cache_refresh_error'); + }); + + it('should drop gauges, which the default sink reports at Info on every read', async () => { + const logger = jest.fn(); + const agent = new Agent( + factories.forestAdminHttpDriverOptions.build({ skipSchemaUpdate: true, logger }), + ).addBff(); + await agent.start(); + logger.mockClear(); + + mockBuildBff.mock.calls[0][0].metrics.gauge('schema_cache_age_ms', 1); + + expect(logger).not.toHaveBeenCalled(); + }); + }); + + describe('the logger handed to the BFF', () => { + it('should keep an Error context readable instead of serializing it to {}', async () => { + const logger = jest.fn(); + const agent = new Agent( + factories.forestAdminHttpDriverOptions.build({ skipSchemaUpdate: true, logger }), + ).addBff(); + await agent.start(); + + mockBuildBff.mock.calls[0][0].logger('Warn', 'bundle unreadable', { + error: new Error('EACCES'), + }); + + expect(logger).toHaveBeenCalledWith('Warn', expect.stringContaining('"message":"EACCES"')); + }); + }); +}); diff --git a/packages/agent/test/agent-bff-missing-dep.test.ts b/packages/agent/test/agent-bff-missing-dep.test.ts new file mode 100644 index 0000000000..4f873814a4 --- /dev/null +++ b/packages/agent/test/agent-bff-missing-dep.test.ts @@ -0,0 +1,58 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +/** + * Covers the optional-dependency failure path: when @forestadmin/agent-bff cannot be loaded, the + * dynamic import() rejects and the agent surfaces an actionable error that still carries the reason. + */ +import { DataSourceCustomizer } from '@forestadmin/datasource-customizer'; + +import * as factories from './__factories__'; +import Agent from '../src/agent'; + +const mockMakeRoutes = jest.fn(); +jest.mock('../src/routes', () => ({ + __esModule: true, + default: (...args) => mockMakeRoutes(...args), +})); +jest.mock('@forestadmin/datasource-customizer'); + +jest.mock('@forestadmin/agent-bff', () => { + throw new Error("Cannot find module '@forestadmin/agent-bff'"); +}); + +beforeEach(() => { + jest.clearAllMocks(); + mockMakeRoutes.mockReturnValue([{ setupRoutes: jest.fn(), bootstrap: jest.fn() }]); + jest + .mocked(DataSourceCustomizer.prototype.getDataSource) + .mockResolvedValue(factories.dataSource.build()); +}); + +describe('Agent.addBff (optional dependency missing)', () => { + function buildAgent() { + return new Agent( + factories.forestAdminHttpDriverOptions.build({ skipSchemaUpdate: true }), + ).addBff(); + } + + it('should tell the developer to install the package', async () => { + await expect(buildAgent().start()).rejects.toThrow( + 'The embedded BFF requires the `@forestadmin/agent-bff` package', + ); + }); + + it('should keep the load failure in the message, since installing does not always fix it', async () => { + await expect(buildAgent().start()).rejects.toThrow( + "Cannot find module '@forestadmin/agent-bff'", + ); + }); + + it('should keep the original error as the cause', async () => { + const error: Error & { cause?: Error } = await buildAgent() + .start() + .then(() => new Error('start() resolved')) + .catch(caught => caught as Error); + + expect(error.cause).toBeInstanceOf(Error); + expect(error.cause?.message).toBe("Cannot find module '@forestadmin/agent-bff'"); + }); +}); diff --git a/packages/agent/test/agent-bff.test.ts b/packages/agent/test/agent-bff.test.ts index ad30d4b029..d116a99511 100644 --- a/packages/agent/test/agent-bff.test.ts +++ b/packages/agent/test/agent-bff.test.ts @@ -37,6 +37,37 @@ describe('Agent.addBff', () => { }); }); + describe('when the MCP server claims /bff under another spelling', () => { + it.each(['/bff/', 'bff', '/bff/ai'])( + 'should refuse basePath %s, which the MCP server normalizes into /bff', + basePath => { + const agent = buildAgent(); + agent.mountAiMcpServer({ basePath }); + + expect(() => agent.addBff()).toThrow('Cannot use addBff together with mountAiMcpServer'); + }, + ); + + it.each(['/bff/', 'bff', '/bff/ai'])( + 'should refuse basePath %s in the other order too', + basePath => { + const agent = buildAgent().addBff(); + + expect(() => agent.mountAiMcpServer({ basePath })).toThrow( + 'Cannot use addBff together with mountAiMcpServer', + ); + }, + ); + + it('should name the basePath the caller actually passed', () => { + const agent = buildAgent().addBff(); + + expect(() => agent.mountAiMcpServer({ basePath: '/bff/ai' })).toThrow( + "mountAiMcpServer({ basePath: '/bff/ai' })", + ); + }); + }); + describe('when the MCP server is mounted elsewhere', () => { it('should accept both', () => { const agent = buildAgent(); diff --git a/packages/agent/test/bff-routes.test.ts b/packages/agent/test/bff-routes.test.ts index 232446f398..48021a3c47 100644 --- a/packages/agent/test/bff-routes.test.ts +++ b/packages/agent/test/bff-routes.test.ts @@ -1,4 +1,4 @@ -import { isBffRoute, stripBffPrefix } from '../src/bff-routes'; +import { collidesWithBff, isBffRoute, stripBffPrefix } from '../src/bff-routes'; describe('isBffRoute', () => { it.each(['/bff', '/bff/health', '/bff/agent/v1/books/list', '/bff?x=1', '/bff/health?x=1'])( @@ -32,3 +32,19 @@ describe('stripBffPrefix', () => { expect(stripBffPrefix('/bff')).not.toBe(''); }); }); + +describe('collidesWithBff', () => { + it.each(['/bff', '/bff/', '//bff//', 'bff', ' /bff ', '/bff/ai'])( + 'should reject %s, which the MCP server normalizes onto a path the BFF answers', + basePath => { + expect(collidesWithBff(basePath)).toBe(true); + }, + ); + + it.each([undefined, '', '/', '/ai', '/bffalo', '/bff-server'])( + 'should accept %s, which lands outside /bff', + basePath => { + expect(collidesWithBff(basePath)).toBe(false); + }, + ); +}); diff --git a/packages/agent/test/bff/embedded-bff.e2e.test.ts b/packages/agent/test/bff/embedded-bff.e2e.test.ts index 40ea375c42..7b2752129f 100644 --- a/packages/agent/test/bff/embedded-bff.e2e.test.ts +++ b/packages/agent/test/bff/embedded-bff.e2e.test.ts @@ -1,9 +1,14 @@ +import type { Server } from 'http'; +import type supertest from 'supertest'; + +import { buildBff, parseConfig } from '@forestadmin/agent-bff'; import express from 'express'; import jsonwebtoken from 'jsonwebtoken'; import { tmpdir } from 'os'; import path from 'path'; -import supertest from 'supertest'; +import request from 'supertest'; +import RecordContractDataSource from './fixtures/record-contract-datasource'; import SearchDataSource from './fixtures/search-datasource'; import Agent from '../../src/agent'; import MockForestServer from '../__helper__/mock-forest-server'; @@ -47,6 +52,13 @@ const COLLECTIONS = [ { field: 'label', type: 'String' }, ], }, + { + name: 'people', + fields: [ + { field: 'id', type: 'Number', isPrimaryKey: true }, + { field: 'first_name', type: 'String' }, + ], + }, ]; /** What the third-party UI presents. Signed like the BFF's own OAuth mode signs it. */ @@ -69,89 +81,28 @@ function sessionToken(): string { ); } -describe('embedded BFF', () => { - let mockForestServer: MockForestServer; - let agent: Agent; - let app: express.Express; +type Post = (url: string, body: unknown) => supertest.Test; - beforeAll(async () => { - mockForestServer = new MockForestServer(); - mockForestServer - // Registered first: the first matching route wins, and the default one only grants the - // collections of the sqlite fixture. - .get('/liana/v4/permissions/environment', { - collections: { - authors: COLLECTION_PERMISSIONS, - books: COLLECTION_PERMISSIONS, - ledgers: COLLECTION_PERMISSIONS, - }, - }) - .setupDefaultRoutes({ envSecret: ENV_SECRET, collections: COLLECTIONS }) - .setupSuperagentMock() - .setupFetchMock(); - - agent = new Agent({ - authSecret: AUTH_SECRET, - envSecret: ENV_SECRET, - forestServerUrl: 'https://api.forestadmin.com', - forestAppUrl: 'https://app.forestadmin.com', - isProduction: false, - schemaPath: path.join(tmpdir(), `.forestadmin-schema-bff-${Date.now()}.json`), - logger: () => undefined, - }) - .addDataSource(async () => new SearchDataSource()) - .addBff({ allowedOrigins: ['https://my-app.com'] }); - - app = express(); - agent.mountOnExpress(app); - - agent.customizeCollection('books', collection => - collection.addManyToOneRelation('author', 'authors', { foreignKey: 'authorId' }), - ); - agent.customizeCollection('ledgers', collection => collection.disableSearch()); - - await agent.start(); - }, BOOT_TIMEOUT_MS); - - afterAll(async () => { - await agent?.stop(); - mockForestServer?.restore(); - }); - - function post(url: string, body: unknown) { - return supertest(app) - .post(url) - .set('Authorization', `Bearer ${sessionToken()}`) - .set('X-Forest-Timezone', 'Europe/Paris') - .send(body as object); - } - - describe('/bff/health', () => { - it('should report ok with the features this deployment switched on', async () => { - const response = await supertest(app).get('/bff/health'); - - expect(response.status).toBe(200); - expect(response.body).toMatchObject({ - status: 'ok', - features: { oauth: false, ai: false, cors: true, openapi: false }, - }); - }); - }); - - function titlesOf(body: { data: Array<{ title: string }> }): string[] { - return body.data.map(record => record.title).sort(); - } +function titlesOf(body: { data: Array<{ title: string }> }): string[] { + return body.data.map(record => record.title).sort(); +} - describe('data routes', () => { - it('should list the records the agent serves, through the in-process transport', async () => { - const response = await post('/bff/agent/v1/books/list', { projection: ['id', 'title'] }); +/** + * The contract the data routes owe whichever transport carries them. Run twice: once over the + * in-process dispatcher an embedded BFF uses, once over the socket a standalone deployment uses. + * Both go through a real agent, so a behaviour that only one transport gets right fails here. + */ +function itServesTheDataContract(post: () => Post) { + describe('list', () => { + it('should list the records the agent serves', async () => { + const response = await post()('/agent/v1/books/list', { projection: ['id', 'title'] }); expect(response.status).toBe(200); expect(titlesOf(response.body)).toEqual(['Foundation', 'I, Robot', 'The Dispossessed']); }); it('should return only what the search matches', async () => { - const response = await post('/bff/agent/v1/books/list', { + const response = await post()('/agent/v1/books/list', { projection: ['id', 'title'], search: 'foundation', }); @@ -161,7 +112,7 @@ describe('embedded BFF', () => { }); it('should not match a related record without searchExtended', async () => { - const response = await post('/bff/agent/v1/books/list', { + const response = await post()('/agent/v1/books/list', { projection: ['id', 'title'], search: 'asimov', }); @@ -171,7 +122,7 @@ describe('embedded BFF', () => { }); it('should reach a relation when searchExtended is set', async () => { - const response = await post('/bff/agent/v1/books/list', { + const response = await post()('/agent/v1/books/list', { projection: ['id', 'title'], search: 'asimov', searchExtended: true, @@ -186,7 +137,7 @@ describe('embedded BFF', () => { // filter draws 422 relation_field_not_supported, so this is the one way a request reaches a // relation column through the top-level routes. it('should cross a relation through the query syntax without searchExtended', async () => { - const response = await post('/bff/agent/v1/books/list', { + const response = await post()('/agent/v1/books/list', { projection: ['id', 'title'], search: 'author.name:asimov', }); @@ -196,7 +147,7 @@ describe('embedded BFF', () => { }); it('should reject the same relation path in a filter, unlike in a search', async () => { - const response = await post('/bff/agent/v1/books/list', { + const response = await post()('/agent/v1/books/list', { projection: ['id', 'title'], filter: { field: 'author:name', operator: 'IContains', value: 'asimov' }, }); @@ -206,7 +157,7 @@ describe('embedded BFF', () => { }); it('should intersect the search with the filter rather than replace it', async () => { - const response = await post('/bff/agent/v1/books/list', { + const response = await post()('/agent/v1/books/list', { projection: ['id', 'title'], search: 'asimov', searchExtended: true, @@ -218,7 +169,7 @@ describe('embedded BFF', () => { }); it('should list everything when the search holds only whitespace', async () => { - const response = await post('/bff/agent/v1/books/list', { + const response = await post()('/agent/v1/books/list', { projection: ['id', 'title'], search: ' ', }); @@ -226,17 +177,19 @@ describe('embedded BFF', () => { expect(response.status).toBe(200); expect(titlesOf(response.body)).toEqual(['Foundation', 'I, Robot', 'The Dispossessed']); }); + }); + describe('count', () => { it('should count the searched rows rather than the collection', async () => { - const searched = await post('/bff/agent/v1/books/count', { search: 'foundation' }); - const all = await post('/bff/agent/v1/books/count', {}); + const searched = await post()('/agent/v1/books/count', { search: 'foundation' }); + const all = await post()('/agent/v1/books/count', {}); expect(all.body).toEqual({ count: 3, countStatus: 'available' }); expect(searched.body).toEqual({ count: 1, countStatus: 'available' }); }); it('should count the rows a relation-extended search returns', async () => { - const response = await post('/bff/agent/v1/books/count', { + const response = await post()('/agent/v1/books/count', { search: 'asimov', searchExtended: true, }); @@ -247,20 +200,8 @@ describe('embedded BFF', () => { }); describe('a collection whose search is disabled', () => { - // Outside production the agent dumps every handled error to stderr, so the two rejections - // asserted here would drown the suite output in expected stack traces. - let consoleError: jest.SpyInstance; - - beforeAll(() => { - consoleError = jest.spyOn(console, 'error').mockImplementation(() => {}); - }); - - afterAll(() => { - consoleError.mockRestore(); - }); - - it('should surface an agent-side refusal as the BFF error contract', async () => { - const response = await post('/bff/agent/v1/ledgers/list', { + it('should surface the agent-side refusal as the BFF error contract', async () => { + const response = await post()('/agent/v1/ledgers/list', { projection: ['id', 'label'], search: 'foundation', }); @@ -273,8 +214,8 @@ describe('embedded BFF', () => { }); }); - it('should reject a count search the same way list does', async () => { - const response = await post('/bff/agent/v1/ledgers/count', { search: 'foundation' }); + it('should refuse a count search the same way list does', async () => { + const response = await post()('/agent/v1/ledgers/count', { search: 'foundation' }); expect(response.status).toBe(400); expect(response.body.error).toMatchObject({ @@ -285,22 +226,170 @@ describe('embedded BFF', () => { }); it('should still serve it when no search is sent', async () => { - const response = await post('/bff/agent/v1/ledgers/list', { projection: ['id', 'label'] }); + const response = await post()('/agent/v1/ledgers/list', { projection: ['id', 'label'] }); expect(response.status).toBe(200); expect(response.body.data).toHaveLength(1); }); }); + describe('the record contract', () => { + it('should carry the flat id as a string while __forest.primaryKey holds it typed', async () => { + const response = await post()('/agent/v1/people/list', { projection: ['id'] }); + + expect(response.status).toBe(200); + expect(response.body.data).toEqual([ + { id: '8', __forest: { collection: 'people', primaryKey: { id: 8 } } }, + ]); + }); + + it('should return a snake_case column under its camelCase key, while projecting its schema name', async () => { + const response = await post()('/agent/v1/people/list', { + projection: ['id', 'first_name'], + }); + + expect(response.status).toBe(200); + expect(response.body.data).toEqual([ + { + id: '8', + firstName: 'Ada', + __forest: { collection: 'people', primaryKey: { id: 8 } }, + }, + ]); + }); + }); +} + +function agentOptions(schemaSuffix: string) { + return { + authSecret: AUTH_SECRET, + envSecret: ENV_SECRET, + forestServerUrl: 'https://api.forestadmin.com', + forestAppUrl: 'https://app.forestadmin.com', + isProduction: false, + schemaPath: path.join(tmpdir(), `.forestadmin-schema-bff-${schemaSuffix}-${Date.now()}.json`), + logger: () => undefined, + }; +} + +describe('embedded BFF', () => { + let mockForestServer: MockForestServer; + let agent: Agent; + let app: express.Express; + + beforeAll(async () => { + mockForestServer = new MockForestServer(); + mockForestServer + // Registered first: the first matching route wins, and the default one only grants the + // collections of the sqlite fixture. + .get('/liana/v4/permissions/environment', { + collections: { + authors: COLLECTION_PERMISSIONS, + books: COLLECTION_PERMISSIONS, + ledgers: COLLECTION_PERMISSIONS, + people: COLLECTION_PERMISSIONS, + }, + }) + .setupDefaultRoutes({ envSecret: ENV_SECRET, collections: COLLECTIONS }) + .setupSuperagentMock() + .setupFetchMock(); + + agent = new Agent(agentOptions('embedded')) + .addDataSource(async () => new SearchDataSource()) + .addDataSource(async () => new RecordContractDataSource()) + .addBff({ allowedOrigins: ['https://my-app.com'] }); + + app = express(); + agent.mountOnExpress(app); + + agent.customizeCollection('books', collection => + collection.addManyToOneRelation('author', 'authors', { foreignKey: 'authorId' }), + ); + agent.customizeCollection('ledgers', collection => collection.disableSearch()); + + await agent.start(); + }, BOOT_TIMEOUT_MS); + + afterAll(async () => { + await agent?.stop(); + mockForestServer?.restore(); + }); + + function authenticated(url: string, body: unknown) { + return request(app) + .post(url) + .set('Authorization', `Bearer ${sessionToken()}`) + .set('X-Forest-Timezone', 'Europe/Paris') + .send(body as object); + } + + describe('/bff/health', () => { + it('should report ok with the features this deployment switched on', async () => { + const response = await request(app).get('/bff/health'); + + expect(response.status).toBe(200); + expect(response.body).toMatchObject({ + status: 'ok', + features: { oauth: false, ai: false, cors: true, openapi: false }, + }); + }); + }); + + describe('over the in-process transport', () => { + itServesTheDataContract(() => (url, body) => authenticated(`/bff${url}`, body)); + }); + + // The standalone deployment's only route to the agent, and the configuration shipping today: a + // BFF built with an AGENT_URL, reaching the agent over a real socket instead of the dispatcher. + describe('over the http transport', () => { + let httpServer: Server; + let bffCallback: Parameters[0]; + + beforeAll(async () => { + httpServer = await new Promise(resolve => { + const server = app.listen(0, () => resolve(server)); + }); + const { port } = httpServer.address() as { port: number }; + + const { callback } = await buildBff({ + config: parseConfig({ + FOREST_AUTH_SECRET: AUTH_SECRET, + FOREST_ENV_SECRET: ENV_SECRET, + FOREST_SERVER_URL: 'https://api.forestadmin.com', + FOREST_APP_URL: 'https://app.forestadmin.com', + AGENT_URL: `http://127.0.0.1:${port}`, + BFF_OPENAPI_ENABLED: 'false', + }), + logger: () => undefined, + }); + bffCallback = callback; + }, BOOT_TIMEOUT_MS); + + afterAll(async () => { + await new Promise(resolve => { + httpServer.close(resolve); + }); + }); + + itServesTheDataContract( + () => (url, body) => + request(bffCallback) + .post(url) + .set('Authorization', `Bearer ${sessionToken()}`) + .set('X-Forest-Timezone', 'Europe/Paris') + .send(body as object), + ); + }); + describe('the agent routes next to it', () => { it('should keep answering on their own prefix', async () => { - const response = await supertest(app).get('/forest'); + const response = await request(app).get('/forest'); expect(response.status).toBe(200); }); it('should not claim a url that merely starts with the same letters', async () => { - const response = await supertest(app).get('/bffalo'); + const response = await request(app).get('/bffalo'); expect(response.status).toBe(404); }); @@ -310,15 +399,7 @@ describe('embedded BFF', () => { it( 'should stop answering rather than dispatch into a dead stack', async () => { - const stoppedAgent = new Agent({ - authSecret: AUTH_SECRET, - envSecret: ENV_SECRET, - forestServerUrl: 'https://api.forestadmin.com', - forestAppUrl: 'https://app.forestadmin.com', - isProduction: false, - schemaPath: path.join(tmpdir(), `.forestadmin-schema-bff-stop-${Date.now()}.json`), - logger: () => undefined, - }) + const stoppedAgent = new Agent(agentOptions('stop')) .addDataSource(async () => new SearchDataSource()) .addBff({}); @@ -327,10 +408,10 @@ describe('embedded BFF', () => { await stoppedAgent.start(); await stoppedAgent.stop(); - const response = await supertest(stoppedApp).get('/bff/health'); + const response = await request(stoppedApp).get('/bff/health'); expect(response.status).toBe(503); - expect(response.body.error).toMatchObject({ type: 'bff_not_started' }); + expect(response.body.error).toMatchObject({ type: 'bff_stopped' }); }, BOOT_TIMEOUT_MS, ); diff --git a/packages/agent/test/bff/record-contract.e2e.test.ts b/packages/agent/test/bff/record-contract.e2e.test.ts deleted file mode 100644 index e5db1817bc..0000000000 --- a/packages/agent/test/bff/record-contract.e2e.test.ts +++ /dev/null @@ -1,126 +0,0 @@ -import express from 'express'; -import jsonwebtoken from 'jsonwebtoken'; -import { tmpdir } from 'os'; -import path from 'path'; -import supertest from 'supertest'; - -import RecordContractDataSource from './fixtures/record-contract-datasource'; -import Agent from '../../src/agent'; -import MockForestServer from '../__helper__/mock-forest-server'; - -const AUTH_SECRET = 'test-auth-secret-32-chars-min!!!'; -const ENV_SECRET = '0'.repeat(64); -const BOOT_TIMEOUT_MS = 30_000; - -const COLLECTION_PERMISSIONS = { - collection: { - browseEnabled: true, - readEnabled: true, - editEnabled: true, - addEnabled: true, - deleteEnabled: true, - exportEnabled: true, - }, - actions: {}, -}; - -const COLLECTIONS = [ - { - name: 'people', - fields: [ - { field: 'id', type: 'Number', isPrimaryKey: true }, - { field: 'first_name', type: 'String' }, - ], - }, -]; - -function sessionToken(): string { - return jsonwebtoken.sign( - { - type: 'bff_access', - sid: 'session-1', - id: 1, - email: 'test@example.com', - first_name: 'Test', - last_name: 'User', - team: 'admin', - rendering_id: '1', - permission_level: 'admin', - tags: {}, - }, - AUTH_SECRET, - { expiresIn: '1h' }, - ); -} - -describe('the record contract of the embedded BFF', () => { - let mockForestServer: MockForestServer; - let agent: Agent; - let app: express.Express; - - beforeAll(async () => { - mockForestServer = new MockForestServer(); - mockForestServer - .get('/liana/v4/permissions/environment', { - collections: { people: COLLECTION_PERMISSIONS }, - }) - .setupDefaultRoutes({ envSecret: ENV_SECRET, collections: COLLECTIONS }) - .setupSuperagentMock() - .setupFetchMock(); - - agent = new Agent({ - authSecret: AUTH_SECRET, - envSecret: ENV_SECRET, - forestServerUrl: 'https://api.forestadmin.com', - forestAppUrl: 'https://app.forestadmin.com', - isProduction: false, - schemaPath: path.join(tmpdir(), `.forestadmin-schema-bff-record-${Date.now()}.json`), - logger: () => undefined, - }) - .addDataSource(async () => new RecordContractDataSource()) - .addBff({}); - - app = express(); - agent.mountOnExpress(app); - - await agent.start(); - }, BOOT_TIMEOUT_MS); - - afterAll(async () => { - await agent?.stop(); - mockForestServer?.restore(); - }); - - function post(url: string, body: unknown) { - return supertest(app) - .post(url) - .set('Authorization', `Bearer ${sessionToken()}`) - .set('X-Forest-Timezone', 'Europe/Paris') - .send(body as object); - } - - it('should carry the flat id as a string while __forest.primaryKey holds it typed', async () => { - const response = await post('/bff/agent/v1/people/list', { projection: ['id'] }); - - expect(response.status).toBe(200); - expect(response.body.data).toEqual([ - { - id: '8', - __forest: { collection: 'people', primaryKey: { id: 8 } }, - }, - ]); - }); - - it('should return a snake_case column under its camelCase key, while projecting its schema name', async () => { - const response = await post('/bff/agent/v1/people/list', { projection: ['id', 'first_name'] }); - - expect(response.status).toBe(200); - expect(response.body.data).toEqual([ - { - id: '8', - firstName: 'Ada', - __forest: { collection: 'people', primaryKey: { id: 8 } }, - }, - ]); - }); -}); From ef02373114a0ae642e1f3a681c22afba3e93afad Mon Sep 17 00:00:00 2001 From: Nicolas Bouliol Date: Thu, 3 Sep 2026 15:08:56 +0200 Subject: [PATCH 04/12] fix(agent): drop the node engines field from the agent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `engines: { node: ">=22.12.0" }` is a hard install failure under yarn 1, not a warning, and this ships as a minor — so every consumer still on Node 20 would fail to install the agent over a version bump that has nothing to do with the BFF they never asked for. The constraint belongs to the package that actually needs it. agent-bff keeps its own `engines`, and it is an optional peer: a Node 20 host installs the agent as before, and only hits the requirement if it opts into addBff() by installing agent-bff. Co-Authored-By: Claude Opus 5 (1M context) --- packages/agent/package.json | 3 --- packages/agent/src/embedded-bff.ts | 5 +++-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/agent/package.json b/packages/agent/package.json index b8e8b9de85..a133c31f9c 100644 --- a/packages/agent/package.json +++ b/packages/agent/package.json @@ -88,8 +88,5 @@ "@forestadmin/workflow-executor": { "optional": true } - }, - "engines": { - "node": ">=22.12.0" } } diff --git a/packages/agent/src/embedded-bff.ts b/packages/agent/src/embedded-bff.ts index 5a3b16b787..5c39b3e57d 100644 --- a/packages/agent/src/embedded-bff.ts +++ b/packages/agent/src/embedded-bff.ts @@ -154,8 +154,9 @@ export default class EmbeddedBff { `The embedded BFF requires the \`@forestadmin/agent-bff\` package, which failed to ` + `load: ${message}. Install it with \`npm install @forestadmin/agent-bff\`.`, ); - // Assigned rather than passed to the constructor: the repo targets ES2020, whose lib has no - // options bag on Error, though the declared engines guarantee a runtime that reads it. + // Assigned rather than passed to the constructor: the repo targets ES2020, whose lib types no + // options bag on Error. A plain assignment carries the cause on every runtime, and the reason + // is in the message regardless of what reads it. (wrapped as Error & { cause?: unknown }).cause = error; throw wrapped; From 95efb67c968b5e91942c6f014488105f7e7b73f5 Mon Sep 17 00:00:00 2001 From: Nicolas Bouliol Date: Thu, 3 Sep 2026 16:05:32 +0200 Subject: [PATCH 05/12] fix(agent-bff): make the origin allow-list deny, and stop /health implying it works MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two things the embedded mode turns from academic into real. The allow-list only omitted a header for a disallowed origin and ran the request anyway, so a list was read and an action was executed for that origin — the browser merely discarded the answer it was never allowed to read. Once a host application sits in front of this app, its own permissive `cors()` answers the preflight with `*` and the real request arrives here regardless, which is measured in the e2e suite. A request carrying a disallowed `Origin` is now refused with `origin_not_allowed`; a caller sending no `Origin` at all — every server-to-server api-key call — is untouched. And `/health` called its map `features`, which reads as "these work". It never meant that: `oauth` is true as soon as an encryption key is set, so a deployment whose Forest server is unreachable answered 200 while advertising oauth and ai. Renamed to `configured`, which is what it has always reported. Co-Authored-By: Claude Opus 5 (1M context) --- packages/agent-bff/src/build-bff.ts | 2 +- .../agent-bff/src/cors/cors-middleware.ts | 24 +++ .../agent-bff/src/http/bff-http-server.ts | 2 +- packages/agent-bff/src/http/health-route.ts | 15 +- packages/agent-bff/test/build-bff.test.ts | 4 +- .../test/cors/cors-middleware.test.ts | 16 +- .../test/http/bff-http-server.test.ts | 8 +- .../agent/test/bff/embedded-bff.e2e.test.ts | 143 +++++++++++++++++- 8 files changed, 197 insertions(+), 17 deletions(-) diff --git a/packages/agent-bff/src/build-bff.ts b/packages/agent-bff/src/build-bff.ts index 0b88ca3937..bf5d7c7fcb 100644 --- a/packages/agent-bff/src/build-bff.ts +++ b/packages/agent-bff/src/build-bff.ts @@ -543,7 +543,7 @@ export default async function buildBff({ // can be served, which is exactly what a probe must see. healthy: (dispatcher !== undefined && Boolean(config.forestAuthSecret)) || config.hasAllRequired, - features: { + configured: { oauth: oauth.middlewares.length > 0, ai: aiMiddlewares.length > 0, cors: config.allowedOrigins.length > 0, diff --git a/packages/agent-bff/src/cors/cors-middleware.ts b/packages/agent-bff/src/cors/cors-middleware.ts index ceaadf1aee..ebdb811803 100644 --- a/packages/agent-bff/src/cors/cors-middleware.ts +++ b/packages/agent-bff/src/cors/cors-middleware.ts @@ -8,6 +8,17 @@ export const ALLOWED_HEADERS = 'Authorization, Content-Type, X-Forest-Timezone, X-Forest-Bff-Key, X-Request-Id, Forest-Projection'; export const PREFLIGHT_MAX_AGE_SECONDS = 600; +/** + * Shaped like every other BFF error so a consumer branches on `error.type`. Built here rather than + * thrown: this middleware runs before the agent-scoped error middleware, so a throw would surface + * as a bare 500 instead of the contract. + */ +export const ORIGIN_NOT_ALLOWED = { + type: 'origin_not_allowed', + status: 403, + message: 'This origin is not allowed to call this BFF', +} as const; + export interface CorsMiddlewareOptions { allowedOrigins: string[]; logger: Logger; @@ -43,6 +54,19 @@ export default function createCorsMiddleware({ return; } + // Refuse rather than run and let the browser discard the answer. Omitting the header only stops + // the caller from READING the response: the request still executed, so a list was read and an + // action was executed for an origin the allow-list names as unwelcome. It also stops being + // theoretical once a host sits in front of this app — a permissive `cors()` of its own answers + // the preflight with `*`, and the real request arrives here regardless. + // A caller with no `Origin` at all is untouched: that is every server-to-server api-key call. + if (origin && !allowed) { + ctx.status = ORIGIN_NOT_ALLOWED.status; + ctx.body = { error: ORIGIN_NOT_ALLOWED }; + + return; + } + await next(); }; } diff --git a/packages/agent-bff/src/http/bff-http-server.ts b/packages/agent-bff/src/http/bff-http-server.ts index 17f6d87f63..83643c9ceb 100644 --- a/packages/agent-bff/src/http/bff-http-server.ts +++ b/packages/agent-bff/src/http/bff-http-server.ts @@ -71,7 +71,7 @@ export default class BFFHttpServer { createHealthRoute({ version, healthy: config.hasAllRequired, - features: { + configured: { oauth: Boolean(config.tokenEncryptionKey), ai: Boolean(config.tokenEncryptionKey), cors: config.allowedOrigins.length > 0, diff --git a/packages/agent-bff/src/http/health-route.ts b/packages/agent-bff/src/http/health-route.ts index fc42ef1386..d5cd3f3f05 100644 --- a/packages/agent-bff/src/http/health-route.ts +++ b/packages/agent-bff/src/http/health-route.ts @@ -2,8 +2,13 @@ import type { Middleware } from 'koa'; export const HEALTH_PATH = '/health'; -/** What the deployment actually serves, and therefore what its configuration switched on. */ -export interface HealthFeatures { +/** + * Which optional surfaces this deployment was CONFIGURED to serve. Deliberately not a statement + * that they work: OAuth is `true` as soon as an encryption key is set, whether or not the Forest + * server ever answers. Naming it `configured` is the whole point — a reader who takes `oauth: true` + * for "OAuth is functional" will believe a misconfigured deployment is healthy. + */ +export interface HealthConfigured { oauth: boolean; ai: boolean; cors: boolean; @@ -14,13 +19,13 @@ export interface HealthRouteOptions { version: string; /** Whether everything this deployment needs is configured. Embedded, it always is. */ healthy: boolean; - features: HealthFeatures; + configured: HealthConfigured; } export default function createHealthRoute({ version, healthy, - features, + configured, }: HealthRouteOptions): Middleware { return async function health(ctx, next) { const isHealthRequest = @@ -33,6 +38,6 @@ export default function createHealthRoute({ } ctx.status = healthy ? 200 : 503; - ctx.body = { status: healthy ? 'ok' : 'degraded', version, features }; + ctx.body = { status: healthy ? 'ok' : 'degraded', version, configured }; }; } diff --git a/packages/agent-bff/test/build-bff.test.ts b/packages/agent-bff/test/build-bff.test.ts index 917df3caad..70755d8b14 100644 --- a/packages/agent-bff/test/build-bff.test.ts +++ b/packages/agent-bff/test/build-bff.test.ts @@ -62,7 +62,7 @@ describe('buildBff', () => { expect(response.body).toEqual({ status: 'ok', version, - features: { oauth: true, ai: true, cors: false, openapi: true }, + configured: { oauth: true, ai: true, cors: false, openapi: true }, }); }); @@ -85,7 +85,7 @@ describe('buildBff', () => { expect(response.body).toEqual({ status: 'degraded', version, - features: { oauth: false, ai: false, cors: false, openapi: true }, + configured: { oauth: false, ai: false, cors: false, openapi: true }, }); }); diff --git a/packages/agent-bff/test/cors/cors-middleware.test.ts b/packages/agent-bff/test/cors/cors-middleware.test.ts index fa65f09e05..978d015a3e 100644 --- a/packages/agent-bff/test/cors/cors-middleware.test.ts +++ b/packages/agent-bff/test/cors/cors-middleware.test.ts @@ -49,15 +49,27 @@ describe('cors middleware (layer 1)', () => { expect(response.headers['access-control-allow-origin']).toBe(`${ALLOWED}:443`); }); - it('sends no CORS origin header for a disallowed origin but still proceeds', async () => { + it('refuses a disallowed origin instead of running the request', async () => { const { app, terminal } = buildApp(); const response = await request(app.callback()) .get('/agent/x') .set('Origin', 'https://evil.example.com'); - expect(response.status).toBe(200); + expect(response.status).toBe(403); + expect(response.body.error).toMatchObject({ type: 'origin_not_allowed', status: 403 }); expect(response.headers['access-control-allow-origin']).toBeUndefined(); + // Le point du refus : sans lui la requête s'exécutait, et seul le navigateur jetait la + // réponse — donc une liste était lue et une action exécutée pour une origine non autorisée. + expect(terminal).not.toHaveBeenCalled(); + }); + + it('leaves a caller with no Origin alone, which is every server-to-server call', async () => { + const { app, terminal } = buildApp(); + + const response = await request(app.callback()).get('/agent/x'); + + expect(response.status).toBe(200); expect(terminal).toHaveBeenCalled(); }); }); diff --git a/packages/agent-bff/test/http/bff-http-server.test.ts b/packages/agent-bff/test/http/bff-http-server.test.ts index 57302af0de..c655785fa0 100644 --- a/packages/agent-bff/test/http/bff-http-server.test.ts +++ b/packages/agent-bff/test/http/bff-http-server.test.ts @@ -79,7 +79,7 @@ function closeServer(server: Server): Promise { describe('BFFHttpServer', () => { describe('when config is complete', () => { - it('should answer GET /health with 200 ok, the version and the features it serves', async () => { + it('should answer GET /health with 200 ok, the version and the surfaces it was configured for', async () => { const server = createServer({ ...VALID_ENV }); const response = await request(server.callback).get('/health'); @@ -88,11 +88,11 @@ describe('BFFHttpServer', () => { expect(response.body).toEqual({ status: 'ok', version: VERSION, - features: { oauth: true, ai: true, cors: false, openapi: true }, + configured: { oauth: true, ai: true, cors: false, openapi: true }, }); }); - it('should report the features a partial configuration leaves off', async () => { + it('should report the surfaces a partial configuration leaves off', async () => { const server = createServer({ ...VALID_ENV, BFF_TOKEN_ENCRYPTION_KEY: undefined, @@ -102,7 +102,7 @@ describe('BFFHttpServer', () => { const response = await request(server.callback).get('/health'); - expect(response.body.features).toEqual({ + expect(response.body.configured).toEqual({ oauth: false, ai: false, cors: true, diff --git a/packages/agent/test/bff/embedded-bff.e2e.test.ts b/packages/agent/test/bff/embedded-bff.e2e.test.ts index 7b2752129f..c3168f6e03 100644 --- a/packages/agent/test/bff/embedded-bff.e2e.test.ts +++ b/packages/agent/test/bff/embedded-bff.e2e.test.ts @@ -324,13 +324,13 @@ describe('embedded BFF', () => { } describe('/bff/health', () => { - it('should report ok with the features this deployment switched on', async () => { + it('should report ok with the surfaces this deployment was configured for', async () => { const response = await request(app).get('/bff/health'); expect(response.status).toBe(200); expect(response.body).toMatchObject({ status: 'ok', - features: { oauth: false, ai: false, cors: true, openapi: false }, + configured: { oauth: false, ai: false, cors: true, openapi: false }, }); }); }); @@ -381,6 +381,145 @@ describe('embedded BFF', () => { ); }); + describe('the allow-list against a host that answers the preflight itself', () => { + /** A permissive host CORS, the kind an Express app registers without thinking about it. */ + function permissiveHostCors(): express.RequestHandler { + return (req, res, next) => { + res.setHeader('Access-Control-Allow-Origin', '*'); + + if (req.method === 'OPTIONS') { + res.statusCode = 204; + res.end(); + + return; + } + + next(); + }; + } + + async function hostWithPermissiveCorsInFront() { + const hosted = new Agent(agentOptions('host-cors')) + .addDataSource(async () => new SearchDataSource()) + .addBff({ allowedOrigins: ['https://my-app.com'] }); + const hostApp = express(); + hostApp.use(permissiveHostCors()); + hosted.mountOnExpress(hostApp); + await hosted.start(); + + return { hosted, hostApp }; + } + + // The preflight is lost to the host — nothing downstream can take it back — so the allow-list + // has to hold on the request that follows, which is the one with the side effects. + it( + 'should refuse the request a permissive host preflight let through', + async () => { + const { hosted, hostApp } = await hostWithPermissiveCorsInFront(); + + try { + const preflight = await request(hostApp) + .options('/bff/agent/v1/books/list') + .set('Origin', 'https://forbidden.example.com') + .set('Access-Control-Request-Method', 'POST'); + + const actual = await request(hostApp) + .post('/bff/agent/v1/books/list') + .set('Origin', 'https://forbidden.example.com') + .set('Authorization', `Bearer ${sessionToken()}`) + .set('X-Forest-Timezone', 'Europe/Paris') + .send({ projection: ['id', 'title'] }); + + // The host did answer the preflight, permissively: that part is out of our hands. + expect(preflight.headers['access-control-allow-origin']).toBe('*'); + // What is in our hands: the collection is never read for that origin. + expect(actual.status).toBe(403); + expect(actual.body.error).toMatchObject({ type: 'origin_not_allowed' }); + } finally { + await hosted.stop(); + } + }, + BOOT_TIMEOUT_MS, + ); + + it( + 'should still serve an allow-listed origin through the same host', + async () => { + const { hosted, hostApp } = await hostWithPermissiveCorsInFront(); + + try { + const response = await request(hostApp) + .post('/bff/agent/v1/books/list') + .set('Origin', 'https://my-app.com') + .set('Authorization', `Bearer ${sessionToken()}`) + .set('X-Forest-Timezone', 'Europe/Paris') + .send({ projection: ['id', 'title'] }); + + expect(response.status).toBe(200); + } finally { + await hosted.stop(); + } + }, + BOOT_TIMEOUT_MS, + ); + }); + + describe('when the host serves the agent under a sub-path of its own', () => { + async function hostWithSubPath() { + const hosted = new Agent(agentOptions('sub-path')) + .addDataSource(async () => new SearchDataSource()) + .addBff({ allowedOrigins: ['https://my-app.com'], openapiEnabled: true }); + const mounted = express(); + hosted.mountOnExpress(mounted); + const hostApp = express(); + hostApp.use('/api', mounted); + await hosted.start(); + + return { hosted, hostApp }; + } + + it( + 'should answer under the host prefix and nowhere else', + async () => { + const { hosted, hostApp } = await hostWithSubPath(); + + try { + expect((await request(hostApp).get('/api/bff/health')).status).toBe(200); + expect((await request(hostApp).get('/bff/health')).status).toBe(404); + } finally { + await hosted.stop(); + } + }, + BOOT_TIMEOUT_MS, + ); + + // Routing works either way — Express strips its own prefix. What breaks without deriving the + // prefix per request is everything the BFF *emits*: a generated client and the docs viewer both + // aim at `/bff/...`, which the host does not serve. + it( + 'should carry the host prefix into the document and the docs page', + async () => { + const { hosted, hostApp } = await hostWithSubPath(); + + try { + const document = await request(hostApp) + .get('/api/bff/agent/openapi.json') + .set('Authorization', `Bearer ${sessionToken()}`); + const page = await request(hostApp).get('/api/bff/docs'); + + expect(document.status).toBe(200); + expect(document.body.servers[0].url).toBe('/api/bff'); + expect(page.status).toBe(200); + expect(page.text).toContain('/api/bff/agent/openapi.json'); + expect(page.text).toContain('/api/bff/docs/redoc.standalone.js'); + } finally { + await hosted.stop(); + } + }, + BOOT_TIMEOUT_MS, + ); + }); + describe('the agent routes next to it', () => { it('should keep answering on their own prefix', async () => { const response = await request(app).get('/forest'); From 5187658c481669d36334d358bb5e7408e7a1236d Mon Sep 17 00:00:00 2001 From: Nicolas Bouliol Date: Thu, 3 Sep 2026 16:30:26 +0200 Subject: [PATCH 06/12] feat(agent): say at startup that BFF requests skip the IP whitelist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The whitelist exempts a caller arriving over a loopback socket with no proxy hop, which is exactly what the in-process transport looks like, so every BFF request escapes it. That is the decision: propagating the caller's ip would make the embedded mode stricter than the standalone one — where the whitelist only ever sees the BFF's own host — and would refuse the browsers a third-party UI is made of. What it must not be is silent. An operator who turned the whitelist on to close a door had no way to learn this door is not part of it: nothing in the logs, nothing in /health. The warning names the consequence and what still protects the route, so it cannot be read as "the BFF is open". Its own read of the configuration rather than the one the IpWhitelist route already fetched: that route keeps it private, and reaching into it would put BFF concerns in an unrelated part of the agent. One round-trip at boot, and it never fails the boot — a warning is not worth refusing to serve over. Co-Authored-By: Claude Opus 5 (1M context) --- packages/agent/src/embedded-bff.ts | 41 ++++++++ .../bff/embedded-bff-ip-whitelist.test.ts | 97 +++++++++++++++++++ 2 files changed, 138 insertions(+) create mode 100644 packages/agent/test/bff/embedded-bff-ip-whitelist.test.ts diff --git a/packages/agent/src/embedded-bff.ts b/packages/agent/src/embedded-bff.ts index 5c39b3e57d..9e62e19e54 100644 --- a/packages/agent/src/embedded-bff.ts +++ b/packages/agent/src/embedded-bff.ts @@ -94,6 +94,47 @@ export default class EmbeddedBff { this.stopped = false; this.options.logger('Info', formatLog(`Embedded BFF mounted on ${BFF_PREFIX}`)); + + await this.warnIfExemptFromIpWhitelist(); + } + + /** + * The whitelist exempts a caller arriving over a loopback socket with no proxy hop, which is + * exactly what the in-process transport looks like — so every BFF request escapes it. That is the + * decision, not an oversight: propagating the caller's ip would make the embedded mode stricter + * than the standalone one, where the whitelist only ever sees the BFF's own host, and would refuse + * the browsers a third-party UI is made of. What it must not be is silent — an operator who turned + * the whitelist on to close a door has no other way to learn this door is not part of it. + * + * Reads the configuration itself rather than borrowing the one `IpWhitelist` already fetched: that + * route keeps it private, and reaching into it would put BFF concerns in an unrelated part of the + * agent. Costs one round-trip at boot. Never fails the boot: a warning is not worth refusing to + * serve over, and this is the same posture as the rest of the BFF's startup. + */ + private async warnIfExemptFromIpWhitelist(): Promise { + try { + const { isFeatureEnabled } = + await this.options.forestAdminClient.getIpWhitelistConfiguration(); + + if (!isFeatureEnabled) return; + + this.options.logger( + 'Warn', + formatLog( + `The IP whitelist is enabled for this environment, but requests served under ` + + `${BFF_PREFIX} are not subject to it: they reach the agent in-process, which the ` + + `whitelist exempts as a trusted loopback caller. A resolved API key or a valid OAuth ` + + `session is still required.`, + ), + ); + } catch (error) { + this.options.logger( + 'Debug', + formatLog('Could not read the IP whitelist configuration', { + cause: error instanceof Error ? error.message : String(error), + }), + ); + } } /** Drop what the BFF read from the SaaS. Called on a restart, which means the schema moved. */ diff --git a/packages/agent/test/bff/embedded-bff-ip-whitelist.test.ts b/packages/agent/test/bff/embedded-bff-ip-whitelist.test.ts new file mode 100644 index 0000000000..8df8051e01 --- /dev/null +++ b/packages/agent/test/bff/embedded-bff-ip-whitelist.test.ts @@ -0,0 +1,97 @@ +import type { LoggerLevel } from '@forestadmin/datasource-toolkit'; + +import express from 'express'; +import { tmpdir } from 'os'; +import path from 'path'; + +import SearchDataSource from './fixtures/search-datasource'; +import Agent from '../../src/agent'; +import MockForestServer from '../__helper__/mock-forest-server'; + +const AUTH_SECRET = 'test-auth-secret-32-chars-min!!!'; +const ENV_SECRET = '0'.repeat(64); +const BOOT_TIMEOUT_MS = 30_000; + +function whitelistRules(useIpWhitelist: boolean) { + return { + data: { + type: 'ip-whitelist-rules', + id: '1', + attributes: { use_ip_whitelist: useIpWhitelist, rules: [] }, + }, + }; +} + +/** + * Boots an agent with an embedded BFF against an environment whose IP whitelist is on or off, and + * returns what the agent logged while starting. + */ +async function bootAndCollectLogs(useIpWhitelist: boolean): Promise { + const mock = new MockForestServer(); + mock + // Registered before the defaults: the first matching route wins. + .get('/liana/v1/ip-whitelist-rules', whitelistRules(useIpWhitelist)) + .setupDefaultRoutes({ envSecret: ENV_SECRET, collections: [] }) + .setupSuperagentMock() + .setupFetchMock(); + + const logs: string[] = []; + const agent = new Agent({ + authSecret: AUTH_SECRET, + envSecret: ENV_SECRET, + forestServerUrl: 'https://api.forestadmin.com', + forestAppUrl: 'https://app.forestadmin.com', + isProduction: false, + schemaPath: path.join(tmpdir(), `.fa-whitelist-${useIpWhitelist}-${Date.now()}.json`), + logger: (level: LoggerLevel, message: string) => logs.push(`${level}: ${message}`), + }) + .addDataSource(async () => new SearchDataSource()) + .addBff({}); + + agent.mountOnExpress(express()); + + try { + await agent.start(); + + return logs; + } finally { + await agent.stop(); + mock.restore(); + } +} + +describe('embedded BFF against the IP whitelist', () => { + const mentionsTheExemption = (logs: string[]) => + logs.filter(line => line.startsWith('Warn') && line.includes('IP whitelist is enabled')); + + describe('when the environment has the whitelist enabled', () => { + // The exemption is deliberate, so the only thing that can go wrong is nobody knowing about it. + it( + 'should warn at startup that BFF requests are not subject to it', + async () => { + const logs = await bootAndCollectLogs(true); + const [warning, ...others] = mentionsTheExemption(logs); + + expect(warning).toBeDefined(); + expect(warning).toContain('/bff'); + expect(warning).toContain('in-process'); + // Says what still protects the route, so the warning cannot be read as "the BFF is open". + expect(warning).toContain('API key or a valid OAuth session is still required'); + expect(others).toHaveLength(0); + }, + BOOT_TIMEOUT_MS, + ); + }); + + describe('when the environment has no whitelist', () => { + it( + 'should stay quiet, since there is no expectation to correct', + async () => { + const logs = await bootAndCollectLogs(false); + + expect(mentionsTheExemption(logs)).toHaveLength(0); + }, + BOOT_TIMEOUT_MS, + ); + }); +}); From 4b9501b636dd56540a0f4fc4d6f083e38d03234d Mon Sep 17 00:00:00 2001 From: Nicolas Bouliol Date: Thu, 3 Sep 2026 16:58:19 +0200 Subject: [PATCH 07/12] style(agent): prefix the embedded BFF logs [BFF], like [MCP] The two in-process surfaces the agent hosts now tag their lines the same way, so a host scanning its own logs reads one convention rather than two. Co-Authored-By: Claude Opus 5 (1M context) --- packages/agent/src/embedded-bff.ts | 6 +++--- packages/agent/test/agent-bff-lifecycle.test.ts | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/agent/src/embedded-bff.ts b/packages/agent/src/embedded-bff.ts index 9e62e19e54..65837f20bd 100644 --- a/packages/agent/src/embedded-bff.ts +++ b/packages/agent/src/embedded-bff.ts @@ -10,7 +10,7 @@ import { BFF_PREFIX, stripBffPrefix } from './bff-routes'; * value worth logging into `{}`. Never throws — logging must not break a request. */ function formatLog(message: string, context?: Record): string { - if (!context || Object.keys(context).length === 0) return `[bff] ${message}`; + if (!context || Object.keys(context).length === 0) return `[BFF] ${message}`; try { const serialized = JSON.stringify(context, (_key, value) => @@ -19,9 +19,9 @@ function formatLog(message: string, context?: Record): string { : value, ); - return `[bff] ${message} ${serialized}`; + return `[BFF] ${message} ${serialized}`; } catch { - return `[bff] ${message} [unserializable context]`; + return `[BFF] ${message} [unserializable context]`; } } diff --git a/packages/agent/test/agent-bff-lifecycle.test.ts b/packages/agent/test/agent-bff-lifecycle.test.ts index 78ab46ef35..d0b49bb5a7 100644 --- a/packages/agent/test/agent-bff-lifecycle.test.ts +++ b/packages/agent/test/agent-bff-lifecycle.test.ts @@ -178,7 +178,7 @@ describe('the embedded BFF lifecycle', () => { mockBuildBff.mock.calls[0][0].metrics.increment('schema_cache_refresh_error'); - expect(logger).toHaveBeenCalledWith('Warn', '[bff] metric schema_cache_refresh_error'); + expect(logger).toHaveBeenCalledWith('Warn', '[BFF] metric schema_cache_refresh_error'); }); it('should drop gauges, which the default sink reports at Info on every read', async () => { From 25652c29bd1aaff021eea7aba7675b8547788ae3 Mon Sep 17 00:00:00 2001 From: Nicolas Bouliol Date: Fri, 4 Sep 2026 11:47:11 +0200 Subject: [PATCH 08/12] fix: close four review findings on the embedded BFF MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A same-origin call carries `Origin` too — the Fetch spec sends it on anything but GET and HEAD, and every BFF data route is a POST — so a host serving its own UI next to a `/bff` mount was refused on every request under the default empty allow-list, for an origin it has no reason to think it must name. The allow-list now applies to cross-origin callers only. Matched on host rather than full origin: a TLS-terminating proxy leaves ctx.protocol at http while the browser reports https, so comparing the scheme would work in development and fail in production. Nothing is weakened — a caller that can forge `Origin` can simply omit it, which was already allowed by design, and a cross-site request never carries this host as its origin. `hasAllRequired` no longer counts the encryption key. env-config already had a test named "it gates OAuth, not boot", and the next one pinned the opposite: a key-only deployment reported degraded, so a load balancer would restart a process serving its api-key and bearer traffic fine. warnMissingConfig never named the key either, so the 503 came with no explanation. Which optional surfaces are on is what `configured` reports. Metric tags are forwarded to the logger. `action_endpoint_error` and `action_endpoint_miss` carry the rendering, collection and action that failed; without them an embedded host learned only that something, somewhere, did not resolve. `addBff()` after `start()` throws instead of registering a BFF nothing will start: the dispatcher hook has no mount left to attach to, so `/bff` answered 503 for the rest of the process while every other route worked. A start() that failed still accepts it. Co-Authored-By: Claude Opus 5 (1M context) --- packages/agent-bff/src/config/env-config.ts | 6 ++- .../agent-bff/src/cors/cors-middleware.ts | 28 ++++++++++- .../agent-bff/test/config/env-config.test.ts | 4 +- .../test/cors/cors-middleware.test.ts | 49 +++++++++++++++++++ .../test/http/bff-http-server.test.ts | 9 ++++ packages/agent/src/agent.ts | 12 +++++ packages/agent/src/embedded-bff.ts | 5 +- .../agent/test/agent-bff-lifecycle.test.ts | 42 ++++++++++++++++ 8 files changed, 150 insertions(+), 5 deletions(-) diff --git a/packages/agent-bff/src/config/env-config.ts b/packages/agent-bff/src/config/env-config.ts index 74bd9bbcdf..853f564ac4 100644 --- a/packages/agent-bff/src/config/env-config.ts +++ b/packages/agent-bff/src/config/env-config.ts @@ -219,6 +219,10 @@ export function parseConfig(env: NodeJS.ProcessEnv): BFFConfig { ), httpPort: parsePort(env.HTTP_PORT), presence, - hasAllRequired: REQUIRED_KEYS.every(key => presence[key]) && tokenEncryptionKey !== undefined, + // The encryption key is deliberately not part of this: it gates OAuth, not boot, so a + // key-only deployment is fully operational and must not report degraded — `/health` would + // otherwise have a load balancer restart a process that serves its api-key and bearer traffic + // fine. Which optional surfaces are on is what `configured` reports. + hasAllRequired: REQUIRED_KEYS.every(key => presence[key]), }; } diff --git a/packages/agent-bff/src/cors/cors-middleware.ts b/packages/agent-bff/src/cors/cors-middleware.ts index ebdb811803..bd10cf86de 100644 --- a/packages/agent-bff/src/cors/cors-middleware.ts +++ b/packages/agent-bff/src/cors/cors-middleware.ts @@ -3,6 +3,31 @@ import type { Middleware } from 'koa'; import { loggableOrigin, originAllowed } from './origin'; +/** + * Whether the caller is the very application serving this BFF. A same-origin request carries + * `Origin` too — the Fetch spec sends it on everything but GET and HEAD, and every BFF data route + * is a POST — so without this exemption a host serving its own UI next to a `/bff` mount would be + * refused on every request under the default empty allow-list, for an origin it has no reason to + * think it must name. + * + * Compared on host rather than on the full origin: a TLS-terminating proxy leaves `ctx.protocol` at + * `http` while the browser reports `https`, so matching the scheme would work in development and + * silently fail in production. The scheme is also the part a same-host attacker already controls. + * + * Forging the header buys nothing: a caller that can set `Origin` can simply omit it, which the + * allow-list lets through by design. What this cannot be reached by is the cross-site request the + * allow-list exists for — there the browser sets `Origin` to the attacking page, never to this host. + */ +function isSameOrigin(origin: string, host: string): boolean { + if (host === '') return false; + + try { + return new URL(origin).host === host; + } catch { + return false; + } +} + export const ALLOWED_METHODS = 'GET, POST, PUT, PATCH, DELETE, OPTIONS'; export const ALLOWED_HEADERS = 'Authorization, Content-Type, X-Forest-Timezone, X-Forest-Bff-Key, X-Request-Id, Forest-Projection'; @@ -60,7 +85,8 @@ export default function createCorsMiddleware({ // theoretical once a host sits in front of this app — a permissive `cors()` of its own answers // the preflight with `*`, and the real request arrives here regardless. // A caller with no `Origin` at all is untouched: that is every server-to-server api-key call. - if (origin && !allowed) { + // Nor is the host's own application, which the allow-list has nothing to say about. + if (origin && !allowed && !isSameOrigin(origin, ctx.host)) { ctx.status = ORIGIN_NOT_ALLOWED.status; ctx.body = { error: ORIGIN_NOT_ALLOWED }; diff --git a/packages/agent-bff/test/config/env-config.test.ts b/packages/agent-bff/test/config/env-config.test.ts index e2fff45821..fa3e943728 100644 --- a/packages/agent-bff/test/config/env-config.test.ts +++ b/packages/agent-bff/test/config/env-config.test.ts @@ -128,12 +128,12 @@ describe('parseConfig', () => { expect(REQUIRED_KEYS).not.toContain('BFF_TOKEN_ENCRYPTION_KEY'); }); - it('should leave the key undefined and mark hasAllRequired false when absent', () => { + it('should leave the key undefined but keep hasAllRequired true when absent', () => { const { BFF_TOKEN_ENCRYPTION_KEY, ...envWithoutKey } = VALID_ENV; const config = parseConfig(envWithoutKey); expect(config.tokenEncryptionKey).toBeUndefined(); - expect(config.hasAllRequired).toBe(false); + expect(config.hasAllRequired).toBe(true); }); it('should expose the key when a valid base64 32-byte value is provided', () => { diff --git a/packages/agent-bff/test/cors/cors-middleware.test.ts b/packages/agent-bff/test/cors/cors-middleware.test.ts index 978d015a3e..60b3611e0b 100644 --- a/packages/agent-bff/test/cors/cors-middleware.test.ts +++ b/packages/agent-bff/test/cors/cors-middleware.test.ts @@ -74,6 +74,55 @@ describe('cors middleware (layer 1)', () => { }); }); + describe('the application serving this BFF', () => { + it('serves its own page without it having to allow-list itself', async () => { + const { app, terminal } = buildApp(); + + const response = await request(app.callback()) + .post('/agent/x') + .set('Host', 'self.example.com') + .set('Origin', 'https://self.example.com'); + + expect(response.status).toBe(200); + expect(terminal).toHaveBeenCalled(); + }); + + it('matches on host, so a proxy terminating TLS does not turn it into a refusal', async () => { + const { app, terminal } = buildApp(); + + const response = await request(app.callback()) + .post('/agent/x') + .set('Host', 'self.example.com') + .set('Origin', 'http://self.example.com'); + + expect(response.status).toBe(200); + expect(terminal).toHaveBeenCalled(); + }); + + it('still refuses the same hostname on another port, which is another origin', async () => { + const { app, terminal } = buildApp(); + + const response = await request(app.callback()) + .post('/agent/x') + .set('Host', 'self.example.com') + .set('Origin', 'https://self.example.com:8443'); + + expect(response.status).toBe(403); + expect(terminal).not.toHaveBeenCalled(); + }); + + it('sends no Access-Control-Allow-Origin, which same-origin never needs', async () => { + const { app } = buildApp(); + + const response = await request(app.callback()) + .post('/agent/x') + .set('Host', 'self.example.com') + .set('Origin', 'https://self.example.com'); + + expect(response.headers['access-control-allow-origin']).toBeUndefined(); + }); + }); + describe('preflight', () => { it('answers an allow-listed OPTIONS with 204, methods, the allowed headers and max-age', async () => { const { app, terminal } = buildApp(); diff --git a/packages/agent-bff/test/http/bff-http-server.test.ts b/packages/agent-bff/test/http/bff-http-server.test.ts index c655785fa0..c5ad7f0314 100644 --- a/packages/agent-bff/test/http/bff-http-server.test.ts +++ b/packages/agent-bff/test/http/bff-http-server.test.ts @@ -110,6 +110,15 @@ describe('BFFHttpServer', () => { }); }); + it('should stay healthy without an encryption key, which gates OAuth and not boot', async () => { + const server = createServer({ ...VALID_ENV, BFF_TOKEN_ENCRYPTION_KEY: undefined }); + + const response = await request(server.callback).get('/health'); + + expect(response.status).toBe(200); + expect(response.body.status).toBe('ok'); + }); + // The features block names capabilities, never values: an anonymous caller learns that OAuth is // configured, not what any secret holds. it('should never expose secret values or the config itself in the response body', async () => { diff --git a/packages/agent/src/agent.ts b/packages/agent/src/agent.ts index a99ddc0c78..c520ad8e7e 100644 --- a/packages/agent/src/agent.ts +++ b/packages/agent/src/agent.ts @@ -79,6 +79,9 @@ export default class Agent extends FrameworkMounter private isRestarting = false; + /** Set once start() has completed, so a builder call made too late can say so. */ + private started = false; + /** * Create a new Agent Builder. * If any options are missing, the default will be applied: @@ -133,6 +136,8 @@ export default class Agent extends FrameworkMounter await this.embeddedExecutor?.start(this.standaloneServerHost, this.standaloneServerPort); // Same reason, without the socket: the dispatcher injects into the stack mount() just built. await this.embeddedBff?.start(this.getInProcessDispatcher()); + + this.started = true; } catch (error) { const { message } = error as Error; this.options.logger('Error', `Forest Admin agent startup failure: ${message}`); @@ -421,6 +426,13 @@ export default class Agent extends FrameworkMounter throw new Error('addBff can only be called once.'); } + // Refused rather than accepted and left dark: after start() the dispatcher hook has no mount to + // attach to and nothing calls the BFF's own start(), so `/bff` would answer 503 for the rest of + // the process while every other route works. A throw on the line the developer wrote instead. + if (this.started) { + throw new Error('addBff must be called before start(): the agent is already started.'); + } + if (collidesWithBff(this.mcpBasePath)) { throw new Error(bffMcpCollision(this.mcpBasePath as string)); } diff --git a/packages/agent/src/embedded-bff.ts b/packages/agent/src/embedded-bff.ts index 65837f20bd..6cc3044b09 100644 --- a/packages/agent/src/embedded-bff.ts +++ b/packages/agent/src/embedded-bff.ts @@ -86,7 +86,10 @@ export default class EmbeddedBff { // do not: they are periodic cache sizes, and the default console sink reports them at Info, // which would flood a host that only asked for a BFF. metrics: { - increment: name => this.options.logger('Warn', formatLog(`metric ${name}`)), + // Tags carried through: `action_endpoint_error` and `action_endpoint_miss` name the + // rendering, collection and action that failed, which is the whole of what makes the line + // actionable — the metric name alone says only that something, somewhere, did not resolve. + increment: (name, tags) => this.options.logger('Warn', formatLog(`metric ${name}`, tags)), gauge: () => undefined, }, logger: (level, message, context) => this.options.logger(level, formatLog(message, context)), diff --git a/packages/agent/test/agent-bff-lifecycle.test.ts b/packages/agent/test/agent-bff-lifecycle.test.ts index d0b49bb5a7..96908a2fbd 100644 --- a/packages/agent/test/agent-bff-lifecycle.test.ts +++ b/packages/agent/test/agent-bff-lifecycle.test.ts @@ -85,6 +85,28 @@ describe('the embedded BFF lifecycle', () => { }); }); + describe('when addBff is called after start()', () => { + it('should refuse, rather than register a BFF nothing will ever start', async () => { + const agent = buildAgent(); + await agent.start(); + + expect(() => agent.addBff()).toThrow( + 'addBff must be called before start(): the agent is already started.', + ); + }); + + it('should still accept it after a start() that failed', async () => { + const options = factories.forestAdminHttpDriverOptions.build({ skipSchemaUpdate: true }); + jest + .mocked(options.forestAdminClient.subscribeToServerEvents) + .mockRejectedValueOnce(new Error('SaaS unreachable')); + const agent = new Agent(options); + await expect(agent.start()).rejects.toThrow('SaaS unreachable'); + + expect(() => agent.addBff()).not.toThrow(); + }); + }); + describe('when the caller options are invalid', () => { it('should reject start() before anything is mounted or subscribed', async () => { mockParseConfig.mockImplementation(() => { @@ -181,6 +203,26 @@ describe('the embedded BFF lifecycle', () => { expect(logger).toHaveBeenCalledWith('Warn', '[BFF] metric schema_cache_refresh_error'); }); + it('should carry the tags naming what failed, not just the metric name', async () => { + const logger = jest.fn(); + const agent = new Agent( + factories.forestAdminHttpDriverOptions.build({ skipSchemaUpdate: true, logger }), + ).addBff(); + await agent.start(); + + mockBuildBff.mock.calls[0][0].metrics.increment('action_endpoint_miss', { + rendering: '1', + collection: 'books', + action: 'Mark as read', + }); + + expect(logger).toHaveBeenCalledWith( + 'Warn', + '[BFF] metric action_endpoint_miss ' + + '{"rendering":"1","collection":"books","action":"Mark as read"}', + ); + }); + it('should drop gauges, which the default sink reports at Info on every read', async () => { const logger = jest.fn(); const agent = new Agent( From 49a808545d0e8aa393bb3e7aae99295031404044 Mon Sep 17 00:00:00 2001 From: Nicolas Bouliol Date: Fri, 4 Sep 2026 12:04:20 +0200 Subject: [PATCH 09/12] fix: close the two follow-up findings on the same-origin exemption MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `isSameOrigin` compared a URL-normalized host against the raw `Host` header, and the two normalize differently: `new URL()` drops a port that is the default for the scheme, while `ctx.host` keeps whatever the proxy sent. So `Host: app.example.com:443` against `Origin: https://app.example.com` read as cross-origin and 403ed — the very case the exemption exists for. Both spellings are accepted now, with the default port derived from the origin scheme. The addBff guard keyed on a flag set at the end of start(), but mount() drains the onFirstStart hooks partway through, so a call landing while start() is still in flight was already too late and slipped past. The flag is set on the first line instead, and cleared when startup fails so a failed start leaves the agent configurable. The README promised /health would report degraded without an encryption key. It reports `configured.oauth: false` and stays ok since that key gates OAuth and not boot. Co-Authored-By: Claude Opus 5 (1M context) --- packages/agent-bff/README.md | 2 +- .../agent-bff/src/cors/cors-middleware.ts | 12 +++++++++- .../test/cors/cors-middleware.test.ts | 24 +++++++++++++++++++ packages/agent/src/agent.ts | 24 ++++++++++++------- .../agent/test/agent-bff-lifecycle.test.ts | 15 ++++++++++-- 5 files changed, 64 insertions(+), 13 deletions(-) diff --git a/packages/agent-bff/README.md b/packages/agent-bff/README.md index bfd3b2974d..dbce76f258 100644 --- a/packages/agent-bff/README.md +++ b/packages/agent-bff/README.md @@ -95,7 +95,7 @@ yarn start:dev # node --env-file=.env dist/cli.js | `FOREST_SERVER_URL` | yes | Forest SaaS API base URL. | | `FOREST_APP_URL` | yes | Forest front base URL, used to build the OAuth front-channel redirect (`src/oauth/oauth-routes.ts`). | | `AGENT_URL` | yes | The customer agent base URL the BFF calls via agent-client. | -| `BFF_TOKEN_ENCRYPTION_KEY` | for OAuth | Base64-encoded 32-byte AES-256 key encrypting stored refresh tokens. Until it is set, the `/oauth/*` token-issuance routes are disabled and `/health` reports `degraded`; already-issued `bff_access` tokens still authenticate on `/agent/*` whenever `FOREST_AUTH_SECRET` is present. | +| `BFF_TOKEN_ENCRYPTION_KEY` | for OAuth | Base64-encoded 32-byte AES-256 key encrypting stored refresh tokens. Until it is set, the `/oauth/*` token-issuance routes are disabled and `/health` reports `configured.oauth: false` — but it stays `ok`, since the key gates OAuth and not boot; already-issued `bff_access` tokens still authenticate on `/agent/*` whenever `FOREST_AUTH_SECRET` is present. | | `HTTP_PORT` | no | Server port, integer 0–65535. Defaults to `3450`. `0` binds an OS-assigned ephemeral port. | | `BFF_ALLOWED_ORIGINS` | no | Comma-separated CORS allow-list of exact origins (scheme + host + port). No wildcard. Empty ⇒ no cross-origin browser access. | | `BFF_DEFAULT_TIMEZONE` | no | Fallback IANA timezone used when a request carries neither an `X-Forest-Timezone` header nor a body `timezone`. | diff --git a/packages/agent-bff/src/cors/cors-middleware.ts b/packages/agent-bff/src/cors/cors-middleware.ts index bd10cf86de..01c9777034 100644 --- a/packages/agent-bff/src/cors/cors-middleware.ts +++ b/packages/agent-bff/src/cors/cors-middleware.ts @@ -21,11 +21,21 @@ import { loggableOrigin, originAllowed } from './origin'; function isSameOrigin(origin: string, host: string): boolean { if (host === '') return false; + let url: URL; + try { - return new URL(origin).host === host; + url = new URL(origin); } catch { return false; } + + // Both spellings are accepted because the two sides normalize differently: `new URL()` drops a + // port that is the default for the scheme, while `ctx.host` is the raw `Host` header, which a + // proxy is free to spell out. Without this, `Host: app.example.com:443` against + // `Origin: https://app.example.com` reads as cross-origin. + const defaultPort = url.protocol === 'https:' ? '443' : '80'; + + return host === url.host || host === `${url.host}:${defaultPort}`; } export const ALLOWED_METHODS = 'GET, POST, PUT, PATCH, DELETE, OPTIONS'; diff --git a/packages/agent-bff/test/cors/cors-middleware.test.ts b/packages/agent-bff/test/cors/cors-middleware.test.ts index 60b3611e0b..2529db7c6b 100644 --- a/packages/agent-bff/test/cors/cors-middleware.test.ts +++ b/packages/agent-bff/test/cors/cors-middleware.test.ts @@ -99,6 +99,30 @@ describe('cors middleware (layer 1)', () => { expect(terminal).toHaveBeenCalled(); }); + it('serves it when a proxy spells out the default https port in Host', async () => { + const { app, terminal } = buildApp(); + + const response = await request(app.callback()) + .post('/agent/x') + .set('Host', 'self.example.com:443') + .set('Origin', 'https://self.example.com'); + + expect(response.status).toBe(200); + expect(terminal).toHaveBeenCalled(); + }); + + it('serves it when a proxy spells out the default http port in Host', async () => { + const { app, terminal } = buildApp(); + + const response = await request(app.callback()) + .post('/agent/x') + .set('Host', 'self.example.com:80') + .set('Origin', 'http://self.example.com'); + + expect(response.status).toBe(200); + expect(terminal).toHaveBeenCalled(); + }); + it('still refuses the same hostname on another port, which is another origin', async () => { const { app, terminal } = buildApp(); diff --git a/packages/agent/src/agent.ts b/packages/agent/src/agent.ts index c520ad8e7e..17823c7a25 100644 --- a/packages/agent/src/agent.ts +++ b/packages/agent/src/agent.ts @@ -79,8 +79,12 @@ export default class Agent extends FrameworkMounter private isRestarting = false; - /** Set once start() has completed, so a builder call made too late can say so. */ - private started = false; + /** + * Set as soon as start() begins, not once it finishes: mount() drains the `onFirstStart` hooks + * partway through, so a builder call landing mid-startup is already too late. Cleared again when + * startup fails, which leaves nothing mounted and makes the agent configurable once more. + */ + private startupBegun = false; /** * Create a new Agent Builder. @@ -115,6 +119,7 @@ export default class Agent extends FrameworkMounter */ async start(): Promise { let mounted = false; + this.startupBegun = true; try { // First, before anything is mounted or subscribed: everything it validates is what the caller @@ -136,9 +141,8 @@ export default class Agent extends FrameworkMounter await this.embeddedExecutor?.start(this.standaloneServerHost, this.standaloneServerPort); // Same reason, without the socket: the dispatcher injects into the stack mount() just built. await this.embeddedBff?.start(this.getInProcessDispatcher()); - - this.started = true; } catch (error) { + this.startupBegun = false; const { message } = error as Error; this.options.logger('Error', `Forest Admin agent startup failure: ${message}`); @@ -426,11 +430,13 @@ export default class Agent extends FrameworkMounter throw new Error('addBff can only be called once.'); } - // Refused rather than accepted and left dark: after start() the dispatcher hook has no mount to - // attach to and nothing calls the BFF's own start(), so `/bff` would answer 503 for the rest of - // the process while every other route works. A throw on the line the developer wrote instead. - if (this.started) { - throw new Error('addBff must be called before start(): the agent is already started.'); + // Refused rather than accepted and left dark: once startup has begun the dispatcher hook has no + // mount left to attach to and nothing calls the BFF's own start(), so `/bff` would answer 503 + // for the rest of the process while every other route works. A throw on the line the developer + // wrote instead. Refused from the first line of start(), not from its last: `start()` is async, + // so a call made while it is still in flight is just as late. + if (this.startupBegun) { + throw new Error('addBff must be called before start(): the agent is already starting.'); } if (collidesWithBff(this.mcpBasePath)) { diff --git a/packages/agent/test/agent-bff-lifecycle.test.ts b/packages/agent/test/agent-bff-lifecycle.test.ts index 96908a2fbd..05037916b2 100644 --- a/packages/agent/test/agent-bff-lifecycle.test.ts +++ b/packages/agent/test/agent-bff-lifecycle.test.ts @@ -85,16 +85,27 @@ describe('the embedded BFF lifecycle', () => { }); }); - describe('when addBff is called after start()', () => { + describe('when addBff is called once startup has begun', () => { it('should refuse, rather than register a BFF nothing will ever start', async () => { const agent = buildAgent(); await agent.start(); expect(() => agent.addBff()).toThrow( - 'addBff must be called before start(): the agent is already started.', + 'addBff must be called before start(): the agent is already starting.', ); }); + it('should refuse while start() is still in flight, which is just as late', async () => { + const agent = buildAgent(); + const starting = agent.start(); + + expect(() => agent.addBff()).toThrow( + 'addBff must be called before start(): the agent is already starting.', + ); + + await starting; + }); + it('should still accept it after a start() that failed', async () => { const options = factories.forestAdminHttpDriverOptions.build({ skipSchemaUpdate: true }); jest From 7bbb38294a6dcee7831d0c88c767d8bf40cf6e84 Mon Sep 17 00:00:00 2001 From: Nicolas Bouliol Date: Fri, 4 Sep 2026 12:15:35 +0200 Subject: [PATCH 10/12] fix(agent-bff): match the same-origin host case-insensitively MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `new URL()` lowercases the host it parses out of `Origin`; `ctx.host` is the raw `Host` header, spelled however the client or proxy sent it. So `Host: APP.EXAMPLE.COM` against `Origin: https://app.example.com` read as cross-origin and 403ed, though DNS hostnames are case-insensitive — the same shape as the default-port mismatch, on the other half of what `new URL()` normalizes. Co-Authored-By: Claude Opus 5 (1M context) --- packages/agent-bff/src/cors/cors-middleware.ts | 12 +++++++----- packages/agent-bff/test/cors/cors-middleware.test.ts | 12 ++++++++++++ 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/packages/agent-bff/src/cors/cors-middleware.ts b/packages/agent-bff/src/cors/cors-middleware.ts index 01c9777034..7e35a398fd 100644 --- a/packages/agent-bff/src/cors/cors-middleware.ts +++ b/packages/agent-bff/src/cors/cors-middleware.ts @@ -29,13 +29,15 @@ function isSameOrigin(origin: string, host: string): boolean { return false; } - // Both spellings are accepted because the two sides normalize differently: `new URL()` drops a - // port that is the default for the scheme, while `ctx.host` is the raw `Host` header, which a - // proxy is free to spell out. Without this, `Host: app.example.com:443` against - // `Origin: https://app.example.com` reads as cross-origin. + // `url.host` is already normalized by `new URL()` — lowercased, and stripped of a port that is + // the default for the scheme — while `ctx.host` is the raw `Host` header, which a client or proxy + // spells however it likes. Both differences have to be absorbed here, or the exemption refuses + // the very requests it exists for: `Host: APP.EXAMPLE.COM` or `Host: app.example.com:443` against + // `Origin: https://app.example.com`. A non-default port still has to match exactly. + const normalizedHost = host.toLowerCase(); const defaultPort = url.protocol === 'https:' ? '443' : '80'; - return host === url.host || host === `${url.host}:${defaultPort}`; + return normalizedHost === url.host || normalizedHost === `${url.host}:${defaultPort}`; } export const ALLOWED_METHODS = 'GET, POST, PUT, PATCH, DELETE, OPTIONS'; diff --git a/packages/agent-bff/test/cors/cors-middleware.test.ts b/packages/agent-bff/test/cors/cors-middleware.test.ts index 2529db7c6b..dff0c15088 100644 --- a/packages/agent-bff/test/cors/cors-middleware.test.ts +++ b/packages/agent-bff/test/cors/cors-middleware.test.ts @@ -123,6 +123,18 @@ describe('cors middleware (layer 1)', () => { expect(terminal).toHaveBeenCalled(); }); + it('serves it whatever the case of the Host header, since hostnames are case-insensitive', async () => { + const { app, terminal } = buildApp(); + + const response = await request(app.callback()) + .post('/agent/x') + .set('Host', 'SELF.EXAMPLE.COM') + .set('Origin', 'https://self.example.com'); + + expect(response.status).toBe(200); + expect(terminal).toHaveBeenCalled(); + }); + it('still refuses the same hostname on another port, which is another origin', async () => { const { app, terminal } = buildApp(); From dbaf740b13d827adb316d80fbba2531aa740a099 Mon Sep 17 00:00:00 2001 From: Nicolas Bouliol Date: Wed, 9 Sep 2026 14:50:40 +0200 Subject: [PATCH 11/12] fix(agent): track the BFF peer range so a release stops breaking the link The exact pin has to equal agent-bff's workspace version or yarn installs the published copy instead of linking the workspace, which fails the build on four symbols the published version predates. agent-bff released four times while this branch was open, so the pin went stale three times. A caret range still links the workspace across those releases, and changes nothing about what ships: multi-semantic-release runs with --deps.bump=override and no --deps.prefix, so resolveNextVersion returns the bare version and the published package still carries an exact pin. --- packages/agent/package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/agent/package.json b/packages/agent/package.json index a133c31f9c..2fcd0fb188 100644 --- a/packages/agent/package.json +++ b/packages/agent/package.json @@ -49,7 +49,7 @@ }, "devDependencies": { "@fastify/express": "^1.1.0", - "@forestadmin/agent-bff": "1.25.4", + "@forestadmin/agent-bff": "^1.28.0", "@forestadmin/datasource-sql": "1.17.14", "@forestadmin/workflow-executor": "1.28.0", "@nestjs/common": "^11.1.24", @@ -75,7 +75,7 @@ }, "peerDependencies": { "@fastify/express": "^1.1.0 || ^2.0.0 || ^3.0.0 || ^4.0.0", - "@forestadmin/agent-bff": "1.25.4", + "@forestadmin/agent-bff": "^1.28.0", "@forestadmin/workflow-executor": "1.28.0" }, "peerDependenciesMeta": { From e05ddeb54779fadd05037f46c3d950cf9e3fe655 Mon Sep 17 00:00:00 2001 From: Nicolas Bouliol Date: Wed, 9 Sep 2026 14:50:50 +0200 Subject: [PATCH 12/12] fix(agent): keep a mounted agent unconfigurable, and let a stop win its race MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two findings on the embedded BFF, both reachable and both now covered. start() cleared startupBegun on every failure, including one raised after mount() — where the host framework is already serving this agent. addBff() then passed its guard and registered a BFF whose start() nothing calls, so /bff answered 503 for the rest of the process: restart() only invalidates, it never starts it. Cleared only when nothing was mounted, which is the distinction the audit-trail close two lines below already makes. start() also assigned the built BFF after awaiting buildBff(), so a stop() landing during that await was overwritten and a stopped agent resumed serving /bff into the stack it had just torn down. The result goes to a local and is dropped when shutdown already happened. The stopped flag is cleared before the await rather than after, so a start() following a stop() still serves. --- packages/agent/src/agent.ts | 6 +- packages/agent/src/embedded-bff.ts | 13 +++- .../agent/test/agent-bff-lifecycle.test.ts | 71 +++++++++++++++++++ 3 files changed, 87 insertions(+), 3 deletions(-) diff --git a/packages/agent/src/agent.ts b/packages/agent/src/agent.ts index 17823c7a25..86c9ddafea 100644 --- a/packages/agent/src/agent.ts +++ b/packages/agent/src/agent.ts @@ -142,7 +142,11 @@ export default class Agent extends FrameworkMounter // Same reason, without the socket: the dispatcher injects into the stack mount() just built. await this.embeddedBff?.start(this.getInProcessDispatcher()); } catch (error) { - this.startupBegun = false; + // Only when nothing was mounted. Past mount() the host framework is already serving this + // agent, so it is not configurable again: clearing the flag would let a later addBff() past + // its guard and register a BFF whose start() nothing calls — /bff would answer 503 for the + // rest of the process, and restart() only invalidates, it never starts it. + if (!mounted) this.startupBegun = false; const { message } = error as Error; this.options.logger('Error', `Forest Admin agent startup failure: ${message}`); diff --git a/packages/agent/src/embedded-bff.ts b/packages/agent/src/embedded-bff.ts index 6cc3044b09..a92e0883dc 100644 --- a/packages/agent/src/embedded-bff.ts +++ b/packages/agent/src/embedded-bff.ts @@ -75,9 +75,13 @@ export default class EmbeddedBff { async start(dispatcher: AgentDispatcher): Promise { if (!this.config) await this.prepare(); + // Cleared before the await, not after it: a start() following a stop() has to drop the previous + // shutdown's flag, while a stop() landing DURING the await must still be seen by the guard below. + this.stopped = false; + const { buildBff } = await this.importPackage(); - this.bff = await buildBff({ + const bff = await buildBff({ config: this.config as BFFConfig, dispatcher, basePath: BFF_PREFIX, @@ -94,7 +98,12 @@ export default class EmbeddedBff { }, logger: (level, message, context) => this.options.logger(level, formatLog(message, context)), }); - this.stopped = false; + + // stop() may have landed while buildBff() was in flight. Assigning anyway would resurrect a + // stopped agent's /bff — dispatching into the stack it just tore down instead of answering 503. + if (this.stopped) return; + + this.bff = bff; this.options.logger('Info', formatLog(`Embedded BFF mounted on ${BFF_PREFIX}`)); diff --git a/packages/agent/test/agent-bff-lifecycle.test.ts b/packages/agent/test/agent-bff-lifecycle.test.ts index 05037916b2..d0136fa55a 100644 --- a/packages/agent/test/agent-bff-lifecycle.test.ts +++ b/packages/agent/test/agent-bff-lifecycle.test.ts @@ -27,6 +27,18 @@ jest.mock('@forestadmin/agent-bff', () => ({ buildBff: (options: unknown) => mockBuildBff(options), })); +const mockExecutorStart = jest.fn(); +const mockExecutorStop = jest.fn(); + +jest.mock('@forestadmin/workflow-executor', () => ({ + __esModule: true, + buildInMemoryExecutor: () => ({ + start: mockExecutorStart, + stop: mockExecutorStop, + state: 'idle', + }), +})); + function responseSpy() { const response = { statusCode: 200, @@ -106,6 +118,19 @@ describe('the embedded BFF lifecycle', () => { await starting; }); + it('should still refuse after a start() that failed once mounted, which stays serving', async () => { + mockExecutorStart.mockRejectedValueOnce(new Error('database unreachable')); + const agent = buildAgent().addWorkflowExecutor({ + agentUrl: 'http://localhost:3310', + inMemory: true, + }); + await expect(agent.start()).rejects.toThrow('database unreachable'); + + expect(() => agent.addBff()).toThrow( + 'addBff must be called before start(): the agent is already starting.', + ); + }); + it('should still accept it after a start() that failed', async () => { const options = factories.forestAdminHttpDriverOptions.build({ skipSchemaUpdate: true }); jest @@ -168,6 +193,52 @@ describe('the embedded BFF lifecycle', () => { }); }); + describe('when stop() lands while the BFF is still being built', () => { + it('should answer 503 bff_stopped rather than serve the stack the agent tore down', async () => { + let release: (bff: unknown) => void = () => undefined; + let building: () => void = () => undefined; + const entered = new Promise(resolve => { + building = resolve; + }); + mockBuildBff.mockImplementationOnce( + () => + new Promise(resolve => { + release = resolve; + building(); + }), + ); + const agent = buildAgent().addBff(); + + const starting = agent.start(); + await entered; + await agent.stop(); + release({ callback: mockBffCallback, invalidate: mockInvalidate }); + await starting; + + const response = responseSpy(); + bffHandlerOf(agent)(requestFor('/bff/agent/v1/books/list'), response); + + expect(response.statusCode).toBe(503); + expect(bodyOf(response).error).toMatchObject({ type: 'bff_stopped' }); + expect(mockBffCallback).not.toHaveBeenCalled(); + }); + }); + + describe('when start() follows a stop()', () => { + it('should serve again, rather than keep the previous shutdown flag', async () => { + const agent = buildAgent().addBff(); + await agent.start(); + await agent.stop(); + + await agent.start(); + + const response = responseSpy(); + bffHandlerOf(agent)(requestFor('/bff/agent/v1/books/list'), response); + + expect(mockBffCallback).toHaveBeenCalledTimes(1); + }); + }); + describe('while serving', () => { it('should hand the BFF the url without the prefix it knows nothing about', async () => { const agent = buildAgent().addBff();