feat(cve/scanner): add CVE research pipeline extension #75
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "cve-scanner"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
@swamp/cve/scanner— a daily CVE research pipeline that queries NVD, GitHub Advisory DB, and CISA KEV for HIGH/CRITICAL vulnerabilities@keeb/discordWhat's included
cve_research.tsresearchmethod — queries 3 CVE sources, deduplicates, classifies, scoresdiscord_webhook_env.ts@keeb/discord/webhookthat readsDISCORD_WEBHOOK_URLfrom envcve_research_report.tscve-research-to-discord.yaml*_test.tsDependencies
@keeb/discord(declared in manifest, auto-pulled)Test plan
deno task test— 32/32 passingswamp extension push manifest.yaml --dry-run— passes with no blocking warningsswamp extension fmt manifest.yaml— cleanCI Security Review
Summary
The diff adds
researcherto thecvematrix in two existing jobs (cve-checkat line 364 andcve-lockfileat line 394). This extends CI coverage to the newcve/researcherextension directory using the same check/lint/fmt/test and lockfile-verification pattern already applied todirtyfragandmini-shai-hulud.Analysis
1. Prompt Injection — No LLM-invoking jobs were modified. Not applicable.
2. Expression Injection —
${{ matrix.cve }}is used inworking-directoryand jobnamefields. The matrix values are hardcoded strings defined in the workflow file (dirtyfrag,mini-shai-hulud,researcher) — not attacker-controlled. No injection risk.3. Dangerous Triggers — Unchanged. The workflow triggers on
pull_request(notpull_request_target), which runs in the PR head context without access to base-repo secrets. Safe.4. Supply Chain — The affected jobs use
actions/checkout@v6anddenoland/setup-deno@v2, both from trusted publishers. No new actions introduced.5. Permissions — The affected jobs inherit the workflow-level
permissions: contents: read, which is the minimum needed for checkout + deno check/test. No escalation.6. Secret Exposure — No secrets are referenced in
cve-checkorcve-lockfile. Not applicable.7. Auto-merge & Trust Boundaries — Not applicable to these jobs.
Critical / High
None.
Medium
None.
Low
None.
Verdict
PASS — Security-neutral change. Adds a new matrix entry (
researcher) to two existing CI jobs using an established, safe pattern. No new triggers, permissions, secrets, actions, or LLM interactions introduced.CI Security Review
Summary
The diff adds
researcherto thecvematrix in two jobs (cve-checkat line 364 andcve-lockfileat line 394). This is a security-neutral change — it extends existing hardcoded matrix values to include an additional CVE extension directory for CI checks.Critical / High
No critical or high severity findings.
Medium
No medium severity findings.
Low
No low severity findings.
Pre-existing observations (not blocking)
The overall workflow has a well-considered security posture:
pull_request(notpull_request_target), preventing secrets from being exposed to fork PR code.contents: readwith job-level escalation topull-requests: writeonly for the review-posting jobs.actions/checkout@v6anddenoland/setup-deno@v2use tag pins which is acceptable for trusted publishers.--allowedTools(read-only git commands,Read,Grep, and specifictee/touchpaths).WriteandEditare explicitly disallowed.base.sha,head.sha) are passed viaenv:blocks rather than direct${{ }}interpolation inrun:blocks, preventing expression injection.Verdict
PASS — Adding
researcherto the CVE matrix is a minimal, security-neutral change that follows the same safe patterns used by the other matrix entries.Code Review
Blocking Issues
npm:zod@4is not an exact version pin —cve_research.ts:1anddiscord_webhook_env.ts:1both import fromnpm:zod@4. This is a major-version specifier (matches any 4.x.y), not an exact pin. CLAUDE.md requires "Pin all npm dependencies to exact versions (no ranges)", and the localcve/researcher/CLAUDE.mdreinforces this with an example of the correct form (npm:lodash-es@4.17.21). Should be e.g.npm:zod@4.3.0(whatever the current 4.x release is).Suggestions
fetchJsonretry backoff only covers 429/503 —cve_research.ts:185-205: for non-429/503 HTTP errors (e.g. 500 from NVD or GitHub), the loop callscontinueimmediately with no sleep, so three rapid-fire requests are made with no delay. Consider adding backoff for all retryable errors, or at minimum not retrying 4xx errors that will never succeed (400, 401, 403).Discord embed field value may exceed the 1024-char Discord limit —
cve_research.ts:605-619: each field value is built by joining up to 5 CVE summaries (up to 300 chars each) with\n\n, yielding up to ~1508 chars in a single field. Discord rejects webhook POSTs with field values over 1024 chars. Consider capping the combined field value length.||vs??for webhook URL fallback —discord_webhook_env.ts:30:context.globalArgs.webhookUrl || Deno.env.get(...)means an empty-stringwebhookUrlsilently falls through to the env var rather than being caught at the "no webhook URL" guard.??is more precise here since an empty string is also invalid and should hit the same error path.No integration tests for the execute paths —
research.execute(network calls to NVD, GitHub, CISA KEV) and the success path ofsendFromEnv.execute(Discord POST) are untested. The existing tests cover schema validation and report rendering well, but the execution paths that do actual I/O have no coverage. Local mock servers (Deno.serve({ port: 0 })) would let these be tested without hitting live APIs.Adversarial Review
Critical / High
Discord embed field values exceed 1024-char limit —
cve/researcher/extensions/models/cve_research.ts:604-619formatDiscordSummaryjoins up to 5 CVE entries into a single embed field value. Each entry includes a title (~60 chars) and acveSummary(up to 300 chars), so the worst-case field value is ~1850 characters. Discord rejects embed field values over 1024 characters with HTTP 400.Breaking input: 4+ extension-candidate CVEs with descriptions longer than 200 characters — a realistic scenario during a busy disclosure day. The Discord API returns 400,
sendFromEnvthrows, the workflow step fails, and no notification is sent for exactly the highest-priority results.Suggested fix: Cap each field value at 1024 characters. Either reduce
cveSummaryto ~150 chars when there are 3+ items, or split across multiple embed fields (Discord allows up to 25 fields per embed). The same issue applies to the monitors field at lines 630-645.Medium
Non-atomic state file write —
cve/researcher/extensions/models/cve_research.ts:156-168saveStateusesDeno.writeTextFile, which is not atomic. If the process is killed mid-write (OOM, CI timeout, SIGKILL), the state file is left partially written. Next run,loadStatecatches the JSON parse error and falls back to{ lastCheck: null, seenIds: new Set() }, causing every CVE in the window to be re-flagged as "new" and producing a duplicate Discord blast.Suggested fix: Write to a temp file, then
Deno.renameit over the target (rename is atomic on POSIX).fetchJsonretries non-transient HTTP errors with zero delay —cve/researcher/extensions/models/cve_research.ts:192-195For HTTP errors other than 429/503 (e.g. 400, 401, 403, 404), the code sets
lastErrorandcontinues with no backoff. This fires 3 requests in rapid succession against APIs that have already returned a definitive error, wasting budget and risking IP-level rate limiting from NVD or GitHub.Suggested fix: Either don't retry 4xx errors at all (they're not transient), or at minimum apply the same exponential backoff as the 429/503 path.
Unbounded
seenIdsset growth —cve/researcher/extensions/models/cve_research.ts:815-819state.seenIdsis append-only — IDs are added every run but never pruned. With 3 sources × 40 results/source × daily runs, the set grows by ~120 IDs/day after dedup. After a year the state file holds 40k+ entries. Not a crash, but the file grows monotonically andloadState/saveStateget progressively slower.Suggested fix: Prune IDs older than 2× the max lookback window (180 days) on each save, or cap the set size.
Discord webhook fetch has no timeout —
cve/researcher/extensions/models/discord_webhook_env.ts:47The
fetchcall to Discord has noAbortSignal.timeout(). Compare withcve_research.ts:182which correctly usesAbortSignal.timeout(30_000). If Discord is unresponsive, the workflow step hangs indefinitely until the CI runner's global timeout kills it.Suggested fix: Add
signal: AbortSignal.timeout(30_000)to the fetch options.Low
Response body not consumed on retry paths —
cve/researcher/extensions/models/cve_research.ts:185-195When
fetchJsongets a 429, 503, or other non-OK response, the response body is never read before the retry loop continues. In Deno, an unconsumed response body keeps the underlying TCP connection alive. Over 3 retries × 3 sources × 2 severities, this could temporarily hold ~18 connections open.Suggested fix: Add
await resp.body?.cancel()beforecontinueon retry paths."dos"category keyword is overly broad —cve/researcher/extensions/models/cve_research.ts:242desc.includes("dos")matches substrings like "todos" or "dosage" in lowercased descriptions, causing false-positive categorization as "dos". Unlikely in CVE descriptions but possible.Suggested fix: Use a word-boundary regex like
/\bdos\b/or match only the full phrase "denial of service".Verdict
FAIL — The Discord embed field-value overflow (finding #1) will cause the notification pipeline to error on the days it matters most (many high-severity CVEs). The missing fetch timeout (finding #5) can hang the workflow indefinitely. Fix these two before merging.
CI Security Review
Summary
The change adds
researcherto thecvematrix in two jobs (cve-checkat line 364 andcve-lockfileat line 394). This is a purely additive change that extends existing CI check/lint/fmt/test and lockfile verification to cover the newcve/researcherextension directory.The added matrix value is a static, hardcoded string — not derived from any user-controlled input. It flows into:
cve/researcher - check, etc.working-directory: cve/researcher(safe — static path)No new triggers, permissions, secrets, actions, or LLM integrations are introduced by this change.
Critical / High
None.
Medium
None.
Low
None.
Verdict
PASS — Security-neutral change. Adding a static matrix entry to run existing CI jobs against a new extension directory introduces no new attack surface.
Adversarial Review
Medium
Deduplication priority is backwards from the comment —
cve/researcher/extensions/models/cve_research.ts:782-804The comment on line 782 says
"preferring CISA KEV > NVD > GitHub", but the iteration order is[...ghResult.findings, ...nvdResult.findings, ...kevResult.findings]. The first occurrence wins for all base fields (description, severity, severityScore, source, published). OnlyknownExploitedandcategoriesare merged from later duplicates.Breaking example: A CVE appears in both GitHub Advisory and CISA KEV. The record is stored with
source: "GITHUB", GitHub's description, and GitHub's severity score. TheknownExploitedflag is correctly merged from KEV, but thesourcefield will show "GitHub Advisory" in reports and Discord embeds instead of "CISA KEV". Users see a misleadingly attributed record for actively exploited vulnerabilities.Suggested fix: Either reverse the iteration order to
[...kevResult.findings, ...nvdResult.findings, ...ghResult.findings](so KEV data is stored first and kept), or update the merge logic to overwrite base fields when a higher-priority source is encountered. Or, if the current behavior is intentional, fix the comment.Discord webhook fetch has no timeout —
cve/researcher/extensions/models/discord_webhook_env.ts:47-51The
fetch(webhookUrl, ...)call has nosignal: AbortSignal.timeout(...), unlikefetchJsonincve_research.tswhich uses a 30-second timeout. If the Discord webhook endpoint hangs (DNS resolution stall, connection accepted but no response), this call blocks indefinitely.Breaking example: Discord experiences a partial outage where connections are accepted but responses stall. The workflow step hangs forever, preventing the workflow from completing or retrying.
Suggested fix: Add
signal: AbortSignal.timeout(15_000)to the fetch options, matching the pattern used infetchJson.categorizeCvefalse-positive on "dos" substring —cve/researcher/extensions/models/cve_research.ts:241desc.includes("dos")matches any description containing the substring "dos" — not just "denial of service" or "DoS". Whiledescis lowercased so proper nouns won't match, product names or technical terms containing "dos" (e.g., "msdos", "freedos", "dosfstools", "kudos") would incorrectly categorize the CVE as"dos".Breaking example: A CVE description mentioning "FreeDOS" or "dosfstools" gets tagged
doswhen it's actually a memory corruption bug, skewing category-based filtering and Discord embed grouping.Suggested fix: Use word-boundary matching:
/\bdos\b/or/\bdenial of service\b|\bdos\b/instead ofdesc.includes("dos"). The "denial of service" long form is already checked separately, so the short-form check could be tightened todesc.includes(" dos ")or a regex with word boundaries.Low
State file write is non-atomic —
cve/researcher/extensions/models/cve_research.ts:160-168Deno.writeTextFileis not atomic. If the process crashes mid-write,cve-research-state.jsoncould be corrupted. On next run,loadStatecatches the parse error and returns empty state, causing all CVEs to be re-flagged as "new". Unlikely in practice since the file is small and writes are fast, but awrite-then-renamepattern would eliminate the risk.seenIdsset grows without bound —cve/researcher/extensions/models/cve_research.ts:833-835Every run adds new CVE IDs to
seenIdsbut old IDs are never pruned. Over months of daily runs, this set grows without limit. At ~50 new IDs/day, after a year that's ~18,000 entries (~500KB JSON). Not operationally dangerous but worth noting — a periodic prune (e.g., drop IDs older than 180 days) would keep the file bounded.Non-retryable HTTP errors retried without delay —
cve/researcher/extensions/models/cve_research.ts:193-195In
fetchJson, HTTP errors other than 429/503 (e.g., 400, 401, 403, 404) hitcontinuewithout any delay, immediately retrying. A 404 will be retried 3 times in rapid succession with no hope of success.Discord webhook has no retry mechanism —
cve/researcher/extensions/models/discord_webhook_env.ts:47-58Unlike the CVE source queries which retry on transient errors, the Discord webhook POST has no retry logic. A transient 429 or 503 from Discord will immediately fail the workflow step.
Verdict
PASS — No critical or high severity issues found. The deduplication priority mismatch (Medium #1) produces incorrect source attribution but doesn't affect the functional correctness of scoring or alerting since
knownExploitedis correctly merged. The missing timeout (Medium #2) is a robustness gap but not a correctness bug. The "dos" substring match (Medium #3) is a minor classification accuracy issue. All three mediums are worth fixing but none block the merge.Code Review
Blocking Issues
None.
Suggestions
research.executepath has no test coverage (cve/researcher/extensions/models/cve_research.ts:739).The HTTP fetching, date-window filtering, deduplication, state load/save, and
formatDiscordSummarylogic are all untested. The report module (cve_research_report_test.ts) demonstrates the right approach — an in-memorymakeContextmock + encoded JSON bytes. A parallel setup usingDeno.serve({ port: 0 })for mock NVD/GitHub/CISA responses would give meaningful coverage for the core pipeline. The Zod schema tests are solid but don't exercise the code that matters most at runtime.fetchJsonretries non-retriable HTTP errors immediately (cve_research.ts:193).The
429/503branch correctly backs off, but the generic!resp.okbranch (continuewith no delay) retries401/403/404responses three times immediately. A 403 from NVD (bad API key) or a 404 from CISA KEV fires three requests before giving up. These errors won't succeed on retry; the behaviour is wasteful and obscures permanent failures. Abreak(or at minimum a shortawaitbeforecontinue) for non-rate-limit errors would fix this.discordSummaryZod description says "under 2000 chars" (cve_research.ts:114).formatDiscordSummaryreturnsJSON.stringify({ embeds: [embed] })— a full Discord webhook payload. With three embed fields each up to 1 024 chars, plus title/description/footer, the JSON easily exceeds 2 000 chars when there are more than a handful of CVEs. The description is misleading to any consumer of this resource field. Consider updating it to something like"JSON-encoded Discord webhook payload (embeds)".errorfield typed asbooleaninstead ofstringin report (cve_research_report.ts:418).The actual runtime value is
string | undefined(matching the Zod schema incve_research.ts). The type assertion masks this under strict mode. The truthy checksrc.nvd.error ? " (error)" : ""happens to work at runtime because a non-empty string is truthy, but the annotation will mislead future readers. Changeerror?: booleantoerror?: string.GitHub Advisory
cveIdfallback can produce"GHSA-undefined"(cve_research.ts:386).If both
cve_idandghsa_idare absent (or null),cveIdbecomes"GHSA-undefined", which will be persisted to state and stored as a resource. The GitHub Advisory API always providesghsa_id, so this is unlikely in practice, but a guard (adv.ghsa_id ?? "unknown") would prevent silent data corruption.