Skip to content

fix: Resolve npm security vulnerabilities#452

Open
darwintantuco wants to merge 5 commits into
bettergovph:mainfrom
darwintantuco:dt-npm-audit-fix
Open

fix: Resolve npm security vulnerabilities#452
darwintantuco wants to merge 5 commits into
bettergovph:mainfrom
darwintantuco:dt-npm-audit-fix

Conversation

@darwintantuco
Copy link
Copy Markdown
Contributor

Summary

  • Ran npm audit fix to resolve security vulnerabilities
  • Fixed 5 vulnerabilities (2 low, 3 moderate)
  • Updated Babel and ESLint-related packages to patched versions

Context

Project seems to have few security vulnerabilities reported by npm audit. Running npm audit fix automatically updates dependencies to their latest secure versions within semver ranges. The main fixes include:

  • @babel/helpers: Updated to fix RegExp complexity vulnerability
  • @eslint/plugin-kit: Updated to fix ReDoS vulnerability
  • brace-expansion: Updated to fix ReDoS vulnerability

All updates are non-breaking changes within the existing semver ranges.

Test plan

  • Run npm audit and verify vulnerabilities are reduced
  • Run npm install and verify no errors
  • Run build and tests to ensure nothing broke
  • Manual testing: Verify the application runs correctly in development mode
  • Regression testing: Test key features to ensure no functionality broke

@Sheape
Copy link
Copy Markdown
Member

Sheape commented Oct 12, 2025

Hi @darwintantuco. Thank you for taking a look at this.
I think it would be beneficial if package.json is also updated to match the updates from package-lock.json.
Also it seems like there are no breaking changes currently but I'll have a look at this later on.

@darwintantuco darwintantuco force-pushed the dt-npm-audit-fix branch 2 times, most recently from 1607ed9 to 8c8217f Compare October 13, 2025 04:17
@darwintantuco
Copy link
Copy Markdown
Contributor Author

Hi @Sheape thank you for the feedback, attempted to fix on last commit: 8c8217f

Copy link
Copy Markdown
Member

@Sheape Sheape left a comment

Choose a reason for hiding this comment

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

Looks good to me for the most part. I guess the only thing to fix left are these:

npm warn deprecated mkdirp@0.3.0: Legacy versions of mkdirp are no longer supported. Please update to mkdirp 1.x. (Note that the API surface has changed to use Promises in 1.x.)
npm warn deprecated @types/react-leaflet@3.0.0: This is a stub types definition. react-leaflet provides its own type definitions, so you do not need this installed.

Do you also intend to fix them in this PR or we can just leave it as it is since these are just soft warnings?

Comment thread package-lock.json
"@babel/template": "^7.27.2",
"@babel/traverse": "^7.28.4",
"@babel/types": "^7.28.4",
"@jridgewell/remapping": "^2.3.5",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ahh turns out this is the new maintained package instead of @ampproject/remapping.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

probably better to fix warnings here, will take a look soon

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Dont worry about it. It was just a comment, since this is the only one that changes from package A to package B instead of updating.

Serves as a reference for other reviewers/maintainers in case they want to check if it's malicious in npmjs.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

hi @Sheape

I think I was able to fix npm warn deprecated @types/react-leaflet@3.0.0: This is a stub types definition. react-leaflet provides its own type definitions, so you do not need this installed.

see recent commit, seems react-leaflet v5 includes its own TypeScript definitions, making the separate @types package redundant

for mkdirp, I agree, I think we can leave it as is for now, as it's a transitive dependency from hogan.js → instantsearch.js → react-instantsearch and seems even latest versions still use the old hogan.js with mkdirp@0.3.0

@niculistana niculistana requested a review from Sheape October 15, 2025 04:49
@niculistana niculistana added the high priority This project is needed and needs to be done fast!!! label Oct 15, 2025
@Sheape Sheape added the pending This is currently being worked on (on-going). label Oct 15, 2025
@Sheape
Copy link
Copy Markdown
Member

Sheape commented Oct 15, 2025

@niculistana I'll mark this as pending for now. There are still on-going changes that needs to be made (specifically those 2 warnings), thus I cant approve of this yet.

@Sheape
Copy link
Copy Markdown
Member

Sheape commented Oct 15, 2025

LGTM. I guess my only concern is this:
image

Tried patching it with npm audit fix --force, it seems to not break anything but I haven't fully tested it.

@github-actions
Copy link
Copy Markdown

Stale pull request message

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

Labels

high priority This project is needed and needs to be done fast!!! no-pr-activity pending This is currently being worked on (on-going). security

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants