Skip to content

Improve NPM issues implementation #539

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
riosje merged 2 commits into from
Aug 14, 2025
Merged

Improve NPM issues implementation #539

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9438ff3
enhancements
riosje Aug 14, 2025
b306689
enhancements
riosje Aug 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/check-vulns.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ jobs:
VULN_DEP_NAME: ${{ matrix.vulnerabilities.dependency }}
VULN_DEP_VERSION: ${{ matrix.vulnerabilities.version }}
VULN_SOURCE: ${{ matrix.vulnerabilities.source }}
VULN_TITLE: ${{ matrix.vulnerabilities.title }}
VULN_MAIN_DEP_NAME: ${{ matrix.vulnerabilities.main_dep_name }}
Comment on lines +113 to 114
Copy link

@coderabbitai coderabbitai bot Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

Suggested change
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".

VULN_MAIN_DEP_PATH: ${{ matrix.vulnerabilities.main_dep_path }}
NODEJS_STREAM: ${{ inputs.nsolidStream }}
Expand Down
7 changes: 7 additions & 0 deletions .github/workflows/create_issue.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ VULN_URL="${VULN_URL}"
VULN_DEP_NAME="${VULN_DEP_NAME}"
VULN_DEP_VERSION="${VULN_DEP_VERSION}"
VULN_SOURCE="${VULN_SOURCE}"
VULN_TITLE="${VULN_TITLE:-}"
VULN_MAIN_DEP_NAME="${VULN_MAIN_DEP_NAME:-}"
VULN_MAIN_DEP_PATH="${VULN_MAIN_DEP_PATH:-}"
NODEJS_STREAM="${NODEJS_STREAM}"
Expand All @@ -35,6 +36,12 @@ ISSUE_BODY="A new vulnerability for ${VULN_DEP_NAME} ${VULN_DEP_VERSION} was fou
Vulnerability ID: ${VULN_ID}
Vulnerability URL: ${VULN_URL}"

# Add vulnerability title if available
if [ -n "${VULN_TITLE}" ]; then
ISSUE_BODY="${ISSUE_BODY}
Vulnerability Title: ${VULN_TITLE}"
fi
Comment on lines +39 to +43
Copy link

@coderabbitai coderabbitai bot Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.


# Add npm-specific info if applicable
if [ "${VULN_SOURCE}" = "npm" ] && [ -n "${VULN_MAIN_DEP_NAME}" ]; then
ISSUE_BODY="${ISSUE_BODY}
Expand Down
8 changes: 8 additions & 0 deletions .github/workflows/format_matrix.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ def generate_labels_for_vulnerability(vuln: Dict[str, Any], nsolid_stream: str)
if vuln.get("source") == "npm":
labels.append("NPM")

# Add main dependency name as label if present
main_dep_name = vuln.get("main_dep_name")
if main_dep_name and main_dep_name != "null":
labels.append(main_dep_name)

# Add severity label if available
severity = vuln.get("severity")
if severity and severity != "null":
Expand Down Expand Up @@ -70,6 +75,9 @@ def build_vulnerability_matrix(vulnerabilities_data: Dict[str, Any], nsolid_stre
matrix_entry["severity"] = vuln["severity"]
if "via" in vuln and vuln["via"]:
matrix_entry["via"] = vuln["via"]
# 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 ""
Comment on lines +78 to +80
Copy link

@coderabbitai coderabbitai bot Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

Suggested change
# 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.

if "main_dep_name" in vuln and vuln["main_dep_name"] is not None:
matrix_entry["main_dep_name"] = vuln["main_dep_name"]
if "main_dep_path" in vuln and vuln["main_dep_path"] is not None:
Expand Down
99 changes: 71 additions & 28 deletions dep_checker/npm_audit.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
# You can also use folder names for broader exclusions (e.g., "test" excludes all test folders)
EXCLUDE_PATHS = [
# Specific path exclusions
"deps/v8/tools/turbolizer",
"deps/v8/tools",

# General folder name exclusions (will match any folder with this name)
"test",
Expand Down Expand Up @@ -179,37 +179,80 @@ def parse_audit_results(self, audit_data: Dict, package_dir: Path, vulnerability
severity = vuln_data.get("severity", "unknown")
via = vuln_data.get("via", [])
fix_available = vuln_data.get("fixAvailable", False)
range_info = vuln_data.get("range", "unknown")

# Handle different via formats
# Handle different via formats - create separate vulnerabilities for each advisory
if isinstance(via, list) and via:
# Get the first vulnerability ID from via
first_via = via[0]
if isinstance(first_via, dict):
vuln_id = first_via.get("source", f"npm-{vuln_name}")
url = first_via.get("url", f"https://npmjs.com/advisories/{vuln_id}")
else:
vuln_id = str(first_via)
url = f"https://npmjs.com/advisories/{vuln_id}"
for via_item in via:
if isinstance(via_item, dict):
# Extract individual advisory information
advisory_id = via_item.get("source")
advisory_url = via_item.get("url")
advisory_title = via_item.get("title", "")
advisory_severity = via_item.get("severity", severity)

# Use advisory ID as vulnerability ID, fallback to package name
if advisory_id:
vuln_id = str(advisory_id)
else:
vuln_id = f"npm-{vuln_name}"

# Use proper GitHub advisory URL if available
if advisory_url:
url = advisory_url
else:
url = f"https://github.com/advisories?query={vuln_name}"

# Create vulnerability with advisory-specific information
vulnerability = vulnerability_class(
id=vuln_id,
url=url,
dependency=vuln_name,
version=str(range_info),
source="npm",
severity=advisory_severity,
via=[advisory_title] if advisory_title else [],
fix_available=bool(fix_available),
main_dep_name=main_dep_name,
main_dep_path=main_dep_path
)
vulnerabilities.append(vulnerability)
else:
# Handle string via items (legacy format)
vuln_id = str(via_item)
url = f"https://github.com/advisories?query={vuln_id}"

vulnerability = vulnerability_class(
id=vuln_id,
url=url,
dependency=vuln_name,
version=str(range_info),
source="npm",
severity=severity,
via=[str(via_item)],
fix_available=bool(fix_available),
main_dep_name=main_dep_name,
main_dep_path=main_dep_path
)
vulnerabilities.append(vulnerability)
else:
# No via information, create basic vulnerability
vuln_id = f"npm-{vuln_name}"
url = f"https://npmjs.com/package/{vuln_name}"

# Get version range
range_info = vuln_data.get("range", "unknown")

vulnerability = vulnerability_class(
id=vuln_id,
url=url,
dependency=vuln_name,
version=str(range_info),
source="npm",
severity=severity,
via=[str(v) for v in via] if via else [],
fix_available=bool(fix_available),
main_dep_name=main_dep_name,
main_dep_path=main_dep_path
)
vulnerabilities.append(vulnerability)
url = f"https://github.com/advisories?query={vuln_name}"

vulnerability = vulnerability_class(
id=vuln_id,
url=url,
dependency=vuln_name,
version=str(range_info),
source="npm",
severity=severity,
via=[],
fix_available=bool(fix_available),
main_dep_name=main_dep_name,
main_dep_path=main_dep_path
)
vulnerabilities.append(vulnerability)

except Exception as e:
logger.error(f"Error parsing vulnerability {vuln_name}: {e}")
Expand Down
Loading