Repository navigation
fix!: run bandit in the action's Python module instead of the bandit-action fork - #87
Merged
Merged
Conversation
…ction fork The lhoupert/bandit-action fork had three problems: - It installed an unpinned bandit on Python 3.9 (bandit 1.8.6), so files using 3.10+ syntax were skipped silently. - It ran bandit with `|| true` under continue-on-error, so a crash or a missing SARIF read as a clean scan. - It passed a comma-separated bandit_scan_dirs through unchanged when working_directory was '.', so bandit scanned nothing. run_bandit() now runs bandit[sarif]>=1.9.4 from the action's own environment (Python 3.13), on each comma-separated scan dir. It runs from the repository root, so SARIF paths stay repo-relative. Finding paths are percent-decoded, and annotation property values are escaped. It fails closed, as pip-audit does since the previous commits. Each of these cases is reported as "bandit did NOT run", and pip-audit results are still reported: - a missing scan dir; - a non-zero exit (with --exit-zero, findings exit 0, so anything else is a crash or a usage error); - a missing or invalid SARIF; - a file bandit could not parse, listed (at most 20) with how to exclude it on purpose in a `.bandit` file; - no Python file scanned; - an OSError while running bandit. In every case results.sarif is deleted, so neither the artifact nor Code Scanning gets an empty or partial report: that would close open alerts. A SARIF without bandit's file metrics only means the scanned file count is unknown, which is a warning. action.yml: - Removes an earlier results.sarif before the audit. - Uploads it to Code Scanning, as the fork did, with its own pinned upload-sarif step. The step runs before the artifact upload and has continue-on-error, so security-events: write stays optional. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Contributor
✅ All test workflows behaved as expected13 passed, 0 failed
|
lhoupert
changed the title
Settings reads GITHUB_STEP_SUMMARY, GITHUB_WORKSPACE, RUNNER_DEBUG and the action inputs from the environment, and CI sets several of them. The tests then wrote to the real job summary (1,945 bytes in a local repro) and ran with debug output. Clear every variable Settings reads, plus PIPENV_PIPFILE. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A file bandit could not parse or read used to discard the whole report and say "bandit did NOT run", hiding the other files' findings. What happens now: - The findings are kept in the summary, the annotations and the PR comment. - The skipped files are a blocking error that lists them (at most 20). - The SARIF is still deleted, because a partial report would close those files' Code Scanning alerts. The message says so. - The "did NOT run" path is now only for a crash, a usage error, or a missing or malformed SARIF. The message also explains how to exclude files on purpose. A `.bandit` `exclude` list replaces bandit's default excludes, so the example repeats them. It uses `*/name/*` globs because bandit compares the patterns with paths such as `./.venv/...` when it scans `.`, and plain names like `.venv` then do not match: checked with bandit 1.9.4 and this invocation. When a skipped file is under a virtual environment or node_modules, the message says so. The bandit run itself is harder to break: - bandit runs as `sys.executable -m bandit`, so it is always the bandit of the action's environment. - Overlapping scan dirs (for example `src,.`) are scanned once: repeated and nested dirs are dropped, otherwise bandit reports their files twice. - bandit's stderr is capped at its last 2,000 characters. - Any malformed SARIF value is an AuditError, instead of a TypeError that crashed the run before pip-audit and left the file to be uploaded. - When the file count is unknown, the summary now says so too, not only an annotation. - The dead FileNotFoundError catch around run_bandit is removed. The pip-audit one stays: _resolve_exe still raises it there. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
action.yml uses contains(inputs.tools, 'bandit'), which is case-insensitive, while enabled_tools compared exact strings. With `tools: Bandit`, the action deleted results.sarif and tried the upload, but the module skipped bandit, so the job passed with nothing scanned. Tools are now lowercased and trimmed. An unknown or empty tool list fails with a clear error instead of scanning nothing. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
upload-sarif aborts a second upload with the same category in the same job
("only one run of the codeql/analyze or codeql/upload-sarif actions is
allowed per job per tool/category"). So in a monorepo job that runs the
action twice, the second project's results were never uploaded.
A working_directory other than '.' now uploads under the category
`bandit/<working_directory>`. For '.', the category stays empty, which
upload-sarif treats as unset (getOptionalInput maps "" to undefined). That
is what the bandit fork did, so existing alerts keep matching.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A missing results.sarif was read as "no bandit findings". The action now deletes the file when bandit did not run or skipped files, so every case (all of them run bandit) must produce one, like the pip-audit "no report" check. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…issions
README:
- Only one `.bandit` file is read, and only inside the scanned
directories. bandit fails when it finds more than one.
- An `exclude` list replaces bandit's default excludes, so the example
repeats them as `*/name/*` globs. A plain `.venv` does not match when
bandit scans `.`.
- With the default `.`, a virtual environment or node_modules is scanned
too.
- In private repositories the upload needs `actions: read`: upload-sarif
calls GET /repos/{owner}/{repo}/actions/runs/{run_id}. The upload is
best effort.
CLAUDE.md: bandit runs as a subprocess (`python -m bandit`), not
in-process.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The tools validator already lowercases, trims and drops empty names. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
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
2 participants
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.
Bandit now runs from the action's own Python environment (
python -m bandit,bandit[sarif]>=1.9.4on Python 3.13) instead of through thelhoupert/bandit-actionfork. The fork ran on Python 3.9, so files using 3.10+ syntax were skipped silently; it hid crashes behind|| true; and it scanned nothing for the README'sbandit_scan_dirs: 'src/,scripts/'. As #85 does for pip-audit, a crash, a missing scan dir or an empty scan now fails the step with "bandit did NOT run". Files bandit cannot parse block the job while the other files' findings are still reported, and no partial SARIF is uploaded to Code Scanning, where eachworking_directorynow has its own category. Closes #6.For reviewers: all 13 integration cases give the fork's rule IDs, so
expected_results.ymlis unchanged. Repos with a missing scan dir or unparseable files now turn red; that includes a.venvunder the default.. The README gives the.banditexcludeline that skips them.Author attestation
AI-assisted: Claude Code wrote the change, the tests and the local end-to-end checks.