Conversation
Signed-off-by: Jefferson <jefferson.rios.caro@gmail.com>
WalkthroughPropagates an extracted vulnerability title through npm-audit parsing, matrix generation, and the GitHub Actions workflow to optionally include it in created issues; also expands npm advisory parsing, normalizes version ranges, broadens exclude paths, and adds a "Main Dependency" label. Changes
Sequence Diagram(s)sequenceDiagram
participant Audit as dep_checker/npm_audit.py
participant Formatter as .github/workflows/format_matrix.py
participant GHA as .github/workflows/check-vulns.yml
participant IssueScript as .github/workflows/create_issue.sh
participant GitHub as GitHub Issues
Audit->>Formatter: Emit vulnerability items (id/url/severity/via[]/range)
Formatter->>GHA: Build matrix entries (include title from via[0], labels)
GHA->>IssueScript: Run step with env VULN_TITLE=matrix.vulnerabilities.title
IssueScript->>GitHub: Create/Update issue (includes Vulnerability Title if set)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these settings in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
riosje
force-pushed
the
jeff/implementNPM
branch
from
591f0e7 to
9438ff3
Compare
![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 (3)
.github/workflows/format_matrix.py (1)
26-29: Good addition; broaden null-check for robustness.Labeling “Main Dependency” when present is useful. Consider treating empty strings and the string "null" as absent to avoid accidental labeling from upstream JSON quirks.
Apply this diff:
- if vuln.get("main_dep_name") and vuln.get("main_dep_name") != "null": + main_dep_name = vuln.get("main_dep_name") + if main_dep_name not in (None, "", "null"): labels.append("Main Dependency").github/workflows/create_issue.sh (1)
24-24: Optional: sanitize title for newlines/CRs.Defaulting VULN_TITLE is good. Consider stripping carriage returns to keep the issue body tidy if titles ever contain CRLF.
For example:
VULN_TITLE="${VULN_TITLE:-}" VULN_TITLE="${VULN_TITLE//$'\r'/}"dep_checker/npm_audit.py (1)
185-220: Prefer direct advisory URL fallback when advisory_id exists.If advisory_url is missing but advisory_id is present (e.g., GHSA-…), linking directly to https://github.com/advisories/ is more precise than a query.
Apply this diff:
- if advisory_url: - url = advisory_url - else: - url = f"https://github.com/advisories?query={vuln_name}" + if advisory_url: + url = advisory_url + elif advisory_id: + url = f"https://github.com/advisories/{advisory_id}" + else: + url = f"https://github.com/advisories?query={vuln_name}"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (4)
.github/workflows/check-vulns.yml(1 hunks).github/workflows/create_issue.sh(2 hunks).github/workflows/format_matrix.py(2 hunks)dep_checker/npm_audit.py(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: create-issues (1105443, GHSA-v6h2-p8h4-qcjw, brace-expansion, 1.0.0...
🔇 Additional comments (4)
dep_checker/npm_audit.py (4)
24-24: Broadened exclusion is appropriate.Excluding the parent directory (deps/v8/tools) is a sensible generalization and works with your prefix check.
182-183: Consistent range handling looks good.Capturing range once from vuln_data and reusing it unifies version/range reporting across variants.
221-237: String via handling is clear and backward-compatible.Creating a separate vulnerability per string via item with a query URL is sensible for legacy formats.
239-255: No-via path is well-defined.Reasonable defaults (id, URL, empty via, consistent version) ensure downstream consumers behave predictably.
| VULN_TITLE: ${{ matrix.vulnerabilities.title }} | ||
| VULN_MAIN_DEP_NAME: ${{ matrix.vulnerabilities.main_dep_name }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Coalesce null/undefined titles to empty string at the source.
This prevents the string "null" from being passed to the shell script and simplifies downstream checks.
Apply this diff:
- VULN_TITLE: ${{ matrix.vulnerabilities.title }}
+ VULN_TITLE: ${{ matrix.vulnerabilities.title || '' }}📝 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.
| VULN_TITLE: ${{ matrix.vulnerabilities.title }} | |
| VULN_MAIN_DEP_NAME: ${{ matrix.vulnerabilities.main_dep_name }} | |
| VULN_TITLE: ${{ matrix.vulnerabilities.title || '' }} | |
| VULN_MAIN_DEP_NAME: ${{ matrix.vulnerabilities.main_dep_name }} |
🤖 Prompt for AI Agents
.github/workflows/check-vulns.yml around lines 113 to 114: the workflow
currently passes matrix.vulnerabilities.title (and main_dep_name) directly which
can yield the literal "null" when values are missing; update the assignments to
coalesce null/undefined to an empty string at the source by replacing the direct
references with an expression that defaults to '' (e.g., use the GitHub Actions
expression operator to fall back to an empty string), doing this for both
VULN_TITLE and VULN_MAIN_DEP_NAME so downstream scripts never receive "null".
| # Add vulnerability title if available | ||
| if [ -n "${VULN_TITLE}" ]; then | ||
| ISSUE_BODY="${ISSUE_BODY} | ||
| Vulnerability Title: ${VULN_TITLE}" | ||
| fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard against the literal "null" value making it into the issue body.
When matrix.vulnerabilities.title is missing, GitHub can pass the string "null". The current check would add “Vulnerability Title: null” to issues.
Apply this diff:
-if [ -n "${VULN_TITLE}" ]; then
+if [ -n "${VULN_TITLE}" ] && [ "${VULN_TITLE}" != "null" ]; then
ISSUE_BODY="${ISSUE_BODY}
Vulnerability Title: ${VULN_TITLE}"
fi🤖 Prompt for AI Agents
.github/workflows/create_issue.sh around lines 39 to 43: the script currently
appends "Vulnerability Title: ${VULN_TITLE}" whenever VULN_TITLE is non-empty,
but GitHub can pass the literal string "null" which should not be treated as a
real title; update the conditional to only append when VULN_TITLE is non-empty
and not the literal "null" (e.g., test that VULN_TITLE is set and [
"$VULN_TITLE" != "null" ]), trimming whitespace if necessary so the string
"null" is excluded from ISSUE_BODY.
| # Extract title from via array for display purposes | ||
| if isinstance(vuln["via"], list) and vuln["via"]: | ||
| matrix_entry["title"] = vuln["via"][0] if vuln["via"][0] else "" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure title is a string; avoid non-string/JSON objects leaking to workflow env.
If via elements are dicts in some legacy/edge cases, this could set a non-string object to "title", which may propagate poorly through GitHub Actions env interpolation. Coerce to str.
Apply this diff:
- if isinstance(vuln["via"], list) and vuln["via"]:
- matrix_entry["title"] = vuln["via"][0] if vuln["via"][0] else ""
+ if isinstance(vuln["via"], list) and vuln["via"]:
+ matrix_entry["title"] = str(vuln["via"][0]) if vuln["via"][0] else ""Optional: If you want to always provide a defined string (to avoid “null” in YAML/env), initialize title to "" in matrix_entry and then override when available.
📝 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.
| # Extract title from via array for display purposes | |
| if isinstance(vuln["via"], list) and vuln["via"]: | |
| matrix_entry["title"] = vuln["via"][0] if vuln["via"][0] else "" | |
| # Extract title from via array for display purposes | |
| if isinstance(vuln["via"], list) and vuln["via"]: | |
| matrix_entry["title"] = str(vuln["via"][0]) if vuln["via"][0] else "" |
🤖 Prompt for AI Agents
In .github/workflows/format_matrix.py around lines 77 to 79,
matrix_entry["title"] can be assigned a non-string (e.g., a dict) from
vuln["via"][0]; initialize matrix_entry["title"] = "" earlier and when
assigning, coerce the value to a string (e.g., matrix_entry["title"] =
str(vuln["via"][0]) if vuln["via"][0] is not None else "") so the workflow
always gets a plain string and no non-JSON objects leak into the GitHub Actions
environment.
Signed-off-by: Jefferson <jefferson.rios.caro@gmail.com>
1 participant
Summary by CodeRabbit
New Features
Chores