Skip to content
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.

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

chore: bump minimatch version due to vulnerability alert #426

Merged
merged 1 commit into from
Nov 2, 2022

Conversation

octonato
Copy link
Member

@octonato octonato commented Nov 2, 2022

Build is failing https://app.circleci.com/pipelines/github/lightbend/kalix-javascript-sdk/1370/workflows/52ebe57d-137d-4763-ba2e-a17a2e17bf1b/jobs/8737?invite=true#step-106-3

This PR fixes it.

#!/bin/bash -eo pipefail
npm audit --production
npm WARN config production Use `--omit=dev` instead.
# npm audit report

minimatch  <3.0.5
Severity: high
minimatch ReDoS vulnerability - https://github.com/advisories/GHSA-f8q6-p94x-37v3
fix available via `npm audit fix`
node_modules/minimatch

1 high severity vulnerability

To address all issues, run:
  npm audit fix

Exited with code exit status 1

@github-actions github-actions bot added the kalix-runtime Runtime and SDKs sub-team label Nov 2, 2022
Copy link
Contributor

@franciscolopezsancho franciscolopezsancho left a comment

Choose a reason for hiding this comment

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

LGTM

Although there are two things I don't quite get.

  1. I can still see minimatch 3.0.4 all over the place but now the build passes.
  2. How did you find what to change? I guess you didn't use https://docs.npmjs.com/cli/v8/configuring-npm/package-json#overrides on the package.json.

I don't expect you to explain 1. but how about 2.?

@octonato
Copy link
Member Author

octonato commented Nov 2, 2022

LGTM

Although there are two things I don't quite get.

  1. I can still see minimatch 3.0.4 all over the place but now the build passes.
  2. How did you find what to change? I guess you didn't use https://docs.npmjs.com/cli/v8/configuring-npm/package-json#overrides on the package.json.

I don't expect you to explain 1. but how about 2.?

If you run npm audit --production in the sdk folder you get the error and the hint to fix.

To fix it, you run npm audit fix.

My understanding is that minimatch is transient dependency and instead of overriding it in package.json, the command modifies the lock file. And I'm guessing that it also takes the opportunity to do some clean-up.

I don't know what is the best way of doing it. I'm just trusting that the recommendation of using npm audit fix is a solid one.

Maybe @katsutoxin can gives us some hints on this?

Anyway, I would like to merge it because it's blocking the build. @katsutoxin, if you believe this is sub-optimal, I can revert and PR again.

@octonato octonato merged commit d645e74 into main Nov 2, 2022
@octonato octonato deleted the octonato/bump-minimatch-version branch November 2, 2022 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kalix-runtime Runtime and SDKs sub-team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants