Skip to content

Wire bandit and pip-audit into CI#1209

Open
nucleusjay wants to merge 1 commit into
safishamsi:v8from
nucleusjay:ci-run-security-scanners
Open

Wire bandit and pip-audit into CI#1209
nucleusjay wants to merge 1 commit into
safishamsi:v8from
nucleusjay:ci-run-security-scanners

Conversation

@nucleusjay

Copy link
Copy Markdown

bandit, pip-audit, and safety are already declared in the [dependency-groups].dev table but nothing in .github/workflows/ci.yml invokes them. So a new HIGH-severity bandit finding or a freshly-disclosed CVE in a pinned dep can land without anyone noticing until the next manual audit.

Adds a security-scan job that runs on every push and PR:

  • bandit -r graphify -ll (HIGH-severity only -- LOW/MEDIUM is noisy without a tuned config)
  • pip-audit --strict (against the committed lock via uv sync)

Marked continue-on-error: true for now so this doesn't block PRs on pre-existing findings. A follow-up should do the cleanup pass and flip the flag to blocking.

safety intentionally omitted -- it now requires a free-tier API key for its commercial backend, which is a setup burden for forks and CI. pip-audit covers the same ground via the PyPI JSON advisory feed and OSV.

Test plan

  • CI runs the new job and posts results.
  • Confirm existing pyproject [tool.bandit] skip (B404) is honored.
  • No churn to uv.lock from the new job.

🤖 Generated with Claude Code

bandit, pip-audit, and safety are already declared in the dev dependency
group but nothing in CI invokes them, so a new HIGH-severity finding or
a newly-disclosed CVE in a pinned dep can land without anyone noticing
until the next manual audit.

Add a security-scan job that runs bandit (-ll, HIGH-severity only) and
pip-audit (--strict) on every push and PR. Marked continue-on-error so
this doesn't block PRs on pre-existing findings -- a follow-up should
do the cleanup pass and flip the flag.

safety intentionally omitted: it requires a free-tier API key for the
new commercial backend, which is a setup burden for forks. pip-audit
covers the same ground using the PyPI JSON advisory feed and OSV.

@safishamsi safishamsi left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The concept is right — non-blocking rollout with continue-on-error: true is the correct approach given the current state of findings. But there are three bugs in the implementation that need fixing before this lands.

Bug 1 — bandit ignores [tool.bandit] config

# current
run: bandit -r graphify -ll

# fix
run: bandit -r graphify -c pyproject.toml -ll

bandit does not auto-discover [tool.bandit] from pyproject.toml-c must be passed explicitly. Without it, the skips = ["B404"] setting is silently ignored and the job surfaces 10 B404 false positives on every run. I verified this locally: with -c pyproject.toml, B404 is correctly suppressed.

Bug 2 — pip-audit misses 6 of 7 real CVEs

# current
run: uv sync --frozen

# fix  
run: uv sync --all-extras --frozen

The test job already uses --all-extras --frozen — this job should match. Without it, pip-audit audits only the base environment and misses the mcp extra entirely, which is where the real findings live:

  • pyjwt — 4 CVEs (in mcp extra)
  • starlette — 2 CVEs (in mcp extra)

With the current setup, the job reports 1 finding (the pip bootstrap tool, not even a shipped dep). With --all-extras, it reports 7. The job as written gives false confidence.

Bug 3 — severity flag mislabeled

The PR description says -ll = "HIGH-severity only" but per bandit --help, -ll is MEDIUM and higher (-lll is HIGH-only). The flag itself is probably fine (MEDIUM+ is a reasonable threshold), but the description needs correcting — or if HIGH-only was the actual intent, change to -lll.


Fix the three items above and this is good to merge. One heads-up: once --all-extras is in place, pip-audit will start surfacing the pyjwt/starlette CVEs on every run. Worth resolving those before flipping continue-on-error to false in a follow-up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants