Repository navigation
fix(ci): scope bandit scan to source dirs instead of excluding tests/ - #28
Merged
Merged
Conversation
Bandit was scanning tests/ and flagging idiomatic pytest patterns as security issues: assert statements (B101), mock credentials like "pw" in fixtures (B106), and stub try/except helpers (B105/B110). These account for ~1,793 of the 1,812 open alerts and are false positives by construction, not vulnerabilities — excluded tests/ from the bandit workflow to close them at the source. The remaining ~19 alerts got individual fixes: - 6 B105 false positives in source (NOON/MIDNIGHT string literals, CalDAV config-key constants) — suppressed with justified # nosec - 2 real B110 findings in config_flow.py: NYC 311 API-key validation silently swallowed all exceptions. Kept the accept-anyway behavior but added debug logging so failures aren't invisible. - 3 B101 false positives in source (type-narrowing assert, dict-sync sanity check) — suppressed with justified # nosec, propagated to the vendored copy via scripts/sync_vendored.py - 4 py/ineffectual-statement alerts (typing.overload stub bodies, mandated PEP 484 syntax) — dismissed via the GitHub API as false positives; removed the stale # lgtm comments that don't actually suppress anything on GitHub's CodeQL Action - 4 remaining CodeQL findings in test files: mixed-return path, missing explanatory comment on an empty except, a dead regex constant, and a duplicate import/import-from of the same module Verified: ruff check, ruff format --check, mypy (src/ and custom_components/), and the full pytest suite (891 passed) are all clean. The 6 test_sign_retrieval.py failures are pre-existing sandbox network-blocking issues, unrelated to this change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…er.py The previous fix on this branch changed `import gps2asp.resolver as resolver_pkg` to `from gps2asp import resolver as resolver_pkg` (commit 1837872) — that's still a from-import of the same module already from-imported at the top of the file (`from gps2asp.resolver import resolve, convert, resolve_segment`), so the CodeQL rule keeps firing. Reuse the already-imported module via sys.modules[resolve.__module__] instead of adding a second import statement for gps2asp.resolver. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
PR #27 set excluded_paths: "tests" on the bandit workflow to stop scanning idiomatic pytest patterns (assert, mock creds in fixtures, stub try/except) as vulnerabilities. It didn't work: after merging to main, the re-run scan still reported 1750+ B101/B105/B106/B110 alerts in tests/. shundor/python-bandit-scan invokes `bandit -r . -x tests`, and bandit's -x glob matching against a `-r .` recursive scan doesn't reliably exclude nested paths. Switched to listing the real source directories (`src custom_components scripts`) as the scan path instead, which sidesteps the exclude-matching behavior entirely. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
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.
Summary
excluded_paths: "tests"on the Bandit workflow did not actually excludetests/— after merging tomain, the re-run scan still opened 1750+ B101/B105/B106/B110 alerts there.shundor/python-bandit-scanrunsbandit -r . -x tests, and bandit's exclude-glob matching against a-r .recursive scan doesn't reliably filter nested paths.path: "src custom_components scripts"scan target, which avoids the exclude-matching behavior entirely.Test plan
mainshows ~0 open alerts undertests/