False-positive vulnerabilities are cleared at a counterintuitive interval

[iansltx]

UPDATE: @noahtalerman: we decided to document the current behavior as the expected behavior. PR is here.

---

Fleet version: <= 4.64.0

---

💥  Actual behavior

False-positive vulnerabilities are cleared out based on a time limit rather than based on when the current vulnerabilities run started. This means that successive vulns runs will appear to persist false-positive issues that were already resolved.

🧑‍💻  Steps to reproduce

1. Introduce a false positive vulnerability (see recent cpe-translations JSON updates for examples)
2. Run vulnerability scanning
3. Resolve the false positive
4. Run vulnerability scanning again

🕯️ More info

This is most obvious when attempting to QA false-positive fixes, but this also trips up customers after they update to a Fleet release that includes false-positive fixes that couldn't be handled by vuln feed updates, e.g. in #25571.

This behavior is documented in contributor docs, albeit in a footnote, and this behavior is counterintuitive to anyone encountering it for the first time.

From what I can gather here, the intent was to clear false-positives while keeping vulnerabilities runs from clearing vulnerabilities that were added within the same vuln processing run, but we can explicitly check for that condition rather than guessing at how long the vulns run might have taken.

For concerns around concurrent vulnerability scans trampling each other's CVEs after implementing this method (which wouldn't have happened before due to longer periodicity), we set a lock in the DB when starting the vulnerability processing process, so that we shouldn't have concurrent vuln processing runs happening anyway (and if we do, we should fix the locking mechanism, not work around it here).

🛠️ To fix

1. Revise DeleteOutOfDateVulnerabilities and DeleteOutOfDateOSVulnerabilities in the datastore to clear vulnerabilities updated before a set timestamp, rather than by a lookback period.
2. Get the current time right before checkNVDVulnerabilities is called in scanVulnerabilities in cron.go (probably directly from the database to avoid clock skew issues between app servers and the database) and pass that to the calls modified in step 1.

[mostlikelee]

Hey team! Please add your planning poker estimate with Zenhub @iansltx @jahzielv @ksykulev

[noahtalerman]

@mostlikelee we decided to document the current behavior as the expected behavior.

You mentioned that this time lag (~2 hrs) makes testing harder. Up to you to file a separate eng-initiated story to tackle that.

[noahtalerman]

@iansltx @mostlikelee we decided to document the current behavior as the expected behavior. PR is here. Plan is to close the bug once the PR is merged.

Please feel free to file an eng initiated story if you think we should consider prioritizing an improvement.

[noahtalerman]

PR is merged: #26329

[fleet-release]

False-positives cleared,
Time, not runs, sets the pace,
Fleet's truth is endeared.