diff --git a/.env.example b/.env.example index a90f4b1..a6ac790 100644 --- a/.env.example +++ b/.env.example @@ -2,19 +2,10 @@ PORT=3000 OpenSearchCveIndex=cve-index-local OpenSearchAllowUnknownSslCerts=false OpenSearchDomainEndpoint= -SearchLogPerformanceTimings=false -SearchPostFilterCacheTtlMs=60000 -SearchPostFilterCacheMaxEntries=100 -SearchPostFilterColdSearchConcurrency=10 -SearchPostFilterColdSearchMaxQueueDepth=500 -SearchPostFilterColdSearchMaxWaitMs=10000 SearchOpenSearchRequestTimeoutMs=20000 SearchRequestDeadlineMs=30000 SearchReadinessTimeoutMs=5000 RequestLogSuccessSampleRate=0.1 RequestLogHealthChecks=false WebSearchEnabled=true -SearchRateLimitEnabled=false -SearchRateLimitWindowMs=60000 -SearchRateLimitMaxRequests=0 NPM_CA_CERT= diff --git a/README.md b/README.md index b1f5cd0..c3e01e7 100644 --- a/README.md +++ b/README.md @@ -8,21 +8,12 @@ Rename `.env.example` to `.env` and replace or check the following values. - `OpenSearchCveIndex` - existing CVE index name in your OpenSearch deployment - `OpenSearchAllowUnknownSslCerts` - set to `true` only for local development against self-signed certificates - `OpenSearchDomainEndpoint` - absolute URL to an existing OpenSearch deployment. This API does not create or manage the OpenSearch index or mapping. -- `SearchLogPerformanceTimings` - optional diagnostic flag; set to `true` temporarily to log redacted `/search` timing breakdowns -- `SearchPostFilterCacheTtlMs` - optional CPE post-filter cache TTL override in milliseconds, default `60000` -- `SearchPostFilterCacheMaxEntries` - optional per-process CPE post-filter cache entry limit, default `100` -- `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` - `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` - `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` -- `SearchRateLimitMaxRequests` - required positive per-process request limit when rate limiting is enabled - `NPM_CA_CERT` - optional absolute host path to a corporate or self-signed CA certificate bundle used only during local Docker Compose builds ```bash @@ -275,68 +266,21 @@ An importable Postman collection for manual `/search` contract and filter testin ### 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. +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. Access logs omit query strings and request bodies. By default, the API retains 10% of successful non-health-check access logs, suppresses successful `/api/health-check` and `/api/ready-check` access logs, and retains every HTTP 4xx or 5xx access log. Set `RequestLogSuccessSampleRate` from `0` through `1` to change successful-request sampling. Set `RequestLogHealthChecks=true` to retain successful health-check logs. -The API always emits a sanitized JSON `search.performance` event for `/search` queue saturation, queue wait timeout, search timeout, client disconnect, OpenSearch connection errors, and upstream search errors. These events include the request ID, outcome, HTTP status, strategy, cache state, total elapsed time, cancellation reason, and queue state when applicable. Search terms, CPE values, request bodies, credentials, and upstream error details are never included. - -Set `SearchLogPerformanceTimings=true` on the API deployment when diagnosing slow `/search` requests. Successful requests then emit the same structured event with additional redacted timing details, including candidate counts, filtered counts, returned count, OpenSearch operation types, maximum observed active OpenSearch requests in the process, OpenSearch elapsed time, OpenSearch `took`, post-filter elapsed time, cold-search queue wait time, page fetch elapsed or wait time, and total handler elapsed time. Leave it disabled during normal operation unless the deployment logging volume is acceptable. - Each `/search` request owns a lifecycle cancellation signal. When its overall deadline expires, the API aborts the active OpenSearch transport request before returning HTTP 504. When the caller disconnects, the API aborts work that is needed only by that caller and does not attempt to write a response. This prevents timed-out or abandoned standard searches from continuing until the transport timeout. -### Application Rate Limiting - -Application rate limiting is disabled by default and applies only to `POST /search`. It does not apply to `/webSearch`, health checks, Swagger, OpenAPI, or static assets. - -The fallback limiter uses one in-memory counter per Node.js process. Set `SearchRateLimitEnabled=true`, provide a positive `SearchRateLimitMaxRequests`, and optionally change `SearchRateLimitWindowMs` to enable it. Invalid enabled configuration prevents the application from starting. Rejected requests return HTTP 429 with `SEARCH_RATE_LIMITED`, `Retry-After`, `RateLimit`, and `RateLimit-Policy` headers, and emit a sanitized `search.performance` event with the `rate_limited` outcome. - -This is a process-wide safety limit, not a precise distributed quota. Each ECS task has an independent counter, so total permitted traffic scales with the number of tasks and load-balancer routing. It deliberately does not use caller IP addresses or forwarded headers until the ALB proxy chain and Express `trust proxy` setting are confirmed. AWS WAF or another infrastructure-level distributed limit remains the preferred public-facing control. - ### Semantic Post-Filtering -CVSS vector filters, SSVC filters, and date-bounded positive `knownExploited` searches use semantic post-filtering because OpenSearch ordinary-object arrays cannot guarantee that multiple conditions matched the same vector or structured metric entry. These searches retrieve up to 10,000 lightweight candidates, apply same-entry checks in Node.js, and fetch the requested page as complete CVE records. Every token supplied to one CVSS vector filter must occur in the same eligible vector. The reported total and pagination are complete only within that bounded candidate set; qualifying records beyond the first 10,000 OpenSearch candidates may not be represented. This preserves the existing bounded post-filter behavior until PIT-based pagination is implemented. CVSS, SSVC, and KEV candidates do not use the temporary CPE cache or cold-search queue. A `knownExploited` filter without date boundaries uses a direct OpenSearch query and retains ordinary exact-total behavior. - -### CPE Post-Filter Cache - -Searches that use `cpeName`, `virtualMatchString`, or `virtualMatchString` with version range parameters use a semantic CPE post-filter. The API first asks OpenSearch for up to 10,000 lightweight candidate records, filters those candidates in Node.js, then fetches the requested page of full CVE records by CVE ID. - -The post-filter cache stores only the filtered CVE ID list and candidate count for that expensive candidate query. It does not cache full CVE records. On a cache hit, the API skips the 10,000-candidate OpenSearch query and semantic post-filtering, slices the cached CVE IDs for the requested page, and fetches that page of full CVE records by ID. - -The cache is intentionally small and short-lived: - -- TTL: 60 seconds -- Max entries: 100 -- Scope: one Node.js process -- Stored value: filtered CVE IDs and candidate count -- In-flight behavior: concurrent identical cold searches share one reference-counted candidate query per process -- Cold-search concurrency: cold CPE candidate queries are limited to 10 per process before reaching OpenSearch -- Cold-search queue depth: up to 500 unique cold searches can wait per process behind the concurrency limit -- Cold-search queue wait: up to 10 seconds before a waiting search returns HTTP 429 -- OpenSearch request timeout: 20 seconds for each operation -- Overall `/search` deadline: 30 seconds -- Eviction: oldest entries are removed when the cache exceeds its max size - -When the cold-search queue is full, new unique CPE post-filter searches fail closed with HTTP 429 and a `SEARCH_QUEUE_FULL` response. Identical requests for a search that is already in flight still wait on the shared in-flight result and do not consume additional queue slots. - -When a cold search waits in the queue for more than 10 seconds, it fails closed with HTTP 429 and a `SEARCH_QUEUE_TIMEOUT` response. OpenSearch operations use one attempt and return a sanitized HTTP 504 `SEARCH_TIMEOUT` response when they exceed 20 seconds. The same response is returned when a `/search` request exceeds the 30-second overall processing deadline, and the active OpenSearch request is aborted. - -Healthy dev timing logs showed the impact for repeated identical `virtualMatchString` range searches: +CVSS vector filters, SSVC filters, date-bounded positive `knownExploited` searches, `cpeName`, and `virtualMatchString` searches use semantic post-filtering where OpenSearch cannot guarantee the required same-entry or version-comparison behavior. These searches retrieve up to 10,000 lightweight candidates, apply semantic checks in Node.js, paginate the matching CVE IDs, and fetch the requested page as complete CVE records. Every token supplied to one CVSS vector filter must occur in the same eligible vector. -| Metric | Cold Cache | Warm Cache | -| ----------------------------- | ---------: | -----------------------: | -| Avg total handler time | 1709.34 ms | 11.72 ms | -| Median total handler time | 1701.51 ms | 7.68 ms | -| p95 total handler time | 2010.52 ms | 50.66 ms | -| Avg candidate OpenSearch time | 1435.57 ms | skipped | -| Avg post-filter time | 169.21 ms | skipped | -| Avg page fetch time | 104.25 ms | 11.53 ms | -| Avg candidates scanned | 10000 | cached from prior result | -| Avg filtered CVEs | 4 | 4 | +The reported total and pagination are complete only within that bounded candidate set; qualifying records beyond the first 10,000 OpenSearch candidates may not be represented. This preserves the existing bounded post-filter behavior until PIT-based pagination is implemented. A `knownExploited` filter without date boundaries uses a direct OpenSearch query and retains ordinary exact-total behavior. -This helps most when multiple users submit the same expensive CPE search within the TTL. The first request is a cold cache miss; later identical requests on the same running task are warm cache hits. If identical cold requests arrive concurrently before the first one finishes, the later requests wait on the same candidate query instead of starting duplicate 10,000-candidate OpenSearch requests. Disconnecting one waiter does not cancel work still needed by another waiter; the shared operation is aborted when its final waiter disconnects or times out. Each active request still fetches its requested page of full CVE records after the shared candidate result is available. +The API does not cache, coalesce, queue, or rate-limit searches in process. Each request executes its own required OpenSearch operations. Public rate enforcement, admission control, shared caching, and performance telemetry must be supplied by the deployment infrastructure when those capabilities are enabled. -The cache does not share entries across ECS tasks. With multiple tasks, each task warms independently depending on load balancer routing. Steady repeated traffic can warm every active task, but unique one-off searches still pay the cold-cache cost. The request lifecycle and in-flight waiter registry are separate from this result cache, so the cache can later be replaced with Redis or ElastiCache without changing cancellation behavior. The per-process TTL, entry limit, cold-search concurrency, queue depth, queue wait, OpenSearch timeout, and overall request deadline can be tuned with `SearchPostFilterCacheTtlMs`, `SearchPostFilterCacheMaxEntries`, `SearchPostFilterColdSearchConcurrency`, `SearchPostFilterColdSearchMaxQueueDepth`, `SearchPostFilterColdSearchMaxWaitMs`, `SearchOpenSearchRequestTimeoutMs`, and `SearchRequestDeadlineMs`. +OpenSearch operations use one attempt and return a sanitized HTTP 504 `SEARCH_TIMEOUT` response when they exceed the 20-second default timeout. The same response is returned when a `/search` request exceeds the 30-second default overall deadline, and the active OpenSearch request is aborted. Docker runtime smoke checks can be run after building the image. The default `Dockerfile` avoids BuildKit `RUN --mount` syntax for AWS builders that do not support it: diff --git a/api-docs/openapi.json b/api-docs/openapi.json index a23c2a8..c1f1f50 100644 --- a/api-docs/openapi.json +++ b/api-docs/openapi.json @@ -44,29 +44,6 @@ "415": { "description": "A non-empty request body did not use the application/json media type with UTF-8 encoding" }, - "429": { - "description": "Application rate limit exceeded, or the cold CPE search queue is full or exceeded its maximum wait", - "headers": { - "Retry-After": { - "description": "Seconds until a rate-limited request can be retried", - "schema": { - "type": "integer" - } - }, - "RateLimit": { - "description": "Current application rate-limit state when application limiting is enabled", - "schema": { - "type": "string" - } - }, - "RateLimit-Policy": { - "description": "Application rate-limit policy when application limiting is enabled", - "schema": { - "type": "string" - } - } - } - }, "503": { "description": "OpenSearch is unavailable", "headers": { diff --git a/api-docs/openapi.test.js b/api-docs/openapi.test.js index af7f573..8adb8d3 100644 --- a/api-docs/openapi.test.js +++ b/api-docs/openapi.test.js @@ -155,7 +155,7 @@ describe('OpenAPI contract', () => { expect(cpeName.description).toContain('virtualMatchString'); expect(cpeName.example).toBe('cpe:2.3:a:freetype:freetype:2.8.1:*:*:*:*:*:*:*'); expect(openapi.paths['/search'].post.responses['404'].description).toBe('Invalid cpeName value'); - expect(openapi.paths['/search'].post.responses['429'].description).toContain('rate limit'); + expect(openapi.paths['/search'].post.responses['429']).toBeUndefined(); }); test('documents fail-closed request and dependency responses for POST /search', () => { diff --git a/config/custom-environment-variables.jsonc b/config/custom-environment-variables.jsonc index 3470a97..4eb1457 100644 --- a/config/custom-environment-variables.jsonc +++ b/config/custom-environment-variables.jsonc @@ -11,12 +11,6 @@ "index": "OpenSearchCveIndex", // allows local development using containers that do not have SSL certs "allowUnknownSslCerts": "OpenSearchAllowUnknownSslCerts", - "logPerformanceTimings": "SearchLogPerformanceTimings", - "postFilterCacheTtlMs": "SearchPostFilterCacheTtlMs", - "postFilterCacheMaxEntries": "SearchPostFilterCacheMaxEntries", - "postFilterColdSearchConcurrency": "SearchPostFilterColdSearchConcurrency", - "postFilterColdSearchMaxQueueDepth": "SearchPostFilterColdSearchMaxQueueDepth", - "postFilterColdSearchMaxWaitMs": "SearchPostFilterColdSearchMaxWaitMs", "openSearchRequestTimeoutMs": "SearchOpenSearchRequestTimeoutMs", "requestDeadlineMs": "SearchRequestDeadlineMs", "readinessTimeoutMs": "SearchReadinessTimeoutMs" @@ -28,13 +22,6 @@ "features": { "webSearchEnabled": "WebSearchEnabled" }, - "rateLimiting": { - "search": { - "enabled": "SearchRateLimitEnabled", - "windowMs": "SearchRateLimitWindowMs", - "maxRequests": "SearchRateLimitMaxRequests" - } - }, // constants for testing node-config // these values are only used to test node-config in AppConfig.test.int.ts // DO NOT USE THIS FOR ANYTHING ELSE diff --git a/config/default.jsonc b/config/default.jsonc index 8cdef78..8759707 100644 --- a/config/default.jsonc +++ b/config/default.jsonc @@ -14,17 +14,6 @@ // The only time this should be allowed to be true is when developing or testing // using containers that do not have SSL certs "allowUnknownSslCerts": "FALSE", - // set to TRUE temporarily when diagnosing slow /search requests - "logPerformanceTimings": "FALSE", - // CPE post-filter candidate result cache. These values are per Node.js process. - "postFilterCacheTtlMs": "60000", - "postFilterCacheMaxEntries": "100", - // Limits concurrent cold CPE candidate OpenSearch requests per Node.js process. - "postFilterColdSearchConcurrency": "10", - // Limits queued cold CPE candidate searches waiting behind the concurrency cap per Node.js process. - "postFilterColdSearchMaxQueueDepth": "500", - // Limits how long a cold CPE candidate search can wait in the per-process queue. - "postFilterColdSearchMaxWaitMs": "10000", // Limits each request from the API process to OpenSearch. "openSearchRequestTimeoutMs": "20000", // Limits total processing time for a /search request. @@ -43,16 +32,6 @@ "features": { // Keep enabled until existing consumers have migrated to POST /search. "webSearchEnabled": "TRUE" - }, - // Application-level fallback protection. Infrastructure-level enforcement remains preferred. - "rateLimiting": { - "search": { - // Disabled until a per-task threshold is approved. - "enabled": "FALSE", - "windowMs": "60000", - // A positive value is required when enabled. - "maxRequests": "0" - } } } } diff --git a/config/runtime-config.test.js b/config/runtime-config.test.js index 350a6b1..c65066b 100644 --- a/config/runtime-config.test.js +++ b/config/runtime-config.test.js @@ -86,18 +86,6 @@ describe('runtime container and compose configuration', () => { expect(customEnvironmentVariables).toContain('"providerEndpoint": "OpenSearchDomainEndpoint"'); expect(customEnvironmentVariables).toContain('"index": "OpenSearchCveIndex"'); expect(customEnvironmentVariables).toContain('"allowUnknownSslCerts": "OpenSearchAllowUnknownSslCerts"'); - expect(customEnvironmentVariables).toContain('"logPerformanceTimings": "SearchLogPerformanceTimings"'); - expect(customEnvironmentVariables).toContain('"postFilterCacheTtlMs": "SearchPostFilterCacheTtlMs"'); - expect(customEnvironmentVariables).toContain('"postFilterCacheMaxEntries": "SearchPostFilterCacheMaxEntries"'); - expect(customEnvironmentVariables).toContain( - '"postFilterColdSearchConcurrency": "SearchPostFilterColdSearchConcurrency"' - ); - expect(customEnvironmentVariables).toContain( - '"postFilterColdSearchMaxQueueDepth": "SearchPostFilterColdSearchMaxQueueDepth"' - ); - expect(customEnvironmentVariables).toContain( - '"postFilterColdSearchMaxWaitMs": "SearchPostFilterColdSearchMaxWaitMs"' - ); expect(customEnvironmentVariables).toContain( '"openSearchRequestTimeoutMs": "SearchOpenSearchRequestTimeoutMs"' ); @@ -106,9 +94,6 @@ describe('runtime container and compose configuration', () => { expect(customEnvironmentVariables).toContain('"successSampleRate": "RequestLogSuccessSampleRate"'); expect(customEnvironmentVariables).toContain('"healthChecks": "RequestLogHealthChecks"'); expect(customEnvironmentVariables).toContain('"webSearchEnabled": "WebSearchEnabled"'); - expect(customEnvironmentVariables).toContain('"enabled": "SearchRateLimitEnabled"'); - expect(customEnvironmentVariables).toContain('"windowMs": "SearchRateLimitWindowMs"'); - expect(customEnvironmentVariables).toContain('"maxRequests": "SearchRateLimitMaxRequests"'); }); test.each([['config/default.jsonc'], ['config/devel.jsonc'], ['config/prod.jsonc']])( diff --git a/controllers/searchController.js b/controllers/searchController.js index 742ce0a..572a925 100644 --- a/controllers/searchController.js +++ b/controllers/searchController.js @@ -1,10 +1,6 @@ const PackageInfo = require('cve-core').packageInfo; const errors = require('../utils/errors'); -const searchPerformance = require('../utils/searchPerformance'); const searchDeadline = require('../utils/searchDeadline'); -const searchPostFilterCache = require('../utils/searchPostFilterCache'); -const searchInFlightRegistry = require('../utils/searchInFlightRegistry'); -const searchPostFilterWorkQueue = require('../utils/searchPostFilterWorkQueue'); const searchProvider = require('../utils/searchProvider'); const searchQueryBuilder = require('../utils/searchQueryBuilder'); const searchPostFilters = require('../utils/searchPostFilters'); @@ -97,220 +93,107 @@ function combineTook(...values) { return numericValues.reduce((sum, value) => sum + value, 0); } -function recordOpenSearchStart(metrics, operation, state) { - metrics.openSearchOperations ??= []; - metrics.openSearchOperations.push(operation); - metrics.maxOpenSearchActiveRequests = Math.max(metrics.maxOpenSearchActiveRequests ?? 0, state.activeCount); +async function executeOpenSearch(body, signal) { + return searchProvider.searchOpenSearch(body, { signal }); } -async function executeOpenSearch(body, operation, signal, metrics) { - return searchProvider.searchOpenSearch(body, { - signal, - operation, - onStart: (state) => recordOpenSearchStart(metrics, operation, state) - }); -} - -async function executeStandardSearch(searchOptions, queryStringObjects, metrics, signal) { - metrics.strategy = 'standard'; - - const searchStart = searchPerformance.markTime(); +async function executeStandardSearch(searchOptions, queryStringObjects, signal) { const body = searchQueryBuilder.buildOpenSearchBody(searchOptions, queryStringObjects); - const searchData = await executeOpenSearch(body, 'standard', signal, metrics); - const searchResp = { + const searchData = await executeOpenSearch(body, signal); + + return { status: 'ok', data: searchData }; - metrics.openSearchElapsedMs = searchPerformance.elapsedMs(searchStart); - metrics.openSearchTookMs = searchData?.took; - - return searchResp; } -async function fetchFullHitsByCveIds(cveIds, metrics, signal) { - if (!cveIds.length) return []; - - const inFlightKey = searchInFlightRegistry.buildKey('pageFetch', cveIds); - const waitStart = searchPerformance.markTime(); - const inFlight = searchInFlightRegistry.acquire( - inFlightKey, - async (operationSignal) => { - const { searchOptions, queryStringObjects } = searchQueryBuilder.buildCveIdsSearchRequest(cveIds); - const body = searchQueryBuilder.buildOpenSearchBody(searchOptions, queryStringObjects); - const pageFetchStart = searchPerformance.markTime(); - const pageData = await executeOpenSearch(body, 'page-fetch', operationSignal, metrics); - metrics.pageFetchElapsedMs = searchPerformance.elapsedMs(pageFetchStart); - metrics.pageFetchTookMs = pageData?.took; - - return orderHitsByCveIds(pageData?.hits?.hits ?? [], cveIds); - }, - { signal } - ); +async function fetchFullHitsByCveIds(cveIds, signal) { + if (!cveIds.length) return { hits: [], took: undefined }; - metrics.pageFetchInFlightHit = inFlight.shared; - const result = await inFlight.promise; - if (inFlight.shared) { - metrics.pageFetchWaitElapsedMs = searchPerformance.elapsedMs(waitStart); - } + const { searchOptions, queryStringObjects } = searchQueryBuilder.buildCveIdsSearchRequest(cveIds); + const body = searchQueryBuilder.buildOpenSearchBody(searchOptions, queryStringObjects); + const pageData = await executeOpenSearch(body, signal); - return result; + return { + hits: orderHitsByCveIds(pageData?.hits?.hits ?? [], cveIds), + took: pageData?.took + }; } -async function buildPostFilterCandidateResult(candidateBody, postFilters, metrics, signal) { - const operation = searchPostFilters.hasCpePostFilters(postFilters) ? 'cpe-candidate' : 'semantic-candidate'; - const candidateStart = searchPerformance.markTime(); - const candidateData = await executeOpenSearch(candidateBody, operation, signal, metrics); - metrics.openSearchElapsedMs = searchPerformance.elapsedMs(candidateStart); - metrics.openSearchTookMs = candidateData?.took; +async function buildPostFilterCandidateResult(candidateBody, postFilters, signal) { + const candidateData = await executeOpenSearch(candidateBody, signal); const candidateResp = { data: candidateData }; - const postFilterStart = searchPerformance.markTime(); - const { candidateCount, filteredHits } = searchPostFilters.filterSearchHits(candidateResp, postFilters); - metrics.postFilterElapsedMs = searchPerformance.elapsedMs(postFilterStart); + const { filteredHits } = searchPostFilters.filterSearchHits(candidateResp, postFilters); return { - candidateCount, filteredCount: filteredHits.length, - filteredCveIds: getHitCveIds(filteredHits) + filteredCveIds: getHitCveIds(filteredHits), + took: candidateData?.took }; } -async function getPostFilterCandidateResult(cacheKey, candidateBody, postFilters, metrics, signal) { - if (!searchPostFilters.hasCpePostFilters(postFilters)) { - metrics.cacheHit = false; - metrics.inFlightHit = false; - return buildPostFilterCandidateResult(candidateBody, postFilters, metrics, signal); - } - - const cachedResult = searchPostFilterCache.get(cacheKey); - if (cachedResult) { - metrics.cacheHit = true; - metrics.inFlightHit = false; - return cachedResult; - } - - metrics.cacheHit = false; - const inFlightKey = searchInFlightRegistry.buildKey('candidate', cacheKey); - const queueStart = searchPerformance.markTime(); - const waitStart = searchPerformance.markTime(); - const inFlight = searchInFlightRegistry.acquire( - inFlightKey, - (operationSignal) => - searchPostFilterWorkQueue - .run( - () => { - metrics.coldSearchQueueWaitElapsedMs = searchPerformance.elapsedMs(queueStart); - metrics.coldSearchQueueState = searchPostFilterWorkQueue.getState(); - return buildPostFilterCandidateResult(candidateBody, postFilters, metrics, operationSignal); - }, - { signal: operationSignal } - ) - .then((result) => { - searchPostFilterCache.set(cacheKey, result); - return result; - }), - { signal } - ); - - metrics.inFlightHit = inFlight.shared; - const result = await inFlight.promise; - if (inFlight.shared) { - metrics.inFlightWaitElapsedMs = searchPerformance.elapsedMs(waitStart); - } - - return result; -} - -async function executeOptimizedPostFilterSearch( - searchOptions, - queryStringObjects, - postFilters, - responseOptions, - metrics, - signal -) { - metrics.strategy = searchPostFilters.hasCpePostFilters(postFilters) ? 'cpe-candidate' : 'semantic-candidate'; - +async function executePostFilterSearch(searchOptions, queryStringObjects, postFilters, responseOptions, signal) { const candidateBody = searchQueryBuilder.buildPostFilterCandidateSearchBody( searchOptions, queryStringObjects, postFilters ); - const cacheKey = searchPostFilterCache.buildCacheKey(candidateBody, postFilters); - const candidateResult = await getPostFilterCandidateResult(cacheKey, candidateBody, postFilters, metrics, signal); + const candidateResult = await buildPostFilterCandidateResult(candidateBody, postFilters, signal); searchDeadline.throwIfAborted(signal); const pageCveIds = paginateHits(candidateResult.filteredCveIds, responseOptions); - metrics.candidateCount = candidateResult.candidateCount; - metrics.filteredCount = candidateResult.filteredCount ?? candidateResult.filteredCveIds.length; - metrics.returnedCount = pageCveIds.length; + const filteredCount = candidateResult.filteredCount ?? candidateResult.filteredCveIds.length; - const fullPageHits = await fetchFullHitsByCveIds(pageCveIds, metrics, signal); + const pageResult = await fetchFullHitsByCveIds(pageCveIds, signal); const response = { status: 'ok', data: { - took: combineTook(metrics.openSearchTookMs, metrics.pageFetchTookMs), + took: combineTook(candidateResult.took, pageResult.took), hits: { total: { - value: metrics.filteredCount + value: filteredCount }, hits: [] } } }; - return buildPagedSearchResponse(response, metrics.filteredCount, fullPageHits); + return buildPagedSearchResponse(response, filteredCount, pageResult.hits); } -async function executeSearch(searchOptions, queryStringObjects, postFilters, responseOptions, metrics, signal) { +async function executeSearch(searchOptions, queryStringObjects, postFilters, responseOptions, signal) { if (!searchPostFilters.hasSearchPostFilters(postFilters)) { - return executeStandardSearch(searchOptions, queryStringObjects, metrics, signal); + return executeStandardSearch(searchOptions, queryStringObjects, signal); } - return executeOptimizedPostFilterSearch( - searchOptions, - queryStringObjects, - postFilters, - responseOptions, - metrics, - signal - ); + return executePostFilterSearch(searchOptions, queryStringObjects, postFilters, responseOptions, signal); } async function searchHandler(req, res, next) { const params = req.searchParams ?? req.query ?? {}; - const metrics = { - requestId: req.requestId - }; - const handlerStart = searchPerformance.markTime(); const lifecycle = searchDeadline.createSearchRequestLifecycle( req, res, searchProvider.getSearchRequestDeadlineMs() ); - let searchOptions; - let postFilters; - let responseOptions; try { const searchRequest = searchQueryBuilder.buildSearchRequest(params); - searchOptions = searchRequest.searchOptions; - postFilters = searchRequest.postFilters; - responseOptions = searchRequest.responseOptions; + const searchOptions = searchRequest.searchOptions; + const postFilters = searchRequest.postFilters; + const responseOptions = searchRequest.responseOptions; const resp = await executeSearch( searchOptions, searchRequest.queryStringObjects, postFilters, responseOptions, - metrics, lifecycle.signal ); lifecycle.throwIfAborted(); const pagingOptions = responseOptions ?? searchOptions; - metrics.outcome = 'success'; - metrics.statusCode = 200; //Calculate current page and total pages to append to JSON output const totalValue = resp.data.hits.total.value; @@ -327,54 +210,22 @@ async function searchHandler(req, res, next) { }); } catch (err) { if (searchDeadline.isSearchClientDisconnectedError(err)) { - metrics.outcome = 'client_disconnected'; - metrics.statusCode = 499; - metrics.abortReason = 'client_disconnect'; return; } - if (searchPostFilterWorkQueue.isQueueFullError(err)) { - metrics.outcome = 'queue_full'; - metrics.statusCode = 429; - metrics.coldSearchQueueState = err.state; - if (typeof res.set === 'function') res.set('Retry-After', '5'); - - return res.status(429).json(error.searchQueueFullError()); - } - - if (searchPostFilterWorkQueue.isQueueTimeoutError(err)) { - metrics.outcome = 'queue_timeout'; - metrics.statusCode = 429; - metrics.coldSearchQueueState = err.state; - metrics.coldSearchQueueWaitElapsedMs = err.waitElapsedMs; - if (typeof res.set === 'function') res.set('Retry-After', '5'); - - return res.status(429).json(error.searchQueueTimeoutError()); - } - if (searchDeadline.isSearchTimeoutError(err)) { - metrics.outcome = 'search_timeout'; - metrics.statusCode = 504; - metrics.timeoutSource = err?.name === 'TimeoutError' ? 'opensearch' : 'request_deadline'; - metrics.abortReason = metrics.timeoutSource; return res.status(504).json(error.searchTimeoutError()); } if (isOpenSearchUnavailableError(err)) { - metrics.outcome = 'opensearch_unavailable'; - metrics.statusCode = 503; if (typeof res.set === 'function') res.set('Retry-After', '5'); return res.status(503).json(error.openSearchError()); } - metrics.outcome = 'upstream_error'; - metrics.statusCode = 502; return res.status(502).json(error.upstreamSearchError()); } finally { lifecycle.complete(); - metrics.totalElapsedMs = searchPerformance.elapsedMs(handlerStart); - searchPerformance.logSearchPerformance(metrics, searchOptions, responseOptions, postFilters); } } diff --git a/controllers/searchController.test.js b/controllers/searchController.test.js index 667013b..ade95e3 100644 --- a/controllers/searchController.test.js +++ b/controllers/searchController.test.js @@ -4,23 +4,10 @@ const mockApiSearch = jest.fn(); const mockBasicSearch = jest.fn(); const mockSearchRaw = jest.fn(); const mockSearchOpenSearch = jest.fn((body, options) => { - const operation = options?.operation; - const search = operation === 'cpe-candidate' ? mockSearchRaw : mockApiSearch; - options?.onStart?.({ - activeCount: 1, - activeByOperation: { - [operation]: 1 - } - }); + const search = Array.isArray(body?._source) ? mockSearchRaw : mockApiSearch; return Promise.resolve(search(body, options)).then((result) => result?.data ?? result); }); -const mockShouldLogSearchPerformance = jest.fn(); -const mockGetPostFilterCacheTtlMs = jest.fn(() => 60000); -const mockGetPostFilterCacheMaxEntries = jest.fn(() => 100); -const mockGetPostFilterColdSearchConcurrency = jest.fn(() => 10); -const mockGetPostFilterColdSearchMaxQueueDepth = jest.fn(() => 500); -const mockGetPostFilterColdSearchMaxWaitMs = jest.fn(() => 10000); const mockGetSearchRequestDeadlineMs = jest.fn(() => 30000); const mockIsSearchProviderContractError = jest.fn((err) => err?.code === 'SEARCH_PROVIDER_CONTRACT'); const mockGetBasicSearchManager = jest.fn(() => ({ @@ -37,20 +24,12 @@ jest.mock('cve-core', () => { jest.mock('../utils/searchProvider', () => ({ getBasicSearchManager: mockGetBasicSearchManager, - getPostFilterCacheMaxEntries: mockGetPostFilterCacheMaxEntries, - getPostFilterCacheTtlMs: mockGetPostFilterCacheTtlMs, - getPostFilterColdSearchConcurrency: mockGetPostFilterColdSearchConcurrency, - getPostFilterColdSearchMaxQueueDepth: mockGetPostFilterColdSearchMaxQueueDepth, - getPostFilterColdSearchMaxWaitMs: mockGetPostFilterColdSearchMaxWaitMs, getSearchRequestDeadlineMs: mockGetSearchRequestDeadlineMs, isSearchProviderContractError: mockIsSearchProviderContractError, - searchOpenSearch: mockSearchOpenSearch, - shouldLogSearchPerformance: mockShouldLogSearchPerformance + searchOpenSearch: mockSearchOpenSearch })); const controller = require('./searchController.js'); -const searchInFlightRegistry = require('../utils/searchInFlightRegistry'); -const searchPostFilterCache = require('../utils/searchPostFilterCache'); function createResponse() { const res = {}; @@ -95,15 +74,6 @@ function basicResponse({ ok = true, status = 'ok', total = 0, hits = [], notes = }; } -function deferred() { - const state = {}; - state.promise = new Promise((resolve, reject) => { - state.resolve = resolve; - state.reject = reject; - }); - return state; -} - function captureTimers() { const timers = []; jest.spyOn(global, 'setTimeout').mockImplementation((callback, delay) => { @@ -149,34 +119,16 @@ describe('Search controller SEARCH handler', () => { mockBasicSearch.mockReset(); mockSearchRaw.mockReset(); mockSearchOpenSearch.mockClear(); - mockShouldLogSearchPerformance.mockReset(); - mockShouldLogSearchPerformance.mockReturnValue(false); mockGetBasicSearchManager.mockClear(); - mockGetPostFilterCacheTtlMs.mockClear(); - mockGetPostFilterCacheMaxEntries.mockClear(); - mockGetPostFilterColdSearchConcurrency.mockClear(); - mockGetPostFilterColdSearchMaxQueueDepth.mockClear(); - mockGetPostFilterColdSearchMaxWaitMs.mockClear(); mockGetSearchRequestDeadlineMs.mockClear(); mockIsSearchProviderContractError.mockClear(); - mockGetPostFilterCacheTtlMs.mockReturnValue(60000); - mockGetPostFilterCacheMaxEntries.mockReturnValue(100); - mockGetPostFilterColdSearchConcurrency.mockReturnValue(10); - mockGetPostFilterColdSearchMaxQueueDepth.mockReturnValue(500); - mockGetPostFilterColdSearchMaxWaitMs.mockReturnValue(10000); mockGetSearchRequestDeadlineMs.mockReturnValue(30000); - searchPostFilterCache.clear(); - searchInFlightRegistry.clear(); - jest.spyOn(console, 'log').mockImplementation(() => {}); res = createResponse(); next = jest.fn(); }); - afterEach(() => { - searchInFlightRegistry.clear(); - jest.restoreAllMocks(); - }); + afterEach(() => jest.restoreAllMocks()); test('SEARCH builds filters, query strings, date ranges, and pagination options', async () => { const resp = advancedResponse(25, [{ _id: 'CVE-2024-0001' }]); @@ -243,9 +195,7 @@ describe('Search controller SEARCH handler', () => { from: 20 }, expect.objectContaining({ - operation: 'standard', - signal: expect.any(AbortSignal), - onStart: expect.any(Function) + signal: expect.any(AbortSignal) }) ); @@ -541,7 +491,6 @@ describe('Search controller SEARCH handler', () => { from: 0 }, expect.objectContaining({ - operation: 'page-fetch', signal: expect.any(AbortSignal) }) ); @@ -609,9 +558,8 @@ describe('Search controller SEARCH handler', () => { containers: { cna: { title: 'complete record' }, adp: [{ title: 'enrichment' }] } } }; - mockApiSearch - .mockResolvedValueOnce(rawResponse(2, [matching, splitAcrossEntries])) - .mockResolvedValueOnce(advancedResponse(1, [fullRecord])); + mockSearchRaw.mockResolvedValue(rawResponse(2, [matching, splitAcrossEntries])); + mockApiSearch.mockResolvedValue(advancedResponse(1, [fullRecord])); await controller.SEARCH( { @@ -628,8 +576,6 @@ describe('Search controller SEARCH handler', () => { ); expect(mockSearchOpenSearch).toHaveBeenCalledTimes(2); - expect(mockSearchOpenSearch.mock.calls[0][1].operation).toBe('semantic-candidate'); - expect(mockSearchOpenSearch.mock.calls[1][1].operation).toBe('page-fetch'); expect(mockSearchOpenSearch.mock.calls[0][0]._source).toStrictEqual([ 'cveMetadata.cveId', 'containers.adp.metrics.other' @@ -673,9 +619,8 @@ describe('Search controller SEARCH handler', () => { containers: { cna: { title: 'complete record' }, adp: [{ title: 'enrichment' }] } } }; - mockApiSearch - .mockResolvedValueOnce(rawResponse(2, [matching, outsideRange])) - .mockResolvedValueOnce(advancedResponse(1, [fullRecord])); + mockSearchRaw.mockResolvedValue(rawResponse(2, [matching, outsideRange])); + mockApiSearch.mockResolvedValue(advancedResponse(1, [fullRecord])); await controller.SEARCH( { @@ -692,8 +637,6 @@ describe('Search controller SEARCH handler', () => { ); expect(mockSearchOpenSearch).toHaveBeenCalledTimes(2); - expect(mockSearchOpenSearch.mock.calls[0][1].operation).toBe('semantic-candidate'); - expect(mockSearchOpenSearch.mock.calls[1][1].operation).toBe('page-fetch'); expect(mockSearchOpenSearch.mock.calls[0][0]._source).toStrictEqual([ 'cveMetadata.cveId', 'containers.adp.metrics.other' @@ -759,7 +702,6 @@ describe('Search controller SEARCH handler', () => { }); expect(mockApiSearch.mock.calls[0][1]).toEqual( expect.objectContaining({ - operation: 'page-fetch', signal: expect.any(AbortSignal) }) ); @@ -780,184 +722,6 @@ describe('Search controller SEARCH handler', () => { }); }); - test('SEARCH reuses cached semantic post-filter IDs for repeated virtualMatchString range searches', async () => { - const applicable = cpeHit('CVE-2024-0001', { - criteria: 'cpe:2.3:o:linux:linux_kernel:2.5:*:*:*:*:*:*:*' - }); - const notApplicable = cpeHit('CVE-2024-0002', { - criteria: 'cpe:2.3:o:linux:linux_kernel:2.6:*:*:*:*:*:*:*' - }); - const fullApplicable = { - _id: 'CVE-2024-0001', - _source: { - cveMetadata: { - cveId: 'CVE-2024-0001' - } - } - }; - const req = { - query: { - virtualMatchString: 'cpe:2.3:o:linux:linux_kernel', - versionStart: '2.2', - versionStartType: 'including', - versionEnd: '2.6', - versionEndType: 'excluding', - resultsPerPage: '1', - pageNumber: '1' - } - }; - - mockSearchRaw.mockResolvedValue(rawResponse(2, [applicable, notApplicable])); - mockApiSearch.mockResolvedValue(advancedResponse(1, [fullApplicable])); - - await controller.SEARCH(req, res, next); - - const secondRes = createResponse(); - await controller.SEARCH(req, secondRes, next); - - expect(mockSearchRaw).toHaveBeenCalledTimes(1); - expect(mockApiSearch).toHaveBeenCalledTimes(2); - expect(mockApiSearch.mock.calls[1][0].query.bool.filter).toStrictEqual([ - { - terms: { - 'cveMetadata.cveId.keyword': ['CVE-2024-0001'] - } - } - ]); - expect(mockApiSearch.mock.calls[1][0]).toMatchObject({ - track_total_hits: true, - size: 1, - from: 0 - }); - expect(secondRes.status).toHaveBeenCalledWith(200); - expect(secondRes.json).toHaveBeenCalledWith({ - page: 1, - pageSize: 1, - totalPages: 1, - status: 'ok', - data: { - took: undefined, - hits: { - total: { - value: 1 - }, - hits: [fullApplicable] - } - } - }); - }); - - test('SEARCH coalesces concurrent identical cold semantic post-filter searches', async () => { - const candidateSearch = deferred(); - const applicable = cpeHit('CVE-2024-0001', { - criteria: 'cpe:2.3:o:linux:linux_kernel:2.5:*:*:*:*:*:*:*' - }); - const notApplicable = cpeHit('CVE-2024-0002', { - criteria: 'cpe:2.3:o:linux:linux_kernel:2.6:*:*:*:*:*:*:*' - }); - const fullApplicable = { - _id: 'CVE-2024-0001', - _source: { - cveMetadata: { - cveId: 'CVE-2024-0001' - } - } - }; - const req = { - query: { - virtualMatchString: 'cpe:2.3:o:linux:linux_kernel', - versionStart: '2.2', - versionStartType: 'including', - versionEnd: '2.6', - versionEndType: 'excluding', - resultsPerPage: '1', - pageNumber: '1' - } - }; - const firstRes = createResponse(); - const secondRes = createResponse(); - - mockSearchRaw.mockReturnValue(candidateSearch.promise); - mockApiSearch.mockResolvedValue(advancedResponse(1, [fullApplicable])); - - const firstSearch = controller.SEARCH(req, firstRes, next); - await Promise.resolve(); - const secondSearch = controller.SEARCH(req, secondRes, next); - - candidateSearch.resolve(rawResponse(2, [applicable, notApplicable])); - await Promise.all([firstSearch, secondSearch]); - - expect(mockSearchRaw).toHaveBeenCalledTimes(1); - expect(mockApiSearch).toHaveBeenCalledTimes(1); - expect(firstRes.status).toHaveBeenCalledWith(200); - expect(secondRes.status).toHaveBeenCalledWith(200); - expect(secondRes.json).toHaveBeenCalledWith({ - page: 1, - pageSize: 1, - totalPages: 1, - status: 'ok', - data: { - took: undefined, - hits: { - total: { - value: 1 - }, - hits: [fullApplicable] - } - } - }); - }); - - test('SEARCH keeps shared CPE work active when one client disconnects', async () => { - const candidateSearch = deferred(); - const applicable = cpeHit('CVE-2024-0001', { - criteria: 'cpe:2.3:o:linux:linux_kernel:2.5:*:*:*:*:*:*:*' - }); - const fullApplicable = { - _id: 'CVE-2024-0001', - _source: { - cveMetadata: { - cveId: 'CVE-2024-0001' - } - } - }; - const firstReq = new EventEmitter(); - const secondReq = new EventEmitter(); - const query = { - virtualMatchString: 'cpe:2.3:o:linux:linux_kernel', - resultsPerPage: 1, - pageNumber: 1 - }; - firstReq.query = query; - secondReq.query = query; - const firstRes = createResponse(); - const secondRes = createResponse(); - let operationSignal; - - mockSearchRaw.mockImplementation((_body, { signal }) => { - operationSignal = signal; - return candidateSearch.promise; - }); - mockApiSearch.mockResolvedValue(advancedResponse(1, [fullApplicable])); - - const firstSearch = controller.SEARCH(firstReq, firstRes, next); - await Promise.resolve(); - const secondSearch = controller.SEARCH(secondReq, secondRes, next); - await Promise.resolve(); - - firstReq.emit('aborted'); - await firstSearch; - - expect(operationSignal.aborted).toBe(false); - candidateSearch.resolve(rawResponse(1, [applicable])); - await secondSearch; - - expect(mockSearchRaw).toHaveBeenCalledTimes(1); - expect(mockApiSearch).toHaveBeenCalledTimes(1); - expect(firstRes.status).not.toHaveBeenCalled(); - expect(secondRes.status).toHaveBeenCalledWith(200); - }); - test('SEARCH applies semantic virtualMatchString filtering before building response metadata', async () => { const applicable = cpeHit('CVE-2024-0001', { criteria: 'cpe:2.3:o:microsoft:windows_10_1809:-:*:*:*:*:*:x86:*' @@ -1013,128 +777,6 @@ describe('Search controller SEARCH handler', () => { }); }); - test('SEARCH returns 429 when the cold semantic post-filter queue is full', async () => { - const candidateSearch = deferred(); - const firstRes = createResponse(); - const secondRes = createResponse(); - - mockGetPostFilterColdSearchConcurrency.mockReturnValue(1); - mockGetPostFilterColdSearchMaxQueueDepth.mockReturnValue(0); - mockSearchRaw.mockReturnValue(candidateSearch.promise); - - const firstSearch = controller.SEARCH( - { - query: { - virtualMatchString: 'cpe:2.3:o:linux:linux_kernel', - resultsPerPage: '1', - pageNumber: '1' - } - }, - firstRes, - next - ); - await Promise.resolve(); - - await controller.SEARCH( - { - requestId: 'queue-full-request', - query: { - virtualMatchString: 'cpe:2.3:o:microsoft:windows_10_1809', - resultsPerPage: '1', - pageNumber: '1' - } - }, - secondRes, - next - ); - - expect(secondRes.set).toHaveBeenCalledWith('Retry-After', '5'); - expect(secondRes.status).toHaveBeenCalledWith(429); - expect(secondRes.json).toHaveBeenCalledWith({ - error: 'SEARCH_QUEUE_FULL', - message: 'Search service is busy. Please retry later.' - }); - expect(JSON.parse(console.log.mock.calls[0][0])).toMatchObject({ - event: 'search.performance', - requestId: 'queue-full-request', - outcome: 'queue_full', - statusCode: 429, - strategy: 'cpe-candidate', - cacheStatus: 'miss', - coldSearchQueueState: { - activeCount: 1, - queuedCount: 0, - maxConcurrent: 1, - maxQueueDepth: 0, - maxWaitMs: 10000 - } - }); - - candidateSearch.resolve(rawResponse(0, [])); - await firstSearch; - }); - - test('SEARCH returns 429 when a cold semantic post-filter search exceeds the queue wait deadline', async () => { - const timers = captureTimers(); - const candidateSearch = deferred(); - const firstRes = createResponse(); - const secondRes = createResponse(); - - mockGetPostFilterColdSearchConcurrency.mockReturnValue(1); - mockGetPostFilterColdSearchMaxQueueDepth.mockReturnValue(1); - mockGetPostFilterColdSearchMaxWaitMs.mockReturnValue(10000); - mockSearchRaw.mockReturnValue(candidateSearch.promise); - - const firstSearch = controller.SEARCH( - { - query: { - virtualMatchString: 'cpe:2.3:o:linux:linux_kernel', - resultsPerPage: '1', - pageNumber: '1' - } - }, - firstRes, - next - ); - await Promise.resolve(); - - const secondSearch = controller.SEARCH( - { - requestId: 'queue-timeout-request', - query: { - virtualMatchString: 'cpe:2.3:o:microsoft:windows_10_1809', - resultsPerPage: '1', - pageNumber: '1' - } - }, - secondRes, - next - ); - await Promise.resolve(); - - timers.find(({ delay }) => delay === 10000).callback(); - await secondSearch; - - expect(secondRes.set).toHaveBeenCalledWith('Retry-After', '5'); - expect(secondRes.status).toHaveBeenCalledWith(429); - expect(secondRes.json).toHaveBeenCalledWith({ - error: 'SEARCH_QUEUE_TIMEOUT', - message: 'Search service is busy. Please retry later.' - }); - expect(JSON.parse(console.log.mock.calls[0][0])).toMatchObject({ - event: 'search.performance', - requestId: 'queue-timeout-request', - outcome: 'queue_timeout', - statusCode: 429, - strategy: 'cpe-candidate', - cacheStatus: 'miss', - coldSearchQueueWaitElapsedMs: expect.any(Number) - }); - - candidateSearch.resolve(rawResponse(0, [])); - await firstSearch; - }); - test('SEARCH returns sanitized upstream errors when optimized post-filter search fails', async () => { mockSearchRaw.mockRejectedValue(new Error('candidate search failed')); @@ -1158,45 +800,6 @@ describe('Search controller SEARCH handler', () => { }); }); - test('SEARCH logs redacted performance timing when enabled', async () => { - mockShouldLogSearchPerformance.mockReturnValue(true); - mockApiSearch.mockResolvedValue(advancedResponse(1, [{ _id: 'CVE-2024-0001' }])); - - await controller.SEARCH( - { - requestId: 'request-123', - query: { - keywordSearch: 'remote code', - resultsPerPage: '1' - } - }, - res, - next - ); - - expect(console.log).toHaveBeenCalledTimes(1); - const timingLog = JSON.parse(console.log.mock.calls[0][0]); - expect(timingLog).toMatchObject({ - event: 'search.performance', - route: '/search', - requestId: 'request-123', - outcome: 'success', - statusCode: 200, - strategy: 'standard', - cacheStatus: 'not_applicable', - optimizedFallback: false, - cacheHit: false, - inFlightHit: false, - openSearchOperations: ['standard'], - maxOpenSearchActiveRequests: 1, - postFilters: [], - requestedFrom: 0, - requestedSize: 1, - openSearchSize: 1 - }); - expect(console.log.mock.calls[0][0]).not.toContain('remote code'); - }); - test('SEARCH builds version-specific CVSS v3 severity filters', async () => { mockApiSearch.mockResolvedValue(advancedResponse(1)); @@ -1227,7 +830,7 @@ describe('Search controller SEARCH handler', () => { }); test('SEARCH builds cvssV2 metric filters', async () => { - mockApiSearch.mockResolvedValue(advancedResponse(1)); + mockSearchRaw.mockResolvedValue(rawResponse(0)); await controller.SEARCH( { @@ -1240,7 +843,7 @@ describe('Search controller SEARCH handler', () => { next ); - expect(mockApiSearch.mock.calls[0][0].query.bool.filter).toStrictEqual([ + expect(mockSearchRaw.mock.calls[0][0].query.bool.filter).toStrictEqual([ { bool: { filter: [ @@ -1273,7 +876,7 @@ describe('Search controller SEARCH handler', () => { }); test('SEARCH builds version-specific CVSS v3 metric filters', async () => { - mockApiSearch.mockResolvedValue(advancedResponse(1)); + mockSearchRaw.mockResolvedValue(rawResponse(0)); await controller.SEARCH( { @@ -1287,7 +890,7 @@ describe('Search controller SEARCH handler', () => { next ); - expect(mockApiSearch.mock.calls[0][0].query.bool.filter).toStrictEqual([ + expect(mockSearchRaw.mock.calls[0][0].query.bool.filter).toStrictEqual([ { bool: { filter: [ @@ -1517,12 +1120,6 @@ describe('Search controller SEARCH handler', () => { message: 'OpenSearch refused connection, please check that OpenSearch is running and accessible then try again.' }); - expect(JSON.parse(console.log.mock.calls[0][0])).toMatchObject({ - event: 'search.performance', - requestId: 'connection-error-request', - outcome: 'opensearch_unavailable', - statusCode: 503 - }); }); test('SEARCH returns a sanitized 504 when OpenSearch times out', async () => { @@ -1545,13 +1142,6 @@ describe('Search controller SEARCH handler', () => { error: 'SEARCH_TIMEOUT', message: 'Search request timed out. Please try again later.' }); - expect(JSON.parse(console.log.mock.calls[0][0])).toMatchObject({ - event: 'search.performance', - requestId: 'timeout-request', - outcome: 'search_timeout', - statusCode: 504, - timeoutSource: 'opensearch' - }); }); test('SEARCH returns a sanitized 504 when the overall request deadline is exceeded', async () => { @@ -1582,14 +1172,6 @@ describe('Search controller SEARCH handler', () => { error: 'SEARCH_TIMEOUT', message: 'Search request timed out. Please try again later.' }); - expect(JSON.parse(console.log.mock.calls[0][0])).toMatchObject({ - event: 'search.performance', - requestId: 'request-deadline-request', - outcome: 'search_timeout', - statusCode: 504, - timeoutSource: 'request_deadline', - abortReason: 'request_deadline' - }); }); test('SEARCH stops without writing a response when the client disconnects', async () => { @@ -1610,14 +1192,6 @@ describe('Search controller SEARCH handler', () => { expect(res.status).not.toHaveBeenCalled(); expect(res.json).not.toHaveBeenCalled(); - expect(JSON.parse(console.log.mock.calls[0][0])).toMatchObject({ - event: 'search.performance', - requestId: 'client-disconnect-request', - outcome: 'client_disconnected', - statusCode: 499, - abortReason: 'client_disconnect', - strategy: 'standard' - }); }); test('SEARCH does not start a page fetch after a client disconnects from a CPE search', async () => { @@ -1669,12 +1243,6 @@ describe('Search controller SEARCH handler', () => { error: 'UPSTREAM_SEARCH_ERROR', message: 'Search backend returned an error. Please try again later.' }); - expect(JSON.parse(console.log.mock.calls[0][0])).toMatchObject({ - event: 'search.performance', - requestId: 'upstream-error-request', - outcome: 'upstream_error', - statusCode: 502 - }); }); }); @@ -1686,21 +1254,14 @@ describe('Search controller WEB_SEARCH handler', () => { mockBasicSearch.mockReset(); mockSearchRaw.mockReset(); mockSearchOpenSearch.mockClear(); - mockShouldLogSearchPerformance.mockReset(); - mockShouldLogSearchPerformance.mockReturnValue(false); mockGetBasicSearchManager.mockClear(); - searchPostFilterCache.clear(); - searchInFlightRegistry.clear(); jest.spyOn(console, 'log').mockImplementation(() => {}); res = createResponse(); next = jest.fn(); }); - afterEach(() => { - searchInFlightRegistry.clear(); - jest.restoreAllMocks(); - }); + afterEach(() => jest.restoreAllMocks()); test('WEB_SEARCH sends cveId ascending sort, paging, and response metadata', async () => { const hits = [{ _id: 'CVE-2024-0001' }, { _id: 'CVE-2024-0002' }]; diff --git a/package-lock.json b/package-lock.json index 2e5012a..48d2e11 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13,7 +13,6 @@ "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", @@ -1513,12 +1512,12 @@ } }, "node_modules/adm-zip": { - "version": "0.5.16", - "resolved": "https://registry.npmjs.org/adm-zip/-/adm-zip-0.5.16.tgz", - "integrity": "sha512-TGw5yVi4saajsSEgz25grObGHEUaDrniwvA2qwSC060KfqGPdglhvPMA2lPIoxs3PQIItj2iag35fONcQqgUaQ==", + "version": "0.6.0", + "resolved": "https://registry.npmjs.org/adm-zip/-/adm-zip-0.6.0.tgz", + "integrity": "sha512-XleryMhbuksdKtofnWZ9Sk+4CUTbms4Mb/EU32SZwToAyZ5RgVos/ki8n+yr0LWHOGKuakbXTuuYNHLQjhddgg==", "license": "MIT", "engines": { - "node": ">=12.0" + "node": ">=14.0" } }, "node_modules/ajv": { @@ -2920,42 +2919,6 @@ "url": "https://opencollective.com/express" } }, - "node_modules/express-rate-limit": { - "version": "8.6.1", - "resolved": "https://registry.npmjs.org/express-rate-limit/-/express-rate-limit-8.6.1.tgz", - "integrity": "sha512-0D493aP61w0TJ2A0wy27riRsO7FMQ7FK+KUHOKCSfPvYo0R55aiC6emCVgFUeShH0fq0ICPVzNcgoS+BsbXQCA==", - "license": "MIT", - "dependencies": { - "debug": "^4.4.3", - "ip-address": "^10.2.0" - }, - "engines": { - "node": ">= 16" - }, - "funding": { - "url": "https://github.com/sponsors/express-rate-limit" - }, - "peerDependencies": { - "express": ">= 4.11" - } - }, - "node_modules/express-rate-limit/node_modules/debug": { - "version": "4.4.3", - "resolved": "https://registry.npmjs.org/debug/-/debug-4.4.3.tgz", - "integrity": "sha512-RGwwWnwQvkVfavKVt22FGLw+xYSdzARwm0ru6DhTVA3umU5hZc28V3kO4stgYryrTlLpuvgI9GiijltAjNbcqA==", - "license": "MIT", - "dependencies": { - "ms": "^2.1.3" - }, - "engines": { - "node": ">=6.0" - }, - "peerDependenciesMeta": { - "supports-color": { - "optional": true - } - } - }, "node_modules/express-validator": { "version": "7.3.1", "resolved": "https://registry.npmjs.org/express-validator/-/express-validator-7.3.1.tgz", @@ -3613,15 +3576,6 @@ "integrity": "sha512-k/vGaX4/Yla3WzyMCvTQOXYeIHvqOKtnqBduzTHpzpQZzAskKMhZ2K+EnBiSM9zGSoIFeMpXKxa4dYeZIQqewQ==", "license": "ISC" }, - "node_modules/ip-address": { - "version": "10.3.1", - "resolved": "https://registry.npmjs.org/ip-address/-/ip-address-10.3.1.tgz", - "integrity": "sha512-1e9d3kb97NHJTIJDZW9rKqW2h6+dFa50Dy0fpPSMQp2ADje5gvKsXmdiK6dwY5t76TaTt5+P5N1Y/LoToIxP6g==", - "license": "MIT", - "engines": { - "node": ">= 12" - } - }, "node_modules/ipaddr.js": { "version": "1.9.1", "resolved": "https://registry.npmjs.org/ipaddr.js/-/ipaddr.js-1.9.1.tgz", diff --git a/package.json b/package.json index cfe69fd..1cd2c2b 100644 --- a/package.json +++ b/package.json @@ -19,7 +19,6 @@ "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", @@ -35,6 +34,7 @@ "swagger-autogen": "^2.23.7" }, "overrides": { + "adm-zip": "0.6.0", "axios": "1.18.0", "brace-expansion": "5.0.9", "js-yaml": "4.3.1", diff --git a/routes/index.js b/routes/index.js index d4882f0..9a62b1f 100644 --- a/routes/index.js +++ b/routes/index.js @@ -7,11 +7,9 @@ const constants = require('../utils/constants'); const featureFlags = require('../utils/featureFlags'); const searchDeadline = require('../utils/searchDeadline'); const searchProvider = require('../utils/searchProvider'); -const searchRateLimiter = require('../utils/searchRateLimiter'); const searchScope = require('../utils/searchScope'); const error = new errors.errors(); -const limitSearchRequests = searchRateLimiter.createSearchRateLimiter(); const SEARCH_BODY_FIELDS = new Set(constants.SEARCH_QUERY_PARAMS); const SEARCH_STRING_BODY_FIELDS = constants.SEARCH_QUERY_PARAMS.filter( (field) => @@ -300,29 +298,6 @@ description: 'Invalid cpeName value', #swagger.responses[415] = { description: 'A non-empty request body did not use the application/json media type with UTF-8 encoding', } -#swagger.responses[429] = { -description: 'Application rate limit exceeded, or the cold CPE search queue is full or exceeded its maximum wait', -headers: { - "Retry-After": { - description: "Seconds until a rate-limited request can be retried", - schema: { - type: "integer" - } - }, - "RateLimit": { - description: "Current application rate-limit state when application limiting is enabled", - schema: { - type: "string" - } - }, - "RateLimit-Policy": { - description: "Application rate-limit policy when application limiting is enabled", - schema: { - type: "string" - } - } -} -} #swagger.responses[503] = { description: 'OpenSearch is unavailable', headers: { @@ -339,7 +314,6 @@ description: 'Search exceeded its OpenSearch or overall processing deadline', } */ requireSearchJsonContentType, - limitSearchRequests, rejectSearchQueryParams, body().custom(isSearchBodyObject).withMessage(error.searchBodyError()), body().custom(hasOnlySearchBodyFields), diff --git a/routes/index.test.js b/routes/index.test.js index 415447d..a041d7a 100644 --- a/routes/index.test.js +++ b/routes/index.test.js @@ -23,13 +23,8 @@ jest.mock('../utils/featureFlags', () => ({ isWebSearchEnabled: jest.fn(() => true) })); -const mockLimitSearchRequests = jest.fn((req, res, next) => next()); const mockCheckOpenSearchReadiness = jest.fn(); -jest.mock('../utils/searchRateLimiter', () => ({ - createSearchRateLimiter: jest.fn(() => mockLimitSearchRequests) -})); - jest.mock('../utils/searchProvider', () => ({ checkOpenSearchReadiness: mockCheckOpenSearchReadiness, getOpenSearchReadinessTimeoutMs: jest.fn(() => 5000) @@ -47,7 +42,6 @@ describe('routes/index router', () => { app = createRouterApp(router); jest.clearAllMocks(); featureFlags.isWebSearchEnabled.mockReturnValue(true); - mockLimitSearchRequests.mockImplementation((req, res, next) => next()); mockCheckOpenSearchReadiness.mockResolvedValue(true); }); @@ -100,7 +94,6 @@ describe('routes/index router', () => { }); expect(response.status).toBe(200); - expect(mockLimitSearchRequests).toHaveBeenCalledTimes(1); expect(mw.validateParams).toHaveBeenCalledTimes(1); expect(controller.SEARCH).toHaveBeenCalledTimes(1); expect(response.body).toStrictEqual({ @@ -120,7 +113,6 @@ describe('routes/index router', () => { }); expect(response.status).toBe(200); - expect(mockLimitSearchRequests).toHaveBeenCalledTimes(1); expect(mw.validateParams).toHaveBeenCalledTimes(1); expect(controller.SEARCH).toHaveBeenCalledTimes(1); expect(response.body).toStrictEqual({ @@ -131,7 +123,7 @@ describe('routes/index router', () => { }); }); - test('POST /search rejects a non-empty body with an unsupported media type before rate limiting', async () => { + test('POST /search rejects a non-empty body with an unsupported media type before validation', async () => { const response = await request(app, { method: 'POST', path: '/search', @@ -146,29 +138,6 @@ describe('routes/index router', () => { error: 'SEARCH_CONTENT_TYPE', message: 'The /search request body must use the application/json media type with UTF-8 encoding.' }); - expect(mockLimitSearchRequests).not.toHaveBeenCalled(); - expect(mw.validateParams).not.toHaveBeenCalled(); - expect(controller.SEARCH).not.toHaveBeenCalled(); - }); - - test('POST /search returns a rate-limit response before validation and controller work', async () => { - mockLimitSearchRequests.mockImplementation((req, res) => - res.status(429).json({ - error: 'SEARCH_RATE_LIMITED', - message: 'Too many search requests. Please retry later.' - }) - ); - - const response = await request(app, { - method: 'POST', - path: '/search', - body: { - keywordSearch: 'test' - } - }); - - expect(response.status).toBe(429); - expect(response.body.error).toBe('SEARCH_RATE_LIMITED'); expect(mw.validateParams).not.toHaveBeenCalled(); expect(controller.SEARCH).not.toHaveBeenCalled(); }); @@ -405,7 +374,6 @@ describe('routes/index router', () => { }); expect(response.status).toBe(200); - expect(mockLimitSearchRequests).not.toHaveBeenCalled(); expect(controller.WEB_SEARCH).toHaveBeenCalledTimes(1); expect(response.body).toStrictEqual({ route: 'webSearch', diff --git a/routes/search.integration.test.js b/routes/search.integration.test.js index 86a3dbf..22b0c81 100644 --- a/routes/search.integration.test.js +++ b/routes/search.integration.test.js @@ -85,11 +85,9 @@ describe('search route integration', () => { size: 1, from: 0 }, - expect.objectContaining({ - operation: 'standard', - signal: expect.any(AbortSignal), - onStart: expect.any(Function) - }) + { + signal: expect.any(AbortSignal) + } ); expect(response.body.page).toBe(1); expect(response.body.pageSize).toBe(1); diff --git a/test/postman/CVE-Search-API.postman_collection.json b/test/postman/CVE-Search-API.postman_collection.json index 4153034..d4a90fa 100644 --- a/test/postman/CVE-Search-API.postman_collection.json +++ b/test/postman/CVE-Search-API.postman_collection.json @@ -6252,177 +6252,6 @@ }, "response": [] }, - { - "name": "Cold-search queue full", - "event": [ - { - "listen": "prerequest", - "script": { - "type": "text/javascript", - "exec": [ - "if (pm.collectionVariables.get('runFaultInjection') !== 'true') {", - " console.log('Skipping Cold-search queue full; set runFaultInjection=true to enable it.');", - " pm.execution.skipRequest();", - "}" - ] - } - }, - { - "listen": "test", - "script": { - "type": "text/javascript", - "exec": [ - "pm.test('status is 429', () => pm.response.to.have.status(429));", - "pm.test('X-Request-ID is present', () => {", - " pm.expect(pm.response.headers.get('X-Request-ID')).to.be.a('string').and.not.empty;", - "});", - "pm.test('response is JSON', () => pm.expect(pm.response.headers.get('Content-Type')).to.include('application/json'));", - "const payload = pm.response.json();", - "const errors = Array.isArray(payload) ? payload : [payload];", - "const expectedError = {\"error\":\"SEARCH_QUEUE_FULL\",\"message\":\"Search service is busy. Please retry later.\"};", - "pm.test('expected error code is returned', () => {", - " pm.expect(errors.map((entry) => entry.error)).to.include(expectedError.error);", - "});", - "pm.test('expected error message is returned', () => {", - " const match = errors.find((entry) => entry.error === expectedError.error);", - " pm.expect(match?.message).to.eql(expectedError.message);", - "});" - ] - } - } - ], - "request": { - "method": "POST", - "header": [{ "key": "Content-Type", "value": "application/json", "type": "text" }], - "url": { - "raw": "{{baseUrl}}/search", - "host": ["{{baseUrl}}"], - "path": ["search"], - "query": [] - }, - "description": "Requires concurrent unique cold CPE searches and a deliberately constrained queue.", - "body": { - "mode": "raw", - "raw": "{\n \"cpeName\": \"cpe:2.3:a:freetype:freetype:2.8.1:*:*:*:*:*:*:*\",\n \"isVulnerable\": true\n}", - "options": { "raw": { "language": "json" } } - } - }, - "response": [] - }, - { - "name": "Cold-search queue timeout", - "event": [ - { - "listen": "prerequest", - "script": { - "type": "text/javascript", - "exec": [ - "if (pm.collectionVariables.get('runFaultInjection') !== 'true') {", - " console.log('Skipping Cold-search queue timeout; set runFaultInjection=true to enable it.');", - " pm.execution.skipRequest();", - "}" - ] - } - }, - { - "listen": "test", - "script": { - "type": "text/javascript", - "exec": [ - "pm.test('status is 429', () => pm.response.to.have.status(429));", - "pm.test('X-Request-ID is present', () => {", - " pm.expect(pm.response.headers.get('X-Request-ID')).to.be.a('string').and.not.empty;", - "});", - "pm.test('response is JSON', () => pm.expect(pm.response.headers.get('Content-Type')).to.include('application/json'));", - "const payload = pm.response.json();", - "const errors = Array.isArray(payload) ? payload : [payload];", - "const expectedError = {\"error\":\"SEARCH_QUEUE_TIMEOUT\",\"message\":\"Search service is busy. Please retry later.\"};", - "pm.test('expected error code is returned', () => {", - " pm.expect(errors.map((entry) => entry.error)).to.include(expectedError.error);", - "});", - "pm.test('expected error message is returned', () => {", - " const match = errors.find((entry) => entry.error === expectedError.error);", - " pm.expect(match?.message).to.eql(expectedError.message);", - "});" - ] - } - } - ], - "request": { - "method": "POST", - "header": [{ "key": "Content-Type", "value": "application/json", "type": "text" }], - "url": { - "raw": "{{baseUrl}}/search", - "host": ["{{baseUrl}}"], - "path": ["search"], - "query": [] - }, - "description": "Requires concurrent cold CPE searches and a deliberately short queue wait timeout.", - "body": { - "mode": "raw", - "raw": "{\n \"cpeName\": \"cpe:2.3:a:freetype:freetype:2.8.1:*:*:*:*:*:*:*\",\n \"isVulnerable\": true\n}", - "options": { "raw": { "language": "json" } } - } - }, - "response": [] - }, - { - "name": "Application rate limit", - "event": [ - { - "listen": "prerequest", - "script": { - "type": "text/javascript", - "exec": [ - "if (pm.collectionVariables.get('runFaultInjection') !== 'true') {", - " console.log('Skipping Application rate limit; set runFaultInjection=true to enable it.');", - " pm.execution.skipRequest();", - "}" - ] - } - }, - { - "listen": "test", - "script": { - "type": "text/javascript", - "exec": [ - "pm.test('status is 429', () => pm.response.to.have.status(429));", - "pm.test('X-Request-ID is present', () => {", - " pm.expect(pm.response.headers.get('X-Request-ID')).to.be.a('string').and.not.empty;", - "});", - "pm.test('response is JSON', () => pm.expect(pm.response.headers.get('Content-Type')).to.include('application/json'));", - "const payload = pm.response.json();", - "const errors = Array.isArray(payload) ? payload : [payload];", - "const expectedError = {\"error\":\"SEARCH_RATE_LIMITED\",\"message\":\"Too many search requests. Please retry later.\"};", - "pm.test('expected error code is returned', () => {", - " pm.expect(errors.map((entry) => entry.error)).to.include(expectedError.error);", - "});", - "pm.test('expected error message is returned', () => {", - " const match = errors.find((entry) => entry.error === expectedError.error);", - " pm.expect(match?.message).to.eql(expectedError.message);", - "});" - ] - } - } - ], - "request": { - "method": "POST", - "header": [{ "key": "Content-Type", "value": "application/json", "type": "text" }], - "url": { - "raw": "{{baseUrl}}/search", - "host": ["{{baseUrl}}"], - "path": ["search"], - "query": [] - }, - "description": "Requires the application limiter to be enabled with an exhausted allowance. A WAF response may use a different contract.", - "body": { - "mode": "raw", - "raw": "{\n \"cveIds\": \"CVE-2024-5981\"\n}", - "options": { "raw": { "language": "json" } } - } - }, - "response": [] - }, { "name": "Unexpected upstream response", "event": [ diff --git a/test/postman/README.md b/test/postman/README.md index a1a042c..4aa8afc 100644 --- a/test/postman/README.md +++ b/test/postman/README.md @@ -65,15 +65,12 @@ Folder `17 - Manual fault injection - skipped by default` describes runtime erro Setting `runFaultInjection` to `true` only enables the requests; it does not create their required failure conditions. Prepare and run one request at a time because the conditions are different and can conflict. -| Request | Required condition | Expected response | -| ----------------------------- | ------------------------------------------------------------------------------------------------------- | --------------------------- | -| OpenSearch readiness failure | Make the configured OpenSearch index unavailable to the API. | `503 SEARCH_NOT_READY` | -| OpenSearch unavailable search | Make OpenSearch connectivity, permissions, or index access unavailable. | `503 OPEN_SEARCH_ERROR` | -| Search timeout | Deliberately lower the request deadline or delay OpenSearch beyond the configured deadline. | `504 SEARCH_TIMEOUT` | -| Cold-search queue full | Constrain cold CPE concurrency and queue depth, then keep all active and queued slots occupied. | `429 SEARCH_QUEUE_FULL` | -| Cold-search queue timeout | Keep cold CPE workers occupied longer than a deliberately shortened queue wait timeout. | `429 SEARCH_QUEUE_TIMEOUT` | -| Application rate limit | Enable the application limiter with a low threshold and exhaust the allowance before sending the check. | `429 SEARCH_RATE_LIMITED` | -| Unexpected upstream response | Use a test double or proxy that returns an invalid or incomplete OpenSearch response. | `502 UPSTREAM_SEARCH_ERROR` | +| Request | Required condition | Expected response | +| ----------------------------- | ------------------------------------------------------------------------------------------- | --------------------------- | +| OpenSearch readiness failure | Make the configured OpenSearch index unavailable to the API. | `503 SEARCH_NOT_READY` | +| OpenSearch unavailable search | Make OpenSearch connectivity, permissions, or index access unavailable. | `503 OPEN_SEARCH_ERROR` | +| Search timeout | Deliberately lower the request deadline or delay OpenSearch beyond the configured deadline. | `504 SEARCH_TIMEOUT` | +| Unexpected upstream response | Use a test double or proxy that returns an invalid or incomplete OpenSearch response. | `502 UPSTREAM_SEARCH_ERROR` | Run a fault check as follows: @@ -83,8 +80,6 @@ Run a fault check as follows: 4. Confirm the asserted HTTP status, sanitized error code, message, and `X-Request-ID`. 5. Set `runFaultInjection` back to `false` and restore the normal application and dependency configuration. -The two queue checks require concurrent external setup because the Postman collection runner sends requests sequentially. A separate request must occupy the worker or queue before sending the Postman assertion request. - Do not enable fault injection against a shared or production environment without the target owner's approval. A WAF or load balancer can return a different error contract than the application-level response asserted by this folder. ## Known Validation Boundary diff --git a/test/postman/buildCollection.js b/test/postman/buildCollection.js index db293fe..f58b62f 100644 --- a/test/postman/buildCollection.js +++ b/test/postman/buildCollection.js @@ -1108,30 +1108,6 @@ const faultInjection = folder( error.searchTimeoutError(), 'Run only with a deliberately reduced request deadline or delayed OpenSearch response.' ), - faultInjectionItem( - 'Cold-search queue full', - '/search', - { cpeName: 'cpe:2.3:a:freetype:freetype:2.8.1:*:*:*:*:*:*:*', isVulnerable: true }, - 429, - error.searchQueueFullError(), - 'Requires concurrent unique cold CPE searches and a deliberately constrained queue.' - ), - faultInjectionItem( - 'Cold-search queue timeout', - '/search', - { cpeName: 'cpe:2.3:a:freetype:freetype:2.8.1:*:*:*:*:*:*:*', isVulnerable: true }, - 429, - error.searchQueueTimeoutError(), - 'Requires concurrent cold CPE searches and a deliberately short queue wait timeout.' - ), - faultInjectionItem( - 'Application rate limit', - '/search', - { cveIds: ids.primary }, - 429, - error.searchRateLimitedError(), - 'Requires the application limiter to be enabled with an exhausted allowance. A WAF response may use a different contract.' - ), faultInjectionItem( 'Unexpected upstream response', '/search', diff --git a/utils/constants.js b/utils/constants.js index d80f19d..a86aecc 100644 --- a/utils/constants.js +++ b/utils/constants.js @@ -20,11 +20,6 @@ const SEARCH_STRING_LENGTH_LIMITS = Object.freeze({ ssvcRole: 256, referenceURL: 2048 }); -const SEARCH_POST_FILTER_CACHE_TTL_MS = 60 * 1000; -const SEARCH_POST_FILTER_CACHE_MAX_ENTRIES = 100; -const SEARCH_POST_FILTER_COLD_SEARCH_CONCURRENCY = 10; -const SEARCH_POST_FILTER_COLD_SEARCH_MAX_QUEUE_DEPTH = 500; -const SEARCH_POST_FILTER_COLD_SEARCH_MAX_WAIT_MS = 10 * 1000; const SEARCH_OPEN_SEARCH_REQUEST_TIMEOUT_MS = 20 * 1000; const SEARCH_REQUEST_DEADLINE_MS = 30 * 1000; const SEARCH_READINESS_TIMEOUT_MS = 5 * 1000; @@ -136,11 +131,6 @@ module.exports = { MAX_KEYWORD_SEARCH_TOKENS, MIN_REFERENCE_URL_LENGTH, SEARCH_STRING_LENGTH_LIMITS, - SEARCH_POST_FILTER_CACHE_TTL_MS, - SEARCH_POST_FILTER_CACHE_MAX_ENTRIES, - SEARCH_POST_FILTER_COLD_SEARCH_CONCURRENCY, - SEARCH_POST_FILTER_COLD_SEARCH_MAX_QUEUE_DEPTH, - SEARCH_POST_FILTER_COLD_SEARCH_MAX_WAIT_MS, SEARCH_OPEN_SEARCH_REQUEST_TIMEOUT_MS, SEARCH_REQUEST_DEADLINE_MS, SEARCH_READINESS_TIMEOUT_MS, diff --git a/utils/errors.js b/utils/errors.js index 6f8115a..d942253 100644 --- a/utils/errors.js +++ b/utils/errors.js @@ -319,21 +319,9 @@ class errors { return makeError('UPSTREAM_SEARCH_ERROR', 'Search backend returned an error. Please try again later.'); } - searchQueueFullError() { - return makeError('SEARCH_QUEUE_FULL', 'Search service is busy. Please retry later.'); - } - - searchQueueTimeoutError() { - return makeError('SEARCH_QUEUE_TIMEOUT', 'Search service is busy. Please retry later.'); - } - searchTimeoutError() { return makeError('SEARCH_TIMEOUT', 'Search request timed out. Please try again later.'); } - - searchRateLimitedError() { - return makeError('SEARCH_RATE_LIMITED', 'Too many search requests. Please retry later.'); - } } module.exports = { diff --git a/utils/errors.test.js b/utils/errors.test.js index 682af30..128e6ad 100644 --- a/utils/errors.test.js +++ b/utils/errors.test.js @@ -175,22 +175,6 @@ describe('error response helpers', () => { message: 'The noRejected field must be a boolean.' } ], - [ - 'searchQueueFullError', - [], - { - error: 'SEARCH_QUEUE_FULL', - message: 'Search service is busy. Please retry later.' - } - ], - [ - 'searchQueueTimeoutError', - [], - { - error: 'SEARCH_QUEUE_TIMEOUT', - message: 'Search service is busy. Please retry later.' - } - ], [ 'searchTimeoutError', [], @@ -199,14 +183,6 @@ describe('error response helpers', () => { message: 'Search request timed out. Please try again later.' } ], - [ - 'searchRateLimitedError', - [], - { - error: 'SEARCH_RATE_LIMITED', - message: 'Too many search requests. Please retry later.' - } - ], [ 'searchNotReadyError', [], diff --git a/utils/searchInFlightRegistry.js b/utils/searchInFlightRegistry.js deleted file mode 100644 index 07f4e85..0000000 --- a/utils/searchInFlightRegistry.js +++ /dev/null @@ -1,118 +0,0 @@ -const searchDeadline = require('./searchDeadline'); - -const entries = new Map(); - -class SearchOperationAbandonedError extends Error { - constructor() { - super('Shared search operation no longer has active waiters.'); - this.name = 'SearchOperationAbandonedError'; - this.code = 'SEARCH_OPERATION_ABANDONED'; - } -} - -function buildKey(type, value) { - return `${type}:${JSON.stringify(value)}`; -} - -function settleEntry(key, entry) { - entry.settled = true; - if (entries.get(key) === entry) { - entries.delete(key); - } -} - -function createEntry(key, createOperation) { - const controller = new AbortController(); - const entry = { - controller, - key, - promise: undefined, - settled: false, - waiterCount: 0 - }; - - entries.set(key, entry); - entry.promise = Promise.resolve().then(() => createOperation(controller.signal)); - entry.promise.then( - () => settleEntry(key, entry), - () => settleEntry(key, entry) - ); - - return entry; -} - -function releaseWaiter(entry) { - entry.waiterCount = Math.max(0, entry.waiterCount - 1); - if (!entry.settled && entry.waiterCount === 0 && !entry.controller.signal.aborted) { - settleEntry(entry.key, entry); - entry.controller.abort(new SearchOperationAbandonedError()); - } -} - -function waitForEntry(entry, signal) { - entry.waiterCount += 1; - - return new Promise((resolve, reject) => { - let settled = false; - - const finish = (callback, value) => { - if (settled) return; - - settled = true; - signal?.removeEventListener('abort', onAbort); - releaseWaiter(entry); - callback(value); - }; - const onAbort = () => finish(reject, searchDeadline.getAbortReason(signal)); - - if (signal?.aborted) { - onAbort(); - return; - } - - signal?.addEventListener('abort', onAbort, { once: true }); - entry.promise.then( - (value) => finish(resolve, value), - (err) => finish(reject, err) - ); - }); -} - -function acquire(key, createOperation, { signal } = {}) { - searchDeadline.throwIfAborted(signal); - - let entry = entries.get(key); - const shared = Boolean(entry); - if (!entry) { - entry = createEntry(key, createOperation); - } - - return { - promise: waitForEntry(entry, signal), - shared - }; -} - -function clear() { - for (const entry of entries.values()) { - if (!entry.controller.signal.aborted) { - entry.controller.abort(new SearchOperationAbandonedError()); - } - } - entries.clear(); -} - -function getState() { - return { - operationCount: entries.size, - waiterCount: [...entries.values()].reduce((total, entry) => total + entry.waiterCount, 0) - }; -} - -module.exports = { - acquire, - buildKey, - clear, - getState, - SearchOperationAbandonedError -}; diff --git a/utils/searchInFlightRegistry.test.js b/utils/searchInFlightRegistry.test.js deleted file mode 100644 index aed7ad9..0000000 --- a/utils/searchInFlightRegistry.test.js +++ /dev/null @@ -1,133 +0,0 @@ -const searchDeadline = require('./searchDeadline'); -const searchInFlightRegistry = require('./searchInFlightRegistry'); - -function deferred() { - const state = {}; - state.promise = new Promise((resolve, reject) => { - state.resolve = resolve; - state.reject = reject; - }); - return state; -} - -describe('searchInFlightRegistry', () => { - beforeEach(() => { - searchInFlightRegistry.clear(); - }); - - afterEach(() => { - searchInFlightRegistry.clear(); - }); - - test('coalesces waiters for the same operation key', async () => { - const operation = deferred(); - const createOperation = jest.fn(() => operation.promise); - const first = searchInFlightRegistry.acquire('same-key', createOperation); - const second = searchInFlightRegistry.acquire('same-key', createOperation); - await Promise.resolve(); - - expect(first.shared).toBe(false); - expect(second.shared).toBe(true); - expect(createOperation).toHaveBeenCalledTimes(1); - expect(searchInFlightRegistry.getState()).toStrictEqual({ - operationCount: 1, - waiterCount: 2 - }); - - operation.resolve('result'); - - await expect(first.promise).resolves.toBe('result'); - await expect(second.promise).resolves.toBe('result'); - expect(searchInFlightRegistry.getState()).toStrictEqual({ - operationCount: 0, - waiterCount: 0 - }); - }); - - test('keeps shared work active while another waiter remains', async () => { - const operation = deferred(); - const firstController = new AbortController(); - const secondController = new AbortController(); - let operationSignal; - const createOperation = jest.fn((signal) => { - operationSignal = signal; - return operation.promise; - }); - const first = searchInFlightRegistry.acquire('same-key', createOperation, { - signal: firstController.signal - }); - const second = searchInFlightRegistry.acquire('same-key', createOperation, { - signal: secondController.signal - }); - await Promise.resolve(); - - firstController.abort(new searchDeadline.SearchClientDisconnectedError()); - - await expect(first.promise).rejects.toMatchObject({ - code: 'SEARCH_CLIENT_DISCONNECTED' - }); - expect(operationSignal.aborted).toBe(false); - expect(searchInFlightRegistry.getState().waiterCount).toBe(1); - - operation.resolve('result'); - await expect(second.promise).resolves.toBe('result'); - }); - - test('aborts shared work after its final waiter cancels', async () => { - const operation = deferred(); - const controller = new AbortController(); - let operationSignal; - const acquired = searchInFlightRegistry.acquire( - 'same-key', - (signal) => { - operationSignal = signal; - return operation.promise; - }, - { - signal: controller.signal - } - ); - await Promise.resolve(); - - controller.abort(new searchDeadline.SearchDeadlineExceededError()); - - await expect(acquired.promise).rejects.toMatchObject({ - code: 'SEARCH_TIMEOUT' - }); - expect(operationSignal.reason).toMatchObject({ - code: 'SEARCH_OPERATION_ABANDONED' - }); - expect(searchInFlightRegistry.getState().operationCount).toBe(0); - - const replacementOperation = jest.fn(() => Promise.resolve('replacement')); - const replacement = searchInFlightRegistry.acquire('same-key', replacementOperation); - - expect(replacement.shared).toBe(false); - await expect(replacement.promise).resolves.toBe('replacement'); - expect(replacementOperation).toHaveBeenCalledTimes(1); - - operation.reject(operationSignal.reason); - await expect(operation.promise).rejects.toMatchObject({ - code: 'SEARCH_OPERATION_ABANDONED' - }); - }); - - test('does not start work for an already-aborted waiter', () => { - const controller = new AbortController(); - const createOperation = jest.fn(); - controller.abort(new searchDeadline.SearchDeadlineExceededError()); - - expect(() => - searchInFlightRegistry.acquire('same-key', createOperation, { - signal: controller.signal - }) - ).toThrow(searchDeadline.SearchDeadlineExceededError); - expect(createOperation).not.toHaveBeenCalled(); - }); - - test('buildKey preserves operation type boundaries', () => { - expect(searchInFlightRegistry.buildKey('candidate', ['CVE-2024-0001'])).not.toBe( - searchInFlightRegistry.buildKey('pageFetch', ['CVE-2024-0001']) - ); - }); -}); diff --git a/utils/searchPerformance.js b/utils/searchPerformance.js deleted file mode 100644 index 7b72bb6..0000000 --- a/utils/searchPerformance.js +++ /dev/null @@ -1,78 +0,0 @@ -const searchProvider = require('./searchProvider'); -const searchPostFilters = require('./searchPostFilters'); - -function markTime() { - return process.hrtime.bigint(); -} - -function elapsedMs(start) { - return Number(process.hrtime.bigint() - start) / 1e6; -} - -function roundMs(value) { - return value === undefined ? undefined : Math.round(value * 100) / 100; -} - -function getCacheStatus(metrics, postFilters) { - if (!searchPostFilters.hasCpePostFilters(postFilters)) return 'not_applicable'; - if (metrics.cacheHit === true) return 'hit'; - if (metrics.inFlightHit === true) return 'in_flight'; - - return 'miss'; -} - -function logSearchPerformance(metrics, searchOptions, responseOptions, postFilters) { - const outcome = metrics.outcome ?? 'success'; - const includeDetailedTimings = searchProvider.shouldLogSearchPerformance(); - if (outcome === 'success' && !includeDetailedTimings) return; - - const event = { - event: 'search.performance', - route: '/search', - requestId: metrics.requestId, - outcome, - statusCode: metrics.statusCode, - timeoutSource: metrics.timeoutSource, - abortReason: metrics.abortReason, - strategy: metrics.strategy, - cacheStatus: getCacheStatus(metrics, postFilters), - coldSearchQueueWaitElapsedMs: roundMs(metrics.coldSearchQueueWaitElapsedMs), - coldSearchQueueState: metrics.coldSearchQueueState, - totalElapsedMs: roundMs(metrics.totalElapsedMs) - }; - - if (includeDetailedTimings) { - Object.assign(event, { - optimizedFallback: metrics.optimizedFallback === true, - cacheHit: metrics.cacheHit === true, - inFlightHit: metrics.inFlightHit === true, - pageFetchInFlightHit: metrics.pageFetchInFlightHit === true, - openSearchOperations: metrics.openSearchOperations ?? [], - maxOpenSearchActiveRequests: metrics.maxOpenSearchActiveRequests, - postFilters: Object.keys(postFilters ?? {}), - requestedFrom: responseOptions?.from ?? searchOptions?.from, - requestedSize: responseOptions?.resultsPerPage ?? searchOptions?.resultsPerPage, - openSearchSize: searchOptions?.resultsPerPage, - candidateCount: metrics.candidateCount, - filteredCount: metrics.filteredCount, - returnedCount: metrics.returnedCount, - openSearchTookMs: metrics.openSearchTookMs, - pageFetchTookMs: metrics.pageFetchTookMs, - openSearchElapsedMs: roundMs(metrics.openSearchElapsedMs), - postFilterElapsedMs: roundMs(metrics.postFilterElapsedMs), - inFlightWaitElapsedMs: roundMs(metrics.inFlightWaitElapsedMs), - pageFetchElapsedMs: roundMs(metrics.pageFetchElapsedMs), - pageFetchWaitElapsedMs: roundMs(metrics.pageFetchWaitElapsedMs) - }); - } - - console.log(JSON.stringify(event)); -} - -module.exports = { - elapsedMs, - getCacheStatus, - logSearchPerformance, - markTime, - roundMs -}; diff --git a/utils/searchPerformance.test.js b/utils/searchPerformance.test.js deleted file mode 100644 index 221521f..0000000 --- a/utils/searchPerformance.test.js +++ /dev/null @@ -1,174 +0,0 @@ -const mockShouldLogSearchPerformance = jest.fn(); - -jest.mock('./searchProvider', () => ({ - shouldLogSearchPerformance: mockShouldLogSearchPerformance -})); - -const searchPerformance = require('./searchPerformance'); - -describe('searchPerformance', () => { - beforeEach(() => { - mockShouldLogSearchPerformance.mockReset(); - jest.spyOn(console, 'log').mockImplementation(() => {}); - }); - - afterEach(() => { - jest.restoreAllMocks(); - }); - - test('logSearchPerformance emits redacted search timing metadata when enabled', () => { - mockShouldLogSearchPerformance.mockReturnValue(true); - - searchPerformance.logSearchPerformance( - { - requestId: 'request-123', - outcome: 'success', - statusCode: 200, - strategy: 'cpe-candidate', - cacheHit: true, - inFlightHit: true, - pageFetchInFlightHit: true, - openSearchOperations: ['cpe-candidate', 'page-fetch'], - maxOpenSearchActiveRequests: 3, - candidateCount: 10000, - filteredCount: 4, - returnedCount: 1, - openSearchTookMs: 97, - pageFetchTookMs: 8, - openSearchElapsedMs: 123.456, - postFilterElapsedMs: 7.891, - coldSearchQueueWaitElapsedMs: 2.345, - coldSearchQueueState: { - activeCount: 1, - queuedCount: 0, - maxConcurrent: 4 - }, - inFlightWaitElapsedMs: 5.678, - pageFetchElapsedMs: 10.234, - pageFetchWaitElapsedMs: 3.456, - totalElapsedMs: 150.567 - }, - { - resultsPerPage: 10000, - from: 0 - }, - { - resultsPerPage: 1, - from: 0 - }, - { - virtualMatchStringRange: {} - } - ); - - expect(console.log).toHaveBeenCalledTimes(1); - expect(JSON.parse(console.log.mock.calls[0][0])).toStrictEqual({ - event: 'search.performance', - route: '/search', - requestId: 'request-123', - outcome: 'success', - statusCode: 200, - strategy: 'cpe-candidate', - cacheStatus: 'hit', - optimizedFallback: false, - cacheHit: true, - inFlightHit: true, - pageFetchInFlightHit: true, - openSearchOperations: ['cpe-candidate', 'page-fetch'], - maxOpenSearchActiveRequests: 3, - postFilters: ['virtualMatchStringRange'], - requestedFrom: 0, - requestedSize: 1, - openSearchSize: 10000, - candidateCount: 10000, - filteredCount: 4, - returnedCount: 1, - openSearchTookMs: 97, - pageFetchTookMs: 8, - openSearchElapsedMs: 123.46, - postFilterElapsedMs: 7.89, - coldSearchQueueWaitElapsedMs: 2.35, - coldSearchQueueState: { - activeCount: 1, - queuedCount: 0, - maxConcurrent: 4 - }, - inFlightWaitElapsedMs: 5.68, - pageFetchElapsedMs: 10.23, - pageFetchWaitElapsedMs: 3.46, - totalElapsedMs: 150.57 - }); - }); - - test('logSearchPerformance does not emit logs when disabled', () => { - mockShouldLogSearchPerformance.mockReturnValue(false); - - searchPerformance.logSearchPerformance( - { - outcome: 'success' - }, - { resultsPerPage: 1, from: 0 }, - undefined, - {} - ); - - expect(console.log).not.toHaveBeenCalled(); - }); - - test.each([{ cvss: {} }, { ssvc: {} }, { kev: {} }])( - 'getCacheStatus reports non-CPE semantic filters as not applicable', - (postFilters) => { - expect(searchPerformance.getCacheStatus({ cacheHit: false, inFlightHit: false }, postFilters)).toBe( - 'not_applicable' - ); - } - ); - - test('logSearchPerformance always emits sanitized failure outcomes', () => { - mockShouldLogSearchPerformance.mockReturnValue(false); - - searchPerformance.logSearchPerformance( - { - requestId: 'request-456', - outcome: 'queue_timeout', - statusCode: 429, - strategy: 'cpe-candidate', - coldSearchQueueWaitElapsedMs: 10000.123, - coldSearchQueueState: { - activeCount: 10, - queuedCount: 499, - maxConcurrent: 10, - maxQueueDepth: 500, - maxWaitMs: 10000 - }, - totalElapsedMs: 10001.456 - }, - undefined, - undefined, - { - virtualMatchString: 'sensitive-value' - } - ); - - expect(console.log).toHaveBeenCalledTimes(1); - expect(JSON.parse(console.log.mock.calls[0][0])).toStrictEqual({ - event: 'search.performance', - route: '/search', - requestId: 'request-456', - outcome: 'queue_timeout', - statusCode: 429, - strategy: 'cpe-candidate', - cacheStatus: 'miss', - coldSearchQueueWaitElapsedMs: 10000.12, - coldSearchQueueState: { - activeCount: 10, - queuedCount: 499, - maxConcurrent: 10, - maxQueueDepth: 500, - maxWaitMs: 10000 - }, - totalElapsedMs: 10001.46 - }); - expect(console.log.mock.calls[0][0]).not.toContain('sensitive-value'); - }); -}); diff --git a/utils/searchPostFilterCache.js b/utils/searchPostFilterCache.js deleted file mode 100644 index f7ed9d8..0000000 --- a/utils/searchPostFilterCache.js +++ /dev/null @@ -1,73 +0,0 @@ -const searchProvider = require('./searchProvider'); - -const entries = new Map(); - -function normalizeForKey(value) { - if (Array.isArray(value)) return value.map(normalizeForKey); - - if (value && typeof value === 'object') { - return Object.keys(value) - .sort() - .reduce((normalized, key) => { - if (value[key] !== undefined) { - normalized[key] = normalizeForKey(value[key]); - } - return normalized; - }, {}); - } - - return value; -} - -function buildCacheKey(candidateBody, postFilters) { - return JSON.stringify( - normalizeForKey({ - candidateBody, - postFilters - }) - ); -} - -function get(cacheKey, now = Date.now()) { - const entry = entries.get(cacheKey); - if (!entry) return null; - - if (entry.expiresAt <= now) { - entries.delete(cacheKey); - return null; - } - - entries.delete(cacheKey); - entries.set(cacheKey, entry); - - return entry.value; -} - -function set(cacheKey, value, now = Date.now()) { - const ttlMs = searchProvider.getPostFilterCacheTtlMs(); - const maxEntries = searchProvider.getPostFilterCacheMaxEntries(); - - if (!maxEntries || maxEntries < 1 || !ttlMs || ttlMs < 1) return; - - entries.set(cacheKey, { - expiresAt: now + ttlMs, - value - }); - - while (entries.size > maxEntries) { - const oldestKey = entries.keys().next().value; - entries.delete(oldestKey); - } -} - -function clear() { - entries.clear(); -} - -module.exports = { - buildCacheKey, - clear, - get, - normalizeForKey, - set -}; diff --git a/utils/searchPostFilterCache.test.js b/utils/searchPostFilterCache.test.js deleted file mode 100644 index 77aff79..0000000 --- a/utils/searchPostFilterCache.test.js +++ /dev/null @@ -1,61 +0,0 @@ -const searchPostFilterCache = require('./searchPostFilterCache'); - -describe('searchPostFilterCache', () => { - beforeEach(() => { - searchPostFilterCache.clear(); - }); - - test('buildCacheKey is stable for equivalent object key ordering', () => { - const left = searchPostFilterCache.buildCacheKey( - { - query: { - bool: { - filter: [{ term: { b: '2' } }], - must: [] - } - }, - size: 10000, - from: 0 - }, - { - virtualMatchStringRange: { - versionEnd: '2.6', - versionStart: '2.2' - } - } - ); - const right = searchPostFilterCache.buildCacheKey( - { - from: 0, - size: 10000, - query: { - bool: { - must: [], - filter: [{ term: { b: '2' } }] - } - } - }, - { - virtualMatchStringRange: { - versionStart: '2.2', - versionEnd: '2.6' - } - } - ); - - expect(left).toBe(right); - }); - - test('get returns cached values until the entry expires', () => { - const key = 'cache-key'; - const value = { - candidateCount: 2, - filteredCveIds: ['CVE-2024-0001'] - }; - - searchPostFilterCache.set(key, value, 1000); - - expect(searchPostFilterCache.get(key, 1001)).toBe(value); - expect(searchPostFilterCache.get(key, 61001)).toBeNull(); - }); -}); diff --git a/utils/searchPostFilterWorkQueue.js b/utils/searchPostFilterWorkQueue.js deleted file mode 100644 index 8180f97..0000000 --- a/utils/searchPostFilterWorkQueue.js +++ /dev/null @@ -1,149 +0,0 @@ -const searchProvider = require('./searchProvider'); -const searchDeadline = require('./searchDeadline'); - -let activeCount = 0; -const queue = []; - -class SearchPostFilterQueueFullError extends Error { - constructor(state) { - super('CPE post-filter cold search queue is full.'); - this.name = 'SearchPostFilterQueueFullError'; - this.code = 'SEARCH_QUEUE_FULL'; - this.state = state; - } -} - -class SearchPostFilterQueueTimeoutError extends Error { - constructor(state, waitElapsedMs) { - super('CPE post-filter cold search queue wait timed out.'); - this.name = 'SearchPostFilterQueueTimeoutError'; - this.code = 'SEARCH_QUEUE_TIMEOUT'; - this.state = state; - this.waitElapsedMs = waitElapsedMs; - } -} - -function getMaxConcurrent() { - return searchProvider.getPostFilterColdSearchConcurrency(); -} - -function getMaxQueueDepth() { - return searchProvider.getPostFilterColdSearchMaxQueueDepth(); -} - -function getMaxWaitMs() { - return searchProvider.getPostFilterColdSearchMaxWaitMs(); -} - -function getState() { - return { - activeCount, - queuedCount: queue.length, - maxConcurrent: getMaxConcurrent(), - maxQueueDepth: getMaxQueueDepth(), - maxWaitMs: getMaxWaitMs() - }; -} - -function isQueueFullError(err) { - return err instanceof SearchPostFilterQueueFullError || err?.code === 'SEARCH_QUEUE_FULL'; -} - -function isQueueTimeoutError(err) { - return err instanceof SearchPostFilterQueueTimeoutError || err?.code === 'SEARCH_QUEUE_TIMEOUT'; -} - -function drain() { - const maxConcurrent = getMaxConcurrent(); - - while (activeCount < maxConcurrent && queue.length) { - const item = queue.shift(); - clearTimeout(item.timeoutId); - item.signal?.removeEventListener('abort', item.onAbort); - activeCount += 1; - - void (async () => { - try { - searchDeadline.throwIfAborted(item.signal); - item.resolve(await item.task(item.signal)); - } catch (err) { - item.reject(err); - } finally { - activeCount -= 1; - drain(); - } - })(); - } -} - -function run(task, { signal } = {}) { - return new Promise((resolve, reject) => { - try { - searchDeadline.throwIfAborted(signal); - } catch (err) { - reject(err); - return; - } - - if (activeCount >= getMaxConcurrent() && queue.length >= getMaxQueueDepth()) { - reject(new SearchPostFilterQueueFullError(getState())); - return; - } - - const item = { - task, - resolve, - reject, - queuedAt: process.hrtime.bigint(), - timeoutId: undefined, - signal, - onAbort: undefined - }; - - if (activeCount >= getMaxConcurrent()) { - item.onAbort = () => { - const queueIndex = queue.indexOf(item); - if (queueIndex === -1) return; - - queue.splice(queueIndex, 1); - clearTimeout(item.timeoutId); - reject(searchDeadline.getAbortReason(signal)); - drain(); - }; - signal?.addEventListener('abort', item.onAbort, { once: true }); - item.timeoutId = setTimeout(() => { - const queueIndex = queue.indexOf(item); - if (queueIndex === -1) return; - - queue.splice(queueIndex, 1); - signal?.removeEventListener('abort', item.onAbort); - const waitElapsedMs = Number(process.hrtime.bigint() - item.queuedAt) / 1e6; - reject(new SearchPostFilterQueueTimeoutError(getState(), waitElapsedMs)); - drain(); - }, getMaxWaitMs()); - item.timeoutId.unref?.(); - } - - queue.push(item); - drain(); - }); -} - -function resetForTests() { - activeCount = 0; - queue.forEach((item) => { - clearTimeout(item.timeoutId); - item.signal?.removeEventListener('abort', item.onAbort); - }); - queue.length = 0; -} - -module.exports = { - SearchPostFilterQueueFullError, - SearchPostFilterQueueTimeoutError, - getState, - isQueueFullError, - isQueueTimeoutError, - resetForTests, - run -}; diff --git a/utils/searchPostFilterWorkQueue.test.js b/utils/searchPostFilterWorkQueue.test.js deleted file mode 100644 index 02ef0b1..0000000 --- a/utils/searchPostFilterWorkQueue.test.js +++ /dev/null @@ -1,200 +0,0 @@ -const mockGetPostFilterColdSearchConcurrency = jest.fn(); -const mockGetPostFilterColdSearchMaxQueueDepth = jest.fn(); -const mockGetPostFilterColdSearchMaxWaitMs = jest.fn(); - -jest.mock('./searchProvider', () => ({ - getPostFilterColdSearchConcurrency: mockGetPostFilterColdSearchConcurrency, - getPostFilterColdSearchMaxQueueDepth: mockGetPostFilterColdSearchMaxQueueDepth, - getPostFilterColdSearchMaxWaitMs: mockGetPostFilterColdSearchMaxWaitMs -})); - -const searchPostFilterWorkQueue = require('./searchPostFilterWorkQueue'); -const searchDeadline = require('./searchDeadline'); - -function deferred() { - const state = {}; - state.promise = new Promise((resolve, reject) => { - state.resolve = resolve; - state.reject = reject; - }); - return state; -} - -function captureTimers() { - const timers = []; - jest.spyOn(global, 'setTimeout').mockImplementation((callback, delay) => { - const timer = { - unref: jest.fn() - }; - timers.push({ - callback, - delay, - timer - }); - return timer; - }); - jest.spyOn(global, 'clearTimeout').mockImplementation(() => {}); - - return timers; -} - -describe('searchPostFilterWorkQueue', () => { - beforeEach(() => { - searchPostFilterWorkQueue.resetForTests(); - mockGetPostFilterColdSearchConcurrency.mockReset(); - mockGetPostFilterColdSearchMaxQueueDepth.mockReset(); - mockGetPostFilterColdSearchMaxWaitMs.mockReset(); - mockGetPostFilterColdSearchConcurrency.mockReturnValue(1); - mockGetPostFilterColdSearchMaxQueueDepth.mockReturnValue(1); - mockGetPostFilterColdSearchMaxWaitMs.mockReturnValue(10000); - }); - - afterEach(() => { - searchPostFilterWorkQueue.resetForTests(); - jest.restoreAllMocks(); - }); - - test('runs no more than the configured number of cold searches concurrently', async () => { - const first = deferred(); - const second = deferred(); - const firstTask = jest.fn(() => first.promise); - const secondTask = jest.fn(() => second.promise); - - const firstRun = searchPostFilterWorkQueue.run(firstTask); - const secondRun = searchPostFilterWorkQueue.run(secondTask); - await Promise.resolve(); - - expect(firstTask).toHaveBeenCalledTimes(1); - expect(secondTask).not.toHaveBeenCalled(); - expect(searchPostFilterWorkQueue.getState()).toStrictEqual({ - activeCount: 1, - queuedCount: 1, - maxConcurrent: 1, - maxQueueDepth: 1, - maxWaitMs: 10000 - }); - - first.resolve('first'); - await firstRun; - await Promise.resolve(); - - expect(secondTask).toHaveBeenCalledTimes(1); - second.resolve('second'); - - await expect(secondRun).resolves.toBe('second'); - }); - - test('continues draining after a cold search rejects', async () => { - const first = deferred(); - const secondTask = jest.fn(() => 'second'); - - const firstRun = searchPostFilterWorkQueue.run(() => first.promise); - const secondRun = searchPostFilterWorkQueue.run(secondTask); - await Promise.resolve(); - - first.reject(new Error('failed')); - await expect(firstRun).rejects.toThrow('failed'); - await expect(secondRun).resolves.toBe('second'); - }); - - test('rejects new cold searches when the waiting queue is full', async () => { - const first = deferred(); - - const firstRun = searchPostFilterWorkQueue.run(() => first.promise); - const secondRun = searchPostFilterWorkQueue.run(() => 'second'); - await Promise.resolve(); - - await expect(searchPostFilterWorkQueue.run(() => 'third')).rejects.toMatchObject({ - code: 'SEARCH_QUEUE_FULL', - state: { - activeCount: 1, - queuedCount: 1, - maxConcurrent: 1, - maxQueueDepth: 1, - maxWaitMs: 10000 - } - }); - - first.resolve('first'); - await expect(firstRun).resolves.toBe('first'); - await expect(secondRun).resolves.toBe('second'); - }); - - test('allows immediate execution when queue depth is zero and concurrency is available', async () => { - mockGetPostFilterColdSearchMaxQueueDepth.mockReturnValue(0); - - await expect(searchPostFilterWorkQueue.run(() => 'first')).resolves.toBe('first'); - }); - - test('rejects queued work that exceeds the configured maximum wait', async () => { - const timers = captureTimers(); - const first = deferred(); - const secondTask = jest.fn(() => 'second'); - - const firstRun = searchPostFilterWorkQueue.run(() => first.promise); - const secondRun = searchPostFilterWorkQueue.run(secondTask); - const timeoutExpectation = expect(secondRun).rejects.toMatchObject({ - code: 'SEARCH_QUEUE_TIMEOUT', - waitElapsedMs: expect.any(Number), - state: { - activeCount: 1, - queuedCount: 0, - maxConcurrent: 1, - maxQueueDepth: 1, - maxWaitMs: 10000 - } - }); - await Promise.resolve(); - - timers.find(({ delay }) => delay === 10000).callback(); - await timeoutExpectation; - expect(secondTask).not.toHaveBeenCalled(); - - first.resolve('first'); - await expect(firstRun).resolves.toBe('first'); - }); - - test('removes queued work when its operation is aborted', async () => { - const first = deferred(); - const controller = new AbortController(); - const secondTask = jest.fn(); - - const firstRun = searchPostFilterWorkQueue.run(() => first.promise); - const secondRun = searchPostFilterWorkQueue.run(secondTask, { - signal: controller.signal - }); - await Promise.resolve(); - - controller.abort(new searchDeadline.SearchDeadlineExceededError()); - - await expect(secondRun).rejects.toMatchObject({ - code: 'SEARCH_TIMEOUT' - }); - expect(secondTask).not.toHaveBeenCalled(); - expect(searchPostFilterWorkQueue.getState().queuedCount).toBe(0); - - first.resolve('first'); - await expect(firstRun).resolves.toBe('first'); - }); - - test('passes the operation signal to active work', async () => { - const controller = new AbortController(); - const task = jest.fn( - (signal) => - new Promise((resolve, reject) => { - signal.addEventListener('abort', () => reject(signal.reason), { once: true }); - }) - ); - const run = searchPostFilterWorkQueue.run(task, { - signal: controller.signal - }); - await Promise.resolve(); - - controller.abort(new searchDeadline.SearchDeadlineExceededError()); - - await expect(run).rejects.toMatchObject({ - code: 'SEARCH_TIMEOUT' - }); - expect(task).toHaveBeenCalledWith(controller.signal); - }); -}); diff --git a/utils/searchProvider.js b/utils/searchProvider.js index e50b3a6..98e373d 100644 --- a/utils/searchProvider.js +++ b/utils/searchProvider.js @@ -14,8 +14,6 @@ class SearchProviderContractError extends Error { let searchProviderConfig; let basicSearchManager; let searchReader; -let activeOpenSearchRequestCount = 0; -const activeOpenSearchRequestsByOperation = new Map(); function configureClientRequestTimeout(client) { if (client?.transport) { @@ -120,31 +118,6 @@ function getSearchReader() { return searchReader; } -function getOpenSearchRequestState() { - return { - activeCount: activeOpenSearchRequestCount, - activeByOperation: Object.fromEntries(activeOpenSearchRequestsByOperation) - }; -} - -function beginOpenSearchRequest(operation) { - activeOpenSearchRequestCount += 1; - activeOpenSearchRequestsByOperation.set(operation, (activeOpenSearchRequestsByOperation.get(operation) ?? 0) + 1); - - return getOpenSearchRequestState(); -} - -function endOpenSearchRequest(operation) { - activeOpenSearchRequestCount = Math.max(0, activeOpenSearchRequestCount - 1); - - const operationCount = activeOpenSearchRequestsByOperation.get(operation) ?? 0; - if (operationCount <= 1) { - activeOpenSearchRequestsByOperation.delete(operation); - } else { - activeOpenSearchRequestsByOperation.set(operation, operationCount - 1); - } -} - function awaitAbortableRequest(request, signal) { if (!signal) return Promise.resolve(request); @@ -177,21 +150,13 @@ function awaitAbortableRequest(request, signal) { }); } -async function executeOpenSearchRequest(createRequest, { signal, operation, onStart } = {}) { +async function executeOpenSearchRequest(createRequest, { signal } = {}) { searchDeadline.throwIfAborted(signal); - const state = beginOpenSearchRequest(operation); - try { - onStart?.(state); - const response = await awaitAbortableRequest(createRequest(), signal); - - return response; - } finally { - endOpenSearchRequest(operation); - } + return awaitAbortableRequest(createRequest(), signal); } -async function searchOpenSearch(body, { signal, operation = 'search', onStart } = {}) { +async function searchOpenSearch(body, { signal } = {}) { searchDeadline.throwIfAborted(signal); const reader = getSearchReader(); const response = await executeOpenSearchRequest( @@ -207,11 +172,7 @@ async function searchOpenSearch(body, { signal, operation = 'search', onStart } requestTimeout: getOpenSearchRequestTimeoutMs() } ), - { - signal, - operation, - onStart - } + { signal } ); return searchResponseValidator.validateOpenSearchResponse(response.body); @@ -240,10 +201,7 @@ async function checkOpenSearchReadiness({ signal } = {}) { requestTimeout: getOpenSearchReadinessTimeoutMs() } ), - { - signal, - operation: 'readiness' - } + { signal } ); searchResponseValidator.validateOpenSearchResponse(response.body); @@ -261,44 +219,6 @@ function getOpenSearchReadinessTimeoutMs() { }); } -function shouldLogSearchPerformance() { - if (!AppConfig.has('search.logPerformanceTimings')) return false; - - return normalizeBoolean(AppConfig.get('search.logPerformanceTimings')); -} - -function getPostFilterCacheTtlMs() { - return getIntegerConfig('search.postFilterCacheTtlMs', constants.SEARCH_POST_FILTER_CACHE_TTL_MS); -} - -function getPostFilterCacheMaxEntries() { - return getIntegerConfig('search.postFilterCacheMaxEntries', constants.SEARCH_POST_FILTER_CACHE_MAX_ENTRIES); -} - -function getPostFilterColdSearchConcurrency() { - return getIntegerConfig( - 'search.postFilterColdSearchConcurrency', - constants.SEARCH_POST_FILTER_COLD_SEARCH_CONCURRENCY, - { min: 1 } - ); -} - -function getPostFilterColdSearchMaxQueueDepth() { - return getIntegerConfig( - 'search.postFilterColdSearchMaxQueueDepth', - constants.SEARCH_POST_FILTER_COLD_SEARCH_MAX_QUEUE_DEPTH, - { min: 0 } - ); -} - -function getPostFilterColdSearchMaxWaitMs() { - return getIntegerConfig( - 'search.postFilterColdSearchMaxWaitMs', - constants.SEARCH_POST_FILTER_COLD_SEARCH_MAX_WAIT_MS, - { min: 1 } - ); -} - function getOpenSearchRequestTimeoutMs() { return getIntegerConfig('search.openSearchRequestTimeoutMs', constants.SEARCH_OPEN_SEARCH_REQUEST_TIMEOUT_MS, { min: 1 @@ -313,8 +233,6 @@ function resetSearchProviderForTests() { searchProviderConfig = undefined; basicSearchManager = undefined; searchReader = undefined; - activeOpenSearchRequestCount = 0; - activeOpenSearchRequestsByOperation.clear(); } module.exports = { @@ -322,13 +240,7 @@ module.exports = { getBasicSearchManager, getAllowUnknownSslCerts, getIntegerConfig, - getOpenSearchRequestState, getOpenSearchReadinessTimeoutMs, - getPostFilterCacheMaxEntries, - getPostFilterCacheTtlMs, - getPostFilterColdSearchConcurrency, - getPostFilterColdSearchMaxQueueDepth, - getPostFilterColdSearchMaxWaitMs, getOpenSearchRequestTimeoutMs, getSearchRequestDeadlineMs, getSearchProviderConfig, @@ -337,7 +249,6 @@ module.exports = { normalizeBoolean, resetSearchProviderForTests, searchOpenSearch, - shouldLogSearchPerformance, validateSearchReader, SearchProviderContractError }; diff --git a/utils/searchProvider.test.js b/utils/searchProvider.test.js index b26888f..d18f6e5 100644 --- a/utils/searchProvider.test.js +++ b/utils/searchProvider.test.js @@ -132,11 +132,7 @@ describe('searchProvider', () => { match_all: {} } }; - const onStart = jest.fn(); - const result = await searchProvider.searchOpenSearch(body, { - operation: 'standard', - onStart - }); + const result = await searchProvider.searchOpenSearch(body); expect(first).toBe(second); expect(mockSearchReader).toHaveBeenCalledTimes(1); @@ -157,16 +153,6 @@ describe('searchProvider', () => { expect(first._client.transport.maxRetries).toBe(0); expect(first._client.transport.requestTimeout).toBe(20000); expect(result).toStrictEqual(openSearchResponse().body); - expect(onStart).toHaveBeenCalledWith({ - activeCount: 1, - activeByOperation: { - standard: 1 - } - }); - expect(searchProvider.getOpenSearchRequestState()).toStrictEqual({ - activeCount: 0, - activeByOperation: {} - }); }); test('checks OpenSearch readiness through the configured CVE index', async () => { @@ -192,10 +178,6 @@ describe('searchProvider', () => { requestTimeout: 5000 } ); - expect(searchProvider.getOpenSearchRequestState()).toStrictEqual({ - activeCount: 0, - activeByOperation: {} - }); }); test('fails readiness when the configured CVE index is unavailable', async () => { @@ -222,10 +204,6 @@ describe('searchProvider', () => { code: 'SEARCH_TIMEOUT' }); expect(request.abort).toHaveBeenCalledTimes(1); - expect(searchProvider.getOpenSearchRequestState()).toStrictEqual({ - activeCount: 0, - activeByOperation: {} - }); }); test('does not construct a SearchReader for an already-canceled readiness check', async () => { @@ -269,8 +247,7 @@ describe('searchProvider', () => { } }, { - signal: controller.signal, - operation: 'standard' + signal: controller.signal } ); await Promise.resolve(); @@ -281,56 +258,6 @@ describe('searchProvider', () => { code: 'SEARCH_TIMEOUT' }); expect(request.abort).toHaveBeenCalledTimes(1); - expect(searchProvider.getOpenSearchRequestState()).toStrictEqual({ - activeCount: 0, - activeByOperation: {} - }); - }); - - test('tracks concurrent OpenSearch requests by operation type', async () => { - const candidate = deferred(); - const pageFetch = deferred(); - const candidateStart = jest.fn(); - const pageFetchStart = jest.fn(); - mockClientSearch.mockReturnValueOnce(candidate.promise).mockReturnValueOnce(pageFetch.promise); - - const candidateSearch = searchProvider.searchOpenSearch( - {}, - { - operation: 'cpe-candidate', - onStart: candidateStart - } - ); - const pageSearch = searchProvider.searchOpenSearch( - {}, - { - operation: 'page-fetch', - onStart: pageFetchStart - } - ); - - expect(candidateStart).toHaveBeenCalledWith({ - activeCount: 1, - activeByOperation: { - 'cpe-candidate': 1 - } - }); - expect(pageFetchStart).toHaveBeenCalledWith({ - activeCount: 2, - activeByOperation: { - 'cpe-candidate': 1, - 'page-fetch': 1 - } - }); - - candidate.resolve(openSearchResponse()); - pageFetch.resolve(openSearchResponse()); - await Promise.all([candidateSearch, pageSearch]); - - expect(searchProvider.getOpenSearchRequestState()).toStrictEqual({ - activeCount: 0, - activeByOperation: {} - }); }); test('does not start an OpenSearch request for an already-aborted signal', async () => { @@ -390,33 +317,12 @@ describe('searchProvider', () => { expect(searchProvider.getSearchProviderConfig().allowUnknownSslCerts).toBe(true); }); - test('reads search performance timing flag from config', () => { - mockAppConfigHas.mockImplementation((key) => key === 'search.logPerformanceTimings'); - mockAppConfigGet.mockImplementation((key) => (key === 'search.logPerformanceTimings' ? 'TRUE' : undefined)); - - expect(searchProvider.shouldLogSearchPerformance()).toBe(true); - }); - - test('reads CPE post-filter runtime tuning from config', () => { + test('reads search timeout runtime settings from config', () => { mockAppConfigHas.mockImplementation((key) => - [ - 'search.postFilterCacheTtlMs', - 'search.postFilterCacheMaxEntries', - 'search.postFilterColdSearchConcurrency', - 'search.postFilterColdSearchMaxQueueDepth', - 'search.postFilterColdSearchMaxWaitMs', - 'search.openSearchRequestTimeoutMs', - 'search.requestDeadlineMs', - 'search.readinessTimeoutMs' - ].includes(key) + ['search.openSearchRequestTimeoutMs', 'search.requestDeadlineMs', 'search.readinessTimeoutMs'].includes(key) ); mockAppConfigGet.mockImplementation((key) => { const values = { - 'search.postFilterCacheTtlMs': '120000', - 'search.postFilterCacheMaxEntries': '250', - 'search.postFilterColdSearchConcurrency': '6', - 'search.postFilterColdSearchMaxQueueDepth': '50', - 'search.postFilterColdSearchMaxWaitMs': '15000', 'search.openSearchRequestTimeoutMs': '25000', 'search.requestDeadlineMs': '45000', 'search.readinessTimeoutMs': '7000' @@ -424,25 +330,15 @@ describe('searchProvider', () => { return values[key]; }); - expect(searchProvider.getPostFilterCacheTtlMs()).toBe(120000); - expect(searchProvider.getPostFilterCacheMaxEntries()).toBe(250); - expect(searchProvider.getPostFilterColdSearchConcurrency()).toBe(6); - expect(searchProvider.getPostFilterColdSearchMaxQueueDepth()).toBe(50); - expect(searchProvider.getPostFilterColdSearchMaxWaitMs()).toBe(15000); expect(searchProvider.getOpenSearchRequestTimeoutMs()).toBe(25000); expect(searchProvider.getSearchRequestDeadlineMs()).toBe(45000); expect(searchProvider.getOpenSearchReadinessTimeoutMs()).toBe(7000); }); - test('falls back to CPE post-filter runtime defaults when config values are invalid', () => { + test('falls back to search timeout defaults when config values are invalid', () => { mockAppConfigHas.mockReturnValue(true); mockAppConfigGet.mockReturnValue('not-a-number'); - expect(searchProvider.getPostFilterCacheTtlMs()).toBe(60000); - expect(searchProvider.getPostFilterCacheMaxEntries()).toBe(100); - expect(searchProvider.getPostFilterColdSearchConcurrency()).toBe(10); - expect(searchProvider.getPostFilterColdSearchMaxQueueDepth()).toBe(500); - expect(searchProvider.getPostFilterColdSearchMaxWaitMs()).toBe(10000); expect(searchProvider.getOpenSearchRequestTimeoutMs()).toBe(20000); expect(searchProvider.getSearchRequestDeadlineMs()).toBe(30000); expect(searchProvider.getOpenSearchReadinessTimeoutMs()).toBe(5000); diff --git a/utils/searchRateLimiter.js b/utils/searchRateLimiter.js deleted file mode 100644 index 00026e1..0000000 --- a/utils/searchRateLimiter.js +++ /dev/null @@ -1,100 +0,0 @@ -const { AppConfig } = require('cve-core'); -const { rateLimit } = require('express-rate-limit'); -const errors = require('./errors'); -const searchPerformance = require('./searchPerformance'); - -const DEFAULT_SEARCH_RATE_LIMIT_ENABLED = false; -const DEFAULT_SEARCH_RATE_LIMIT_WINDOW_MS = 60000; -const GLOBAL_SEARCH_RATE_LIMIT_KEY = 'search-api-process'; - -const error = new errors.errors(); - -function parseBoolean(value) { - if (typeof value === 'boolean') return value; - - const normalized = String(value).trim().toLowerCase(); - if (['true', '1', 'yes', 'y', 'on'].includes(normalized)) return true; - if (['false', '0', 'no', 'n', 'off'].includes(normalized)) return false; - - return undefined; -} - -function readBooleanConfig(path, defaultValue) { - if (!AppConfig.has(path)) return defaultValue; - - const value = parseBoolean(AppConfig.get(path)); - if (value === undefined) throw new Error(`${path} must be a boolean.`); - - return value; -} - -function readPositiveIntegerConfig(path, defaultValue) { - if (!AppConfig.has(path)) return defaultValue; - - const value = Number(AppConfig.get(path)); - if (!Number.isInteger(value) || value < 1) return undefined; - - return value; -} - -function getSearchRateLimitConfig() { - const enabled = readBooleanConfig('rateLimiting.search.enabled', DEFAULT_SEARCH_RATE_LIMIT_ENABLED); - if (!enabled) { - return Object.freeze({ - enabled, - windowMs: DEFAULT_SEARCH_RATE_LIMIT_WINDOW_MS - }); - } - - const windowMs = readPositiveIntegerConfig('rateLimiting.search.windowMs', DEFAULT_SEARCH_RATE_LIMIT_WINDOW_MS); - const maxRequests = readPositiveIntegerConfig('rateLimiting.search.maxRequests'); - - if (!windowMs) throw new Error('rateLimiting.search.windowMs must be a positive integer when enabled.'); - if (!maxRequests) throw new Error('rateLimiting.search.maxRequests must be a positive integer when enabled.'); - - return Object.freeze({ - enabled, - windowMs, - maxRequests - }); -} - -function disabledRateLimiter(_req, _res, next) { - next(); -} - -function handleRateLimit(req, res, _next, options) { - searchPerformance.logSearchPerformance( - { - requestId: req.requestId, - outcome: 'rate_limited', - statusCode: options.statusCode - }, - undefined, - undefined, - undefined - ); - - return res.status(options.statusCode).json(error.searchRateLimitedError()); -} - -function createSearchRateLimiter(config = getSearchRateLimitConfig()) { - if (!config.enabled) return disabledRateLimiter; - - return rateLimit({ - windowMs: config.windowMs, - limit: config.maxRequests, - standardHeaders: 'draft-8', - legacyHeaders: false, - identifier: GLOBAL_SEARCH_RATE_LIMIT_KEY, - keyGenerator: () => GLOBAL_SEARCH_RATE_LIMIT_KEY, - handler: handleRateLimit, - passOnStoreError: false - }); -} - -module.exports = { - createSearchRateLimiter, - getSearchRateLimitConfig, - handleRateLimit -}; diff --git a/utils/searchRateLimiter.test.js b/utils/searchRateLimiter.test.js deleted file mode 100644 index da00ea6..0000000 --- a/utils/searchRateLimiter.test.js +++ /dev/null @@ -1,127 +0,0 @@ -const express = require('express'); -const { request } = require('../test/http'); - -const mockAppConfigHas = jest.fn(); -const mockAppConfigGet = jest.fn(); -const mockLogSearchPerformance = jest.fn(); - -jest.mock('cve-core', () => ({ - AppConfig: { - get: mockAppConfigGet, - has: mockAppConfigHas - } -})); - -jest.mock('./searchPerformance', () => ({ - logSearchPerformance: mockLogSearchPerformance -})); - -const searchRateLimiter = require('./searchRateLimiter'); - -describe('searchRateLimiter', () => { - beforeEach(() => { - mockAppConfigHas.mockReset(); - mockAppConfigGet.mockReset(); - mockLogSearchPerformance.mockReset(); - mockAppConfigHas.mockReturnValue(false); - }); - - test('defaults to a disabled limiter that passes requests through', () => { - const next = jest.fn(); - const limiter = searchRateLimiter.createSearchRateLimiter(); - - limiter({}, {}, next); - - expect(next).toHaveBeenCalledTimes(1); - }); - - test('resolves enabled rate limit configuration from AppConfig', () => { - const values = { - 'rateLimiting.search.enabled': 'TRUE', - 'rateLimiting.search.windowMs': '30000', - 'rateLimiting.search.maxRequests': '25' - }; - mockAppConfigHas.mockImplementation((path) => Object.hasOwn(values, path)); - mockAppConfigGet.mockImplementation((path) => values[path]); - - expect(searchRateLimiter.getSearchRateLimitConfig()).toStrictEqual({ - enabled: true, - windowMs: 30000, - maxRequests: 25 - }); - }); - - test.each([ - [ - { - 'rateLimiting.search.enabled': 'invalid' - }, - 'rateLimiting.search.enabled must be a boolean.' - ], - [ - { - 'rateLimiting.search.enabled': 'TRUE', - 'rateLimiting.search.windowMs': '0', - 'rateLimiting.search.maxRequests': '25' - }, - 'rateLimiting.search.windowMs must be a positive integer when enabled.' - ], - [ - { - 'rateLimiting.search.enabled': 'TRUE', - 'rateLimiting.search.windowMs': '60000', - 'rateLimiting.search.maxRequests': '0' - }, - 'rateLimiting.search.maxRequests must be a positive integer when enabled.' - ] - ])('rejects invalid explicit configuration', (values, expectedMessage) => { - mockAppConfigHas.mockImplementation((path) => Object.hasOwn(values, path)); - mockAppConfigGet.mockImplementation((path) => values[path]); - - expect(() => searchRateLimiter.getSearchRateLimitConfig()).toThrow(expectedMessage); - }); - - test('enforces a process-wide limit with standard headers and sanitized telemetry', async () => { - const app = express(); - const limiter = searchRateLimiter.createSearchRateLimiter({ - enabled: true, - windowMs: 60000, - maxRequests: 1 - }); - - app.use((req, res, next) => { - req.requestId = 'request-123'; - next(); - }); - app.post('/search', limiter, (req, res) => res.status(200).json({ ok: true })); - - const accepted = await request(app, { - method: 'POST', - path: '/search' - }); - const rejected = await request(app, { - method: 'POST', - path: '/search' - }); - - expect(accepted.status).toBe(200); - expect(accepted.headers.ratelimit).toBeDefined(); - expect(accepted.headers['ratelimit-policy']).toBeDefined(); - expect(rejected.status).toBe(429); - expect(rejected.headers['retry-after']).toBeDefined(); - expect(rejected.body).toStrictEqual({ - error: 'SEARCH_RATE_LIMITED', - message: 'Too many search requests. Please retry later.' - }); - expect(mockLogSearchPerformance).toHaveBeenCalledWith( - { - requestId: 'request-123', - outcome: 'rate_limited', - statusCode: 429 - }, - undefined, - undefined, - undefined - ); - }); -});