feat(security): vulnerability-centric security-burndown command#30
Merged
Conversation
- ghas_alerts.py: refactor _fetch_dependabot_counts to return (counts, details) tuple; fetch_ghas_alerts attaches dependabot_details sibling key to each repo entry alongside the unchanged dependabot counts dict - security_burndown.py: new module — build_security_burndown filters to runtime-scope fixable critical/high alerts, groups by advisory (ghsa_id or ecosystem+package+version fallback), deduplicates clone-repos, ranks critical-before-high then repo-count desc; render_burndown_markdown produces a # Security Burndown markdown doc with ranked table - cli.py: adds `audit security-burndown <username>` subcommand with own dedicated parser; detects counts-only (pre-detail) GHAS files and prints a clear re-run warning; writes output/security-burndown-<user>-<date>.md - tests/test_security_burndown.py: 31 new tests covering detail extraction, defensiveness, filtering (dev/null scope, no-fix, medium/low), grouping-dedup (same ghsa 3 repos → 1 entry), ranking, empty-state, non-breaking counts shape assertion
![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: ed2e4ce296
ℹ️ 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".
| repo_result: dict = { | ||
| "dependabot": _fetch_dependabot_counts(s, owner, repo), | ||
| "dependabot": dep_counts, | ||
| "dependabot_details": dep_details, |
There was a problem hiding this comment.
When a user has an existing ghas-alerts-* cache entry from before this change, fetch_ghas_alerts returns it unchanged from the cache branch above and never reaches this fresh-fetch path that adds dependabot_details. A normal audit report <user> --ghas-alerts rerun within the GHAS cache TTL will therefore write another counts-only file, and security-burndown will keep warning the user to rerun without fixing itself. Please version the cache params/key or reject cached repo entries that lack the new sibling before writing the report.
Useful? React with 👍 / 👎.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
CodeQL py/clear-text-logging-sensitive-data flagged the GHAS fetch exception handlers: the authenticated session carries the token, so logging the raw exception (`exc`) or response-derived `status` is a potential secret-in-logs sink. Harden all three fetch handlers (dependabot/code-scanning/secret-scanning) to log only the repo identity plus the exception class name (`type(exc).__name__`) — never the exception object or response status. Keeps useful diagnostics (which repo, which error class) without routing session/response-derived data to the log. Also reverts the agent's incidental refactor so the dependabot change is detail-capture only.
…t_details (zero-diff ghas_alerts) Move per-alert Dependabot detail fetching out of ghas_alerts.py into a new module src/ghas_alert_details.py so ghas_alerts.py ends up byte-for-byte identical to main, preventing ruff-format reflows that CodeQL flags as clear-text-logging sinks. - src/ghas_alerts.py: reverted to main (no changes) - src/ghas_alert_details.py: new module — fetch_dependabot_details() paginates the same endpoint as fetch_ghas_alerts but extracts flat detail dicts; all except handlers use static-string-only log messages (zero format args) to satisfy the CodeQL clear-text-logging contract; best-effort per-repo (errors yield [] and continue) - src/cli.py: ghas-alerts block calls fetch_dependabot_details after counts fetch, merges dependabot_details into each repo entry before JSON write - tests/test_security_burndown.py: replace TestFetchDependabotDetail / TestCountsShapeUnchanged (targeted reverted approach) with TestFetchDependabotDetails covering extraction, defensiveness, error paths, partial-failure continuation, and static-log assertion
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.
What
The Automation arc — makes the security radar actionable. Counts alone ("307 high across 72 repos") don't tell you what to fix. This adds
audit security-burndown <username>: a ranked, vulnerability-centric burndown that answers "what do I fix next, and how many repos does each fix clear."How it works
Per-alert detail capture (
src/ghas_alerts.py) —_fetch_dependabot_countsnow returns(counts, details)from the same alert pagination (zero extra API calls). Each repo's GHAS entry gains a siblingdependabot_detailslist (package, ecosystem, scope, severity, ghsa_id, first_patched_version, manifest_path). Thedependabotcounts dict is byte-for-byte unchanged — the SecurityFields overlay (_build_security_fields,_load_security_alerts_by_name) is untouched.Burndown builder (
src/security_burndown.py) — filters to runtime-scope (prod-reachable) + fixable (a patched version exists) + critical/high, then groups by advisory (ghsa_id, or ecosystem+package+version). Grouping is what collapses the clone-repo inflation: oneaxiosadvisory across IncidentWorkbench + its 3 branch-clones becomes a single entry listing 4 repos. Ranked critical→high, then affected-repo-count desc, then package asc.CLI —
audit security-burndown <user>loads the latestghas-alerts-<user>-*.json, builds + prints + writesoutput/security-burndown-<user>-<date>.md. Old counts-only files (pre-detail fetch) hit a clean warning to re-run the fetch, exit 0 — no crash.Sample output
Tests
31 new behavior-focused tests (detail extraction + defensiveness, all filter exclusions, dedup grouping, ranking tiebreaks, rendering edge cases, and 3 non-breaking assertions that the counts contract is intact). Full suite 2181 passed, 2 skipped;
ruff check .clean.Note
Scheduling (a launchd job to keep the overlay fresh) is a deliberate follow-on, not in this PR. Dead
_SEVERITY_HIGHESTmap caught in review and removed.