fix(watch): enrich re-parsed SBOMs to prevent false resolved-vuln alerts#220
Closed
matrosov wants to merge 1 commit into
Closed
fix(watch): enrich re-parsed SBOMs to prevent false resolved-vuln alerts#220matrosov wants to merge 1 commit into
matrosov wants to merge 1 commit into
Conversation
process_sbom_change parsed modified files without enrichment and diffed the bare parse against the enriched previous snapshot. Since resolved vulnerabilities are computed as in-old-not-new, every file touch alerted all OSV-enriched vulnerabilities as resolved, and the next enrichment cycle re-alerted them as new. Separately, run_enrichment_cycle re-enriched already-enriched SBOMs while OsvEnricher::enrich extended component.vulnerabilities without dedup, inflating vulnerability counts on every cycle. Fix: - process_sbom_change now enriches the re-parsed SBOM (OSV/EOL/VEX) when config.enrichment.enabled, via a shared enrich_watched_sbom helper extracted from process_initial/run_enrichment_cycle. Periodic cycles keep bypassing caches; initial scans and re-parses use the cache. - OSV enrichment merges results by vulnerability id, skipping ids the component already carries, so repeated enrichment is idempotent. - EnrichmentConfig gains an optional api_base override (wired through build_enrichment_config) so watch-loop enrichment can target a mock or private OSV endpoint. Tests: end-to-end watch loop test (tempdir SBOM + httpmock OSV + NDJSON sink) asserting no resolved/new-vuln alerts on a file touch while the vulnerability is still present and a stable vuln count across multiple enrichment cycles; enricher-level re-enrichment idempotency test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Alex Matrosov <alex.matrosov@gmail.com>
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.
Note
Stacked on #219 (
fix/osv-hydration, OSV querybatch hydration). Merge that PR first; this one targets its branch.Summary
Root cause 1 — false resolved/new alerts on every file touch.
process_sbom_changeparsed modified files without enrichment (the_configparameter was unused,src/watch/loop_impl.rs:240-312pre-fix) and diffed the bare parse against the enriched previous snapshot. Resolved vulnerabilities are computed as in-old-not-new (src/diff/changes/vulnerabilities.rs:145-150), so every file touch alerted all OSV-enriched vulnerabilities as resolved, and the next enrichment cycle re-alerted them as new.Root cause 2 — vuln-count inflation.
run_enrichment_cyclere-enriches already-enriched SBOMs, andOsvEnricher::enrichextendedcomponent.vulnerabilitieswithout dedup (src/enrichment/osv/mod.rs, both the cached-results and batch-fetch apply sites), so counts grew on every cycle.Fix
process_sbom_changenow enriches the re-parsed SBOM whenconfig.enrichment.enabled, through a sharedenrich_watched_sbomhelper extracted from the previously duplicated OSV/EOL/VEX blocks inprocess_initialandrun_enrichment_cycle(src/watch/loop_impl.rs:181-209). Periodic cycles keep bypassing caches; initial scans and re-parses use the cache.merge_vulnerabilities, which skips vulnerability ids the component already carries, making repeated enrichment idempotent (src/enrichment/osv/mod.rs:262-276).EnrichmentConfiggains an optionalapi_baseoverride wired throughpipeline::build_enrichment_configso the watch loop's enrichment path can target a mock (or private mirror) OSV endpoint — required to drive the end-to-end test, and the only knob the watch path lacked compared toOsvEnricherConfig.Test plan
tests/watch_tests.rs::enrichment_loop::test_watch_loop_enriched_reparse_no_false_resolved_alerts: drivesrun_watch_loopin a thread against a tempdir SBOM with httpmock OSV endpoints (querybatch + per-vuln hydration + health check) and an NDJSONAlertSink. The first cycle enriches; the file is then touched with a component version bump. Asserts: noresolved_vulns/new_vulnsin any change event, nonew_vulnsenrichment events, andvulnsstays at 1 in every status event across >=3 enrichment passes (verified via mock hit counts). Ran 6x locally with no flakes.tests/enrichment_http_tests.rs::repeated_enrichment_does_not_duplicate_vulnerabilities: enriching the same component twice (second pass served from cache) yields exactly one vulnerability.cargo fmt --all -- --check,cargo clippy -- -D warnings,cargo clippy --all-features -- -D warnings(toolchain 1.88): clean.cargo test: all suites pass.🤖 Generated with Claude Code