From 4db3f0c0055b39b3fba93f6065c84855b6f18eab Mon Sep 17 00:00:00 2001 From: Andrew Foote Date: Thu, 30 Jul 2026 13:28:25 -0400 Subject: [PATCH 1/6] Add graceful runtime shutdown handling --- .env.example | 1 + README.md | 13 +- api-docs/openapi.json | 14 +- api-docs/openapi.test.js | 5 +- config/custom-environment-variables.jsonc | 3 + config/default.jsonc | 4 + config/runtime-config.test.js | 1 + routes/index.js | 20 ++- routes/index.test.js | 18 +++ server.js | 177 ++++++++++++++++++++- server.test.js | 179 ++++++++++++++++++++++ utils/constants.js | 2 + utils/errors.js | 8 + utils/errors.test.js | 16 ++ utils/runtimeLifecycle.js | 64 ++++++++ utils/runtimeLifecycle.test.js | 72 +++++++++ utils/searchProvider.js | 16 ++ utils/searchProvider.test.js | 21 +++ 18 files changed, 627 insertions(+), 7 deletions(-) create mode 100644 utils/runtimeLifecycle.js create mode 100644 utils/runtimeLifecycle.test.js diff --git a/.env.example b/.env.example index 823f158..795db04 100644 --- a/.env.example +++ b/.env.example @@ -15,6 +15,7 @@ SearchRequestDeadlineMs=30000 SearchReadinessTimeoutMs=5000 RequestLogSuccessSampleRate=0.1 RequestLogHealthChecks=false +ShutdownTimeoutMs=35000 WebSearchEnabled=true SearchRateLimitEnabled=false SearchRateLimitWindowMs=60000 diff --git a/README.md b/README.md index 1155e76..429fe3d 100644 --- a/README.md +++ b/README.md @@ -14,11 +14,14 @@ Rename `.env.example` to `.env` and replace or check the following values. - `SearchPostFilterColdSearchConcurrency` - optional per-process concurrent cold CPE candidate search limit, default `10` - `SearchPostFilterColdSearchMaxQueueDepth` - optional per-process waiting queue depth for cold CPE candidate searches, default `500` - `SearchPostFilterColdSearchMaxWaitMs` - optional maximum cold CPE candidate queue wait, default `10000` +- `SearchInFlightMaxWaitersPerOperation` - optional per-process caller limit for one shared semantic search operation, default `500` +- `SearchPostFilterEvaluationChunkSize` - optional semantic CPE cancellation/yield interval, default `250` - `SearchOpenSearchRequestTimeoutMs` - optional timeout for each OpenSearch request, default `20000` - `SearchRequestDeadlineMs` - optional overall `/search` processing deadline, default `30000` - `SearchReadinessTimeoutMs` - optional timeout for the OpenSearch index readiness check, default `5000` - `RequestLogSuccessSampleRate` - optional fraction of successful non-health-check access logs to retain, default `0.1` - `RequestLogHealthChecks` - optional flag to retain successful health-check access logs, default `false` +- `ShutdownTimeoutMs` - optional graceful HTTP shutdown deadline, default `35000` - `WebSearchEnabled` - optional deprecated `/webSearch` feature flag, default `true` - `SearchRateLimitEnabled` - optional per-process `/search` rate-limit flag, default `false` - `SearchRateLimitWindowMs` - optional rate-limit window in milliseconds, default `60000` @@ -89,10 +92,18 @@ The Search API will be served at `localhost:3000`. `GET /api/health-check` is a process liveness check. It returns HTTP 200 when the Node.js process is running and deliberately does not contact OpenSearch. Use it where a dependency outage should not restart otherwise healthy API tasks. -`GET /api/ready-check` sends a minimal zero-result search to the configured CVE index. This verifies connectivity, index availability, and the same search permission used by the API without retrieving CVE records or counting all matches. It returns HTTP 200 when search requests can be served. Configuration errors, connection failures, timeouts, incompatible `cve-core` SearchReader contracts, and a missing index return a sanitized HTTP 503 `SEARCH_NOT_READY` response with `Retry-After: 5`. The OpenSearch request is limited to five seconds by default and can be changed with `SearchReadinessTimeoutMs`. +`GET /api/ready-check` sends a minimal zero-result search to the configured CVE index. This verifies connectivity, index availability, and the same search permission used by the API without retrieving CVE records or counting all matches. It returns HTTP 200 when search requests can be served. Configuration errors, connection failures, timeouts, incompatible `cve-core` SearchReader contracts, a missing index, and an in-progress process shutdown return a sanitized HTTP 503 `SEARCH_NOT_READY` response with `Retry-After: 5`. The OpenSearch request is limited to five seconds by default and can be changed with `SearchReadinessTimeoutMs`. Once shutdown starts, readiness fails immediately without contacting OpenSearch. An AWS load balancer can use `/api/ready-check` to stop routing search traffic to an unready task while the container orchestrator continues using `/api/health-check` for process liveness. Coordinate that probe selection with the infrastructure team before changing deployed health-check behavior. +## Graceful Shutdown + +When the runtime receives `SIGTERM` or `SIGINT`, it marks the process as shutting down before closing the HTTP listener. Readiness changes to HTTP 503, while liveness remains HTTP 200 for as long as the process is running. Existing requests are allowed to drain. New non-probe requests attempted over existing keep-alive connections receive HTTP 503 `SERVICE_SHUTTING_DOWN`, `Retry-After: 5`, and `Connection: close`. + +The server closes idle connections and waits up to `ShutdownTimeoutMs`, which defaults to 35 seconds. A successful drain exits normally. If the deadline expires, the process force-closes remaining connections and exits with a failure code. Configure the container orchestrator's stop timeout to exceed `ShutdownTimeoutMs`; otherwise the orchestrator can terminate the process before its application-level drain completes. + +Shutdown lifecycle events are emitted as sanitized `runtime.lifecycle` JSON logs. Unexpected Express errors emit a sanitized `request.error` event and return HTTP 500 `INTERNAL_SERVER_ERROR`. Neither event includes exception messages, stacks, query strings, request bodies, credentials, or upstream response details. + ## API Input Validation `POST /search` accepts an optional JSON object request body and validates supported fields, boolean parameters, pagination values, CVSS severity values, required parameter combinations, CPE Name requirements, and date-time formats. A non-empty body must use the `application/json` media type with UTF-8 encoding; unsupported media types or encodings return HTTP 415, and malformed JSON returns HTTP 400. If OpenSearch is unavailable, the endpoint returns HTTP 503 with a `Retry-After` header. diff --git a/api-docs/openapi.json b/api-docs/openapi.json index 22f7452..3d69e9e 100644 --- a/api-docs/openapi.json +++ b/api-docs/openapi.json @@ -109,6 +109,16 @@ } } }, + "500": { + "description": "An unexpected internal server error occurred", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ErrorResponse" + } + } + } + }, "502": { "description": "OpenSearch returned an invalid response or an unexpected upstream error occurred", "content": { @@ -120,7 +130,7 @@ } }, "503": { - "description": "OpenSearch is unavailable", + "description": "The service is shutting down or OpenSearch is unavailable", "headers": { "Retry-After": { "description": "Seconds before retrying an unavailable OpenSearch dependency", @@ -258,7 +268,7 @@ } }, "503": { - "description": "Search service is not ready", + "description": "Search service is shutting down or its OpenSearch dependency is not ready", "headers": { "Retry-After": { "description": "Seconds before retrying the readiness check", diff --git a/api-docs/openapi.test.js b/api-docs/openapi.test.js index 35da775..3bee554 100644 --- a/api-docs/openapi.test.js +++ b/api-docs/openapi.test.js @@ -202,11 +202,13 @@ describe('OpenAPI contract', () => { } ]); - ['404', '415', '429', '502', '503', '504'].forEach((status) => { + ['404', '415', '429', '500', '502', '503', '504'].forEach((status) => { expect(responses[status].content['application/json'].schema).toStrictEqual({ $ref: '#/components/schemas/ErrorResponse' }); }); + + expect(responses['503'].description).toContain('shutting down'); }); test('documents request correlation headers', () => { @@ -219,6 +221,7 @@ describe('OpenAPI contract', () => { expect(openapi.paths['/api/health-check'].get.responses['200'].headers['X-Request-ID']).toBeDefined(); expect(openapi.paths['/api/ready-check'].get.responses['200'].headers['X-Request-ID']).toBeDefined(); expect(openapi.paths['/api/ready-check'].get.responses['503'].headers['Retry-After']).toBeDefined(); + expect(openapi.paths['/api/ready-check'].get.responses['503'].description).toContain('shutting down'); expect(openapi.paths['/webSearch'].post.responses['200'].headers['X-Request-ID']).toBeDefined(); }); diff --git a/config/custom-environment-variables.jsonc b/config/custom-environment-variables.jsonc index 8898e50..205aa1f 100644 --- a/config/custom-environment-variables.jsonc +++ b/config/custom-environment-variables.jsonc @@ -27,6 +27,9 @@ "successSampleRate": "RequestLogSuccessSampleRate", "healthChecks": "RequestLogHealthChecks" }, + "runtime": { + "shutdownTimeoutMs": "ShutdownTimeoutMs" + }, "features": { "webSearchEnabled": "WebSearchEnabled" }, diff --git a/config/default.jsonc b/config/default.jsonc index 58208d9..33561f2 100644 --- a/config/default.jsonc +++ b/config/default.jsonc @@ -43,6 +43,10 @@ // Set to TRUE to retain successful health-check access logs. "healthChecks": "FALSE" }, + "runtime": { + // Must remain shorter than the container orchestrator's stop timeout. + "shutdownTimeoutMs": "35000" + }, // Feature flags used for controlled endpoint migrations. "features": { // Keep enabled until existing consumers have migrated to POST /search. diff --git a/config/runtime-config.test.js b/config/runtime-config.test.js index d07d258..2ee6a0f 100644 --- a/config/runtime-config.test.js +++ b/config/runtime-config.test.js @@ -111,6 +111,7 @@ describe('runtime container and compose configuration', () => { expect(customEnvironmentVariables).toContain('"readinessTimeoutMs": "SearchReadinessTimeoutMs"'); expect(customEnvironmentVariables).toContain('"successSampleRate": "RequestLogSuccessSampleRate"'); expect(customEnvironmentVariables).toContain('"healthChecks": "RequestLogHealthChecks"'); + expect(customEnvironmentVariables).toContain('"shutdownTimeoutMs": "ShutdownTimeoutMs"'); expect(customEnvironmentVariables).toContain('"webSearchEnabled": "WebSearchEnabled"'); expect(customEnvironmentVariables).toContain('"enabled": "SearchRateLimitEnabled"'); expect(customEnvironmentVariables).toContain('"windowMs": "SearchRateLimitWindowMs"'); diff --git a/routes/index.js b/routes/index.js index 658d590..7a2ff47 100644 --- a/routes/index.js +++ b/routes/index.js @@ -8,6 +8,7 @@ const featureFlags = require('../utils/featureFlags'); const searchDeadline = require('../utils/searchDeadline'); const searchProvider = require('../utils/searchProvider'); const searchRateLimiter = require('../utils/searchRateLimiter'); +const runtimeLifecycle = require('../utils/runtimeLifecycle'); const error = new errors.errors(); const limitSearchRequests = searchRateLimiter.createSearchRateLimiter(); @@ -24,6 +25,11 @@ function healthCheck(req, res) { } async function readinessCheck(req, res) { + if (runtimeLifecycle.isShuttingDown()) { + if (typeof res.set === 'function') res.set('Retry-After', '5'); + return res.status(503).json(error.searchNotReadyError()); + } + const lifecycle = searchDeadline.createSearchRequestLifecycle( req, res, @@ -330,7 +336,7 @@ content: { } } #swagger.responses[503] = { -description: 'OpenSearch is unavailable', +description: 'The service is shutting down or OpenSearch is unavailable', headers: { "Retry-After": { description: "Seconds before retrying an unavailable OpenSearch dependency", @@ -357,6 +363,16 @@ content: { } } } +#swagger.responses[500] = { +description: 'An unexpected internal server error occurred', +content: { + "application/json": { + schema: { + $ref: "#/components/schemas/ErrorResponse" + } + } +} +} */ requireSearchJsonContentType, limitSearchRequests, @@ -432,7 +448,7 @@ headers: { } } #swagger.responses[503] = { -description: 'Search service is not ready', +description: 'Search service is shutting down or its OpenSearch dependency is not ready', headers: { "Retry-After": { description: "Seconds before retrying the readiness check", diff --git a/routes/index.test.js b/routes/index.test.js index c1cd095..9b7aff1 100644 --- a/routes/index.test.js +++ b/routes/index.test.js @@ -38,6 +38,7 @@ jest.mock('../utils/searchProvider', () => ({ const mw = require('../controllers/middleware'); const controller = require('../controllers/searchController'); const featureFlags = require('../utils/featureFlags'); +const runtimeLifecycle = require('../utils/runtimeLifecycle'); const router = require('./index'); describe('routes/index router', () => { @@ -47,6 +48,7 @@ describe('routes/index router', () => { app = createRouterApp(router); jest.clearAllMocks(); featureFlags.isWebSearchEnabled.mockReturnValue(true); + runtimeLifecycle.resetForTests(); mockLimitSearchRequests.mockImplementation((req, res, next) => next()); mockCheckOpenSearchReadiness.mockResolvedValue(true); }); @@ -89,6 +91,22 @@ describe('routes/index router', () => { expect(response.text).not.toContain('sensitive internal OpenSearch details'); }); + test('GET /api/ready-check fails without contacting OpenSearch during shutdown', async () => { + runtimeLifecycle.beginShutdown(); + + const response = await request(app, { + path: '/api/ready-check' + }); + + expect(response.status).toBe(503); + expect(response.headers['retry-after']).toBe('5'); + expect(response.body).toStrictEqual({ + error: 'SEARCH_NOT_READY', + message: 'Search service is not ready. Please try again later.' + }); + expect(mockCheckOpenSearchReadiness).not.toHaveBeenCalled(); + }); + test('POST /search reaches middleware and controller for valid JSON body fields', async () => { const response = await request(app, { method: 'POST', diff --git a/server.js b/server.js index 4c94a56..1a34142 100644 --- a/server.js +++ b/server.js @@ -9,10 +9,13 @@ const path = require('path'); const router = require('./routes/index'); const errors = require('./utils/errors'); const requestLogging = require('./utils/requestLogging'); +const runtimeLifecycle = require('./utils/runtimeLifecycle'); +const searchProvider = require('./utils/searchProvider'); dotenv.load(); const error = new errors.errors(); +const SHUTDOWN_PROBE_PATHS = new Set(['/api/health-check', '/api/ready-check']); const swaggerSetupOptions = { swaggerOptions: { @@ -50,12 +53,51 @@ function handleRequestError(err, req, res, next) { next(err); } +function getRouteCategory(req) { + const requestPath = requestLogging.getRedactedRequestPath(req); + if (['/search', '/webSearch', '/api/health-check', '/api/ready-check'].includes(requestPath)) { + return requestPath; + } + if (requestPath.startsWith('/api-docs')) return '/api-docs'; + + return 'other'; +} + +function rejectRequestsDuringShutdown(req, res, next) { + const requestPath = requestLogging.getRedactedRequestPath(req); + if (!runtimeLifecycle.isShuttingDown() || SHUTDOWN_PROBE_PATHS.has(requestPath)) { + return next(); + } + + res.set('Retry-After', '5'); + res.set('Connection', 'close'); + return res.status(503).json(error.serviceShuttingDownError()); +} + +function handleUnexpectedError(err, req, res, next) { + console.error( + JSON.stringify({ + event: 'request.error', + outcome: 'internal_error', + requestId: req.requestId, + route: getRouteCategory(req), + statusCode: 500 + }) + ); + + if (res.headersSent) return next(err); + + return res.status(500).json(error.internalServerError()); +} + function createApp() { const app = express(); app.use(requestLogging.assignRequestId); app.use(createRequestLogger()); app.use(cors()); + app.use(runtimeLifecycle.trackRequest); + app.use(rejectRequestsDuringShutdown); app.use(express.static(path.join(__dirname, 'public'))); app.use(express.json()); @@ -64,6 +106,7 @@ function createApp() { app.use('/', router); app.use(handleRequestError); + app.use(handleUnexpectedError); return app; } @@ -83,15 +126,147 @@ function startServer(port = process.env.PORT || 3000) { }); } +function logRuntimeEvent(log, event) { + log(JSON.stringify(event)); +} + +function shutdownServer( + server, + { + closeDependencies = searchProvider.closeSearchProviderClients, + exit = (code) => process.exit(code), + log = console.log, + setExitCode = (code) => { + if (process.exitCode === undefined) process.exitCode = code; + }, + signal = 'unknown', + timeoutMs = runtimeLifecycle.getShutdownTimeoutMs() + } = {} +) { + const startState = runtimeLifecycle.beginShutdown(); + logRuntimeEvent(log, { + event: 'runtime.lifecycle', + outcome: 'shutdown_started', + signal, + activeRequestCount: startState.activeRequestCount, + timeoutMs + }); + + return new Promise((resolve) => { + let settled = false; + + const complete = (outcome, exitCode, forceExit = false) => { + if (settled) return; + + settled = true; + clearTimeout(timeoutId); + const state = runtimeLifecycle.getState(); + const result = { + outcome, + exitCode, + activeRequestCount: state.activeRequestCount + }; + logRuntimeEvent(log, { + event: 'runtime.lifecycle', + ...result + }); + resolve(result); + + if (forceExit) { + exit(exitCode); + } else { + setExitCode(exitCode); + } + }; + + const timeoutId = setTimeout(() => { + if (settled) return; + + settled = true; + server.closeAllConnections?.(); + const state = runtimeLifecycle.getState(); + const result = { + outcome: 'shutdown_timeout', + exitCode: 1, + activeRequestCount: state.activeRequestCount + }; + logRuntimeEvent(log, { + event: 'runtime.lifecycle', + ...result + }); + resolve(result); + exit(1); + }, timeoutMs); + timeoutId.unref?.(); + + try { + server.close(async (err) => { + if (err) { + complete('shutdown_error', 1, true); + return; + } + + try { + await closeDependencies(); + complete('shutdown_complete', 0); + } catch (_err) { + complete('shutdown_error', 1, true); + } + }); + server.closeIdleConnections?.(); + } catch (_err) { + complete('shutdown_error', 1, true); + } + }); +} + +function installShutdownHandlers(server, { processRef = process, shutdownOptions = {} } = {}) { + let shutdownPromise; + + const handleSignal = (signal) => { + if (!shutdownPromise) { + shutdownPromise = shutdownServer(server, { + exit: (code) => processRef.exit(code), + setExitCode: (code) => { + if (processRef.exitCode === undefined) processRef.exitCode = code; + }, + ...shutdownOptions, + signal + }); + } + + return shutdownPromise; + }; + const handleSigterm = () => void handleSignal('SIGTERM'); + const handleSigint = () => void handleSignal('SIGINT'); + + processRef.on('SIGTERM', handleSigterm); + processRef.on('SIGINT', handleSigint); + + return { + dispose() { + processRef.removeListener('SIGTERM', handleSigterm); + processRef.removeListener('SIGINT', handleSigint); + }, + getShutdownPromise: () => shutdownPromise, + handleSignal + }; +} + if (require.main === module) { - startServer(); + const server = startServer(); + installShutdownHandlers(server); } module.exports = { createApp, createRequestLogger, + handleUnexpectedError, + installShutdownHandlers, getRedactedRequestPath: requestLogging.getRedactedRequestPath, getRuntimeConfig, + rejectRequestsDuringShutdown, + shutdownServer, startServer, swaggerSetupOptions }; diff --git a/server.test.js b/server.test.js index 51ce3c8..c847901 100644 --- a/server.test.js +++ b/server.test.js @@ -1,4 +1,5 @@ const http = require('http'); +const { EventEmitter } = require('events'); const { request } = require('./test/http'); const mockValidateParams = jest.fn((req, res, next) => next()); @@ -34,9 +35,13 @@ const { createRequestLogger, getRedactedRequestPath, getRuntimeConfig, + handleUnexpectedError, + installShutdownHandlers, + shutdownServer, startServer, swaggerSetupOptions } = require('./server'); +const runtimeLifecycle = require('./utils/runtimeLifecycle'); function restoreEnv(name, value) { if (value === undefined) { @@ -56,6 +61,8 @@ describe('server application bootstrap', () => { restoreEnv('BASE_URL', originalBaseUrl); restoreEnv('NODE_ENV', originalNodeEnv); restoreEnv('PORT', originalPort); + runtimeLifecycle.resetForTests(); + jest.useRealTimers(); jest.restoreAllMocks(); jest.clearAllMocks(); }); @@ -142,6 +149,37 @@ describe('server application bootstrap', () => { }); }); + test('createApp rejects non-probe requests after shutdown starts', async () => { + runtimeLifecycle.beginShutdown(); + + const response = await request(createApp(), { + method: 'POST', + path: '/search', + body: { + keywordSearch: 'apache' + } + }); + + expect(response.status).toBe(503); + expect(response.headers['retry-after']).toBe('5'); + expect(response.headers.connection).toBe('close'); + expect(response.body).toStrictEqual({ + error: 'SERVICE_SHUTTING_DOWN', + message: 'Search service is shutting down. Please retry later.' + }); + expect(mockSearch).not.toHaveBeenCalled(); + }); + + test('createApp keeps process liveness available during shutdown', async () => { + runtimeLifecycle.beginShutdown(); + + const response = await request(createApp(), { + path: '/api/health-check' + }); + + expect(response.status).toBe(200); + }); + test('createApp returns a sanitized error for malformed JSON', async () => { const response = await request(createApp(), { method: 'POST', @@ -266,6 +304,147 @@ describe('server application bootstrap', () => { expect(fakeServer.listen).toHaveBeenCalledWith('4567', expect.any(Function)); }); + test('shutdownServer marks readiness unavailable and drains the HTTP server', async () => { + const fakeServer = { + close: jest.fn((callback) => callback()), + closeIdleConnections: jest.fn() + }; + const exit = jest.fn(); + const setExitCode = jest.fn(); + const log = jest.fn(); + const closeDependencies = jest.fn().mockResolvedValue(); + + await expect( + shutdownServer(fakeServer, { + closeDependencies, + exit, + log, + setExitCode, + signal: 'SIGTERM', + timeoutMs: 1000 + }) + ).resolves.toStrictEqual({ + outcome: 'shutdown_complete', + exitCode: 0, + activeRequestCount: 0 + }); + + expect(runtimeLifecycle.isShuttingDown()).toBe(true); + expect(fakeServer.close).toHaveBeenCalledTimes(1); + expect(fakeServer.closeIdleConnections).toHaveBeenCalledTimes(1); + expect(closeDependencies).toHaveBeenCalledTimes(1); + expect(setExitCode).toHaveBeenCalledWith(0); + expect(exit).not.toHaveBeenCalled(); + expect(log.mock.calls.map(([entry]) => JSON.parse(entry).outcome)).toEqual([ + 'shutdown_started', + 'shutdown_complete' + ]); + }); + + test('shutdownServer force-closes connections after its deadline', async () => { + const fakeServer = { + close: jest.fn(), + closeAllConnections: jest.fn(), + closeIdleConnections: jest.fn() + }; + const exit = jest.fn(); + const shutdown = shutdownServer(fakeServer, { + closeDependencies: jest.fn(), + exit, + log: jest.fn(), + setExitCode: jest.fn(), + timeoutMs: 5 + }); + + await expect(shutdown).resolves.toStrictEqual({ + outcome: 'shutdown_timeout', + exitCode: 1, + activeRequestCount: 0 + }); + expect(fakeServer.closeAllConnections).toHaveBeenCalledTimes(1); + expect(exit).toHaveBeenCalledWith(1); + }); + + test('installShutdownHandlers coalesces repeated shutdown signals', async () => { + const processRef = new EventEmitter(); + processRef.exit = jest.fn(); + processRef.exitCode = undefined; + let closeServer; + const fakeServer = { + close: jest.fn((callback) => { + closeServer = callback; + }), + closeIdleConnections: jest.fn() + }; + const handlers = installShutdownHandlers(fakeServer, { + processRef, + shutdownOptions: { + closeDependencies: jest.fn().mockResolvedValue(), + log: jest.fn(), + timeoutMs: 1000 + } + }); + + processRef.emit('SIGTERM'); + const firstShutdown = handlers.getShutdownPromise(); + processRef.emit('SIGTERM'); + processRef.emit('SIGINT'); + + expect(handlers.getShutdownPromise()).toBe(firstShutdown); + expect(fakeServer.close).toHaveBeenCalledTimes(1); + + closeServer(); + await expect(firstShutdown).resolves.toMatchObject({ + outcome: 'shutdown_complete' + }); + handlers.dispose(); + }); + + test('handleUnexpectedError returns and logs a sanitized internal error', () => { + const res = {}; + res.headersSent = false; + res.status = jest.fn(() => res); + res.json = jest.fn(() => res); + const next = jest.fn(); + jest.spyOn(console, 'error').mockImplementation(() => {}); + + handleUnexpectedError( + new Error('sensitive stack and credential details'), + { + path: '/private/value', + requestId: 'request-123' + }, + res, + next + ); + + expect(res.status).toHaveBeenCalledWith(500); + expect(res.json).toHaveBeenCalledWith({ + error: 'INTERNAL_SERVER_ERROR', + message: 'An unexpected server error occurred.' + }); + expect(next).not.toHaveBeenCalled(); + expect(console.error).toHaveBeenCalledTimes(1); + expect(JSON.parse(console.error.mock.calls[0][0])).toStrictEqual({ + event: 'request.error', + outcome: 'internal_error', + requestId: 'request-123', + route: 'other', + statusCode: 500 + }); + expect(console.error.mock.calls[0][0]).not.toContain('sensitive'); + }); + + test('handleUnexpectedError delegates after response headers were sent', () => { + const err = new Error('request failed after headers'); + const next = jest.fn(); + jest.spyOn(console, 'error').mockImplementation(() => {}); + + handleUnexpectedError(err, { path: '/search' }, { headersSent: true }, next); + + expect(next).toHaveBeenCalledWith(err); + }); + test('importing server does not start listening', () => { const createServer = jest.spyOn(http, 'createServer'); diff --git a/utils/constants.js b/utils/constants.js index 696bebd..6233699 100644 --- a/utils/constants.js +++ b/utils/constants.js @@ -26,6 +26,7 @@ const SEARCH_POST_FILTER_EVALUATION_CHUNK_SIZE = 250; const SEARCH_OPEN_SEARCH_REQUEST_TIMEOUT_MS = 20 * 1000; const SEARCH_REQUEST_DEADLINE_MS = 30 * 1000; const SEARCH_READINESS_TIMEOUT_MS = 5 * 1000; +const RUNTIME_SHUTDOWN_TIMEOUT_MS = 35 * 1000; const rejectedCveStates = ['REJECT', 'REJECTED', 'Rejected']; const rejectedCveStateFields = ['cveMetadata.state', 'containers.cna.x_legacyV4Record.CVE_data_meta.STATE']; @@ -156,6 +157,7 @@ module.exports = { SEARCH_OPEN_SEARCH_REQUEST_TIMEOUT_MS, SEARCH_REQUEST_DEADLINE_MS, SEARCH_READINESS_TIMEOUT_MS, + RUNTIME_SHUTDOWN_TIMEOUT_MS, RESULTS_PER_PAGE, REJECTED_CVE_STATES: rejectedCveStates, CPE_VERSION_RANGE_TYPES: cpeVersionRangeTypes, diff --git a/utils/errors.js b/utils/errors.js index 824b804..6b46440 100644 --- a/utils/errors.js +++ b/utils/errors.js @@ -285,6 +285,14 @@ class errors { return makeError('SEARCH_NOT_READY', 'Search service is not ready. Please try again later.'); } + serviceShuttingDownError() { + return makeError('SERVICE_SHUTTING_DOWN', 'Search service is shutting down. Please retry later.'); + } + + internalServerError() { + return makeError('INTERNAL_SERVER_ERROR', 'An unexpected server error occurred.'); + } + upstreamSearchError() { return makeError('UPSTREAM_SEARCH_ERROR', 'Search backend returned an error. Please try again later.'); } diff --git a/utils/errors.test.js b/utils/errors.test.js index 3258bf4..3d09f08 100644 --- a/utils/errors.test.js +++ b/utils/errors.test.js @@ -223,6 +223,22 @@ describe('error response helpers', () => { message: 'Search service is not ready. Please try again later.' } ], + [ + 'serviceShuttingDownError', + [], + { + error: 'SERVICE_SHUTTING_DOWN', + message: 'Search service is shutting down. Please retry later.' + } + ], + [ + 'internalServerError', + [], + { + error: 'INTERNAL_SERVER_ERROR', + message: 'An unexpected server error occurred.' + } + ], [ 'searchBodyError', [], diff --git a/utils/runtimeLifecycle.js b/utils/runtimeLifecycle.js new file mode 100644 index 0000000..cc2c86f --- /dev/null +++ b/utils/runtimeLifecycle.js @@ -0,0 +1,64 @@ +const { AppConfig } = require('cve-core'); +const constants = require('./constants'); + +let shuttingDown = false; +let activeRequestCount = 0; + +function getShutdownTimeoutMs() { + if (!AppConfig.has('runtime.shutdownTimeoutMs')) { + return constants.RUNTIME_SHUTDOWN_TIMEOUT_MS; + } + + const value = Number(AppConfig.get('runtime.shutdownTimeoutMs')); + if (Number.isInteger(value) && value >= 1) return value; + + return constants.RUNTIME_SHUTDOWN_TIMEOUT_MS; +} + +function getState() { + return { + activeRequestCount, + shuttingDown + }; +} + +function isShuttingDown() { + return shuttingDown; +} + +function beginShutdown() { + shuttingDown = true; + return getState(); +} + +function trackRequest(_req, res, next) { + activeRequestCount += 1; + let completed = false; + + const complete = () => { + if (completed) return; + + completed = true; + activeRequestCount = Math.max(0, activeRequestCount - 1); + res.removeListener?.('finish', complete); + res.removeListener?.('close', complete); + }; + + res.once('finish', complete); + res.once('close', complete); + next(); +} + +function resetForTests() { + shuttingDown = false; + activeRequestCount = 0; +} + +module.exports = { + beginShutdown, + getShutdownTimeoutMs, + getState, + isShuttingDown, + resetForTests, + trackRequest +}; diff --git a/utils/runtimeLifecycle.test.js b/utils/runtimeLifecycle.test.js new file mode 100644 index 0000000..a62c806 --- /dev/null +++ b/utils/runtimeLifecycle.test.js @@ -0,0 +1,72 @@ +const { EventEmitter } = require('events'); + +const mockAppConfigHas = jest.fn(); +const mockAppConfigGet = jest.fn(); + +jest.mock('cve-core', () => ({ + AppConfig: { + get: mockAppConfigGet, + has: mockAppConfigHas + } +})); + +const runtimeLifecycle = require('./runtimeLifecycle'); + +describe('runtimeLifecycle', () => { + beforeEach(() => { + runtimeLifecycle.resetForTests(); + mockAppConfigHas.mockReset(); + mockAppConfigGet.mockReset(); + mockAppConfigHas.mockReturnValue(false); + }); + + test('beginShutdown changes process readiness state idempotently', () => { + expect(runtimeLifecycle.getState()).toStrictEqual({ + activeRequestCount: 0, + shuttingDown: false + }); + + expect(runtimeLifecycle.beginShutdown()).toStrictEqual({ + activeRequestCount: 0, + shuttingDown: true + }); + expect(runtimeLifecycle.beginShutdown()).toStrictEqual({ + activeRequestCount: 0, + shuttingDown: true + }); + }); + + test('trackRequest counts a request until its response finishes or closes', () => { + const res = new EventEmitter(); + const next = jest.fn(); + + runtimeLifecycle.trackRequest({}, res, next); + + expect(next).toHaveBeenCalledTimes(1); + expect(runtimeLifecycle.getState().activeRequestCount).toBe(1); + + res.emit('finish'); + res.emit('close'); + + expect(runtimeLifecycle.getState().activeRequestCount).toBe(0); + expect(res.listenerCount('finish')).toBe(0); + expect(res.listenerCount('close')).toBe(0); + }); + + test('reads a positive shutdown timeout from config', () => { + mockAppConfigHas.mockImplementation((path) => path === 'runtime.shutdownTimeoutMs'); + mockAppConfigGet.mockReturnValue('45000'); + + expect(runtimeLifecycle.getShutdownTimeoutMs()).toBe(45000); + }); + + test.each(['not-a-number', '0', '-1', '1.5'])( + 'falls back to the shutdown timeout default for invalid value %s', + (value) => { + mockAppConfigHas.mockReturnValue(true); + mockAppConfigGet.mockReturnValue(value); + + expect(runtimeLifecycle.getShutdownTimeoutMs()).toBe(35000); + } + ); +}); diff --git a/utils/searchProvider.js b/utils/searchProvider.js index 18c0ea2..e9244f1 100644 --- a/utils/searchProvider.js +++ b/utils/searchProvider.js @@ -170,6 +170,21 @@ function getSearchReader() { return searchReader; } +async function closeSearchProviderClients() { + const clients = new Set( + [basicSearchManager?._searchReader?._client, searchReader?._client].filter( + (client) => typeof client?.close === 'function' + ) + ); + + const results = await Promise.allSettled([...clients].map((client) => client.close())); + basicSearchManager = undefined; + searchReader = undefined; + + const failedClose = results.find((result) => result.status === 'rejected'); + if (failedClose) throw failedClose.reason; +} + function getOpenSearchRequestState() { return { activeCount: activeOpenSearchRequestCount, @@ -399,6 +414,7 @@ function resetSearchProviderForTests() { module.exports = { checkOpenSearchReadiness, + closeSearchProviderClients, getBasicSearchManager, getAllowUnknownSslCerts, getIntegerConfig, diff --git a/utils/searchProvider.test.js b/utils/searchProvider.test.js index 8d06bed..f9ceae7 100644 --- a/utils/searchProvider.test.js +++ b/utils/searchProvider.test.js @@ -2,6 +2,7 @@ const mockBasicSearchManager = jest.fn(function BasicSearchManager(spec) { this.spec = spec; this._searchReader = { _client: { + close: jest.fn().mockResolvedValue(), transport: { maxRetries: 3, requestTimeout: 30000 @@ -10,11 +11,13 @@ const mockBasicSearchManager = jest.fn(function BasicSearchManager(spec) { }; }); const mockClientSearch = jest.fn(); +const mockClientClose = jest.fn(); const mockSearchReader = jest.fn(function SearchReader(endpoint, index, options) { this._openSearchDomainEndpoint = endpoint; this._cveIndex = index; this._options = options; this._client = { + close: mockClientClose, search: mockClientSearch, transport: { maxRetries: 3, @@ -57,6 +60,8 @@ describe('searchProvider', () => { mockBasicSearchManager.mockClear(); mockSearchReader.mockClear(); mockClientSearch.mockReset(); + mockClientClose.mockReset(); + mockClientClose.mockResolvedValue(); mockGetDefaultSearchProviderSpec.mockReset(); mockGetDefaultSearchProviderSpec.mockReturnValue({ providerEndpoint: 'https://search.example', @@ -167,6 +172,22 @@ describe('searchProvider', () => { }); }); + test('closes initialized OpenSearch clients and allows them to be recreated', async () => { + const manager = searchProvider.getBasicSearchManager(); + searchProvider.getSearchReader(); + + await searchProvider.closeSearchProviderClients(); + + expect(manager._searchReader._client.close).toHaveBeenCalledTimes(1); + expect(mockClientClose).toHaveBeenCalledTimes(1); + + searchProvider.getBasicSearchManager(); + searchProvider.getSearchReader(); + + expect(mockBasicSearchManager).toHaveBeenCalledTimes(2); + expect(mockSearchReader).toHaveBeenCalledTimes(2); + }); + test('caps an OpenSearch request timeout to the remaining request budget', async () => { mockClientSearch.mockResolvedValue({ body: { From 429fbad19124911e736350b21f9c21b91c2d7b83 Mon Sep 17 00:00:00 2001 From: Andrew Foote Date: Thu, 30 Jul 2026 14:07:08 -0400 Subject: [PATCH 2/6] Expand CI runtime validation coverage --- .github/workflows/quality.yml | 50 +++++++++++++++++++++ README.md | 19 ++++++++ config/runtime-config.test.js | 12 +++++ package.json | 3 +- server.test.js | 80 ++++++++++++++++++++++++++++++++++ test/contract/opensearch.js | 65 +++++++++++++++++++++++++++ utils/runtimeLifecycle.test.js | 5 +++ utils/searchProvider.test.js | 18 ++++++++ 8 files changed, 251 insertions(+), 1 deletion(-) create mode 100644 test/contract/opensearch.js diff --git a/.github/workflows/quality.yml b/.github/workflows/quality.yml index 95eff65..8de9c02 100644 --- a/.github/workflows/quality.yml +++ b/.github/workflows/quality.yml @@ -31,3 +31,53 @@ jobs: - name: Run quality checks run: npm run quality + + container: + name: Runtime Container + runs-on: ubuntu-latest + timeout-minutes: 15 + + steps: + - name: Check out repository + uses: actions/checkout@v4 + + - name: Build production image + run: docker build --tag cve-search-api:pr . + + - name: Start production image + run: | + docker run --detach \ + --name cve-search-api-pr \ + --publish 4100:3000 \ + --env NODE_CONFIG_ENV=prod \ + --env OpenSearchDomainEndpoint=https://search.example.com \ + --env OpenSearchCveIndex=cves \ + cve-search-api:pr + + - name: Validate runtime image + run: | + for attempt in $(seq 1 30); do + if curl --fail --silent --show-error http://127.0.0.1:4100/api/health-check; then + break + fi + if [ "$attempt" -eq 30 ]; then + exit 1 + fi + sleep 1 + done + + curl --fail --silent --show-error http://127.0.0.1:4100/api-docs/openapi.json \ + | jq --exit-status \ + '.openapi == "3.0.2" and .info.title == "CVE Search API"' \ + > /dev/null + + test "$(docker image inspect cve-search-api:pr --format '{{.Config.User}}')" = "65532:65532" + test "$(docker image inspect cve-search-api:pr --format '{{json .Config.Cmd}}')" = '["server.js"]' + + - name: Capture container logs + if: always() + run: docker logs cve-search-api-pr || true + + - name: Remove test container + if: always() + run: docker rm --force cve-search-api-pr || true diff --git a/README.md b/README.md index 429fe3d..cf7fa7e 100644 --- a/README.md +++ b/README.md @@ -264,6 +264,25 @@ npm run coverage npm run quality ``` +Pull requests run the quality gate and independently build the production `Dockerfile`. The container job starts the built image, checks process liveness and the packaged OpenAPI document, and verifies the runtime user and command. It intentionally does not call `/api/ready-check` or `/search`, because pull-request CI does not own or provision an OpenSearch index. + +An opt-in, read-only OpenSearch contract check is available for an approved environment that can reach an existing index: + +```bash +npm run test:opensearch-contract +``` + +Set `OpenSearchDomainEndpoint` and `OpenSearchCveIndex` in the environment or local `.env` before running it. `OpenSearchAllowUnknownSslCerts` remains optional and should be enabled only for local development against a self-signed certificate. The contract check sends a minimal readiness query and one single-hit search, validates the response shape, closes its OpenSearch client, and emits sanitized JSON output. It never creates, updates, maps, or deletes indexes. + +The contract check is not part of public pull-request CI. A future automated invocation should use a protected GitHub environment or self-hosted runner with network access to an approved test index. Connection values and credentials must remain in environment secrets rather than source files or workflow text. + +The validation tiers have separate purposes: + +- Jest and `npm run quality` verify application behavior, API contracts, and regression coverage with controlled test doubles. +- Pull-request container validation verifies production-image packaging, non-root execution, startup, liveness, and packaged API documentation. +- The optional OpenSearch contract check verifies that the installed client can communicate with an externally managed index using the response contract expected by this application. +- Load and performance testing remain in the separate automation performance-testing repository. + ### Operational Logging Every request receives an `X-Request-ID` response header. A caller-provided `X-Request-ID` is preserved when it contains only letters, numbers, `.`, `_`, `:`, or `-` and is no longer than 128 characters; otherwise the API generates a UUID. The same value appears as `request-id` in access logs and `requestId` in structured `/search` events. diff --git a/config/runtime-config.test.js b/config/runtime-config.test.js index 2ee6a0f..4c969ec 100644 --- a/config/runtime-config.test.js +++ b/config/runtime-config.test.js @@ -69,6 +69,18 @@ describe('runtime container and compose configuration', () => { expect(dockerignore).toContain('**/reports'); }); + test('pull request CI builds and smoke tests the production runtime image', () => { + const workflow = readProjectFile('.github', 'workflows', 'quality.yml'); + + expect(workflow).toContain('name: Runtime Container'); + expect(workflow).toContain('docker build --tag cve-search-api:pr .'); + expect(workflow).toContain('http://127.0.0.1:4100/api/health-check'); + expect(workflow).toContain('http://127.0.0.1:4100/api-docs/openapi.json'); + expect(workflow).toContain(`--format '{{.Config.User}}'`); + expect(workflow).toContain(`--format '{{json .Config.Cmd}}'`); + expect(workflow).not.toContain('/api/ready-check'); + }); + test('server-only compose file joins the existing OpenSearch 3.5 network', () => { const compose = readProjectFile('docker-compose.yaml'); diff --git a/package.json b/package.json index 2a59a9f..346a0c2 100644 --- a/package.json +++ b/package.json @@ -11,7 +11,8 @@ "quality": "npm run lint && npm run format:check && npm run coverage", "start": "node server.js", "swagger-autogen": "node routes/swagger.js", - "test": "jest" + "test": "jest", + "test:opensearch-contract": "node test/contract/opensearch.js" }, "dependencies": { "cors": "^2.8.5", diff --git a/server.test.js b/server.test.js index c847901..bb829df 100644 --- a/server.test.js +++ b/server.test.js @@ -365,6 +365,86 @@ describe('server application bootstrap', () => { expect(exit).toHaveBeenCalledWith(1); }); + test('shutdownServer exits with failure when the HTTP server reports a close error', async () => { + const closeDependencies = jest.fn(); + const exit = jest.fn(); + const log = jest.fn(); + const fakeServer = { + close: jest.fn((callback) => callback(new Error('sensitive server close failure'))), + closeIdleConnections: jest.fn() + }; + + await expect( + shutdownServer(fakeServer, { + closeDependencies, + exit, + log, + setExitCode: jest.fn(), + timeoutMs: 1000 + }) + ).resolves.toMatchObject({ + outcome: 'shutdown_error', + exitCode: 1 + }); + + expect(closeDependencies).not.toHaveBeenCalled(); + expect(exit).toHaveBeenCalledWith(1); + expect(log.mock.calls.map(([entry]) => entry).join(' ')).not.toContain('sensitive'); + }); + + test('shutdownServer exits with failure when dependency cleanup rejects', async () => { + const exit = jest.fn(); + const log = jest.fn(); + const fakeServer = { + close: jest.fn((callback) => callback()), + closeIdleConnections: jest.fn() + }; + + await expect( + shutdownServer(fakeServer, { + closeDependencies: jest.fn().mockRejectedValue(new Error('sensitive dependency close failure')), + exit, + log, + setExitCode: jest.fn(), + timeoutMs: 1000 + }) + ).resolves.toMatchObject({ + outcome: 'shutdown_error', + exitCode: 1 + }); + + expect(exit).toHaveBeenCalledWith(1); + expect(log.mock.calls.map(([entry]) => entry).join(' ')).not.toContain('sensitive'); + }); + + test('shutdownServer exits with failure when closing the HTTP server throws', async () => { + const exit = jest.fn(); + const log = jest.fn(); + const fakeServer = { + close: jest.fn(() => { + throw new Error('sensitive synchronous close failure'); + }), + closeIdleConnections: jest.fn() + }; + + await expect( + shutdownServer(fakeServer, { + closeDependencies: jest.fn(), + exit, + log, + setExitCode: jest.fn(), + timeoutMs: 1000 + }) + ).resolves.toMatchObject({ + outcome: 'shutdown_error', + exitCode: 1 + }); + + expect(fakeServer.closeIdleConnections).not.toHaveBeenCalled(); + expect(exit).toHaveBeenCalledWith(1); + expect(log.mock.calls.map(([entry]) => entry).join(' ')).not.toContain('sensitive'); + }); + test('installShutdownHandlers coalesces repeated shutdown signals', async () => { const processRef = new EventEmitter(); processRef.exit = jest.fn(); diff --git a/test/contract/opensearch.js b/test/contract/opensearch.js new file mode 100644 index 0000000..0467d2d --- /dev/null +++ b/test/contract/opensearch.js @@ -0,0 +1,65 @@ +const assert = require('assert'); +require('dotenv').config(); + +const REQUIRED_ENVIRONMENT_VARIABLES = ['OpenSearchDomainEndpoint', 'OpenSearchCveIndex']; + +function requireConfiguration() { + const missing = REQUIRED_ENVIRONMENT_VARIABLES.filter((name) => !process.env[name]?.trim()); + if (missing.length > 0) { + throw new Error(`Missing required environment variables: ${missing.join(', ')}`); + } +} + +function getSanitizedFailure(err) { + const statusCode = Number(err?.meta?.statusCode ?? err?.statusCode); + + return { + event: 'opensearch.contract', + outcome: 'failed', + errorCode: typeof err?.code === 'string' ? err.code : undefined, + failureType: typeof err?.failureType === 'string' ? err.failureType : undefined, + statusCode: Number.isInteger(statusCode) ? statusCode : undefined + }; +} + +async function run() { + requireConfiguration(); + + const searchProvider = require('../../utils/searchProvider'); + try { + await searchProvider.checkOpenSearchReadiness(); + const body = await searchProvider.searchOpenSearch( + { + query: { + match_all: {} + }, + size: 1, + track_total_hits: false, + _source: ['cveMetadata.cveId'] + }, + { + operation: 'contract-check' + } + ); + + assert.ok(Array.isArray(body.hits.hits)); + if (body.hits.hits.length > 0) { + assert.strictEqual(typeof body.hits.hits[0]._id, 'string'); + } + + process.stdout.write( + `${JSON.stringify({ + event: 'opensearch.contract', + outcome: 'passed', + returnedHits: body.hits.hits.length + })}\n` + ); + } finally { + await searchProvider.closeSearchProviderClients(); + } +} + +run().catch((err) => { + process.stderr.write(`${JSON.stringify(getSanitizedFailure(err))}\n`); + process.exitCode = 1; +}); diff --git a/utils/runtimeLifecycle.test.js b/utils/runtimeLifecycle.test.js index a62c806..47b2571 100644 --- a/utils/runtimeLifecycle.test.js +++ b/utils/runtimeLifecycle.test.js @@ -60,6 +60,11 @@ describe('runtimeLifecycle', () => { expect(runtimeLifecycle.getShutdownTimeoutMs()).toBe(45000); }); + test('uses the default shutdown timeout when no override is configured', () => { + expect(runtimeLifecycle.getShutdownTimeoutMs()).toBe(35000); + expect(mockAppConfigGet).not.toHaveBeenCalled(); + }); + test.each(['not-a-number', '0', '-1', '1.5'])( 'falls back to the shutdown timeout default for invalid value %s', (value) => { diff --git a/utils/searchProvider.test.js b/utils/searchProvider.test.js index f9ceae7..159aac2 100644 --- a/utils/searchProvider.test.js +++ b/utils/searchProvider.test.js @@ -188,6 +188,24 @@ describe('searchProvider', () => { expect(mockSearchReader).toHaveBeenCalledTimes(2); }); + test('attempts to close every initialized OpenSearch client before reporting a close failure', async () => { + const manager = searchProvider.getBasicSearchManager(); + searchProvider.getSearchReader(); + const closeFailure = new Error('sensitive client close failure'); + manager._searchReader._client.close.mockRejectedValue(closeFailure); + + await expect(searchProvider.closeSearchProviderClients()).rejects.toBe(closeFailure); + + expect(manager._searchReader._client.close).toHaveBeenCalledTimes(1); + expect(mockClientClose).toHaveBeenCalledTimes(1); + + searchProvider.getBasicSearchManager(); + searchProvider.getSearchReader(); + + expect(mockBasicSearchManager).toHaveBeenCalledTimes(2); + expect(mockSearchReader).toHaveBeenCalledTimes(2); + }); + test('caps an OpenSearch request timeout to the remaining request budget', async () => { mockClientSearch.mockResolvedValue({ body: { From efc87796c78f72c0a721ad1a2867371be905ca13 Mon Sep 17 00:00:00 2001 From: Andrew Foote Date: Thu, 30 Jul 2026 14:28:11 -0400 Subject: [PATCH 3/6] Remove stale search maintenance paths --- .github/workflows/quality.yml | 6 +- controllers/middleware.js | 2 +- controllers/middleware.test.js | 4 +- controllers/searchController.js | 64 +---------- controllers/searchController.test.js | 156 ++++++++++++--------------- jest.config.js | 8 +- package-lock.json | 45 -------- package.json | 2 - server.js | 2 - utils/constants.js | 15 --- utils/searchPerformance.js | 1 - utils/searchPerformance.test.js | 1 - 12 files changed, 81 insertions(+), 225 deletions(-) diff --git a/.github/workflows/quality.yml b/.github/workflows/quality.yml index 8de9c02..7671e70 100644 --- a/.github/workflows/quality.yml +++ b/.github/workflows/quality.yml @@ -18,10 +18,10 @@ jobs: steps: - name: Check out repository - uses: actions/checkout@v4 + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 - name: Set up Node.js - uses: actions/setup-node@v4 + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 with: node-version: 24.14.1 cache: npm @@ -39,7 +39,7 @@ jobs: steps: - name: Check out repository - uses: actions/checkout@v4 + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 - name: Build production image run: docker build --tag cve-search-api:pr . diff --git a/controllers/middleware.js b/controllers/middleware.js index 524d052..cdbba5a 100644 --- a/controllers/middleware.js +++ b/controllers/middleware.js @@ -117,7 +117,7 @@ function isValidDateTime(value) { //Ensures parameters are valid and checks for parameters that can't be called together function validateParams(req, res, next) { try { - const params = req.searchParams ?? req.query ?? {}; + const params = req.searchParams ?? {}; //Ensures all specified parameter names are valid for (const param of Object.keys(params)) { diff --git a/controllers/middleware.test.js b/controllers/middleware.test.js index 0261e9b..c793a8f 100644 --- a/controllers/middleware.test.js +++ b/controllers/middleware.test.js @@ -926,8 +926,8 @@ describe('middleware validation function', () => { ); test('ValidateParams delegates unexpected errors to next()', () => { - const err = new Error('query failure'); - Object.defineProperty(req, 'query', { + const err = new Error('search parameter failure'); + Object.defineProperty(req, 'searchParams', { get: () => { throw err; } diff --git a/controllers/searchController.js b/controllers/searchController.js index f7e00e0..e954f28 100644 --- a/controllers/searchController.js +++ b/controllers/searchController.js @@ -56,18 +56,6 @@ function buildEmptyFilteredResult(hitsContainer) { }; } -function filterSearchHits(resp, postFilters) { - const hitsContainer = getHitsContainer(resp); - if (!Array.isArray(hitsContainer?.hits)) return buildEmptyFilteredResult(hitsContainer); - - const cpeEvaluationContext = cpe.createEvaluationContext(); - return { - hitsContainer, - candidateCount: hitsContainer.hits.length, - filteredHits: hitsContainer.hits.filter((hit) => hitMatchesPostFilters(hit, postFilters, cpeEvaluationContext)) - }; -} - function yieldToEventLoop() { return new Promise((resolve) => setImmediate(resolve)); } @@ -153,15 +141,6 @@ function buildPaginationMetadata(resp, pagingOptions) { }; } -function buildPostFilteredResponse( - resp, - filteredHits, - responseOptions, - pagedHits = paginateHits(filteredHits, responseOptions) -) { - return buildPagedSearchResponse(resp, filteredHits.length, pagedHits); -} - function getOpenSearchErrorStatusCode(err) { return Number(err?.statusCode ?? err?.meta?.statusCode ?? err?.meta?.body?.status); } @@ -208,22 +187,6 @@ function buildWebSearchSort(sort) { return undefined; } -function applySearchPostFilters(resp, postFilters, responseOptions, metrics) { - if (!hasSearchPostFilters(postFilters)) return resp; - - const { hitsContainer, candidateCount, filteredHits } = filterSearchHits(resp, postFilters); - if (!Array.isArray(hitsContainer?.hits)) return resp; - - if (metrics) { - const pagedHits = paginateHits(filteredHits, responseOptions); - metrics.candidateCount = candidateCount; - metrics.filteredCount = filteredHits.length; - metrics.returnedCount = pagedHits.length; - } - - return buildPostFilteredResponse(resp, filteredHits, responseOptions); -} - function getHitCveId(hit) { return hit?._source?.cveMetadata?.cveId ?? hit?._id; } @@ -260,14 +223,7 @@ async function executeOpenSearch(body, operation, signal, metrics, requestTimeou }); } -async function executeStandardSearch( - searchOptions, - queryStringObjects, - postFilters, - responseOptions, - metrics, - lifecycle -) { +async function executeStandardSearch(searchOptions, queryStringObjects, metrics, lifecycle) { metrics.strategy = 'standard'; const searchStart = searchPerformance.markTime(); @@ -280,11 +236,7 @@ async function executeStandardSearch( metrics.openSearchElapsedMs = searchPerformance.elapsedMs(searchStart); metrics.openSearchTookMs = searchData?.took; - const postFilterStart = searchPerformance.markTime(); - const resp = applySearchPostFilters(searchResp, postFilters, responseOptions, metrics); - metrics.postFilterElapsedMs = searchPerformance.elapsedMs(postFilterStart); - - return resp; + return searchResp; } async function fetchFullHitsByCveIds(cveIds, metrics, signal) { @@ -422,14 +374,7 @@ async function executeOptimizedPostFilterSearch( async function executeSearch(searchOptions, queryStringObjects, postFilters, responseOptions, metrics, lifecycle) { if (!hasSearchPostFilters(postFilters)) { - return executeStandardSearch( - searchOptions, - queryStringObjects, - postFilters, - responseOptions, - metrics, - lifecycle - ); + return executeStandardSearch(searchOptions, queryStringObjects, metrics, lifecycle); } return executeOptimizedPostFilterSearch( @@ -443,7 +388,7 @@ async function executeSearch(searchOptions, queryStringObjects, postFilters, res } async function searchHandler(req, res, next) { - const params = req.searchParams ?? req.query ?? {}; + const params = req.searchParams ?? {}; const metrics = { requestId: req.requestId }; @@ -641,7 +586,6 @@ async function webSearchHandler(req, res, next) { } module.exports = { - applySearchPostFilters, buildWebSearchSort, filterSearchHitsInChunks, SEARCH: searchHandler, diff --git a/controllers/searchController.test.js b/controllers/searchController.test.js index 69fb4f2..245415d 100644 --- a/controllers/searchController.test.js +++ b/controllers/searchController.test.js @@ -212,7 +212,7 @@ describe('Search controller SEARCH handler', () => { await controller.SEARCH( { - query: { + searchParams: { keywordSearch: 'remote code', cveIds: 'CVE-2024-0001', pubStartDate: '2024-01-01T00:00:00.000Z', @@ -302,7 +302,7 @@ describe('Search controller SEARCH handler', () => { await controller.SEARCH( { - query: { + searchParams: { keywordSearch: 'remote code', keywordExactMatch: 'true' } @@ -328,7 +328,7 @@ describe('Search controller SEARCH handler', () => { await controller.SEARCH( { - query: { + searchParams: { cveIds: 'CVE-2024-0001,CVE-2024-0002' } }, @@ -351,7 +351,7 @@ describe('Search controller SEARCH handler', () => { await controller.SEARCH( { - query: { + searchParams: { noRejected: true } }, @@ -389,7 +389,7 @@ describe('Search controller SEARCH handler', () => { await controller.SEARCH( { - query: { + searchParams: { virtualMatchString: 'cpe:2.3:a:microsoft:windows:*:*:*:*:*:*:*:*' } }, @@ -438,7 +438,7 @@ describe('Search controller SEARCH handler', () => { await controller.SEARCH( { - query: { + searchParams: { cweId: 'CWE-79', cpeName: 'cpe:2.3:a:vendor:product:1.0:*:*:*:*:*:*:*', isVulnerable: 'true' @@ -503,7 +503,7 @@ describe('Search controller SEARCH handler', () => { await controller.SEARCH( { - query: { + searchParams: { cpeName: 'cpe:2.3:a:vendor:product:1.0:*:*:*:*:*:*:*', isVulnerable: true } @@ -547,7 +547,7 @@ describe('Search controller SEARCH handler', () => { await controller.SEARCH( { - query: { + searchParams: { cpeName: 'cpe:2.3:a:vendor:product:2.10:*:*:*:*:*:*:*', resultsPerPage: '1', pageNumber: '1' @@ -604,7 +604,7 @@ describe('Search controller SEARCH handler', () => { }); }); - test('applySearchPostFilters does not combine platform and version matches from different CPE entries', () => { + test('chunked filtering does not combine platform and version matches from different CPE entries', async () => { const candidate = cpeHit('CVE-2026-0250', { criteria: 'cpe:2.3:a:palo_alto_networks:globalprotect_app:*:*:macos:*:*:*:*:*', vulnerable: true, @@ -632,23 +632,20 @@ describe('Search controller SEARCH handler', () => { } ); - const response = controller.applySearchPostFilters( + const { filteredHits } = await controller.filterSearchHitsInChunks( advancedResponse(1, [candidate]), { cpeName: 'cpe:2.3:a:palo_alto_networks:globalprotect_app:6.1.1:*:macos:*:*:*:*:*', isVulnerable: true }, - { - from: 0, - resultsPerPage: 20 - } + undefined, + 100 ); - expect(response.data.hits.total.value).toBe(0); - expect(response.data.hits.hits).toStrictEqual([]); + expect(filteredHits).toStrictEqual([]); }); - test('applySearchPostFilters matches CPE string literals without lexical case', () => { + test('chunked filtering matches CPE string literals without lexical case', async () => { const candidate = cpeHit('CVE-2026-0251', { criteria: 'cpe:2.3:a:palo_alto_networks:globalprotect_app:*:*:*:*:*:macOS:*:*', vulnerable: true, @@ -656,23 +653,20 @@ describe('Search controller SEARCH handler', () => { versionEndExcluding: '6.1' }); - const response = controller.applySearchPostFilters( + const { filteredHits } = await controller.filterSearchHitsInChunks( advancedResponse(1, [candidate]), { cpeName: 'cpe:2.3:a:palo_alto_networks:globalprotect_app:6.0.1:*:*:*:*:macos:*:*', isVulnerable: true }, - { - from: 0, - resultsPerPage: 20 - } + undefined, + 100 ); - expect(response.data.hits.total.value).toBe(1); - expect(response.data.hits.hits).toStrictEqual([candidate]); + expect(filteredHits).toStrictEqual([candidate]); }); - test('applySearchPostFilters preserves exact version padding while normalizing numeric range segments', () => { + test('chunked filtering preserves exact version padding while normalizing numeric range segments', async () => { const exactCandidate = cpeHit('CVE-2025-9392', { criteria: 'cpe:2.3:o:linksys:re6250_firmware:1.0.04.001:*:*:*:*:*:*:*', vulnerable: true @@ -683,46 +677,40 @@ describe('Search controller SEARCH handler', () => { versionEndExcluding: '6.02' }); - const exactResponse = controller.applySearchPostFilters( + const exactResult = await controller.filterSearchHitsInChunks( advancedResponse(1, [exactCandidate]), { cpeName: 'cpe:2.3:o:linksys:re6250_firmware:1.0.4.1:*:*:*:*:*:*:*', isVulnerable: true }, - { - from: 0, - resultsPerPage: 20 - } + undefined, + 100 ); - const includedRangeResponse = controller.applySearchPostFilters( + const includedRangeResult = await controller.filterSearchHitsInChunks( advancedResponse(1, [rangeCandidate]), { cpeName: 'cpe:2.3:o:buffalo:wsr3600be4-kh_firmware:6.1:*:*:*:*:*:*:*', isVulnerable: true }, - { - from: 0, - resultsPerPage: 20 - } + undefined, + 100 ); - const excludedRangeResponse = controller.applySearchPostFilters( + const excludedRangeResult = await controller.filterSearchHitsInChunks( advancedResponse(1, [rangeCandidate]), { cpeName: 'cpe:2.3:o:buffalo:wsr3600be4-kh_firmware:6.2:*:*:*:*:*:*:*', isVulnerable: true }, - { - from: 0, - resultsPerPage: 20 - } + undefined, + 100 ); - expect(exactResponse.data.hits.total.value).toBe(0); - expect(includedRangeResponse.data.hits.hits).toStrictEqual([rangeCandidate]); - expect(excludedRangeResponse.data.hits.total.value).toBe(0); + expect(exactResult.filteredHits).toStrictEqual([]); + expect(includedRangeResult.filteredHits).toStrictEqual([rangeCandidate]); + expect(excludedRangeResult.filteredHits).toStrictEqual([]); }); - test('applySearchPostFilters preserves CPE NA matches across numeric version ranges', () => { + test('chunked filtering preserves CPE NA matches across numeric version ranges', async () => { const criteria = 'cpe:2.3:a:microsoft:edge:*:*:*:*:*:android:*:*'; const candidates = [ cpeHit('CVE-2020-17153', { @@ -744,20 +732,17 @@ describe('Search controller SEARCH handler', () => { }) ]; - const response = controller.applySearchPostFilters( + const { filteredHits } = await controller.filterSearchHitsInChunks( advancedResponse(candidates.length, candidates), { cpeName: 'cpe:2.3:a:microsoft:edge:-:*:*:*:*:android:*:*', isVulnerable: true }, - { - from: 0, - resultsPerPage: 20 - } + undefined, + 100 ); - expect(response.data.hits.total.value).toBe(3); - expect(response.data.hits.hits.map((hit) => hit._id)).toStrictEqual([ + expect(filteredHits.map((hit) => hit._id)).toStrictEqual([ 'CVE-2020-17153', 'CVE-2024-38208', 'CVE-2026-35429' @@ -790,7 +775,7 @@ describe('Search controller SEARCH handler', () => { await controller.SEARCH( { - query: { + searchParams: { virtualMatchString: 'cpe:2.3:o:linux:linux_kernel', versionStart: '2.2', versionStartType: 'including', @@ -857,7 +842,7 @@ describe('Search controller SEARCH handler', () => { } }; const req = { - query: { + searchParams: { virtualMatchString: 'cpe:2.3:o:linux:linux_kernel', versionStart: '2.2', versionStartType: 'including', @@ -925,7 +910,7 @@ describe('Search controller SEARCH handler', () => { } }; const req = { - query: { + searchParams: { virtualMatchString: 'cpe:2.3:o:linux:linux_kernel', versionStart: '2.2', versionStartType: 'including', @@ -984,13 +969,13 @@ describe('Search controller SEARCH handler', () => { }; const firstReq = new EventEmitter(); const secondReq = new EventEmitter(); - const query = { + const searchParams = { virtualMatchString: 'cpe:2.3:o:linux:linux_kernel', resultsPerPage: 1, pageNumber: 1 }; - firstReq.query = query; - secondReq.query = query; + firstReq.searchParams = searchParams; + secondReq.searchParams = searchParams; const firstRes = createResponse(); const secondRes = createResponse(); let operationSignal; @@ -1045,7 +1030,7 @@ describe('Search controller SEARCH handler', () => { await controller.SEARCH( { - query: { + searchParams: { virtualMatchString: 'cpe:2.3:o:microsoft:windows_10_1809', resultsPerPage: '1', pageNumber: '1' @@ -1109,7 +1094,7 @@ describe('Search controller SEARCH handler', () => { const firstSearch = controller.SEARCH( { - query: { + searchParams: { virtualMatchString: 'cpe:2.3:o:linux:linux_kernel', resultsPerPage: '1', pageNumber: '1' @@ -1123,7 +1108,7 @@ describe('Search controller SEARCH handler', () => { await controller.SEARCH( { requestId: 'queue-full-request', - query: { + searchParams: { virtualMatchString: 'cpe:2.3:o:microsoft:windows_10_1809', resultsPerPage: '1', pageNumber: '1' @@ -1172,7 +1157,7 @@ describe('Search controller SEARCH handler', () => { const firstSearch = controller.SEARCH( { - query: { + searchParams: { virtualMatchString: 'cpe:2.3:o:linux:linux_kernel', resultsPerPage: '1', pageNumber: '1' @@ -1186,7 +1171,7 @@ describe('Search controller SEARCH handler', () => { const secondSearch = controller.SEARCH( { requestId: 'queue-timeout-request', - query: { + searchParams: { virtualMatchString: 'cpe:2.3:o:microsoft:windows_10_1809', resultsPerPage: '1', pageNumber: '1' @@ -1225,7 +1210,7 @@ describe('Search controller SEARCH handler', () => { const firstRes = createResponse(); const secondRes = createResponse(); const req = { - query: { + searchParams: { virtualMatchString: 'cpe:2.3:o:linux:linux_kernel', resultsPerPage: '1', pageNumber: '1' @@ -1272,7 +1257,7 @@ describe('Search controller SEARCH handler', () => { await controller.SEARCH( { - query: { + searchParams: { virtualMatchString: 'cpe:2.3:o:microsoft:windows_10_1809', resultsPerPage: '1', pageNumber: '1' @@ -1296,7 +1281,7 @@ describe('Search controller SEARCH handler', () => { failureType: 'partial_shards' }); const req = { - query: { + searchParams: { virtualMatchString: 'cpe:2.3:o:microsoft:windows_10_1809', resultsPerPage: '1', pageNumber: '1' @@ -1324,7 +1309,7 @@ describe('Search controller SEARCH handler', () => { await controller.SEARCH( { requestId: 'request-123', - query: { + searchParams: { keywordSearch: 'remote code', resultsPerPage: '1' } @@ -1343,7 +1328,6 @@ describe('Search controller SEARCH handler', () => { statusCode: 200, strategy: 'standard', cacheStatus: 'not_applicable', - optimizedFallback: false, cacheHit: false, inFlightHit: false, openSearchOperations: ['standard'], @@ -1361,7 +1345,7 @@ describe('Search controller SEARCH handler', () => { await controller.SEARCH( { - query: { + searchParams: { cvssV3_0Severity: 'high', cvssV3_1Severity: 'critical' } @@ -1389,7 +1373,7 @@ describe('Search controller SEARCH handler', () => { await controller.SEARCH( { - query: { + searchParams: { cvssV2Metrics: 'AV:N/AC:L/Au:N' } }, @@ -1407,7 +1391,7 @@ describe('Search controller SEARCH handler', () => { await controller.SEARCH( { - query: { + searchParams: { cvssV3_0Metrics: 'CVSS:3.0/AV:N/AC:L', cvssV3_1Metrics: 'CVSS:3.1/PR:N/UI:R' } @@ -1427,7 +1411,7 @@ describe('Search controller SEARCH handler', () => { await controller.SEARCH( { - query: { + searchParams: { keywordSearch: 'c++ path:/tmp (test)' } }, @@ -1451,7 +1435,7 @@ describe('Search controller SEARCH handler', () => { await controller.SEARCH( { - query: { + searchParams: { keywordSearch: ' ' } }, @@ -1475,7 +1459,7 @@ describe('Search controller SEARCH handler', () => { await controller.SEARCH( { - query: { + searchParams: { virtualMatchString: 'cpe:2.3:a:vendor:product\\:edition:*:*:*:*:*:*:*:*' } }, @@ -1515,7 +1499,7 @@ describe('Search controller SEARCH handler', () => { await controller.SEARCH( { - query: { + searchParams: { virtualMatchString: 'cpe:2.3:*:*:*:*:*:*:*:*:*:*:*' } }, @@ -1542,7 +1526,7 @@ describe('Search controller SEARCH handler', () => { await controller.SEARCH( { - query: {} + searchParams: {} }, res, next @@ -1563,7 +1547,7 @@ describe('Search controller SEARCH handler', () => { await controller.SEARCH( { - query: { + searchParams: { resultsPerPage: 10, pageNumber: 2 } @@ -1587,7 +1571,7 @@ describe('Search controller SEARCH handler', () => { await controller.SEARCH( { - query: { + searchParams: { resultsPerPage: 30, pageNumber: 333 } @@ -1616,7 +1600,7 @@ describe('Search controller SEARCH handler', () => { await controller.SEARCH( { requestId: 'connection-error-request', - query: {} + searchParams: {} }, res, next @@ -1651,7 +1635,7 @@ describe('Search controller SEARCH handler', () => { await controller.SEARCH( { requestId: 'throttled-request', - query: {} + searchParams: {} }, res, next @@ -1685,7 +1669,7 @@ describe('Search controller SEARCH handler', () => { await controller.SEARCH( { requestId: 'response-timeout-request', - query: {} + searchParams: {} }, res, next @@ -1717,7 +1701,7 @@ describe('Search controller SEARCH handler', () => { await controller.SEARCH( { requestId: `${failureType}-request`, - query: {} + searchParams: {} }, res, next @@ -1747,7 +1731,7 @@ describe('Search controller SEARCH handler', () => { await controller.SEARCH( { requestId: 'timeout-request', - query: {} + searchParams: {} }, res, next @@ -1779,7 +1763,7 @@ describe('Search controller SEARCH handler', () => { const search = controller.SEARCH( { requestId: 'request-deadline-request', - query: {} + searchParams: {} }, res, next @@ -1808,7 +1792,7 @@ describe('Search controller SEARCH handler', () => { test('SEARCH stops without writing a response when the client disconnects', async () => { const req = new EventEmitter(); req.requestId = 'client-disconnect-request'; - req.query = {}; + req.searchParams = {}; mockApiSearch.mockImplementation( (_body, { signal }) => new Promise((resolve, reject) => { @@ -1836,7 +1820,7 @@ describe('Search controller SEARCH handler', () => { test('SEARCH does not start a page fetch after a client disconnects from a CPE search', async () => { const req = new EventEmitter(); req.requestId = 'cpe-client-disconnect-request'; - req.query = { + req.searchParams = { virtualMatchString: 'cpe:2.3:o:linux:linux_kernel', resultsPerPage: 1, pageNumber: 1 @@ -1871,7 +1855,7 @@ describe('Search controller SEARCH handler', () => { await controller.SEARCH( { requestId: 'upstream-error-request', - query: {} + searchParams: {} }, res, next diff --git a/jest.config.js b/jest.config.js index dd834cb..36247ba 100644 --- a/jest.config.js +++ b/jest.config.js @@ -2,13 +2,7 @@ const config = { clearMocks: true, collectCoverage: false, - collectCoverageFrom: [ - 'controllers/**/*.js', - 'routes/index.js', - 'server.js', - 'utils/**/*.js', - '!controllers/wrapper.js' - ], + collectCoverageFrom: ['controllers/**/*.js', 'routes/index.js', 'server.js', 'utils/**/*.js'], coverageDirectory: 'coverage', coverageProvider: 'v8', coverageReporters: ['json', 'text', 'lcov'], diff --git a/package-lock.json b/package-lock.json index 61f936e..5a06297 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,12 +10,10 @@ "dependencies": { "cors": "^2.8.5", "cve-core": "github:CVEProject/cve-core.git#dev-package-test", - "debug": "~3.2.7", "dotenv": "^6.1.0", "express": "^4.22.2", "express-rate-limit": "^8.6.1", "express-validator": "^7.2.1", - "http-errors": "~1.8.1", "morgan": "~1.11.0", "swagger-ui-express": "^5.0.1" }, @@ -2323,15 +2321,6 @@ "date-fns": "2.x" } }, - "node_modules/debug": { - "version": "3.2.7", - "resolved": "https://registry.npmjs.org/debug/-/debug-3.2.7.tgz", - "integrity": "sha512-CFjzYYAi4ThfiQvizrFQevTTXHtnCqWfe7x1AhgEscTz6ZbLbfoLRLPugTQyBth6f8ZERVUSyWHFD/7Wu4t1XQ==", - "license": "MIT", - "dependencies": { - "ms": "^2.1.1" - } - }, "node_modules/dedent": { "version": "0.7.0", "resolved": "https://registry.npmjs.org/dedent/-/dedent-0.7.0.tgz", @@ -3467,40 +3456,6 @@ "dev": true, "license": "MIT" }, - "node_modules/http-errors": { - "version": "1.8.1", - "resolved": "https://registry.npmjs.org/http-errors/-/http-errors-1.8.1.tgz", - "integrity": "sha512-Kpk9Sm7NmI+RHhnj6OIWDI1d6fIoFAtFt9RLaTMRlg/8w49juAStsrBgp0Dp4OdxdVbRIeKhtCUvoi/RuAhO4g==", - "license": "MIT", - "dependencies": { - "depd": "~1.1.2", - "inherits": "2.0.4", - "setprototypeof": "1.2.0", - "statuses": ">= 1.5.0 < 2", - "toidentifier": "1.0.1" - }, - "engines": { - "node": ">= 0.6" - } - }, - "node_modules/http-errors/node_modules/depd": { - "version": "1.1.2", - "resolved": "https://registry.npmjs.org/depd/-/depd-1.1.2.tgz", - "integrity": "sha512-7emPTl6Dpo6JRXOXjLRxck+FlLRX5847cLKEn00PLAgc3g2hTZZgr+e4c2v6QpSmLeFP3n5yUo7ft6avBK/5jQ==", - "license": "MIT", - "engines": { - "node": ">= 0.6" - } - }, - "node_modules/http-errors/node_modules/statuses": { - "version": "1.5.0", - "resolved": "https://registry.npmjs.org/statuses/-/statuses-1.5.0.tgz", - "integrity": "sha512-OpZ3zP+jT1PI7I8nemJX4AKmAX070ZkYPVWV/AaKTJl+tXCTGyVdC1a4SL8RUQYEwk/f34ZX8UTykN68FwrqAA==", - "license": "MIT", - "engines": { - "node": ">= 0.6" - } - }, "node_modules/https-proxy-agent": { "version": "5.0.1", "resolved": "https://registry.npmjs.org/https-proxy-agent/-/https-proxy-agent-5.0.1.tgz", diff --git a/package.json b/package.json index 346a0c2..1b752e5 100644 --- a/package.json +++ b/package.json @@ -17,12 +17,10 @@ "dependencies": { "cors": "^2.8.5", "cve-core": "github:CVEProject/cve-core.git#dev-package-test", - "debug": "~3.2.7", "dotenv": "^6.1.0", "express": "^4.22.2", "express-rate-limit": "^8.6.1", "express-validator": "^7.2.1", - "http-errors": "~1.8.1", "morgan": "~1.11.0", "swagger-ui-express": "^5.0.1" }, diff --git a/server.js b/server.js index 1a34142..a388c54 100644 --- a/server.js +++ b/server.js @@ -5,7 +5,6 @@ const swaggerUi = require('swagger-ui-express'); const openApiSpecification = require('./api-docs/openapi.json'); const http = require('http'); const logger = require('morgan'); -const path = require('path'); const router = require('./routes/index'); const errors = require('./utils/errors'); const requestLogging = require('./utils/requestLogging'); @@ -98,7 +97,6 @@ function createApp() { app.use(cors()); app.use(runtimeLifecycle.trackRequest); app.use(rejectRequestsDuringShutdown); - app.use(express.static(path.join(__dirname, 'public'))); app.use(express.json()); app.get('/api-docs/openapi.json', (req, res) => res.json(openApiSpecification)); diff --git a/utils/constants.js b/utils/constants.js index 6233699..6b3c979 100644 --- a/utils/constants.js +++ b/utils/constants.js @@ -92,20 +92,6 @@ const fields = { virtualMatchString: cpeMatchFields.criteria }; -const searchControlParams = [ - 'keywordExactMatch', - 'resultsPerPage', - 'pageNumber', - 'pubStartDate', - 'pubEndDate', - 'versionStart', - 'versionStartType', - 'versionEnd', - 'versionEndType', - 'lastModStartDate', - 'lastModEndDate' -]; - const searchQueryParams = [ 'keywordSearch', 'keywordExactMatch', @@ -164,6 +150,5 @@ module.exports = { CPE_MATCH_FIELDS: cpeMatchFields, CVSS_VECTOR_VERSION_FIELDS: cvssVectorVersionFields, FIELDS: fields, - SEARCH_CONTROL_PARAMS: searchControlParams, SEARCH_QUERY_PARAMS: searchQueryParams }; diff --git a/utils/searchPerformance.js b/utils/searchPerformance.js index 19c690f..7dfa764 100644 --- a/utils/searchPerformance.js +++ b/utils/searchPerformance.js @@ -45,7 +45,6 @@ function logSearchPerformance(metrics, searchOptions, responseOptions, postFilte if (includeDetailedTimings) { Object.assign(event, { - optimizedFallback: metrics.optimizedFallback === true, cacheHit: metrics.cacheHit === true, inFlightHit: metrics.inFlightHit === true, pageFetchInFlightHit: metrics.pageFetchInFlightHit === true, diff --git a/utils/searchPerformance.test.js b/utils/searchPerformance.test.js index b4d9a75..4d93959 100644 --- a/utils/searchPerformance.test.js +++ b/utils/searchPerformance.test.js @@ -72,7 +72,6 @@ describe('searchPerformance', () => { statusCode: 200, strategy: 'cpe-candidate', cacheStatus: 'hit', - optimizedFallback: false, cacheHit: true, inFlightHit: true, pageFetchInFlightHit: true, From 2867dfcc64b6549dfda34bd87c711f24a296ce15 Mon Sep 17 00:00:00 2001 From: Andrew Foote Date: Thu, 30 Jul 2026 15:30:52 -0400 Subject: [PATCH 4/6] Fail closed on malformed search candidates --- controllers/searchController.js | 35 +++++-- controllers/searchController.test.js | 56 +++++++++++- routes/search.integration.test.js | 38 ++++++++ utils/searchProvider.js | 78 +++++++++++++++- utils/searchProvider.test.js | 131 ++++++++++++++++++++++++++- 5 files changed, 319 insertions(+), 19 deletions(-) diff --git a/controllers/searchController.js b/controllers/searchController.js index e954f28..249ca2b 100644 --- a/controllers/searchController.js +++ b/controllers/searchController.js @@ -11,6 +11,7 @@ const searchQueryBuilder = require('../utils/searchQueryBuilder'); const constants = require('../utils/constants'); const error = new errors.errors(); const openSearchUnavailableErrorNames = new Set(['ConnectionError', 'NoLivingConnectionsError']); +const openSearchUpstreamErrorNames = new Set(['ResponseError', 'SearchError']); const openSearchUnavailableStatusCodes = new Set([502, 503, 504]); function hasSearchPostFilters(postFilters) { @@ -156,11 +157,25 @@ function recordUpstreamFailure(metrics, err, failureType) { function isOpenSearchUnavailableError(err) { return ( searchProvider.isSearchProviderContractError?.(err) === true || + searchProvider.isSearchProviderConfigurationError?.(err) === true || openSearchUnavailableErrorNames.has(err?.name) || openSearchUnavailableStatusCodes.has(getOpenSearchErrorStatusCode(err)) ); } +function getOpenSearchUnavailableFailureType(err) { + if (searchProvider.isSearchProviderContractError?.(err)) return 'provider_contract'; + if (searchProvider.isSearchProviderConfigurationError?.(err)) return 'provider_configuration'; + + return 'unavailable'; +} + +function isOpenSearchUpstreamError(err) { + const statusCode = getOpenSearchErrorStatusCode(err); + + return openSearchUpstreamErrorNames.has(err?.name) || (Number.isInteger(statusCode) && statusCode >= 400); +} + function buildWebSearchSort(sort) { if (!sort) return undefined; @@ -500,11 +515,7 @@ async function searchHandler(req, res, next) { } if (isOpenSearchUnavailableError(err)) { - recordUpstreamFailure( - metrics, - err, - searchProvider.isSearchProviderContractError?.(err) ? 'provider_contract' : 'unavailable' - ); + recordUpstreamFailure(metrics, err, getOpenSearchUnavailableFailureType(err)); metrics.outcome = 'opensearch_unavailable'; metrics.statusCode = 503; if (typeof res.set === 'function') res.set('Retry-After', '5'); @@ -512,10 +523,16 @@ async function searchHandler(req, res, next) { return res.status(503).json(error.openSearchError()); } - recordUpstreamFailure(metrics, err, 'upstream_error'); - metrics.outcome = 'upstream_error'; - metrics.statusCode = 502; - return res.status(502).json(error.upstreamSearchError()); + if (isOpenSearchUpstreamError(err)) { + recordUpstreamFailure(metrics, err, 'upstream_error'); + metrics.outcome = 'upstream_error'; + metrics.statusCode = 502; + return res.status(502).json(error.upstreamSearchError()); + } + + metrics.outcome = 'internal_error'; + metrics.statusCode = 500; + return next(err); } finally { lifecycle.complete(); metrics.totalElapsedMs = searchPerformance.elapsedMs(handlerStart); diff --git a/controllers/searchController.test.js b/controllers/searchController.test.js index 245415d..5fdaf3c 100644 --- a/controllers/searchController.test.js +++ b/controllers/searchController.test.js @@ -25,6 +25,7 @@ const mockGetInFlightMaxWaitersPerOperation = jest.fn(() => 500); const mockGetPostFilterEvaluationChunkSize = jest.fn(() => 250); const mockGetSearchRequestDeadlineMs = jest.fn(() => 30000); const mockIsOpenSearchResponseError = jest.fn((err) => err?.code === 'OPEN_SEARCH_RESPONSE_INVALID'); +const mockIsSearchProviderConfigurationError = jest.fn((err) => err?.code === 'SEARCH_PROVIDER_CONFIGURATION'); const mockIsSearchProviderContractError = jest.fn((err) => err?.code === 'SEARCH_PROVIDER_CONTRACT'); const mockGetBasicSearchManager = jest.fn(() => ({ search: mockBasicSearch @@ -49,6 +50,7 @@ jest.mock('../utils/searchProvider', () => ({ getPostFilterEvaluationChunkSize: mockGetPostFilterEvaluationChunkSize, getSearchRequestDeadlineMs: mockGetSearchRequestDeadlineMs, isOpenSearchResponseError: mockIsOpenSearchResponseError, + isSearchProviderConfigurationError: mockIsSearchProviderConfigurationError, isSearchProviderContractError: mockIsSearchProviderContractError, searchOpenSearch: mockSearchOpenSearch, shouldLogSearchPerformance: mockShouldLogSearchPerformance @@ -149,12 +151,20 @@ function cpeHit(id, cpeMatch) { return { _id: id, _source: { + cveMetadata: { + cveId: id + }, containers: { cna: { cpeApplicability: { nodes: [ { - cpeMatch: [cpeMatch] + cpeMatch: [ + { + vulnerable: true, + ...cpeMatch + } + ] } ] } @@ -184,6 +194,7 @@ describe('Search controller SEARCH handler', () => { mockGetPostFilterEvaluationChunkSize.mockClear(); mockGetSearchRequestDeadlineMs.mockClear(); mockIsOpenSearchResponseError.mockClear(); + mockIsSearchProviderConfigurationError.mockClear(); mockIsSearchProviderContractError.mockClear(); mockGetPostFilterCacheTtlMs.mockReturnValue(60000); mockGetPostFilterCacheMaxEntries.mockReturnValue(100); @@ -1253,7 +1264,13 @@ describe('Search controller SEARCH handler', () => { }); test('SEARCH returns sanitized upstream errors when optimized post-filter search fails', async () => { - mockSearchRaw.mockRejectedValue(new Error('candidate search failed')); + mockSearchRaw.mockRejectedValue({ + name: 'ResponseError', + message: 'candidate search failed', + meta: { + statusCode: 500 + } + }); await controller.SEARCH( { @@ -1273,12 +1290,16 @@ describe('Search controller SEARCH handler', () => { error: 'UPSTREAM_SEARCH_ERROR', message: 'Search backend returned an error. Please try again later.' }); + expect(next).not.toHaveBeenCalled(); }); - test('SEARCH does not cache a partial OpenSearch candidate response', async () => { + test.each([ + ['partial', 'partial_shards'], + ['malformed', 'malformed_candidate'] + ])('SEARCH does not cache a %s OpenSearch candidate response', async (_caseName, failureType) => { mockSearchRaw.mockRejectedValue({ code: 'OPEN_SEARCH_RESPONSE_INVALID', - failureType: 'partial_shards' + failureType }); const req = { searchParams: { @@ -1592,6 +1613,7 @@ describe('Search controller SEARCH handler', () => { test.each([ ['ConnectionError', { name: 'ConnectionError' }], ['NoLivingConnectionsError', { name: 'NoLivingConnectionsError' }], + ['search provider configuration error', { code: 'SEARCH_PROVIDER_CONFIGURATION' }], ['cve-core provider contract error', { code: 'SEARCH_PROVIDER_CONTRACT' }], ['OpenSearch 503 response', { name: 'ResponseError', meta: { statusCode: 503 } }] ])('SEARCH returns a retryable 503 when OpenSearch is unavailable through %s', async (_caseName, thrownError) => { @@ -1872,6 +1894,32 @@ describe('Search controller SEARCH handler', () => { outcome: 'upstream_error', statusCode: 502 }); + expect(next).not.toHaveBeenCalled(); + }); + + test('SEARCH delegates unexpected application errors to the sanitized Express error handler', async () => { + const err = new TypeError('sensitive internal implementation details'); + mockApiSearch.mockRejectedValue(err); + + await controller.SEARCH( + { + requestId: 'internal-error-request', + searchParams: {} + }, + res, + next + ); + + expect(next).toHaveBeenCalledWith(err); + expect(res.status).not.toHaveBeenCalled(); + expect(res.json).not.toHaveBeenCalled(); + expect(JSON.parse(console.log.mock.calls[0][0])).toMatchObject({ + event: 'search.performance', + requestId: 'internal-error-request', + outcome: 'internal_error', + statusCode: 500 + }); + expect(console.log.mock.calls[0][0]).not.toContain('sensitive'); }); }); diff --git a/routes/search.integration.test.js b/routes/search.integration.test.js index d4ff51a..9aab62a 100644 --- a/routes/search.integration.test.js +++ b/routes/search.integration.test.js @@ -53,6 +53,10 @@ describe('search route integration', () => { mockBasicSearch.mockReset(); }); + afterEach(() => { + jest.restoreAllMocks(); + }); + test('POST /search validates, builds the OpenSearch request, and formats pagination metadata', async () => { mockSearchOpenSearch.mockResolvedValue(advancedResponse(1, [{ _id: 'CVE-2024-0001' }]).data); @@ -111,6 +115,40 @@ describe('search route integration', () => { expect(response.body.status).toBe('ok'); }); + test('POST /search returns a sanitized 500 for unexpected application errors', async () => { + const internalError = new TypeError('sensitive internal implementation details'); + const performanceLog = jest.spyOn(console, 'log').mockImplementation(() => {}); + const requestErrorLog = jest.spyOn(console, 'error').mockImplementation(() => {}); + mockSearchOpenSearch.mockRejectedValue(internalError); + + const response = await request(createApp(), { + method: 'POST', + path: '/search', + body: { + keywordSearch: 'remote code' + } + }); + + expect(response.status).toBe(500); + expect(response.body).toStrictEqual({ + error: 'INTERNAL_SERVER_ERROR', + message: 'An unexpected server error occurred.' + }); + expect(JSON.parse(performanceLog.mock.calls[0][0])).toMatchObject({ + event: 'search.performance', + outcome: 'internal_error', + statusCode: 500 + }); + expect(JSON.parse(requestErrorLog.mock.calls[0][0])).toMatchObject({ + event: 'request.error', + outcome: 'internal_error', + route: '/search', + statusCode: 500 + }); + expect(performanceLog.mock.calls[0][0]).not.toContain('sensitive'); + expect(requestErrorLog.mock.calls[0][0]).not.toContain('sensitive'); + }); + test('POST /search rejects invalid dates before invoking OpenSearch', async () => { const response = await request(createApp(), { method: 'POST', diff --git a/utils/searchProvider.js b/utils/searchProvider.js index e9244f1..4d8b9eb 100644 --- a/utils/searchProvider.js +++ b/utils/searchProvider.js @@ -1,5 +1,6 @@ const { AppConfig, BasicSearchManager, SearchProviderSpec, SearchReader } = require('cve-core'); const constants = require('./constants'); +const cpe = require('./cpe'); const searchDeadline = require('./searchDeadline'); class SearchProviderContractError extends Error { @@ -10,11 +11,28 @@ class SearchProviderContractError extends Error { } } +class SearchProviderConfigurationError extends Error { + constructor() { + super('OpenSearch providerEndpoint and index must be configured.'); + this.name = 'SearchProviderConfigurationError'; + this.code = 'SEARCH_PROVIDER_CONFIGURATION'; + } +} + const OPEN_SEARCH_RESPONSE_FAILURE_TYPES = Object.freeze({ + MALFORMED_CANDIDATE: 'malformed_candidate', MALFORMED_RESPONSE: 'malformed_response', PARTIAL_SHARDS: 'partial_shards', TIMEOUT: 'timeout' }); +const CPE_CANDIDATE_OPERATION = 'cpe-candidate'; +const CVE_ID_PATTERN = /^CVE-\d{4}-\d{4,19}$/; +const CPE_RANGE_FIELDS = [ + 'versionStartIncluding', + 'versionStartExcluding', + 'versionEndIncluding', + 'versionEndExcluding' +]; class OpenSearchResponseError extends Error { constructor(failureType) { @@ -79,7 +97,7 @@ function normalizeSearchProviderConfig(spec) { const index = spec?.index; if (!providerEndpoint || !index) { - throw new Error('OpenSearch providerEndpoint and index must be configured.'); + throw new SearchProviderConfigurationError(); } return Object.freeze({ @@ -123,7 +141,51 @@ function throwOpenSearchResponseError(failureType) { throw new OpenSearchResponseError(failureType); } -function validateOpenSearchResponse(body) { +function isObject(value) { + return Boolean(value) && typeof value === 'object' && !Array.isArray(value); +} + +function isValidCpeMatch(match) { + if (!isObject(match) || typeof match.criteria !== 'string' || match.criteria.trim() === '') return false; + if (!cpe.parseCpeMatchCriteria(match.criteria).valid) return false; + if (typeof match.vulnerable !== 'boolean') return false; + + return CPE_RANGE_FIELDS.every( + (field) => match[field] === undefined || (typeof match[field] === 'string' && match[field].trim() !== '') + ); +} + +function isValidCpeCandidateHit(hit) { + if (!isObject(hit) || !isObject(hit._source)) return false; + + const cveId = hit._source.cveMetadata?.cveId; + if (typeof cveId !== 'string' || !CVE_ID_PATTERN.test(cveId)) return false; + + const cpeApplicability = hit._source.containers?.cna?.cpeApplicability; + const applicabilityItems = Array.isArray(cpeApplicability) ? cpeApplicability : [cpeApplicability]; + if (!applicabilityItems.length || applicabilityItems.some((item) => !isObject(item))) return false; + + let matchCount = 0; + for (const applicability of applicabilityItems) { + if (!Array.isArray(applicability.nodes)) return false; + + for (const node of applicability.nodes) { + if (!isObject(node) || !Array.isArray(node.cpeMatch)) return false; + if (!node.cpeMatch.every(isValidCpeMatch)) return false; + matchCount += node.cpeMatch.length; + } + } + + return matchCount > 0; +} + +function validateCpeCandidateHits(hits) { + if (hits.every(isValidCpeCandidateHit)) return; + + throwOpenSearchResponseError(OPEN_SEARCH_RESPONSE_FAILURE_TYPES.MALFORMED_CANDIDATE); +} + +function validateOpenSearchResponse(body, { operation } = {}) { if (!body || typeof body !== 'object' || Array.isArray(body)) { throwOpenSearchResponseError(OPEN_SEARCH_RESPONSE_FAILURE_TYPES.MALFORMED_RESPONSE); } @@ -152,6 +214,10 @@ function validateOpenSearchResponse(body) { throwOpenSearchResponseError(OPEN_SEARCH_RESPONSE_FAILURE_TYPES.MALFORMED_RESPONSE); } + if (operation === CPE_CANDIDATE_OPERATION) { + validateCpeCandidateHits(body.hits.hits); + } + return body; } @@ -290,7 +356,7 @@ async function searchOpenSearch(body, { signal, operation = 'search', onStart, r } ); - return validateOpenSearchResponse(response.body); + return validateOpenSearchResponse(response.body, { operation }); } async function checkOpenSearchReadiness({ signal } = {}) { @@ -330,6 +396,10 @@ function isSearchProviderContractError(err) { return err instanceof SearchProviderContractError || err?.code === 'SEARCH_PROVIDER_CONTRACT'; } +function isSearchProviderConfigurationError(err) { + return err instanceof SearchProviderConfigurationError || err?.code === 'SEARCH_PROVIDER_CONFIGURATION'; +} + function isOpenSearchResponseError(err) { return err instanceof OpenSearchResponseError || err?.code === 'OPEN_SEARCH_RESPONSE_INVALID'; } @@ -432,6 +502,7 @@ module.exports = { getSearchProviderConfig, getSearchReader, isOpenSearchResponseError, + isSearchProviderConfigurationError, isSearchProviderContractError, normalizeBoolean, resetSearchProviderForTests, @@ -441,5 +512,6 @@ module.exports = { validateSearchReader, OPEN_SEARCH_RESPONSE_FAILURE_TYPES, OpenSearchResponseError, + SearchProviderConfigurationError, SearchProviderContractError }; diff --git a/utils/searchProvider.test.js b/utils/searchProvider.test.js index 159aac2..8abc85d 100644 --- a/utils/searchProvider.test.js +++ b/utils/searchProvider.test.js @@ -54,6 +54,36 @@ function deferred() { return state; } +function cpeCandidateHit(cpeMatch = {}) { + return { + _id: 'CVE-2024-0001', + _source: { + cveMetadata: { + cveId: 'CVE-2024-0001' + }, + containers: { + cna: { + cpeApplicability: [ + { + nodes: [ + { + cpeMatch: [ + { + criteria: 'cpe:2.3:a:vendor:product:*:*:*:*:*:*:*:*', + vulnerable: true, + ...cpeMatch + } + ] + } + ] + } + ] + } + } + } + }; +} + describe('searchProvider', () => { beforeEach(() => { searchProvider.resetSearchProviderForTests(); @@ -297,6 +327,103 @@ describe('searchProvider', () => { }); }); + test('accepts schema-shaped CPE candidate hits for candidate operations', async () => { + const body = { + hits: { + hits: [cpeCandidateHit()] + } + }; + mockClientSearch.mockResolvedValue({ body }); + + await expect( + searchProvider.searchOpenSearch( + { + query: { + match_all: {} + } + }, + { + operation: 'cpe-candidate' + } + ) + ).resolves.toBe(body); + }); + + test('accepts an empty CPE candidate result', async () => { + const body = { + hits: { + hits: [] + } + }; + mockClientSearch.mockResolvedValue({ body }); + + await expect( + searchProvider.searchOpenSearch( + { + query: { + match_all: {} + } + }, + { + operation: 'cpe-candidate' + } + ) + ).resolves.toBe(body); + }); + + test.each([ + [ + 'missing projected CVE ID', + { + ...cpeCandidateHit(), + _source: { + ...cpeCandidateHit()._source, + cveMetadata: {} + } + } + ], + [ + 'missing projected CPE applicability', + { + ...cpeCandidateHit(), + _source: { + ...cpeCandidateHit()._source, + containers: { + cna: {} + } + } + } + ], + ['missing CPE criteria', cpeCandidateHit({ criteria: undefined })], + ['invalid CPE criteria', cpeCandidateHit({ criteria: 'not-a-cpe-name' })], + ['missing CPE vulnerability flag', cpeCandidateHit({ vulnerable: undefined })], + ['invalid CPE range boundary', cpeCandidateHit({ versionEndExcluding: 2 })] + ])('rejects CPE candidate hits with %s', async (_caseName, hit) => { + mockClientSearch.mockResolvedValue({ + body: { + hits: { + hits: [hit] + } + } + }); + + await expect( + searchProvider.searchOpenSearch( + { + query: { + match_all: {} + } + }, + { + operation: 'cpe-candidate' + } + ) + ).rejects.toMatchObject({ + code: 'OPEN_SEARCH_RESPONSE_INVALID', + failureType: 'malformed_candidate' + }); + }); + test('checks OpenSearch readiness through the configured CVE index', async () => { mockClientSearch.mockResolvedValue({ body: { @@ -513,9 +640,7 @@ describe('searchProvider', () => { index: 'test-index' }); - expect(() => searchProvider.getSearchProviderConfig()).toThrow( - 'OpenSearch providerEndpoint and index must be configured.' - ); + expect(() => searchProvider.getSearchProviderConfig()).toThrow(searchProvider.SearchProviderConfigurationError); }); test('normalizes string FALSE from AppConfig before constructing managers', () => { From 239942633f1b27b6ada50079a7a9270d41ec40b0 Mon Sep 17 00:00:00 2001 From: Andrew Foote Date: Thu, 30 Jul 2026 15:38:45 -0400 Subject: [PATCH 5/6] Handle oversized JSON requests explicitly --- README.md | 2 +- api-docs/openapi.json | 11 +++++++++++ api-docs/openapi.test.js | 2 ++ routes/index.js | 11 +++++++++++ server.js | 7 ++++++- server.test.js | 23 +++++++++++++++++++++++ utils/constants.js | 4 ++++ utils/errors.js | 7 +++++++ utils/errors.test.js | 8 ++++++++ 9 files changed, 73 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index cf7fa7e..9993afd 100644 --- a/README.md +++ b/README.md @@ -106,7 +106,7 @@ Shutdown lifecycle events are emitted as sanitized `runtime.lifecycle` JSON logs ## API Input Validation -`POST /search` accepts an optional JSON object request body and validates supported fields, boolean parameters, pagination values, CVSS severity values, required parameter combinations, CPE Name requirements, and date-time formats. A non-empty body must use the `application/json` media type with UTF-8 encoding; unsupported media types or encodings return HTTP 415, and malformed JSON returns HTTP 400. If OpenSearch is unavailable, the endpoint returns HTTP 503 with a `Retry-After` header. +`POST /search` accepts an optional JSON object request body and validates supported fields, boolean parameters, pagination values, CVSS severity values, required parameter combinations, CPE Name requirements, and date-time formats. The JSON request body cannot exceed 100 KB; larger bodies return HTTP 413 before routing or OpenSearch access. A non-empty body must use the `application/json` media type with UTF-8 encoding; unsupported media types or encodings return HTTP 415, and malformed JSON returns HTTP 400. If OpenSearch is unavailable, the endpoint returns HTTP 503 with a `Retry-After` header. String inputs are bounded before query construction: diff --git a/api-docs/openapi.json b/api-docs/openapi.json index 3d69e9e..1b67784 100644 --- a/api-docs/openapi.json +++ b/api-docs/openapi.json @@ -69,6 +69,16 @@ } } }, + "413": { + "description": "The JSON request body exceeded the 100 KB limit", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ErrorResponse" + } + } + } + }, "415": { "description": "A non-empty request body did not use the application/json media type with UTF-8 encoding", "content": { @@ -160,6 +170,7 @@ }, "requestBody": { "required": false, + "description": "Optional JSON filters and pagination controls. The JSON request body cannot exceed 100 KB.", "content": { "application/json": { "schema": { diff --git a/api-docs/openapi.test.js b/api-docs/openapi.test.js index 3bee554..f7c9d24 100644 --- a/api-docs/openapi.test.js +++ b/api-docs/openapi.test.js @@ -63,7 +63,9 @@ describe('OpenAPI contract', () => { const mediaType = operation.requestBody.content['application/json']; expect(operation.parameters || []).toStrictEqual([]); + expect(operation.requestBody.description).toContain(`${constants.JSON_REQUEST_BODY_LIMIT_KB} KB`); expect(mediaType.schema).toStrictEqual({ $ref: '#/components/schemas/SearchRequest' }); + expect(operation.responses['413'].description).toContain(`${constants.JSON_REQUEST_BODY_LIMIT_KB} KB`); expect(schema.additionalProperties).toBe(false); expect(Object.keys(schema.properties)).toStrictEqual(constants.SEARCH_QUERY_PARAMS); Object.values(schema.properties).forEach((property) => { diff --git a/routes/index.js b/routes/index.js index 7a2ff47..ff71317 100644 --- a/routes/index.js +++ b/routes/index.js @@ -172,6 +172,7 @@ router.post(

