Fix #nosec with test ID not counted in Total lines skipped#1408
Open
jonasboos wants to merge 2 commits into
Open
Fix #nosec with test ID not counted in Total lines skipped#1408jonasboos wants to merge 2 commits into
jonasboos wants to merge 2 commits into
Conversation
When a #nosec comment includes a specific test ID (e.g., #nosec B608), the line was only counted in skipped_tests but not in the nosec metric. This meant 'Total lines skipped (#nosec)' showed 0 even when lines had nosec comments, which was confusing. Now both note_nosec() and note_skipped_test() are called when a specific test ID nosec is encountered, so the total accurately reflects all lines with any #nosec comment. Also adds the skipped_tests metric display to the screen and HTML formatters for consistency with the text formatter. Resolves: PyCQA#1205
jonasboos
requested review from
ericwb,
lukehinds and
sigmavirus24
as code owners
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Pull request overview
This pull request fixes Bandit’s Total lines skipped (#nosec) metric so that # nosec BXXX (test-specific) suppressions are counted in the nosec total, not only in the skipped_tests total. It also surfaces the skipped_tests metric in the screen and HTML formatters for consistency with the text formatter.
Changes:
- Increment
nosecfor test-specific# nosec BXXXsuppressions (in addition toskipped_tests) when a finding is actually skipped. - Display the
skipped_testsmetric in the screen and HTML reports. - Update functional/unit tests to reflect the new counting behavior and avoid missing metric keys in formatter tests.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
bandit/core/tester.py |
Counts test-specific nosec skips in both nosec and skipped_tests. |
bandit/formatters/screen.py |
Adds skipped_tests to the on-screen metrics summary. |
bandit/formatters/html.py |
Adds skipped_tests to the HTML metrics summary/template. |
tests/functional/test_functional.py |
Updates expected nosec totals for the functional run. |
tests/unit/formatters/test_screen.py |
Updates mocked metrics totals to include skipped_tests. |
tests/unit/formatters/test_html.py |
Updates mocked metrics totals to include skipped_tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| get_issue_list.return_value = [issue_a, issue_b] | ||
|
|
||
| self.manager.metrics.data["_totals"] = {"loc": 1000, "nosec": 50} | ||
| self.manager.metrics.data["_totals"] = { |
| @mock.patch("bandit.core.manager.BanditManager.get_issue_list") | ||
| def test_report_contents(self, get_issue_list, get_code): | ||
| self.manager.metrics.data["_totals"] = {"loc": 1000, "nosec": 50} | ||
| self.manager.metrics.data["_totals"] = { |
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
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.
Problem
When a
# noseccomment includes a specific test ID (e.g.,# nosec B608), the line is not counted in the "Total lines skipped (#nosec)" metric. Only bare# noseccomments increment the nosec counter, while specific# nosec BXXXcomments only increment theskipped_testscounter.This means if your code uses
# nosec B608, the output shows:even though the line was explicitly skipped by a nosec comment.
Fix
When a
# noseccomment with a specific test ID is encountered, bothnote_nosec()andnote_skipped_test()are now called. This accurately reflects that the line was skipped by a nosec comment, regardless of whether it was bare or specific.The
skipped_testsmetric still tracks how many skips were test-specific, and thenosecmetric now correctly counts all lines with any#noseccomment.Additional changes
skipped_testsmetric display to the screen and HTML formatters for consistency with the text formatter (partially addresses PR Add missing metric to Screen formatted report #1206)Resolves: #1205