Add date range filtering and fix test infrastructure#25
Open
Add date range filtering and fix test infrastructure#25
Conversation
Feature: - Add parseDateFilter() and filterByDateRange() to local-repository.ts - Support 'YYYY-MM-DD' (single day) and 'YYYY-MM-DD..YYYY-MM-DD' (range) filters - Improve Zod schema descriptions with examples in advisories.ts - Add E2E tests for date range filtering Fixes: - Preserve CodeQL fix: use %s format in local-server.ts error logging (not template literal) - Fix cross-platform path separator in unit test (use path.join) - Add mkdirSync before git clone in refresh-database.ts to prevent ENOENT - Update OpenTelemetry and AI SDK dependency versions Test infrastructure: - Add globalSetup.ts fixture that clones advisory-database before tests - Disable ADVISORY_REFRESH_ON_START in E2E test server to avoid fetch timeout - Add warmup call in List Advisories beforeAll to pre-build index cache - Wire globalSetup into vitest.config.ts
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Express req.query values can be string | string[] | undefined at runtime. The previous code used bare 'as string' TypeScript casts which don't validate at runtime, allowing arrays to flow through as unexpected types. Changes: - Add queryString() helper in local-server.ts to safely coerce query params to string (picks first element if array, returns undefined if not string) - Apply queryString() to all scalar query parameters - Add defense-in-depth in parseDateFilter() to handle array inputs - Validate sort/direction values with explicit equality checks instead of casts Fixes CodeQL alert: Type confusion through parameter tampering
There was a problem hiding this comment.
Pull request overview
This pull request adds date range filtering functionality to advisory queries and includes several test infrastructure improvements to enable reliable local development and CI. The PR preserves security fixes from PR #15 and addresses cross-platform compatibility issues in tests.
Changes:
- Implements date range filtering for the
publishedandupdatedparameters in two formats: single date (YYYY-MM-DD) and date range (YYYY-MM-DD..YYYY-MM-DD) - Adds global setup for E2E tests to clone the advisory database once before tests run, preventing timeout issues
- Fixes ENOENT error when cloning advisory database by creating parent directory first
- Updates several OpenTelemetry and AI SDK dependencies to their latest minor versions
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
src/datasources/local-repository.ts |
Adds parseDateFilter() and filterByDateRange() methods to support single-date and date-range filtering |
src/tools/advisories.ts |
Enhances Zod schema descriptions with format examples and usage hints for date filters |
src/local-server.ts |
Introduces queryString() helper for type-safe query parameter extraction and preserves CodeQL security fix |
src/refresh-database.ts |
Adds mkdirSync() call to create parent directory before git clone to prevent ENOENT errors |
test/e2e/globalSetup.ts |
New global setup fixture that clones advisory-database once before all tests |
test/e2e/mcp-server.test.ts |
Adds three new E2E tests for date filtering and warmup call to trigger index build |
test/test-utils.ts |
Sets ADVISORY_REFRESH_ON_START: "false" to prevent redundant git operations during tests |
test/unit/refresh-database.test.ts |
Uses path.join() for cross-platform path handling in test assertions |
vitest.config.ts |
Wires up the globalSetup fixture |
package.json |
Updates OpenTelemetry resources, metrics, and trace packages from 2.2.0 to 2.5.0; updates @ai-sdk/azure and ai dev dependencies |
package-lock.json |
Lock file updates corresponding to package.json changes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+313
to
+320
| // End date: include full day by using next day midnight | ||
| const endDate = new Date(end + 'T00:00:00Z'); | ||
| endDate.setUTCDate(endDate.getUTCDate() + 1); | ||
| return { start: start + 'T00:00:00Z', end: endDate.toISOString() }; | ||
| } | ||
| // Single date: filter for that specific day | ||
| const startDate = new Date(str + 'T00:00:00Z'); | ||
| const endDate = new Date(str + 'T00:00:00Z'); |
There was a problem hiding this comment.
The parseDateFilter method lacks input validation for invalid dates. When an invalid date string is provided (e.g., "2026-13-45", "invalid-date", or "2026-02-30"), new Date() will create an Invalid Date object. The code will still proceed and call toISOString() on this Invalid Date, which will throw an error at runtime.
Add validation to check if the parsed dates are valid before proceeding. For example, after creating a Date object, check if date.getTime() returns NaN to detect invalid dates, and throw a descriptive error message.
Suggested change
| // End date: include full day by using next day midnight | |
| const endDate = new Date(end + 'T00:00:00Z'); | |
| endDate.setUTCDate(endDate.getUTCDate() + 1); | |
| return { start: start + 'T00:00:00Z', end: endDate.toISOString() }; | |
| } | |
| // Single date: filter for that specific day | |
| const startDate = new Date(str + 'T00:00:00Z'); | |
| const endDate = new Date(str + 'T00:00:00Z'); | |
| // Validate start and end dates | |
| const startDate = new Date(start + 'T00:00:00Z'); | |
| const endDate = new Date(end + 'T00:00:00Z'); | |
| if (isNaN(startDate.getTime()) || isNaN(endDate.getTime())) { | |
| throw new Error(`Invalid date filter: "${str}"`); | |
| } | |
| // End date: include full day by using next day midnight | |
| endDate.setUTCDate(endDate.getUTCDate() + 1); | |
| // Preserve original behavior for the start bound string | |
| return { start: start + 'T00:00:00Z', end: endDate.toISOString() }; | |
| } | |
| // Single date: filter for that specific day | |
| const startDate = new Date(str + 'T00:00:00Z'); | |
| const endDate = new Date(str + 'T00:00:00Z'); | |
| if (isNaN(startDate.getTime()) || isNaN(endDate.getTime())) { | |
| throw new Error(`Invalid date filter: "${str}"`); | |
| } |
| // Defense-in-depth: ensure dateStr is a string (HTTP query params can be arrays) | ||
| const str = Array.isArray(dateStr) ? String(dateStr[0]) : String(dateStr); | ||
| if (str.includes('..')) { | ||
| const [start, end] = str.split('..'); |
There was a problem hiding this comment.
The split operation on line 312 doesn't limit the number of parts, so a malformed input like "2026-01-01..2026-02-01..2026-03-01" would silently use only the first two parts without warning the user. This could lead to confusion when debugging unexpected filter behavior.
Consider using split('..', 2) to limit the split to 2 parts, and then validate that exactly 2 parts were returned. If more than 2 parts exist, throw a descriptive error indicating the correct format.
Suggested change
| const [start, end] = str.split('..'); | |
| const parts = str.split('..'); | |
| if (parts.length !== 2 || !parts[0] || !parts[1]) { | |
| throw new Error('Invalid date range format. Expected "YYYY-MM-DD..YYYY-MM-DD".'); | |
| } | |
| const [start, end] = parts; |
Comment on lines
+312
to
+316
| const [start, end] = str.split('..'); | ||
| // End date: include full day by using next day midnight | ||
| const endDate = new Date(end + 'T00:00:00Z'); | ||
| endDate.setUTCDate(endDate.getUTCDate() + 1); | ||
| return { start: start + 'T00:00:00Z', end: endDate.toISOString() }; |
There was a problem hiding this comment.
The parseDateFilter method doesn't validate that the start date comes before the end date in a range filter. If a user provides "2026-12-31..2026-01-01", the filter will produce an empty result set without any error message, which could confuse users.
Add validation to ensure start is less than or equal to end when parsing a date range, and throw a descriptive error if the range is invalid.
Suggested change
| const [start, end] = str.split('..'); | |
| // End date: include full day by using next day midnight | |
| const endDate = new Date(end + 'T00:00:00Z'); | |
| endDate.setUTCDate(endDate.getUTCDate() + 1); | |
| return { start: start + 'T00:00:00Z', end: endDate.toISOString() }; | |
| const [startRaw, endRaw] = str.split('..'); | |
| const startDate = new Date(startRaw + 'T00:00:00Z'); | |
| const endDate = new Date(endRaw + 'T00:00:00Z'); | |
| if (startDate > endDate) { | |
| throw new Error( | |
| `Invalid date range "${str}": start date must be less than or equal to end date.`, | |
| ); | |
| } | |
| // End date: include full day by using next day midnight (end is exclusive) | |
| endDate.setUTCDate(endDate.getUTCDate() + 1); | |
| return { start: startDate.toISOString(), end: endDate.toISOString() }; |
Comment on lines
+122
to
+131
| const query = req.query.q as string; | ||
| if (!query) { | ||
| res.status(400).json({ error: 'Missing query parameter: q' }); | ||
| return; | ||
| } | ||
|
|
||
| const options: AdvisoryListOptions = { | ||
| ecosystem: req.query.ecosystem as string, | ||
| severity: req.query.severity as string, | ||
| per_page: req.query.per_page ? parseInt(req.query.per_page as string) : undefined, |
There was a problem hiding this comment.
The search endpoint still uses unsafe type assertions (as string) for query parameters on lines 122, 129, 130, and 131, while the /advisories endpoint was refactored to use the safer queryString helper function. This inconsistency means the search endpoint is less type-safe and doesn't handle array query parameters correctly.
Apply the same queryString helper pattern used in the /advisories endpoint to ensure consistent and safe handling of query parameters throughout the codebase.
Suggested change
| const query = req.query.q as string; | |
| if (!query) { | |
| res.status(400).json({ error: 'Missing query parameter: q' }); | |
| return; | |
| } | |
| const options: AdvisoryListOptions = { | |
| ecosystem: req.query.ecosystem as string, | |
| severity: req.query.severity as string, | |
| per_page: req.query.per_page ? parseInt(req.query.per_page as string) : undefined, | |
| const query = queryString(req.query.q); | |
| if (!query) { | |
| res.status(400).json({ error: 'Missing query parameter: q' }); | |
| return; | |
| } | |
| const ecosystem = queryString(req.query.ecosystem); | |
| const severity = queryString(req.query.severity); | |
| const perPageStr = queryString(req.query.per_page); | |
| const options: AdvisoryListOptions = { | |
| ecosystem, | |
| severity, | |
| per_page: perPageStr ? parseInt(perPageStr, 10) : undefined, |
Comment on lines
+45
to
+63
| "@opentelemetry/resources": "^2.5.0", | ||
| "@opentelemetry/sdk-logs": "^0.211.0", | ||
| "@opentelemetry/sdk-metrics": "^2.2.0", | ||
| "@opentelemetry/sdk-metrics": "^2.5.0", | ||
| "@opentelemetry/sdk-node": "^0.211.0", | ||
| "@opentelemetry/sdk-trace-base": "^2.2.0", | ||
| "@opentelemetry/sdk-trace-node": "^2.2.0", | ||
| "@opentelemetry/sdk-trace-base": "^2.5.0", | ||
| "@opentelemetry/sdk-trace-node": "^2.5.0", | ||
| "@opentelemetry/semantic-conventions": "^1.39.0", | ||
| "cors": "^2.8.6", | ||
| "express": "^5.0.1", | ||
| "winston": "^3.18.3", | ||
| "zod": "^4.3.6" | ||
| }, | ||
| "devDependencies": { | ||
| "@ai-sdk/azure": "^3.0.27", | ||
| "@ai-sdk/azure": "^3.0.28", | ||
| "@azure/identity": "^4.13.0", | ||
| "@types/cors": "^2.8.17", | ||
| "@types/express": "^5.0.0", | ||
| "@types/node": "^25.2.3", | ||
| "ai": "^6.0.79", | ||
| "ai": "^6.0.81", |
There was a problem hiding this comment.
The PR description lists incorrect dependency version updates. The description claims updates to @opentelemetry/api (1.9.0 → 2.0.0), @opentelemetry/sdk-node (0.57.2 → 0.200.0), @opentelemetry/auto-instrumentations-node (0.57.2 → 0.200.0), @ai-sdk/azure (1.3.22 → 2.0.6), and ai (4.3.15 → 4.3.16).
However, the actual changes are:
- @opentelemetry/resources: ^2.2.0 → ^2.5.0
- @opentelemetry/sdk-metrics: ^2.2.0 → ^2.5.0
- @opentelemetry/sdk-trace-base: ^2.2.0 → ^2.5.0
- @opentelemetry/sdk-trace-node: ^2.2.0 → ^2.5.0
- @ai-sdk/azure: ^3.0.27 → ^3.0.28
- ai: ^6.0.79 → ^6.0.81
The PR description should be updated to reflect the actual dependency changes made.
Comment on lines
+304
to
+334
| /** | ||
| * Parse date filter string and return start/end dates | ||
| * Supports: "2026-01-27" (single day) or "2026-01-01..2026-01-31" (range) | ||
| */ | ||
| private parseDateFilter(dateStr: string): { start: string; end: string } { | ||
| // Defense-in-depth: ensure dateStr is a string (HTTP query params can be arrays) | ||
| const str = Array.isArray(dateStr) ? String(dateStr[0]) : String(dateStr); | ||
| if (str.includes('..')) { | ||
| const [start, end] = str.split('..'); | ||
| // End date: include full day by using next day midnight | ||
| const endDate = new Date(end + 'T00:00:00Z'); | ||
| endDate.setUTCDate(endDate.getUTCDate() + 1); | ||
| return { start: start + 'T00:00:00Z', end: endDate.toISOString() }; | ||
| } | ||
| // Single date: filter for that specific day | ||
| const startDate = new Date(str + 'T00:00:00Z'); | ||
| const endDate = new Date(str + 'T00:00:00Z'); | ||
| endDate.setUTCDate(endDate.getUTCDate() + 1); | ||
| return { start: startDate.toISOString(), end: endDate.toISOString() }; | ||
| } | ||
|
|
||
| /** | ||
| * Filter advisories by date range | ||
| */ | ||
| private filterByDateRange(advisories: Advisory[], field: 'published_at' | 'updated_at', dateStr: string): Advisory[] { | ||
| const { start, end } = this.parseDateFilter(dateStr); | ||
| return advisories.filter(a => { | ||
| const date = a[field]; | ||
| return date >= start && date < end; | ||
| }); | ||
| } |
There was a problem hiding this comment.
The new parseDateFilter and filterByDateRange methods lack unit test coverage. While there are E2E tests that exercise these methods indirectly, there are no unit tests that validate edge cases such as invalid dates, reversed date ranges, malformed input, or boundary conditions.
Consider adding unit tests for the LocalRepositoryDataSource class to cover these critical date parsing scenarios, following the pattern established in test/unit/refresh-database.test.ts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds date range filtering support for advisory queries and fixes several issues with the test infrastructure to enable reliable local development and CI.
Feature: Date Range Filtering
src/datasources/local-repository.tsparseDateFilter()— parses date filter strings in two formats:"YYYY-MM-DD"— single day (matches advisories published on that exact date)"YYYY-MM-DD..YYYY-MM-DD"— date range (inclusive start/end)filterByDateRange()— replaces the previous simple>=date comparison with proper range filtering using the parsed date filterpublishedparameter is provided tolist_advisoriessrc/tools/advisories.tspublished,ecosystem,severity, and other parameterstest/e2e/mcp-server.test.tsYYYY-MM-DD..YYYY-MM-DD)Bug Fixes
src/local-server.ts— CodeQL regression fix%sformat specifier inconsole.error()instead of template literal${req.params.ghsa_id}src/refresh-database.ts— ENOENT fixmkdirSync(parentDir, { recursive: true })beforegit cloneto create theexternal/directory if it doesn't existspawn("git", [...], { cwd: nonExistentDir })would fail with a misleading ENOENT errortest/unit/refresh-database.test.ts— Cross-platform path fixpath.join()in thecwdassertion instead of hardcoded Unix path"/path/to"path.join("/path/to/advisory-database", "..")returns\path\to, causing the assertion to failTest Infrastructure Improvements
test/e2e/globalSetup.ts(new file)git clone --depth=1with a 180s timeouttest/test-utils.tsADVISORY_REFRESH_ON_START: "false"to the spawned server's environmentgit fetchon the 630K-object advisory-database during startup, which was causing the E2EbeforeAllhook to time out at 180stest/e2e/mcp-server.test.tsbeforeAllhook (120s timeout) that triggers the advisory database index buildlist_advisoriescall reads all advisory JSON files from disk (~22-40s for the full repo); subsequent calls use the in-memory cache and complete in millisecondsvitest.config.tsglobalSetupfixture:globalSetup: ["./test/e2e/globalSetup.ts"]Dependency Updates
package.json@opentelemetry/api: 1.9.0 → 2.0.0@opentelemetry/sdk-node: 0.57.2 → 0.200.0@opentelemetry/auto-instrumentations-node: 0.57.2 → 0.200.0@opentelemetry/exporter-trace-otlp-http: 0.57.2 → 0.200.0@ai-sdk/azure(dev): 1.3.22 → 2.0.6ai(dev): 4.3.15 → 4.3.16Test Results
All 37 tests pass (12 unit + 4 integration + 21 E2E):
Files Changed (11 files, +389 / -215)
src/datasources/local-repository.tssrc/local-server.tssrc/refresh-database.tssrc/tools/advisories.tstest/e2e/globalSetup.tstest/e2e/mcp-server.test.tstest/test-utils.tstest/unit/refresh-database.test.tsvitest.config.tspackage.jsonpackage-lock.json