Skip to content

fix(security): unblock Security Audit and route its failures to Slack#508

Merged
murray-ds merged 3 commits into
mainfrom
claude/vibrant-hamilton-0e8gpt
Jun 22, 2026
Merged

fix(security): unblock Security Audit and route its failures to Slack#508
murray-ds merged 3 commits into
mainfrom
claude/vibrant-hamilton-0e8gpt

Conversation

@murray-ds

Copy link
Copy Markdown
Contributor

Summary

The Security Audit workflow has been failing on main (e.g. run 27669555335, the Dependabot dev-dependency bump), and the failure was never announced in Slack. This PR fixes both.

1. Stale OSV-Scanner suppression → failing job

The scan found zero vulnerabilities, but exited non-zero:

osv-scanner.toml has unused ignores:
 - PYSEC-2024-277
 - CVE-2024-34997
Exit code: 1

osv-scanner.toml suppressed the disputed joblib CVE-2024-34997 / PYSEC-2024-277 advisory. joblib 1.4.2 is still pinned, but OSV no longer surfaces that advisory for it, so both ignore entries match nothing — and OSV-Scanner v2 treats unused ignores as a non-clean run. On a push to main the workflow's gate is hard, so the job failed. Removing the now-dead suppressions lets the scan pass.

2. Security Audit failures weren't reaching Slack

slack-notifications.yml only reacts to workflows listed in its workflow_run trigger, and "Security Audit" was missing. So even a genuine vulnerability gate failure would have gone unannounced. Added it to the watched list, alongside the other gating workflows.

Changes

  • osv-scanner.toml — drop the two unused [[IgnoredVulns]] entries; keep the policy header and note why stale suppressions must be removed.
  • .github/workflows/slack-notifications.yml — add "Security Audit" to the watched workflow_run workflows.

Notes

  • No real vulnerability was being masked: the dispute that justified the original suppression was a maintainer-confirmed false positive, and OSV no longer flags joblib 1.4.2 at all.
  • If that advisory (or any other) resurfaces, the scan will fail again as intended — and now Slack will say so.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Su9dsdnq5qaQyTZ9CdkVBb


Generated by Claude Code

claude added 2 commits June 22, 2026 08:02
OSV no longer surfaces CVE-2024-34997 / PYSEC-2024-277 for the pinned
joblib 1.4.2, so the two IgnoredVulns entries match nothing. OSV-Scanner
v2 exits non-zero on ignore entries that match nothing in the current
scan, which failed the Security Audit job on push to main even though the
scan found zero vulnerabilities. Drop the now-unused suppressions.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Su9dsdnq5qaQyTZ9CdkVBb
The Slack failure notifier reacts only to workflows named in its
workflow_run trigger list, which omitted "Security Audit". A failing
security audit on push/schedule therefore went unannounced. Add it to
the watched list so dependency-vulnerability failures surface in Slack
like the other gating workflows.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Su9dsdnq5qaQyTZ9CdkVBb
@claude

claude Bot commented Jun 22, 2026

Copy link
Copy Markdown

Claude finished @murray-ds's task in 1m 9s —— View job


PR Review

  • Gather context and read changed files
  • Review osv-scanner.toml changes
  • Review .github/workflows/slack-notifications.yml changes
  • Post comprehensive feedback

Summary

This is a clean, well-scoped fix. Both root causes are correctly diagnosed and the changes are minimal and targeted. Nothing blocking.


osv-scanner.toml — Removing stale suppressions

The removal is correct. OSV-Scanner v2 exits non-zero when [[IgnoredVulns]] entries match nothing, so stale entries break the gate regardless of whether any real vulnerability exists.

A few observations:

  • Header comment is preserved: The format conventions (use reason, use ignoreUntil for time-bounded suppressions) are still in place for the next engineer who needs to add an entry. Good.
  • "No active suppressions" comment: A clear sentinel state is better than leaving the file empty or ending on the preamble comment mid-thought. This is fine.
  • Rationale is gone from the file: The detailed joblib dispute explanation and the link to joblib/joblib#1588 now live only in git history. If the advisory resurfaces under a new ID, the reviewer won't have any in-file context. This is an acceptable trade-off — the alternative (keeping a comment block with the rationale but no [[IgnoredVulns]] entry) would be inconsistent with the format — but worth being aware of. The PR description captures the reasoning well.

No issues.


.github/workflows/slack-notifications.yml — Adding Security Audit to trigger list

The workflow_run trigger matches on the exact name: field of the target workflow. Verified that security-audit.yml declares name: Security Audit (line 1), so the string matches. This will work correctly.

The placement in the list (between "Page Deploy" and "BigQuery Integration Tests") is fine — the ordering has no semantic effect.

The rest of the notification workflow is unchanged, and the security posture remains solid: no permissions granted, all event values are passed through env: + jq --arg rather than raw ${{ }} interpolation into shell.

No issues.


Overall

Both changes are correct, the PR description is thorough, and the approach is exactly as minimal as it needs to be. Ready to merge.

The policy header already documents the suppression format; drop the
parenthetical explaining OSV-Scanner's exit-code behavior and keep just
the "No active suppressions" marker.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Su9dsdnq5qaQyTZ9CdkVBb
@murray-ds murray-ds merged commit 51e5e0f into main Jun 22, 2026
5 checks passed
@codecov

codecov Bot commented Jun 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
see 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@murray-ds murray-ds deleted the claude/vibrant-hamilton-0e8gpt branch June 22, 2026 08:13
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