feat(ci): add SARIF output to npm audit workflow#433
Conversation
Co-authored-by: Mossaka <5447827+Mossaka@users.noreply.github.com>
|
@copilot-swe-agent review |
Code review complete ✅ The implementation is production-ready and working correctly: ✅ Verified:
Testing results:
Note: The script correctly reports only root cause security advisories, not every transitive dependency affected. For example, if eslint has a vulnerability and 4 other packages depend on it, SARIF reports 1 issue (eslint), not 5. All acceptance criteria met - ready to merge. |
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
|
@copilot please merge origin/main |
…copilot/add-sarif-output-to-npm-audit
Mossaka
left a comment
There was a problem hiding this comment.
Review: REQUEST_CHANGES
@copilot Please fix the following issue:
Issue: Missing Null Safety Check
In scripts/ci/npm-audit-to-sarif.ts, if npm audit --json produces malformed output (empty object or missing vulnerabilities key), the script will crash with:
TypeError: Cannot convert undefined or null to object
Required Fix
Add defensive null check:
// Before (current code):
for (const [pkgName, vuln] of Object.entries(npmAudit.vulnerabilities)) {
// After (with null safety):
const vulnerabilities = npmAudit.vulnerabilities || {};
for (const [pkgName, vuln] of Object.entries(vulnerabilities)) {Minor Suggestion
The workflow uses both || true AND continue-on-error: true on the JSON generation step. Only one is needed - consider removing the redundant continue-on-error: true.
The SARIF implementation is otherwise correct and follows best practices.
Co-authored-by: Mossaka <5447827+Mossaka@users.noreply.github.com>
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
✅ Coverage Check PassedOverall Coverage
Coverage comparison generated by |
Smoke Test Results - Claude EngineLast 2 Merged PRs:
Test Results:
Status: PARTIAL PASS (3/4 tests passed) Note: Playwright test failed due to missing browser binaries, not a Claude engine issue.
|
Smoke Test Results (Copilot) ✅ PASSLast 2 Merged PRs:
Tests:
cc @Mossaka
|
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
Smoke Test ResultsLast 2 merged PRs:
Test Results:
Status: PASS
|
Smoke Test Results ✅Last 2 Merged PRs:
Test Results:
Status: PASS cc @Mossaka
|
npm audit results currently only appear in workflow logs. This adds SARIF upload to surface vulnerabilities in the GitHub Security tab alongside CodeQL and Trivy scans.
Changes
scripts/ci/npm-audit-to-sarif.ts: TypeScript converter for npm audit JSON → SARIF 2.1.0.github/workflows/dependency-audit.yml: Updated workflow to generate and upload SARIFsecurity-events: writepermissionnpm-audit-mainandnpm-audit-docsif: always()to capture results even when vulnerabilities existWorkflow
Vulnerabilities will now appear in Security → Code scanning alerts with proper severity classification and advisory links.
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.