diff --git a/.env.dfe.example b/.env.dfe.example index 11c53082..38f83a3b 100644 --- a/.env.dfe.example +++ b/.env.dfe.example @@ -16,6 +16,8 @@ DFE_AUTH_MODE= # serves JWKS at this path -- see dfe-engine docs/LOCAL-DEV.md. DFE_ENGINE_JWKS_URL=http://localhost:8003/.well-known/jwks.json DFE_ENGINE_ISSUER=https://dfe.local/api +# Milliseconds any request to the engine may take before it is abandoned. +# DFE_ENGINE_TIMEOUT_MS=5000 # Optional: auto-provision a ClickHouse connection on team creation # (array of {name, host, username, password}; omit to skip). diff --git a/docs/architecture/oidc-authentication.md b/docs/architecture/oidc-authentication.md index 10026fc4..252861e6 100644 --- a/docs/architecture/oidc-authentication.md +++ b/docs/architecture/oidc-authentication.md @@ -130,6 +130,7 @@ invite step in DFE mode. | `DFE_AUTH_MODE` | unset | `oidc-proxy`, `header-dev`, or unset for stock upstream. Any other value stops the API | | `DFE_ENGINE_JWKS_URL` | - | engine JWKS URL, and `oidc-proxy` mode will not start without it | | `DFE_ENGINE_ISSUER` | unset | enforced as the `iss` claim when set | +| `DFE_ENGINE_TIMEOUT_MS` | `5000` | how long any request to the engine may take before it is abandoned | | `DFE_AUTH_HEADER_EMAIL` | `x-forwarded-email` | `header-dev` only, which will not start with `NODE_ENV=production` | | `DFE_AUTH_HEADER_GROUPS` | `x-forwarded-groups` | `header-dev` only | | `DFE_AUTH_DEFAULT_TEAM` | unset | the engine service identity's team, and the `header-dev` team when no group header is present | diff --git a/packages/api/src/dfe/__tests__/config.test.ts b/packages/api/src/dfe/__tests__/config.test.ts index 321f5585..b89f9e6a 100644 --- a/packages/api/src/dfe/__tests__/config.test.ts +++ b/packages/api/src/dfe/__tests__/config.test.ts @@ -7,11 +7,13 @@ const KEYS: readonly string[] = [ 'DFE_AUTH_MODE', 'DFE_ENGINE_JWKS_URL', + 'DFE_ENGINE_TIMEOUT_MS', 'NODE_ENV', ]; type Env = { DFE_AUTH_MODE?: string; DFE_ENGINE_JWKS_URL?: string; + DFE_ENGINE_TIMEOUT_MS?: string; NODE_ENV?: string; }; @@ -107,3 +109,25 @@ describe('header-dev in production', () => { ).not.toThrow(); }); }); + +describe('DFE_ENGINE_TIMEOUT_MS', () => { + test.each([ + [undefined, 5000], + ['', 5000], + ['250', 250], + ['15000', 15000], + ])('%j reads as %d ms', (value, ms) => { + expect( + loadConfigWith({ DFE_ENGINE_TIMEOUT_MS: value })?.DFE_ENGINE_TIMEOUT_MS, + ).toBe(ms); + }); + + test.each(['0', '-5', '1.5', 'soon', '5s', ' '])( + 'refuses to start on %j', + value => { + expect(() => loadConfigWith({ DFE_ENGINE_TIMEOUT_MS: value })).toThrow( + /DFE_ENGINE_TIMEOUT_MS/, + ); + }, + ); +}); diff --git a/packages/api/src/dfe/__tests__/engine-session.test.ts b/packages/api/src/dfe/__tests__/engine-session.test.ts index d876992f..48241f17 100644 --- a/packages/api/src/dfe/__tests__/engine-session.test.ts +++ b/packages/api/src/dfe/__tests__/engine-session.test.ts @@ -26,6 +26,7 @@ jest.mock('@/dfe/controllers/org-connection', () => ({ engineOrigin: jest.fn(() => 'http://engine.test:8000'), })); +import { DFE_ENGINE_TIMEOUT_MS } from '@/dfe/config'; import { clearEngineSessionCache, resolveEngineSession, @@ -84,6 +85,15 @@ describe('the decision', () => { ); }); + test('bounds the engine call by the configured timeout', async () => { + const timeout = jest.spyOn(AbortSignal, 'timeout'); + grants(['sre']); + + await resolveEngineSession('tok-a', FAR); + + expect(timeout).toHaveBeenCalledWith(DFE_ENGINE_TIMEOUT_MS); + }); + test('names the team after the ClickHouse identity the engine hands the caller', async () => { grants(['acme-analysts'], ORG); diff --git a/packages/api/src/dfe/__tests__/org-connection.test.ts b/packages/api/src/dfe/__tests__/org-connection.test.ts index 3f90202a..71618f77 100644 --- a/packages/api/src/dfe/__tests__/org-connection.test.ts +++ b/packages/api/src/dfe/__tests__/org-connection.test.ts @@ -37,8 +37,13 @@ jest.mock('@/controllers/sources', () => ({ jest.mock('@/dfe/config', () => ({ DFE_ENGINE_JWKS_URL: 'http://engine.test:8000/.well-known/jwks.json', + get DFE_ENGINE_TIMEOUT_MS() { + return mockEngineTimeoutMs; + }, })); +let mockEngineTimeoutMs = 5000; + jest.mock('@/tasks/provisionDashboards', () => ({ syncDashboards: jest.fn(), })); @@ -263,6 +268,18 @@ describe('ensureOrgConnection - the team identity', () => { expect(mockCreateConn).not.toHaveBeenCalled(); }); + test('bounds the engine read by the configured timeout', async () => { + mockEngineTimeoutMs = 4321; + const timeout = jest.spyOn(AbortSignal, 'timeout'); + mockConns.mockResolvedValue([]); + (global.fetch as jest.Mock).mockResolvedValue({ ok: false, status: 403 }); + + await ensureOrgConnection('tok', 'team-1', ORG); + + expect(timeout).toHaveBeenCalledWith(4321); + mockEngineTimeoutMs = 5000; + }); + test.each([500, 502, 503])( 'an engine %i is unavailable, not a refusal', async status => { diff --git a/packages/api/src/dfe/__tests__/query-export.test.ts b/packages/api/src/dfe/__tests__/query-export.test.ts index b02e1336..4a8c6d3d 100644 --- a/packages/api/src/dfe/__tests__/query-export.test.ts +++ b/packages/api/src/dfe/__tests__/query-export.test.ts @@ -53,6 +53,7 @@ import request from 'supertest'; import { getConnectionById } from '@/controllers/connection'; import { getSource } from '@/controllers/sources'; import { makeDocument } from '@/dfe/__tests__/doubles'; +import { DFE_ENGINE_TIMEOUT_MS } from '@/dfe/config'; import { extractToken } from '@/dfe/middleware/jwt-verify'; import router, { MAX_EXPORT_SQL_BODY_CHARS } from '@/dfe/routers/query-export'; import { getNonNullUserWithTeam } from '@/middleware/auth'; @@ -134,6 +135,15 @@ describe('POST /dfe/create-rule status mapping', () => { expect(res.body.detail).toBe('rule:write required'); }); + it('bounds the engine call by the configured timeout', async () => { + const timeout = jest.spyOn(AbortSignal, 'timeout'); + engineReplies(200, { id: 'rule-1' }); + + await createRule().send(RULE_BODY); + + expect(timeout).toHaveBeenCalledWith(DFE_ENGINE_TIMEOUT_MS); + }); + it('forwards the created rule on success', async () => { engineReplies(200, { id: 'rule-1', display_name: 'noisy logins' }); diff --git a/packages/api/src/dfe/config.ts b/packages/api/src/dfe/config.ts index 8f1f2bb8..680b587b 100644 --- a/packages/api/src/dfe/config.ts +++ b/packages/api/src/dfe/config.ts @@ -47,6 +47,26 @@ export const DFE_ENGINE_JWKS_URL = env.DFE_ENGINE_JWKS_URL as export const DFE_ENGINE_ISSUER = env.DFE_ENGINE_ISSUER as string | undefined; +// How long a request to the engine may take before it is abandoned, so a slow +// or unreachable control plane fails the request instead of hanging the client. +// Every DFE call to the engine reads this one value. +const DEFAULT_ENGINE_TIMEOUT_MS = 5000; + +function engineTimeoutMs(raw: string | undefined): number { + if (raw === undefined || raw === '') { + return DEFAULT_ENGINE_TIMEOUT_MS; + } + const ms = Number(raw); + if (!Number.isInteger(ms) || ms <= 0) { + throw new Error( + `DFE_ENGINE_TIMEOUT_MS=${JSON.stringify(raw)} is not a whole number of milliseconds above zero.`, + ); + } + return ms; +} + +export const DFE_ENGINE_TIMEOUT_MS = engineTimeoutMs(env.DFE_ENGINE_TIMEOUT_MS); + export const isDfeEnabled = DFE_AUTH_MODE === 'oidc-proxy' || DFE_AUTH_MODE === 'header-dev'; diff --git a/packages/api/src/dfe/controllers/engine-session.ts b/packages/api/src/dfe/controllers/engine-session.ts index b2b5a840..e3613b49 100644 --- a/packages/api/src/dfe/controllers/engine-session.ts +++ b/packages/api/src/dfe/controllers/engine-session.ts @@ -15,11 +15,10 @@ import { createHash } from 'node:crypto'; import { z } from 'zod'; +import { DFE_ENGINE_TIMEOUT_MS } from '@/dfe/config'; import { engineOrigin } from '@/dfe/controllers/org-connection'; import logger from '@/utils/logger'; -const ENGINE_TIMEOUT_MS = 5000; - // A revoked group, a changed role or a deleted account reaches HyperDX within this window. const CACHE_TTL_MS = 30_000; @@ -90,7 +89,7 @@ async function identityFromConnection( ): Promise { const resp = await fetch(`${origin}/api/v1/hyperdx/connection`, { headers: { Authorization: `Bearer ${token}` }, - signal: AbortSignal.timeout(ENGINE_TIMEOUT_MS), + signal: AbortSignal.timeout(DFE_ENGINE_TIMEOUT_MS), }); if (resp.status === 401) { return refused(401, 'engine_refused'); @@ -122,7 +121,7 @@ async function askEngine(token: string): Promise { try { const resp = await fetch(`${origin}/api/v1/auth/me`, { headers: { Authorization: `Bearer ${token}` }, - signal: AbortSignal.timeout(ENGINE_TIMEOUT_MS), + signal: AbortSignal.timeout(DFE_ENGINE_TIMEOUT_MS), }); if (resp.status === 401 || resp.status === 403) { return refused(resp.status === 401 ? 401 : 403, 'engine_refused'); diff --git a/packages/api/src/dfe/controllers/org-connection.ts b/packages/api/src/dfe/controllers/org-connection.ts index 030d03c0..1dc16591 100644 --- a/packages/api/src/dfe/controllers/org-connection.ts +++ b/packages/api/src/dfe/controllers/org-connection.ts @@ -68,8 +68,6 @@ const OrgConnectionSchema = z.object({ type OrgConnection = z.infer; -const ENGINE_TIMEOUT_MS = 5000; - // The engine API shares an origin with its JWKS endpoint, so we derive the base // URL from DFE_ENGINE_JWKS_URL rather than carrying a second env var. // Exported as the single definition of the engine origin so DFE routers reuse it @@ -97,7 +95,7 @@ async function fetchOrgConnection( try { const resp = await fetch(`${origin}/api/v1/hyperdx/connection`, { headers: { Authorization: `Bearer ${token}` }, - signal: AbortSignal.timeout(ENGINE_TIMEOUT_MS), + signal: AbortSignal.timeout(dfeConfig.DFE_ENGINE_TIMEOUT_MS), }); if (resp.status === 401 || resp.status === 403) { logger.warn( diff --git a/packages/api/src/dfe/routers/query-export.ts b/packages/api/src/dfe/routers/query-export.ts index fb5e1848..854bb998 100644 --- a/packages/api/src/dfe/routers/query-export.ts +++ b/packages/api/src/dfe/routers/query-export.ts @@ -19,6 +19,7 @@ import { validateRequest } from 'zod-express-middleware'; import { getConnectionById } from '@/controllers/connection'; import { getSource } from '@/controllers/sources'; +import { DFE_ENGINE_TIMEOUT_MS } from '@/dfe/config'; import { engineOrigin } from '@/dfe/controllers/org-connection'; import { requireNonSimpleRequest } from '@/dfe/middleware/cross-site'; import { extractToken } from '@/dfe/middleware/jwt-verify'; @@ -48,10 +49,6 @@ const createRuleBodySchema = z.object({ source: z.string().optional(), }); -// Bound the wait on the engine so a slow/unreachable control plane fails the -// request instead of hanging the client. Matches org-connection's ceiling. -const ENGINE_TIMEOUT_MS = 5000; - // renderChartConfig's expression regexes run in time quadratic in their input // and the API accepts 32 MB bodies, so the export body is capped at a size whose // worst case holds the event loop, and every team with it, under half a second. @@ -230,7 +227,7 @@ router.post( hunt_name, source, }), - signal: AbortSignal.timeout(ENGINE_TIMEOUT_MS), + signal: AbortSignal.timeout(DFE_ENGINE_TIMEOUT_MS), }); // Forward the engine's status and JSON body verbatim so RBAC failures