fix(ci): move CVE scan to separate workflow to fix permission conflict#282
Conversation
The reusable workflow (check-code.yaml) called by release-github.yaml cannot have jobs requesting issues:write. Moved check-vulnerabilities to its own workflow with proper permissions and a weekly schedule. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe ChangesVulnerability check workflow relocation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Scheduler as PR/Schedule Trigger
participant Workflow as check-vulnerabilities workflow
participant PipAudit as pip-audit
participant OSV as OSV API
participant GitHubIssues as GitHub Issues
Scheduler->>Workflow: trigger run
Workflow->>PipAudit: run audit on exported requirements
PipAudit-->>Workflow: JSON audit results
Workflow->>OSV: query severity per unique CVE
OSV-->>Workflow: severity scores
Workflow->>Workflow: classify critical/high findings
alt critical or high findings exist
Workflow->>GitHubIssues: create or update automated security issue
end
Workflow->>Workflow: emit warnings for critical CVEs (non-blocking)
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/check-vulnerabilities.yaml (1)
93-133: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winResolved-vulnerability case leaves a stale open issue.
The create/update block only executes when
critical.length > 0 || high.length > 0. When a later scan comes back clean, any previously openedsecurity,automatedissue is neither updated nor closed, so it lingers as an open, out-of-date alert. Consider closing (or commenting on) the existing automated issue when no critical/high CVEs remain.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/check-vulnerabilities.yaml around lines 93 - 133, The issue is that the automated security issue management in the vulnerability scan workflow only handles non-empty critical/high results, leaving a previously created automated issue open when a later scan is clean. Update the logic around the existing issue lookup in the workflow step to also handle the zero-vulnerability case by finding the current issue created by the automated flow and closing it, or otherwise marking it resolved, when both critical and high arrays are empty. Keep the change centered on the current create/update block and the existing issue search that uses the automated label.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/check-vulnerabilities.yaml:
- Around line 3-10: The workflow currently lets the vulnerability-filing logic
run on pull_request events, which can fail on forked PRs and create noisy
duplicate issues on normal PRs. Update the check-vulnerabilities workflow so the
issue creation/update logic in the Process results step only runs for scheduled
executions by checking github.event_name in that step, or otherwise make the
pull_request path reporting-only. Keep the change focused around the Process
results github-script and the existing on: trigger block.
- Around line 37-38: The audit-results handling in the workflow’s Node.js
snippet should guard against missing or empty /tmp/audit-results.json before
calling readFileSync and JSON.parse. Update the logic around the existing fs
usage so it first checks the file exists and has content, then only parses when
valid, otherwise skips JSON loading and handles the no-results case gracefully
in the audit step.
- Around line 53-76: The severity scoring logic in the OSV fetch loop currently
relies on non-standard database_specific fields, which can leave CVSS
vulnerabilities at 0. Update the parsing in the uniqueIds processing block to
read the top-level severity[] entries first, convert any CVSS vector information
to a base score, and then fall back to database_specific.cvss_v3 or
database_specific.severity if needed. Keep the existing severityMap assignment
in place, but ensure the score calculation in this fetch path prefers OSV
severity data before the fallback fields.
---
Nitpick comments:
In @.github/workflows/check-vulnerabilities.yaml:
- Around line 93-133: The issue is that the automated security issue management
in the vulnerability scan workflow only handles non-empty critical/high results,
leaving a previously created automated issue open when a later scan is clean.
Update the logic around the existing issue lookup in the workflow step to also
handle the zero-vulnerability case by finding the current issue created by the
automated flow and closing it, or otherwise marking it resolved, when both
critical and high arrays are empty. Keep the change centered on the current
create/update block and the existing issue search that uses the automated label.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 828935d6-5cfe-47b5-a51b-d24287b7a55a
📒 Files selected for processing (2)
.github/workflows/check-code.yaml.github/workflows/check-vulnerabilities.yaml
💤 Files with no reviewable changes (1)
- .github/workflows/check-code.yaml
| on: | ||
| pull_request: | ||
| schedule: | ||
| - cron: '0 8 * * 1' | ||
|
|
||
| permissions: | ||
| contents: read | ||
| issues: write |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
pull_request trigger will write GitHub issues from PR context (fails on fork PRs, noisy on same-repo PRs).
The issue create/update in the Process results step runs on every pull_request event when vulns are found. For PRs from forks, GITHUB_TOKEN is downgraded to read-only regardless of the permissions: block, so issues.create/issues.update return 403 — and since this github-script step isn't continue-on-error, the job fails. For same-repo PRs it will create/update the shared security issue on every PR, duplicating the scheduled run.
Consider restricting the issue-filing logic to the scheduled event (e.g. gate the write on github.event_name == 'schedule'), or scope the PR run to reporting-only.
🔧 Example: gate issue writes to scheduled runs
// Create/update GH issue for high+critical
- if (critical.length > 0 || high.length > 0) {
+ if ((critical.length > 0 || high.length > 0) && context.eventName === 'schedule') {Also applies to: 32-33
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/check-vulnerabilities.yaml around lines 3 - 10, The
workflow currently lets the vulnerability-filing logic run on pull_request
events, which can fail on forked PRs and create noisy duplicate issues on normal
PRs. Update the check-vulnerabilities workflow so the issue creation/update
logic in the Process results step only runs for scheduled executions by checking
github.event_name in that step, or otherwise make the pull_request path
reporting-only. Keep the change focused around the Process results github-script
and the existing on: trigger block.
| const fs = require('fs'); | ||
| const results = JSON.parse(fs.readFileSync('/tmp/audit-results.json', 'utf8')); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Guard against a missing/empty audit-results file.
steps.audit.outcome == 'failure' fires whenever pip-audit exits non-zero, which includes tool errors — not only when vulnerabilities are found. In those cases /tmp/audit-results.json may be absent or empty, and readFileSync/JSON.parse will throw and fail the step. A small existence/parse guard makes this robust.
🛡️ Proposed guard
const fs = require('fs');
- const results = JSON.parse(fs.readFileSync('/tmp/audit-results.json', 'utf8'));
+ if (!fs.existsSync('/tmp/audit-results.json')) {
+ console.log('No audit results file found; pip-audit may have errored. Skipping.');
+ return;
+ }
+ let results;
+ try {
+ results = JSON.parse(fs.readFileSync('/tmp/audit-results.json', 'utf8'));
+ } catch (e) {
+ core.warning(`Could not parse audit results: ${e.message}`);
+ return;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const fs = require('fs'); | |
| const results = JSON.parse(fs.readFileSync('/tmp/audit-results.json', 'utf8')); | |
| const fs = require('fs'); | |
| if (!fs.existsSync('/tmp/audit-results.json')) { | |
| console.log('No audit results file found; pip-audit may have errored. Skipping.'); | |
| return; | |
| } | |
| let results; | |
| try { | |
| results = JSON.parse(fs.readFileSync('/tmp/audit-results.json', 'utf8')); | |
| } catch (e) { | |
| core.warning(`Could not parse audit results: ${e.message}`); | |
| return; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/check-vulnerabilities.yaml around lines 37 - 38, The
audit-results handling in the workflow’s Node.js snippet should guard against
missing or empty /tmp/audit-results.json before calling readFileSync and
JSON.parse. Update the logic around the existing fs usage so it first checks the
file exists and has content, then only parses when valid, otherwise skips JSON
loading and handles the no-results case gracefully in the audit step.
| for (const id of uniqueIds) { | ||
| try { | ||
| const resp = await fetch(`https://api.osv.dev/v1/vulns/${id}`); | ||
| if (!resp.ok) continue; | ||
| const data = await resp.json(); | ||
| let score = 0; | ||
| if (data.database_specific?.cvss_v3) { | ||
| score = typeof data.database_specific.cvss_v3 === 'number' | ||
| ? data.database_specific.cvss_v3 | ||
| : parseFloat(data.database_specific.cvss_v3) || 0; | ||
| } | ||
| if (data.database_specific?.severity) { | ||
| const sev = data.database_specific.severity.toUpperCase(); | ||
| if (sev === 'CRITICAL') score = Math.max(score, 9.0); | ||
| else if (sev === 'HIGH') score = Math.max(score, 7.0); | ||
| } | ||
| if (score === 0 && data.affected) { | ||
| for (const a of data.affected) { | ||
| if (a.database_specific?.cvss_v3) { | ||
| score = Math.max(score, parseFloat(a.database_specific.cvss_v3) || 0); | ||
| } | ||
| } | ||
| } | ||
| severityMap[id] = score; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
OSV API v1 vulns response severity field schema CVSS_V3 database_specific
💡 Result:
In the OSV (Open Source Vulnerability) schema, the severity of a vulnerability is represented by the severity field, which is an array of objects [1][2]. Each object in this array contains a type and a score [1][2]. The type field specifies the quantitative scoring method used [1][2]. When using CVSS, the type should be set to CVSS_V3 (or CVSS_V2 / CVSS_V4 as appropriate) [2]. The score field is a string containing the CVSS vector string (e.g., "CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H") [1]. The database_specific field is a flexible, optional JSON object [2]. It is intended for storing custom or non-standardized metadata that is specific to the database providing the record [2]. It is distinct from the standardized severity field; while some databases might optionally include severity-related metadata within database_specific, such data is not part of the core, machine-readable severity schema [1][2][3]. Summary of relevant fields: - severity: A list of objects, each with a type (e.g., CVSS_V3) and score (e.g., the vector string) [1][2]. - database_specific: A general-purpose JSON structure for database-defined metadata [2][3]. It should not be used as a substitute for the severity field if the goal is to provide standardized, interoperable vulnerability scoring [1][2].
Citations:
- 1: https://github.com/ossf/osv-schema/blob/main/docs/schema.md
- 2: https://github.com/ossf/osv-schema/blob/main/proto/vulnerability.proto
- 3: Easier way to connect between OSV advisories and other metadata (e.g. CWEs, CVSS, EPSS, CISA KEV) google/osv.dev#3245
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the workflow and inspect the relevant section with line numbers.
git ls-files .github/workflows/check-vulnerabilities.yaml
sed -n '1,220p' .github/workflows/check-vulnerabilities.yaml
# Find any other OSV parsing or severity handling in the repo.
rg -n "database_specific|severity\\]|CVSS_V3|osv.dev/v1/vulns|cvss_v3" .Repository: AgentToolkit/altk-evolve
Length of output: 5885
Severity scoring should use OSV severity[] first. database_specific.cvss_v3/severity is non-standard here, so CVSS-based vulns can fall through to 0 and be skipped. Parse the top-level severity entries (CVSS vector → base score) and keep database_specific as a fallback.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/check-vulnerabilities.yaml around lines 53 - 76, The
severity scoring logic in the OSV fetch loop currently relies on non-standard
database_specific fields, which can leave CVSS vulnerabilities at 0. Update the
parsing in the uniqueIds processing block to read the top-level severity[]
entries first, convert any CVSS vector information to a base score, and then
fall back to database_specific.cvss_v3 or database_specific.severity if needed.
Keep the existing severityMap assignment in place, but ensure the score
calculation in this fetch path prefers OSV severity data before the fallback
fields.
1 participant
The reusable workflow (check-code.yaml) called by release-github.yaml cannot have jobs requesting issues:write. Moved check-vulnerabilities to its own workflow with proper permissions and a weekly schedule.
Summary by CodeRabbit
New Features
Chores