Retrieves all matching CVE records after applying the JSON request body as filters

#swagger.requestBody = { required: false, + description: "Optional JSON filters and pagination controls. The JSON request body cannot exceed 100 KB.", content: { "application/json": { schema: { @@ -295,6 +296,16 @@ content: { } } } +#swagger.responses[413] = { +description: 'The JSON request body exceeded the 100 KB limit', +content: { + "application/json": { + schema: { + $ref: "#/components/schemas/ErrorResponse" + } + } +} +} #swagger.responses[429] = { description: 'Application rate limit exceeded, the cold CPE search queue is full or exceeded its maximum wait, or a shared semantic search operation reached its waiter limit', headers: { diff --git a/server.js b/server.js index a388c54..1340374 100644 --- a/server.js +++ b/server.js @@ -6,6 +6,7 @@ const openApiSpecification = require('./api-docs/openapi.json'); const http = require('http'); const logger = require('morgan'); const router = require('./routes/index'); +const constants = require('./utils/constants'); const errors = require('./utils/errors'); const requestLogging = require('./utils/requestLogging'); const runtimeLifecycle = require('./utils/runtimeLifecycle'); @@ -41,6 +42,10 @@ function createRequestLogger() { } function handleRequestError(err, req, res, next) { + if (err?.type === 'entity.too.large' && err?.status === 413) { + return res.status(413).json(error.requestBodyTooLargeError()); + } + if (err?.type === 'entity.parse.failed') { return res.status(400).json(error.malformedJsonError()); } @@ -97,7 +102,7 @@ function createApp() { app.use(cors()); app.use(runtimeLifecycle.trackRequest); app.use(rejectRequestsDuringShutdown); - app.use(express.json()); + app.use(express.json({ limit: constants.JSON_REQUEST_BODY_LIMIT_BYTES })); app.get('/api-docs/openapi.json', (req, res) => res.json(openApiSpecification)); app.use('/api-docs', swaggerUi.serve, swaggerUi.setup(openApiSpecification, swaggerSetupOptions)); diff --git a/server.test.js b/server.test.js index bb829df..61248a8 100644 --- a/server.test.js +++ b/server.test.js @@ -1,6 +1,7 @@ const http = require('http'); const { EventEmitter } = require('events'); const { request } = require('./test/http'); +const constants = require('./utils/constants'); const mockValidateParams = jest.fn((req, res, next) => next()); const mockSearch = jest.fn((req, res) => @@ -199,6 +200,28 @@ describe('server application bootstrap', () => { expect(mockSearch).not.toHaveBeenCalled(); }); + test('createApp returns a sanitized error for oversized JSON before routing', async () => { + const oversizedValue = 'x'.repeat(constants.JSON_REQUEST_BODY_LIMIT_BYTES); + const response = await request(createApp(), { + method: 'POST', + path: '/search', + body: { + keywordSearch: oversizedValue + } + }); + + expect(response.status).toBe(413); + expect(response.body).toStrictEqual({ + error: 'REQUEST_BODY_TOO_LARGE', + message: `The JSON request body cannot exceed ${constants.JSON_REQUEST_BODY_LIMIT_KB} KB.` + }); + expect(response.text).not.toContain(oversizedValue); + expect(response.text).not.toContain('entity.too.large'); + expect(response.headers['access-control-allow-origin']).toBe('*'); + expect(mockValidateParams).not.toHaveBeenCalled(); + expect(mockSearch).not.toHaveBeenCalled(); + }); + test('createApp returns a sanitized error for unsupported JSON encoding', async () => { const response = await request(createApp(), { method: 'POST', diff --git a/utils/constants.js b/utils/constants.js index 6b3c979..0093369 100644 --- a/utils/constants.js +++ b/utils/constants.js @@ -7,6 +7,8 @@ const MAX_CVSS_METRIC_TOKEN_LENGTH = 32; const MAX_CVE_IDS = 100; const MAX_CVE_ID_LENGTH = 64; const MAX_KEYWORD_SEARCH_TOKENS = 64; +const JSON_REQUEST_BODY_LIMIT_KB = 100; +const JSON_REQUEST_BODY_LIMIT_BYTES = JSON_REQUEST_BODY_LIMIT_KB * 1024; const SEARCH_STRING_LENGTH_LIMITS = Object.freeze({ keywordSearch: 512, cveIds: 4096, @@ -132,6 +134,8 @@ module.exports = { MAX_CVE_IDS, MAX_CVE_ID_LENGTH, MAX_KEYWORD_SEARCH_TOKENS, + JSON_REQUEST_BODY_LIMIT_KB, + JSON_REQUEST_BODY_LIMIT_BYTES, SEARCH_STRING_LENGTH_LIMITS, SEARCH_POST_FILTER_CACHE_TTL_MS, SEARCH_POST_FILTER_CACHE_MAX_ENTRIES, diff --git a/utils/errors.js b/utils/errors.js index 6b46440..d001e55 100644 --- a/utils/errors.js +++ b/utils/errors.js @@ -139,6 +139,13 @@ class errors { return makeError('MALFORMED_JSON', 'The request body contains malformed JSON.'); } + requestBodyTooLargeError() { + return makeError( + 'REQUEST_BODY_TOO_LARGE', + `The JSON request body cannot exceed ${constants.JSON_REQUEST_BODY_LIMIT_KB} KB.` + ); + } + searchQueryUnsupportedError() { return makeError( 'SEARCH_QUERY_UNSUPPORTED', diff --git a/utils/errors.test.js b/utils/errors.test.js index 3d09f08..b40efc8 100644 --- a/utils/errors.test.js +++ b/utils/errors.test.js @@ -263,6 +263,14 @@ describe('error response helpers', () => { message: 'The request body contains malformed JSON.' } ], + [ + 'requestBodyTooLargeError', + [], + { + error: 'REQUEST_BODY_TOO_LARGE', + message: `The JSON request body cannot exceed ${constants.JSON_REQUEST_BODY_LIMIT_KB} KB.` + } + ], [ 'searchQueryUnsupportedError', [], From b0b93dae43789da55f0f9776fd5e9665d73f6b3a Mon Sep 17 00:00:00 2001 From: Andrew Foote Date: Thu, 30 Jul 2026 15:43:03 -0400 Subject: [PATCH 6/6] Prevent readiness success during shutdown --- README.md | 2 +- api-docs/openapi.json | 2 +- api-docs/openapi.test.js | 3 +++ routes/index.js | 17 ++++++++++++----- routes/index.test.js | 35 +++++++++++++++++++++++++++++++++++ 5 files changed, 52 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 9993afd..fb9c40d 100644 --- a/README.md +++ b/README.md @@ -92,7 +92,7 @@ The Search API will be served at `localhost:3000`. `GET /api/health-check` is a process liveness check. It returns HTTP 200 when the Node.js process is running and deliberately does not contact OpenSearch. Use it where a dependency outage should not restart otherwise healthy API tasks. -`GET /api/ready-check` sends a minimal zero-result search to the configured CVE index. This verifies connectivity, index availability, and the same search permission used by the API without retrieving CVE records or counting all matches. It returns HTTP 200 when search requests can be served. Configuration errors, connection failures, timeouts, incompatible `cve-core` SearchReader contracts, a missing index, and an in-progress process shutdown return a sanitized HTTP 503 `SEARCH_NOT_READY` response with `Retry-After: 5`. The OpenSearch request is limited to five seconds by default and can be changed with `SearchReadinessTimeoutMs`. Once shutdown starts, readiness fails immediately without contacting OpenSearch. +`GET /api/ready-check` sends a minimal zero-result search to the configured CVE index. This verifies connectivity, index availability, and the same search permission used by the API without retrieving CVE records or counting all matches. It returns HTTP 200 only when search requests can still be served after the dependency check completes. Configuration errors, connection failures, timeouts, incompatible `cve-core` SearchReader contracts, a missing index, and an in-progress process shutdown return a sanitized HTTP 503 `SEARCH_NOT_READY` response with `Retry-After: 5`. The OpenSearch request is limited to five seconds by default and can be changed with `SearchReadinessTimeoutMs`. If shutdown is already active, readiness fails immediately without contacting OpenSearch. If shutdown begins while the dependency check is pending, the completed check is not reported as ready. An AWS load balancer can use `/api/ready-check` to stop routing search traffic to an unready task while the container orchestrator continues using `/api/health-check` for process liveness. Coordinate that probe selection with the infrastructure team before changing deployed health-check behavior. diff --git a/api-docs/openapi.json b/api-docs/openapi.json index 1b67784..6b1f60e 100644 --- a/api-docs/openapi.json +++ b/api-docs/openapi.json @@ -264,7 +264,7 @@ "/api/ready-check": { "get": { "summary": "OpenSearch readiness check", - "description": "Sends a minimal zero-result search to the configured CVE index. Returns 200 when the index is available with search permission, or a sanitized 503 response when configuration, connectivity, timeout, permission, or index availability checks fail.", + "description": "Sends a minimal zero-result search to the configured CVE index. Returns 200 only when the index is available with search permission and the process is still accepting traffic after the dependency check. Returns a sanitized 503 response when shutdown is already active, shutdown begins while the dependency check is pending, or configuration, connectivity, timeout, permission, or index availability checks fail.", "operationId": "readinessCheck", "responses": { "200": { diff --git a/api-docs/openapi.test.js b/api-docs/openapi.test.js index f7c9d24..4f7e846 100644 --- a/api-docs/openapi.test.js +++ b/api-docs/openapi.test.js @@ -29,6 +29,9 @@ describe('OpenAPI contract', () => { expect(openapi.paths['/search'].post.operationId).toBe('search'); expect(openapi.paths['/api/health-check'].get.operationId).toBe('healthCheck'); expect(openapi.paths['/api/ready-check'].get.operationId).toBe('readinessCheck'); + expect(openapi.paths['/api/ready-check'].get.description).toContain( + 'shutdown begins while the dependency check is pending' + ); expect(openapi.paths['/webSearch'].post.operationId).toBe('webSearch'); }); diff --git a/routes/index.js b/routes/index.js index ff71317..2d30dbc 100644 --- a/routes/index.js +++ b/routes/index.js @@ -24,10 +24,14 @@ function healthCheck(req, res) { res.status(200).send(); } +function sendSearchNotReady(res) { + if (typeof res.set === 'function') res.set('Retry-After', '5'); + return res.status(503).json(error.searchNotReadyError()); +} + async function readinessCheck(req, res) { if (runtimeLifecycle.isShuttingDown()) { - if (typeof res.set === 'function') res.set('Retry-After', '5'); - return res.status(503).json(error.searchNotReadyError()); + return sendSearchNotReady(res); } const lifecycle = searchDeadline.createSearchRequestLifecycle( @@ -42,12 +46,15 @@ async function readinessCheck(req, res) { }); lifecycle.throwIfAborted(); + if (runtimeLifecycle.isShuttingDown()) { + return sendSearchNotReady(res); + } + return res.status(200).send(); } catch (err) { if (searchDeadline.isSearchClientDisconnectedError(err)) return; - if (typeof res.set === 'function') res.set('Retry-After', '5'); - return res.status(503).json(error.searchNotReadyError()); + return sendSearchNotReady(res); } finally { lifecycle.complete(); } @@ -446,7 +453,7 @@ router.get( /* #swagger.operationId = 'readinessCheck' #swagger.summary = "OpenSearch readiness check" -#swagger.description = "Sends a minimal zero-result search to the configured CVE index. Returns 200 when the index is available with search permission, or a sanitized 503 response when configuration, connectivity, timeout, permission, or index availability checks fail." +#swagger.description = "Sends a minimal zero-result search to the configured CVE index. Returns 200 only when the index is available with search permission and the process is still accepting traffic after the dependency check. Returns a sanitized 503 response when shutdown is already active, shutdown begins while the dependency check is pending, or configuration, connectivity, timeout, permission, or index availability checks fail." #swagger.responses[200] = { description: 'API is ready to serve search requests', headers: { diff --git a/routes/index.test.js b/routes/index.test.js index 9b7aff1..5aa1d56 100644 --- a/routes/index.test.js +++ b/routes/index.test.js @@ -41,6 +41,15 @@ const featureFlags = require('../utils/featureFlags'); const runtimeLifecycle = require('../utils/runtimeLifecycle'); const router = require('./index'); +function deferred() { + const state = {}; + state.promise = new Promise((resolve, reject) => { + state.resolve = resolve; + state.reject = reject; + }); + return state; +} + describe('routes/index router', () => { let app; @@ -107,6 +116,32 @@ describe('routes/index router', () => { expect(mockCheckOpenSearchReadiness).not.toHaveBeenCalled(); }); + test('GET /api/ready-check returns 503 when shutdown begins during the OpenSearch check', async () => { + const readinessStarted = deferred(); + const readinessResult = deferred(); + mockCheckOpenSearchReadiness.mockImplementation(() => { + readinessStarted.resolve(); + return readinessResult.promise; + }); + + const responsePromise = request(app, { + path: '/api/ready-check' + }); + await readinessStarted.promise; + runtimeLifecycle.beginShutdown(); + readinessResult.resolve(true); + + const response = await responsePromise; + + expect(response.status).toBe(503); + expect(response.headers['retry-after']).toBe('5'); + expect(response.body).toStrictEqual({ + error: 'SEARCH_NOT_READY', + message: 'Search service is not ready. Please try again later.' + }); + expect(mockCheckOpenSearchReadiness).toHaveBeenCalledTimes(1); + }); + test('POST /search reaches middleware and controller for valid JSON body fields', async () => { const response = await request(app, { method: 'POST',