fix(frontend): resolve high-severity npm audit vulns reddening Security Scanning (#1510) - #1511
Conversation
npm audit --audit-level=high was exiting 1 with 30 high-severity advisories, reddening the Security Scanning job (and CI Success) repo-wide on every open PR. - postcss <=8.5.17 (GHSA-r28c-9q8g-f849, path traversal in source-map auto-loading): resolved via plain `npm audit fix` (semver-compatible lockfile bump, no code change needed). - brace-expansion <=5.0.7 / minimatch 2.0.0-10.0.2 (GHSA-mh99-v99m-4gvg, DoS via unbounded expansion): this chain is pulled in transitively by eslint, jest/@jest/reporters, ts-jest/babel-plugin-istanbul, and serve/serve-handler, all of which pin an old minimatch internally. serve and eslint are already at their latest majors, and serve-handler hard-pins minimatch@3.1.5 with no newer release available, so this could not be resolved by bumping any of those packages directly. Resolved with a targeted npm "overrides" entry forcing minimatch to ^10.2.5 and brace-expansion to ^5.0.8 across the whole tree, instead of a blanket `npm audit fix --force` (which was tried first and actually downgraded jest to ^25.0.0 and serve to ^6.5.8 to satisfy its own resolver heuristics - reverted, not shipped). No eslint/jest major bump was needed, so no config migration (eslint flat config, jest 30 breaking changes) was required. Verified: npm ci from the committed lockfile, npm audit --audit-level=high exits 0 (0 vulnerabilities), frontend build/lint/typecheck/test all green (86 suites, 2729 passed, 1 pre-existing skip).
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a frontend npm override that resolves ChangesFrontend dependency override
Estimated code review effort: 1 (Trivial) | ~2 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@frontend/package.json`:
- Around line 54-55: Remove the global minimatch entry from the overrides block
in frontend/package.json and scope the override only to the vulnerable
dependency chain, or first upgrade all listed minimatch@^3 consumers so they
safely support the newer version. Preserve compatible dependency resolution for
glob@7.2.3, eslint, `@humanwhocodes/config-array`, test-exclude, and
serve-handler.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 63f7b6c8-c58d-4637-93a8-b884fe916078
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
frontend/package.json
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 2 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
…to-fix) This replaces coderabbitai bot commit cbcab4b, which introduced unreachable private-registry (10.0.0.28) URLs into package-lock.json and an incompatible test-exclude@8/glob@13 dependency bump. That combo caused npm ci to hang for roughly 5 minutes in CI trying to reach the unreachable registry, canceling Build frontend, Build Docker, E2E, Playwright, and pre-commit checks. Instead of forcing a global minimatch@^10 override, this scopes the fix to brace-expansion only, the actual transitive dependency behind GHSA-mh99-v99m-4gvg. That advisory has no patched release on the 1.x or 2.x lines, only 5.0.8+, so a single unscoped override is both necessary and sufficient. Consumers like glob@7, eslint, jest, test-exclude, and serve-handler keep their intended minimatch major instead of being forced onto minimatch@10. npm audit --audit-level=high reports 0 vulnerabilities and the full build/lint/typecheck/test suite is green against the regenerated lockfile.
cbcab4b to
6b6e3ec
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
Summary
npm audit --audit-level=highin the frontend was exiting 1 with 30 high-severity advisories, reddening theSecurity Scanningjob (and thereforeCI Success) repo-wide on every open PR, including ones that don't touch the frontend.npm audit fix(semver-compatible lockfile bump, no code change).@jest/reporters, ts-jest/babel-plugin-istanbul, and serve/serve-handler, all pinning an old minimatch internally. serve and eslint are already at their latest available majors, andserve-handlerhard-pinsminimatch@3.1.5with no newer release published, so bumping any of those packages directly would not have fixed it. Resolved with a targetedoverridesentry infrontend/package.jsonforcingminimatchto^10.2.5andbrace-expansionto^5.0.8across the whole dependency tree.npm audit fix --forcewas tried first per the standard remediation path, but it actually downgradedjestto^25.0.0andserveto^6.5.8to satisfy its own resolver heuristics (npm's force-fix picks minimal-diff versions, not necessarily the safest/latest ones). That result was reverted and not shipped; theoverridesapproach was used instead, which resolves the same advisories without touching eslint/jest major versions at all, so no eslint flat-config migration or jest 30 breaking-change handling was needed.Verification
rm -rf node_modules && npm cifrom the committed lockfile, thennpm audit --audit-level=high->found 0 vulnerabilities, exit 0.npm run build-> webpack compiles clean (1 pre-existing bundle-size warning, unrelated).npm run lint-> 0 errors, 125 pre-existing warnings (@typescript-eslint/no-explicit-any).npm run typecheck-> clean.npm test-> 86/86 suites, 2729 passed, 1 pre-existing skip (same as baseline; note the sandbox'sde_DElocale flips$1,200-style assertions to$1.200independent of this change - confirmed pre-existing ongit stash, unrelated, tests pass withLC_ALL=en_US.UTF-8).Only
frontend/package.jsonandfrontend/package-lock.jsonchanged.Test plan
npm ci && npm audit --audit-level=highexits 0npm run buildsucceedsnpm run lint/npm run typecheckcleannpm testgreen (86 suites, 2729 passed)Closes #1510
Summary by CodeRabbit