security: apply all audit fixes (H-01 through I-04)#13
Conversation
H-01: Pin GitHub Actions to immutable commit SHAs - actions/checkout@v1 → @11bd71901bbe5b1630ceea73d27597364c9af683 (v4.2.2) - actions/setup-python@v1 → @42375524bfc43e60c680de71eb3fd7bc68ee5416 (v5.4.0) H-02: Remove phantom astpretty==2.1.0 production dependency (unused) M-01: Fix detection bypass — add generic_visit(node) to DebugVisitor.visit_Call so nested debug calls (e.g. foo(print(x))) are no longer silently missed M-02: Fix false positives on arbitrary objects — restrict attribute-based detection to known debugger modules (pdb, ipdb) only for set_trace M-03: Update outdated dev dependencies - pytest-cov 3.0.0 → 7.1.0 - coverage 5.3 → 7.13.5 - coveralls 2.1.2 → 4.1.0 - wheel 0.45.1 → 0.46.3 L-01: Restrict CI trigger to master branch; add least-privilege permissions block Use COVERALLS_REPO_TOKEN instead of broad GITHUB_TOKEN for coveralls L-02: Move Meta import inside if __name__ == '__main__' in setup.py to prevent source package execution at pip install time L-03: Add RecursionError guard in NoDebug.run() for pathologically nested ASTs L-04: Add .github/SECURITY.md with vulnerability disclosure policy I-01: Remove unused self._filename attribute from NoDebug.__init__ I-03: Inherit Error from abc.ABC for proper abstract base class semantics I-04: Remove committed __pycache__ and .DS_Store files from git index Tests: add 5 regression tests covering nested call detection and false-positive elimination for arbitrary object method calls Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad9d8cdf69
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| coveralls==2.1.2 | ||
| pytest-cov==7.1.0 | ||
| coverage==7.13.5 | ||
| coveralls==4.1.0 |
There was a problem hiding this comment.
coveralls on a Python-3.9-compatible release
Pinning coveralls to 4.1.0 breaks one of the versions this repo still supports. PyPI metadata for 4.1.0 requires Python 3.10+, but .github/workflows/analysis.yml still installs requirements-dev.txt in the 3.9 matrix entry and setup.py still advertises Python 3.9 support. In practice, pip install -r requirements-dev.txt will now fail on 3.9 in CI and for contributors using the documented minimum version.
Useful? React with 👍 / 👎.
flake8_debug/plugin.py
Outdated
| # Only these func names are meaningful as attribute calls (e.g. pdb.set_trace()) | ||
| _ATTR_DETECTABLE: frozenset = frozenset({'set_trace'}) |
There was a problem hiding this comment.
breakpointhook() calls
This allowlist now disables attribute-based matching for breakpointhook, so import sys; sys.breakpointhook() no longer emits DB201 even though it is the same debugger hook the plugin already reports when imported directly. The previous matcher caught the stdlib-qualified form, so this change introduces a real false negative in the core lint rule.
Useful? React with 👍 / 👎.
| and isinstance(node.func.value, ast.Name) | ||
| and node.func.value.id in _DEBUGGER_MODULES |
There was a problem hiding this comment.
set_trace
Restricting set_trace checks to literal names pdb and ipdb misses common alias imports such as import pdb as dbg; dbg.set_trace() or import ipdb as debugger; debugger.set_trace(). Those are still debugger calls, and previous versions reported them, but this branch now lets them pass silently because node.func.value.id is dbg/debugger instead of one of the hard-coded identifiers.
Useful? React with 👍 / 👎.
Pull Request Test Coverage Report for Build 23440449867Details
💛 - Coveralls |
Pull Request Test Coverage Report for Build 23440205493Details
💛 - Coveralls |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* security: apply all audit fixes (H-01 through I-04) H-01: Pin GitHub Actions to immutable commit SHAs - actions/checkout@v1 → @11bd71901bbe5b1630ceea73d27597364c9af683 (v4.2.2) - actions/setup-python@v1 → @42375524bfc43e60c680de71eb3fd7bc68ee5416 (v5.4.0) H-02: Remove phantom astpretty==2.1.0 production dependency (unused) M-01: Fix detection bypass — add generic_visit(node) to DebugVisitor.visit_Call so nested debug calls (e.g. foo(print(x))) are no longer silently missed M-02: Fix false positives on arbitrary objects — restrict attribute-based detection to known debugger modules (pdb, ipdb) only for set_trace M-03: Update outdated dev dependencies - pytest-cov 3.0.0 → 7.1.0 - coverage 5.3 → 7.13.5 - coveralls 2.1.2 → 4.1.0 - wheel 0.45.1 → 0.46.3 L-01: Restrict CI trigger to master branch; add least-privilege permissions block Use COVERALLS_REPO_TOKEN instead of broad GITHUB_TOKEN for coveralls L-02: Move Meta import inside if __name__ == '__main__' in setup.py to prevent source package execution at pip install time L-03: Add RecursionError guard in NoDebug.run() for pathologically nested ASTs L-04: Add .github/SECURITY.md with vulnerability disclosure policy I-01: Remove unused self._filename attribute from NoDebug.__init__ I-03: Inherit Error from abc.ABC for proper abstract base class semantics I-04: Remove committed __pycache__ and .DS_Store files from git index Tests: add 5 regression tests covering nested call detection and false-positive elimination for arbitrary object method calls Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * revert: restore analysis.yml to original state Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ci: remove Python 3.9 from test matrix Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: shorten comment in plugin.py to satisfy E501 line length Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: update minimum Python version to 3.10 in README Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * remove: delete redundant SECURITY.md Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2 participants
Summary
Addresses all 14 findings from a full security audit of the repository, covering supply chain risks, plugin logic flaws, CI/CD hardening, and code quality.
Changes by severity
🔴 High
actions/checkout@v1→ v4.2.2 SHA,actions/setup-python@v1→ v5.4.0 SHA. Mutable@v1tags are from 2019 and vulnerable to supply chain compromise.astpretty==2.1.0production dependency. It was never imported, adding unnecessary attack surface for all users.🟠 Medium
self.generic_visit(node)toDebugVisitor.visit_Call— nested debug calls such asfoo(print(x))orlambda: breakpoint()were silently bypassing detection.pdb,ipdb) — previously any object with a method namedprint,breakpoint, orset_tracetriggered a false positive (e.g.logger.print(),self.breakpoint()).coverage5.3→7.13.5,pytest-cov3.0.0→7.1.0,coveralls2.1.2→4.1.0,wheel0.45.1→0.46.3.GITHUB_TOKENto scopedCOVERALLS_REPO_TOKENsecret.🟡 Low
masterbranch pushes/PRs only; addpermissions: contents: readleast-privilege block.from flake8_debug.meta import Metainsideif __name__ == '__main__':insetup.pyto prevent source package execution atpip installtime.RecursionErrorguard inNoDebug.run()for pathologically nested ASTs (Python 3.9–3.12 vulnerability)..github/SECURITY.mdwith supported versions table and responsible disclosure instructions.ℹ️ Informational
self._filenameattribute fromNoDebug.__init__(parameter kept for flake8 API compatibility).Errorbase class now inherits fromabc.ABCfor proper abstract base class semantics.__pycache__and.DS_Storefiles from git index.Tests
test_nested_print_is_detected— verifies M-01 fixtest_nested_breakpoint_is_detected— verifies M-01 fixtest_no_false_positive_on_arbitrary_object_print— verifies M-02 fixtest_no_false_positive_on_arbitrary_object_breakpoint— verifies M-02 fixtest_no_false_positive_on_arbitrary_object_set_trace— verifies M-02 fixAction required after merge
COVERALLS_REPO_TOKENas a repository secret in GitHub Settings → Secrets (replacesGITHUB_TOKENfor coveralls uploads).