the-difference-engine / ksf

7 stars 1 forks source link

node package security vulnerabilities partial fix #264

Closed somersbmatthews closed 3 years ago

somersbmatthews commented 3 years ago

Zenhub Link:

Describe the problem being solved:

npm security vulnerabilities in root, i want to fix the ones in /packages/api and /packages/app but not sure what the protocol behind fixing these security vulnerabilities is, so stopped at just the root dir package.json security vulnerabilities.

Impacted areas in the application:

node

Describe the steps you took to test your changes:

I re-ran express app and react app and manually tested the "sync nominations" button List general components of the application that this PR will affect: none, there are still two security vulnerabilites, but not fixable: ❯ npm audit

                   === npm audit security report ===

┌──────────────────────────────────────────────────────────────────────────────┐ │ Manual Review │ │ Some vulnerabilities require your attention to resolve │ │ │ │ Visit https://go.npm.me/audit-guide for additional guidance │ └──────────────────────────────────────────────────────────────────────────────┘ ┌───────────────┬──────────────────────────────────────────────────────────────┐ │ High │ Regular Expression Denial of Service │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Package │ trim-newlines │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Patched in │ >=3.0.1 <4.0.0 || >=4.0.1 │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Dependency of │ lerna [dev] │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Path │ lerna > @lerna/publish > @lerna/version > │ │ │ @lerna/conventional-commits > conventional-changelog-core > │ │ │ get-pkg-repo > meow > trim-newlines │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ More info │ https://npmjs.com/advisories/1753 │ └───────────────┴──────────────────────────────────────────────────────────────┘ ┌───────────────┬──────────────────────────────────────────────────────────────┐ │ High │ Regular Expression Denial of Service │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Package │ trim-newlines │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Patched in │ >=3.0.1 <4.0.0 || >=4.0.1 │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Dependency of │ lerna [dev] │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Path │ lerna > @lerna/version > @lerna/conventional-commits > │ │ │ conventional-changelog-core > get-pkg-repo > meow > │ │ │ trim-newlines │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ More info │ https://npmjs.com/advisories/1753 │ └───────────────┴──────────────────────────────────────────────────────────────┘ found 2 high severity vulnerabilities in 691 scanned packages 2 vulnerabilities require manual review. See the full report for details.

PR checklist

aschey commented 3 years ago

@somersbmatthews seems like it's probably fine in this case, but in general it's good to be careful about running audit fix (or any command for that matter) with --force. I've seen it break things in subtly annoying ways before.