Allow setting default vuln chart filters via GitOps - #47634
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #47634 +/- ##
==========================================
+ Coverage 67.31% 67.33% +0.01%
==========================================
Files 3655 3655
Lines 231251 231339 +88
Branches 12075 12091 +16
==========================================
+ Hits 155667 155761 +94
+ Misses 61620 61614 -6
Partials 13964 13964
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
WalkthroughThis PR adds GitOps support for configuring default filter state on the vulnerability exposure dashboard chart. On the backend, a new Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/pages/DashboardPage/cards/ChartCard/ChartFilterModal/ChartFilterModal.tsx (1)
206-222:⚠️ Potential issue | 🟡 MinorPass
filterDefaultsto ChartFilterModal so "Clear all" respects GitOps-managed baseline.The
handleClearfunction resets to hardcoded app defaults (ALL_CVE_SOFTWARE_CATEGORY_VALUESwith all categories, EPSS unset, etc.), not the GitOps-managedfilterDefaultspassed to ChartCard. This creates a UX inconsistency:
- Initial load: respects GitOps defaults via
buildInitialChartFilters(filterDefaults)(e.g., only "os" category if configured)- User clicks "Clear all": resets to hardcoded app defaults (all categories)
- Team switch: re-seeds from GitOps defaults
To align behavior, pass
filterDefaultsas a prop to ChartFilterModal and callbuildInitialChartFilters(filterDefaults)inhandleClearinstead of hardcoding reset values.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/pages/DashboardPage/cards/ChartCard/ChartFilterModal/ChartFilterModal.tsx` around lines 206 - 222, The handleClear function in ChartFilterModal uses hardcoded app defaults instead of respecting the GitOps-managed filterDefaults. Add filterDefaults as a prop to ChartFilterModal, then update the handleClear function to call buildInitialChartFilters(filterDefaults) and apply those results to all state variables instead of manually setting hardcoded values like ALL_CVE_SOFTWARE_CATEGORY_VALUES. This ensures the "Clear all" behavior aligns with the initial load behavior and respects any GitOps configuration.
🧹 Nitpick comments (1)
tools/charts-backfill/main.go (1)
123-125: 💤 Low valueUpdate stale comment reference.
The comment still references
TrackedCriticalCVEs, but the code now usesCollectibleCVEs(line 145). Update the comment to match.📝 Suggested fix
// sqlx wraps the raw connection so we can hand it to the chart bootstrap - // helpers (TrackedCriticalCVEs) without opening a second pool. + // helpers (CollectibleCVEs) without opening a second pool. db := sqlx.NewDb(rawDB, "mysql")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/charts-backfill/main.go` around lines 123 - 125, The comment above the sqlx.NewDb assignment references TrackedCriticalCVEs as the helper function being used, but the code now uses CollectibleCVEs. Update the comment to reference CollectibleCVEs instead of the stale TrackedCriticalCVEs reference to accurately reflect which chart bootstrap helper is being utilized.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/pages/DashboardPage/cards/ChartCard/ChartCard.tsx`:
- Around line 223-232: The description text in the ChartCard component currently
states "All critical vulnerabilities" but the PR changes the CVE collection to
include all-severity vulnerabilities rather than critical-only. Update the text
string that precedes the br tags and CustomLink component to reflect the broader
scope, such as "All tracked vulnerabilities" or "Vulnerabilities across all
severities," to accurately represent the all-severity collection model now being
used.
In `@server/chart/internal/service/service_test.go`:
- Around line 326-407: The "client severity bounds are overridden to critical"
subtest (lines 358-378) verifies that the filter passed to resolveCVEEntitiesFn
has the hard-coded critical severity values (9.0/10.0), but it does not verify
what values are echoed back in the response. Capture the response returned by
svc.GetChartData in that subtest and add assertions to verify that
response.Filters.SeverityMin and response.Filters.SeverityMax are 9.0 and 10.0
respectively (the critical hard-coded values), not the client-supplied 1.0 and
5.0 values. This will ensure the API contract correctly reflects what data was
actually returned.
In `@server/chart/internal/service/service.go`:
- Around line 146-163: The issue is that when the metric is CVE, the code
hard-codes CVSSMin to 9.0 and CVSSMax to 10.0 in the cveFilter to enforce
critical-only severity filtering, but the response's Filters field is still
populated with the client-supplied opts.SeverityMin and opts.SeverityMax,
creating a mismatch where the response claims different filters were applied
than what actually happened. To fix this, locate where the response Filters are
being set with severity bounds and update it to echo back the hard-coded
critical-only values (9.0 for min and 10.0 for max) instead of the
client-supplied values, ensuring the response accurately reflects the filters
that were actually applied to the chart data.
- Around line 150-158: Add input validation in
server/chart/internal/service/handler.go before constructing RequestOpts to
validate client-supplied EPSS and CVSS bounds. Validate that EPSS values are
within 0.0-1.0, CVSS values are within 0.0-10.0, and that minimum values do not
exceed maximum values for both bounds. Follow the validation pattern used by
validateBounds in server/fleet/app.go and return an appropriate error response
if any bounds are invalid. This will prevent invalid values from reaching the
service layer where they are used in cveFilter construction and SQL queries, as
shown in the cveFilter struct initialization in service.go.
In `@server/fleet/app.go`:
- Around line 1404-1413: The new pointer field
VulnerabilityExposureHistoricalReporting added to the AppConfig struct requires
a corresponding deep-copy implementation in the AppConfig.Copy() method.
Currently this field is being shallow-copied, which allows multiple cloned
AppConfig values to share the same underlying VulnExposureFilterSettings object,
causing mutations in one copy to affect all other copies. Update
AppConfig.Copy() to check if VulnerabilityExposureHistoricalReporting is not
nil, and if so, create a new copy of the pointed-to object rather than just
copying the pointer reference.
---
Outside diff comments:
In
`@frontend/pages/DashboardPage/cards/ChartCard/ChartFilterModal/ChartFilterModal.tsx`:
- Around line 206-222: The handleClear function in ChartFilterModal uses
hardcoded app defaults instead of respecting the GitOps-managed filterDefaults.
Add filterDefaults as a prop to ChartFilterModal, then update the handleClear
function to call buildInitialChartFilters(filterDefaults) and apply those
results to all state variables instead of manually setting hardcoded values like
ALL_CVE_SOFTWARE_CATEGORY_VALUES. This ensures the "Clear all" behavior aligns
with the initial load behavior and respects any GitOps configuration.
---
Nitpick comments:
In `@tools/charts-backfill/main.go`:
- Around line 123-125: The comment above the sqlx.NewDb assignment references
TrackedCriticalCVEs as the helper function being used, but the code now uses
CollectibleCVEs. Update the comment to reference CollectibleCVEs instead of the
stale TrackedCriticalCVEs reference to accurately reflect which chart bootstrap
helper is being utilized.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 837e766a-919e-4740-b17e-d390907e2794
📒 Files selected for processing (33)
changes/44746-collect-and-filter-more-cvesee/server/service/teams.gofrontend/interfaces/charts.tsfrontend/interfaces/config.tsfrontend/pages/DashboardPage/DashboardPage.tsxfrontend/pages/DashboardPage/cards/ChartCard/ChartCard.tests.tsxfrontend/pages/DashboardPage/cards/ChartCard/ChartCard.tsxfrontend/pages/DashboardPage/cards/ChartCard/ChartFilterModal/ChartFilterModal.tsxfrontend/pages/DashboardPage/cards/ChartCard/ChartFilterModal/SoftwareFilters/SoftwareFilters.tests.tsxfrontend/pages/DashboardPage/cards/ChartCard/ChartFilterModal/SoftwareFilters/SoftwareFilters.tsxfrontend/pages/DashboardPage/cards/ChartCard/ChartFilterModal/SoftwareFilters/_styles.scssfrontend/pages/DashboardPage/cards/ChartCard/ChartFilterModal/SoftwareFilters/helpers.tests.tsfrontend/pages/DashboardPage/cards/ChartCard/ChartFilterModal/SoftwareFilters/helpers.tsfrontend/pages/DashboardPage/cards/ChartCard/ChartFilterModal/SoftwareFilters/index.tsfrontend/pages/DashboardPage/cards/ChartCard/ChartFilterModal/index.tsfrontend/pages/DashboardPage/cards/ChartCard/_styles.scssfrontend/services/entities/charts.tsserver/chart/api/chart.goserver/chart/api/http/types.goserver/chart/bootstrap/bootstrap.goserver/chart/datasets.goserver/chart/internal/mysql/charts.goserver/chart/internal/mysql/cve_filter_test.goserver/chart/internal/service/handler.goserver/chart/internal/service/service.goserver/chart/internal/service/service_test.goserver/chart/internal/testutils/testutils.goserver/chart/internal/types/chart.goserver/fleet/app.goserver/fleet/vuln_exposure_filters_test.goserver/service/appconfig.goserver/service/appconfig_test.gotools/charts-backfill/main.go
|
@coderabbitai review |
✅ Action performedReview finished.
|
bf8065a to
f388d9d
Compare
3c26a02 to
953355b
Compare
f3561cd to
faa48b0
Compare
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
|
@getvictor there's a little bit of backend here because it adds app/teams config, but nothing very meaty. The bulk of it is front-end, assigning to @lukeheath for now. |
#47470) <!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** Resolves #44746 # Details * Adds the ability to filter historical CVE data by software type, EPSS, CVSS, CVE ID (exclude only) and "has known exploit" * Hard-codes the CVSS filter to 9.0+ for now, since that's the only data that's been collected thus far * Un-gates the collection code so that it will collect CVE data for _all_ severities (but still in the restricted set of software) Related PRs [update the front-end](#47674) to allow sending these filters, and [update GitOps](#47634) to allow changing the default filters. # Checklist for submitter If some of the following don't apply, delete the relevant line. - [X] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. - [X] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters. ## Testing - [X] Added/updated automated tests - [X] QA'd all new/changed functionality manually ### Manual test plan — CVE chart filtering (backend smoke test) #### Setup - Premium dev server running with a few hosts carrying vulnerable software (so `cve_meta` / `software_cve` / `operating_system_vulnerabilities` are populated) - Chart data present — collector ran once, or seeded: `go run ./tools/charts-backfill --dataset cve --use-tracked-cves --days 7` - API token exported and helper set: ```bash BASE=https://localhost:8080/api/v1/fleet/charts peak() { curl -sk -H "Authorization: Bearer $TOKEN" "$BASE/$1" | jq '[.data[].value] | max'; } #### Checks (compare against the no-filter baseline) - [x] Baseline returns data — GET /charts/cve?days=7 returns a data series; .filters is empty/default - [x] Severity force-pinned to critical — cve?days=7 and cve?days=7&severity_min=0&severity_max=10 give identical peaks (no low-severity leak; client severity ignored) - [x] Category narrowing — software_categories=browsers ≤ baseline; software_categories=os,browsers,office,adobe == baseline - [x] OS category includes kernel — software_categories=os returns OS-vuln + Linux-kernel CVE counts - [x] Known-exploit narrowing — known_exploit=true ≤ baseline - [x] EPSS narrowing — epss_min=0.9 ≤ baseline; epss_min=0&epss_max=1 == baseline (EPSS is 0.0–1.0 on the API) - [x] Exclude is subtractive + tolerant — excluding a visible CVE lowers/keeps counts; exclude_cves=CVE-0000-00000 == baseline (no-op) - [x] Filters echo back — filtered requests return applied values under .filters - [x] Uptime untouched — GET /charts/uptime?days=7 returns its normal series - [x] Free-tier safety (optional) — on non-Premium, /charts/cve returns an empty series, no error - [x] > 0 rows from: SELECT COUNT(DISTINCT scd.entity_id) AS below_critical FROM host_scd_data scd JOIN cve_meta cm ON cm.cve = scd.entity_id WHERE scd.dataset='cve' AND cm.cvss_score < 9.0; - (confirms lower-severity CVEs are stored) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Summary of changes * **New Features** * Added advanced CVE chart request filters: software categories, known-exploit flag, EPSS min/max, severity min/max, and excluded CVEs. * Expanded CVE chart coverage to use the full “collectible” CVE set, with filtering applied when serving chart data. * **Tests** * Added coverage for collecting collectible CVEs and resolving chart entities based on filter combinations and exclusions. * **Chores** * Updated CVE chart backfill to use collectible CVE discovery. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
a938394 to
d60bcd6
Compare
The base branch was changed.
AppConfig.Copy hand-copies Features sub-fields rather than delegating to Features.Copy, so the new VulnerabilityExposureHistoricalReporting pointer was left aliased between clones. Copy it explicitly (nil-safe).
A present software_filters list must now include at least one category. On the chart read path an empty selection collapses to nil = all categories, so an empty list can never produce the empty chart it implies — it silently shows everything. This mirrors the frontend rule (Apply blocked until at least one category is selected) added on the #44746 branch.
faa48b0 to
7cc74b4
Compare
There was a problem hiding this comment.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Pull request overview
This PR adds support for configuring default “Vulnerability exposure” chart filter settings via GitOps, including backend persistence/validation + premium gating and frontend seeding of the chart filter UI from those persisted defaults.
Changes:
- Backend: Introduces
VulnExposureFilterSettingsonFeatureswith deep-copy + validation, and enforces premium gating/validation when applying GitOps org config and team specs. - Frontend: Plumbs
features.vulnerability_exposure_historical_reportingintoChartCardand seeds/resets chart filter state from persisted defaults on scope/config changes. - Tests: Adds unit tests for validation/copy and frontend seeding behavior.
Reviewed changes
Copilot reviewed 13 out of 15 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/cloner-check/generated_files/teamconfig.txt | Updates cloner-check output for new team config fields. |
| tools/cloner-check/generated_files/features.txt | Updates cloner-check output for new feature field. |
| tools/cloner-check/generated_files/appconfig.txt | Updates cloner-check output for new app config field. |
| server/fleet/app.go | Adds persisted vuln exposure filter defaults struct, copy, and validation. |
| server/fleet/vuln_exposure_filters_test.go | Adds unit tests for validation and deep copy. |
| server/service/appconfig.go | Premium-gates + validates vuln exposure filter defaults on app config modify/apply. |
| server/service/appconfig_test.go | Adds service-level tests for premium gating + validation rejection. |
| ee/server/service/teams.go | Validates team-scoped vuln exposure defaults during team spec apply. |
| frontend/interfaces/charts.ts | Adds IVulnExposureFilterDefaults interface for persisted defaults. |
| frontend/interfaces/config.ts | Exposes the persisted defaults on IConfigFeatures. |
| frontend/pages/DashboardPage/DashboardPage.tsx | Passes persisted defaults into ChartCard. |
| frontend/pages/DashboardPage/cards/ChartCard/ChartCard.tsx | Seeds/resets chart filter state from persisted defaults. |
| frontend/pages/DashboardPage/cards/ChartCard/ChartCard.tests.tsx | Adds tests for seeding behavior. |
Files excluded by content exclusion policy (2)
- changes/44746-set-vuln-filters-in-gitops
- docs/Configuration/yaml-files.md
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Seed the chart's initial filter state from the persisted, GitOps-managed | ||
| // defaults. Sparse/per-field: an undefined field falls back to the built-in | ||
| // DEFAULT_CHART_FILTERS value, while a present field (including an explicit | ||
| // empty software_filters list, meaning "no categories") is respected. EPSS | ||
| // bounds are numbers (0–100) in the config and strings in the filter state. | ||
| // cvss_min/cvss_max are intentionally NOT wired — there is no severity control | ||
| // yet (#47326). | ||
| export const buildInitialChartFilters = ( | ||
| defaults?: IVulnExposureFilterDefaults | ||
| ): IChartFilterState => { | ||
| if (!defaults) return DEFAULT_CHART_FILTERS; | ||
| return { | ||
| ...DEFAULT_CHART_FILTERS, | ||
| softwareFilters: | ||
| defaults.software_filters !== undefined | ||
| ? [...defaults.software_filters] | ||
| : DEFAULT_CHART_FILTERS.softwareFilters, | ||
| knownExploit: | ||
| defaults.has_known_exploit !== undefined | ||
| ? defaults.has_known_exploit | ||
| : DEFAULT_CHART_FILTERS.knownExploit, |
| it("honors an explicit empty software_filters list as 'none'", () => { | ||
| const filters = buildInitialChartFilters({ software_filters: [] }); | ||
| expect(filters.softwareFilters).toEqual([]); | ||
| }); | ||
|
|
||
| it("seeds the exclude-CVE list", () => { | ||
| const filters = buildInitialChartFilters({ |
| if veFilters := newAppConfig.Features.VulnerabilityExposureHistoricalReporting; veFilters != nil { | ||
| if !lic.IsPremium() { | ||
| invalid.Append("org_settings.features.vulnerability_exposure_historical_reporting", ErrMissingLicense.Error()) | ||
| } else { | ||
| veFilters.Validate("org_settings.features", invalid) | ||
| } | ||
| if invalid.HasErrors() { | ||
| return nil, ctxerr.Wrap(ctx, invalid) | ||
| } | ||
| } |
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
Related issue: For #44746
Checklist for submitter
If some of the following don't apply, delete the relevant line.
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
Testing
Preconditions
sgress454/47327-add-vuln-filters-to-gitops, server running.Workstations) for per-fleet cases.fleetctlconfigured against the instance; a GitOps repo/dir you canfleetctl gitops -ffrom.1. Org-level happy path
YAML (
org_settings.features):echoes exactly these values (EPSS as 10/90, not 0.1/0.9).
filter modal. Software categories = OS + Browsers, "known exploit" on, EPSS slider
shows 10–90, excluded CVE list contains CVE-2025-50897.
YAML (settings.features on a team file, different values from org):
team Workstations
and fields not set on the fleet (e.g. has_known_exploit) fall back to the
built-in chart default — not the org value (per-fleet scope is independent).
org-level values from §1 (no stale fleet values linger).
whole struct nulled; chart opens with all built-in defaults.
Run each as its own GitOps apply; confirm a clear 422/invalid-argument error citing the
field, and that nothing was persisted (re-check GET /config).
lists valid values os, browsers, office, adobe.
exclude_vulnerabilities; valid IDs like CVE-2025-50897 accepted (case-insensitive).
vulnerability_exposure_historical_reporting key.
these values. (Confirms accepted+stored but not consumed.)
Vulnerability-exposure data request → EPSS params are sent as 0.1/0.9 (0–1),
while the YAML/UI/GET /config all stay 10/90.
vulnerability_exposure_historical_reporting block; re-applying it is a no-op (idempotent).
Regression sanity
Summary by CodeRabbit
Release Notes
New Features
Tests