Skip to content

ci(security): add Retire.js workflow to detect bundled JS vulnerabilities#27315

Open
harsh-vador wants to merge 7 commits intomainfrom
add-bundled-security
Open

ci(security): add Retire.js workflow to detect bundled JS vulnerabilities#27315
harsh-vador wants to merge 7 commits intomainfrom
add-bundled-security

Conversation

@harsh-vador
Copy link
Copy Markdown
Contributor

@harsh-vador harsh-vador commented Apr 13, 2026

Describe your changes:

Summary

  • Adds a retire-js-scan job to the existing .github/workflows/security-scan.yml — a lightweight scan that checks node_modules/ for known vulnerable JavaScript libraries bundled inside package dist files using Retire.js

When it runs

  • On PRs — automatically when package.json or yarn.lock changes
  • Every 2 days — on the existing security-scan schedule, to catch newly disclosed CVEs against existing dependencies
  • Manual — via workflow_dispatch

What it does

  1. Installs UI dependencies (yarn install --ignore-scripts)
  2. Runs npx retire@5 --path node_modules/ --severity medium
  3. Verifies the report was generated (fails loudly if not)
  4. Uploads the full JSON report as a 30-day artifact
  5. Posts a structured summary to the GitHub Actions Step Summary grouped by library with severity icons and linked CVEs
  6. Fails the job if any medium/high/critical vulnerability is found

Impact on existing Snyk scan

None. The existing security-scan job is unchanged — only a single if: github.event_name != 'pull_request' guard was added so Snyk continues to run only on schedule/dispatch (not on PRs, where it has no secrets access anyway).

Type of change:

  • Bug fix
  • Improvement
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

@harsh-vador harsh-vador self-assigned this Apr 13, 2026
@harsh-vador harsh-vador added safe to test Add this label to run secure Github workflows on PRs security labels Apr 13, 2026
@harsh-vador harsh-vador changed the title ci(security): add Retire.js workflow to detect bundled JS vulnerabili… ci(security): add Retire.js workflow to detect bundled JS vulnerabilities Apr 13, 2026
@gitar-bot
Copy link
Copy Markdown

gitar-bot bot commented Apr 14, 2026

Code Review ✅ Approved 6 resolved / 6 findings

Adds Retire.js workflow to detect bundled JavaScript vulnerabilities in CI, addressing missing report handling, JSON sanitization in inline Python, retire.js version pinning, yarn lockfile verification, and Slack notification logic. All findings resolved.

✅ 6 resolved
Edge Case: retire-report.json missing causes silent pass with no summary

📄 .github/workflows/retire-js-scan.yml:44-53 📄 .github/workflows/retire-js-scan.yml:126-128
If the Retire.js scan step fails for a reason other than finding vulnerabilities (e.g., npx retire is not found, network error downloading the vulnerability DB, or an invalid flag), continue-on-error: true swallows the failure. The report file won't be created, so the summary step prints 'Report file not found' and exits 0. The final 'Force failure' step checks steps.retire-scan.outcome == 'failure' — but a scan that crashed also sets outcome to failure, which would correctly fail the job.

However, if retire exits 0 but produces no output file (e.g., wrong --outputpath), the workflow silently passes with no vulnerabilities reported. Consider adding an explicit file-existence check before the summary step.

Security: Inline Python in workflow parses untrusted JSON without sanitization

📄 .github/workflows/retire-js-scan.yml:67-81
The inline Python script reads fields like component, version, and summary from the Retire.js JSON report and writes them directly into the GitHub Step Summary (which renders Markdown). If a malicious package name or summary contained Markdown injection (e.g., links or images), it would render in the summary. The risk is low since the data comes from the Retire.js vulnerability database (not user input), but consider escaping pipe characters and backticks in table cell values to avoid broken rendering at minimum.

Quality: Pin retire.js version for reproducible scans

📄 .github/workflows/retire-js-scan.yml:49
npx retire fetches the latest version each run. A breaking change in Retire.js CLI (flag renames, output format changes) would silently break the workflow. Pin the version for reproducibility, e.g., npx retire@5.0.0.

Bug: Missing --frozen-lockfile flag on yarn install in CI

📄 .github/workflows/security-scan.yml:40
Line 40 uses yarn install --ignore-scripts without --frozen-lockfile. Every other workflow in this repo (ui-checkstyle.yml, etc.) consistently passes --frozen-lockfile to prevent yarn from silently modifying yarn.lock during CI runs. Without it, the install may resolve different dependency versions than what's committed, making the scan results non-reproducible and potentially masking vulnerabilities present in the actual lockfile.

Edge Case: Success Slack uses negated check; may fire on cancelled/skipped

📄 .github/workflows/security-scan.yml:140
The "Slack on Success" step (line 140) uses steps.retire-scan.outcome != 'failure', which will also match when the outcome is 'cancelled' or 'skipped', sending a misleading success notification. The existing security-scan job (line 233) uses the stricter positive check steps.security-report.outcome == 'success', which is the safer pattern.

...and 1 more resolved from earlier reviews

Options

Display: compact → Showing less information.

Comment with these commands to change:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test Add this label to run secure Github workflows on PRs security

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants