Don't fail when no vulnerabilities were detected #187
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @DerGuteMoritz, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request modifies the fail-build? function in src/nvd/report.clj to prevent the build from failing when no vulnerabilities with a score greater than 0 are detected, especially in cases where the NVD hasn't assigned scores to newly reported CVEs yet. It achieves this by ensuring that the highest-score is only compared to the fail-threshold if there are actually scores to consider.
Highlights
- Build Failure Logic: The pull request changes the logic for determining build failure based on vulnerability scores.
- NVD Score Handling: It addresses the scenario where NVD scores might be missing or zero, preventing premature build failures.
- Threshold Configuration: It allows setting
fail-thresholdto -1 to fail only when vulnerabilities with a score of 0 are detected.
Changelog
- src/nvd/report.clj
- Modified the
fail-build?function to check if there are any scores before comparing thehighest-scorewith thefail-threshold. - Added
(seq all-scores)check to ensure that the build only fails if there are vulnerabilities detected.
- Modified the
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
When CVEs new make their debut,
And scores from NVD are askew,
A build's false alarm,
Can cause great harm,
This PR ensures checks are true.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
The pull request modifies the fail-build? function to consider cases where no vulnerabilities are detected or only vulnerabilities with a score of 0 are detected. This is achieved by adding a check to ensure that all-scores is not empty before comparing highest-score with fail-threshold. The change seems reasonable and addresses the issue described in the pull request description.
Summary of Findings
- Clarity of Condition: The added condition
(and (seq all-scores) (> highest-score fail-threshold))could be made more explicit for better readability. Consider adding a comment to explain the purpose of checking(seq all-scores).
Merge Readiness
The code change appears to address the intended issue. However, adding a comment to clarify the purpose of the (seq all-scores) check would improve readability. I am unable to directly approve the pull request, and recommend that others review and approve this code before merging.
| (assoc :failed? (and (seq all-scores) | ||
| (> highest-score fail-threshold)))))) |
There was a problem hiding this comment.
Consider adding a comment here to explain why (seq all-scores) is being checked. This makes the logic more explicit and easier to understand at a glance.
(assoc :failed? (and (seq all-scores) ; Ensure there are scores before comparing
(> highest-score fail-threshold))))))
There was a problem hiding this comment.
Does any actual person care for such a comment? Seems a bit silly to me given how small and obvious the function is.
There was a problem hiding this comment.
Also, the comment doesn't even explain why we do this, it only re-states what we do. As the saying goes: The I in LLM stands for intelligence 😜
This allows setting fail-threshold to -1 which will result in a failure in case only vulnerabilities with a score of 0 were detected. This is a pertty common occurrence these days as the NVD struggles to keep up with assigning scores to newly reported CVEs in a timely manner.
DerGuteMoritz
force-pushed
the
dont-fail-when-no-vulnerabilities-detected
branch
from
46598c0 to
ac6a0b1
Compare
This allows setting fail-threshold to -1 which will result in a failure in case only vulnerabilities with a score of 0 were detected. This is a pretty common occurrence these days as the NVD struggles to keep up with assigning scores to newly reported CVEs in a timely manner.