Enable flake8-bandit (Ruff "S") lint rules#3606
Open
rivassec wants to merge 1 commit into
Open
Conversation
Addresses the `# TODO: "S"` marker in pyproject.toml. Adds a per-file ignore for `pelican/tests/*` covering S-rules that are universally appropriate to suppress in test code (S101 assert, S105/S106 hardcoded test passwords, S108 /tmp paths, S311 non-cryptographic randomness, S603/S607 subprocess fixtures, S104 loopback bindings). Each remaining library finding gets a targeted `# noqa` with a justification comment explaining why the call is acceptable in context. Two `S101` suppressions in `pelican_import.py` and one `S504` in `server.py` flag latent issues that warrant follow-up but are intentionally deferred so this commit stays purely lint-enabling. No behavior changes.
Author
|
FWIW the main role of this PR is to start rolling out bandid |
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
1 participant
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.
Summary
This PR resolves the
# TODO: "S"marker inpyproject.tomlby enabling theflake8-banditsecurity rule set with a clean, low-noise suppression strategy.per-file-ignoresrule forpelican/tests/*. This cleanly suppresses rules that don't make sense for test suites, likeS101(asserts),S105/S106(mock credentials),S108(/tmppaths), andS311(non-crypto random numbers).# noqacomment with a clear context justification.There are no logic or runtime behavior changes in this commit.
Why
This check has been sitting on the maintainer wishlist for a while but was blocked by the backlog of existing violations. By unblocking it now, we ensure every future contribution gets automatic static security analysis. The actual code issues we found are now cleanly surfaced and tracked rather than being buried in generic technical debt.
Test plan
pdm lint --diffto confirm the formatting is green.pdm run pytest pelican/tests -qto verify all 312 tests still pass.ruff check --select Sto ensure the new ruleset runs cleanly.Follow-up work
These items were intentionally left out of this PR to avoid breaking changes, but they should be tackled next:
assertstatements used for control flow inpelican_import.py(which get completely stripped out if Python runs with optimizations).pelican/server.pyaway from the deprecatedssl.wrap_sockettowardssl.SSLContext.shell=Truesubprocess call used during the WordPress pandoc import step.