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

Upgrade yargs to ^17.2.1 #117

Merged
merged 1 commit into from Dec 27, 2021
Merged

Conversation

kiskoza
Copy link
Contributor

@kiskoza kiskoza commented Oct 11, 2021

There's no activity on #115 from @shenoyguru , so I'm opening a new PR with all the changes we need to apply to fix the current security vulnerability. A copied over my original comment with a few small changes. I would recoment releasing 4.0.0 as dropping older node versions could be a breaking change for dependants.


Hi. Recently I got some audit warnings coming from this package's dependencies.

How to reproduce

Create an empty folder and run the following:

yarn init --yes
yarn add sass-graph
yarn audit
Yarn output
yarn init v1.22.4
warning The yes flag has been set. This will automatically answer yes to all questions, which may have security implications.
success Saved package.json
Done in 0.03s.
yarn add v1.22.4
info No lockfile found.
[1/4] Resolving packages...
[2/4] Fetching packages...
[3/4] Linking dependencies...
[4/4] Building fresh packages...

success Saved lockfile.
success Saved 39 new dependencies.
info Direct dependencies
└─ sass-graph@3.0.5
info All dependencies
├─ ansi-regex@4.1.0
├─ ansi-styles@3.2.1
├─ balanced-match@1.0.2
├─ brace-expansion@1.1.11
├─ camelcase@5.3.1
├─ cliui@5.0.0
├─ color-convert@1.9.3
├─ color-name@1.1.3
├─ concat-map@0.0.1
├─ decamelize@1.2.0
├─ emoji-regex@7.0.3
├─ find-up@3.0.0
├─ fs.realpath@1.0.0
├─ get-caller-file@2.0.5
├─ glob@7.2.0
├─ inflight@1.0.6
├─ inherits@2.0.4
├─ is-fullwidth-code-point@2.0.0
├─ js-base64@2.6.4
├─ locate-path@3.0.0
├─ lodash@4.17.21
├─ minimatch@3.0.4
├─ p-limit@2.3.0
├─ p-locate@3.0.0
├─ p-try@2.2.0
├─ path-exists@3.0.0
├─ path-is-absolute@1.0.1
├─ require-directory@2.1.1
├─ require-main-filename@2.0.0
├─ sass-graph@3.0.5
├─ scss-tokenizer@0.3.0
├─ set-blocking@2.0.0
├─ source-map@0.7.3
├─ strip-ansi@5.2.0
├─ which-module@2.0.0
├─ wrap-ansi@5.1.0
├─ y18n@4.0.3
├─ yargs-parser@13.1.2
└─ yargs@13.3.2
Done in 3.06s.
yarn audit v1.22.4
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ moderate      │  Inefficient Regular Expression Complexity in                │
│               │ chalk/ansi-regex                                             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ ansi-regex                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=5.0.1                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ sass-graph                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ sass-graph > yargs > cliui > string-width > strip-ansi >     │
│               │ ansi-regex                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://www.npmjs.com/advisories/1002401                     │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ moderate      │  Inefficient Regular Expression Complexity in                │
│               │ chalk/ansi-regex                                             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ ansi-regex                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=5.0.1                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ sass-graph                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ sass-graph > yargs > cliui > wrap-ansi > string-width >      │
│               │ strip-ansi > ansi-regex                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://www.npmjs.com/advisories/1002401                     │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ moderate      │  Inefficient Regular Expression Complexity in                │
│               │ chalk/ansi-regex                                             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ ansi-regex                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=5.0.1                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ sass-graph                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ sass-graph > yargs > string-width > strip-ansi > ansi-regex  │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://www.npmjs.com/advisories/1002401                     │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ moderate      │  Inefficient Regular Expression Complexity in                │
│               │ chalk/ansi-regex                                             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ ansi-regex                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=5.0.1                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ sass-graph                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ sass-graph > yargs > cliui > strip-ansi > ansi-regex         │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://www.npmjs.com/advisories/1002401                     │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ moderate      │  Inefficient Regular Expression Complexity in                │
│               │ chalk/ansi-regex                                             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ ansi-regex                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=5.0.1                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ sass-graph                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ sass-graph > yargs > cliui > wrap-ansi > strip-ansi >        │
│               │ ansi-regex                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://www.npmjs.com/advisories/1002401                     │
└───────────────┴──────────────────────────────────────────────────────────────┘
5 vulnerabilities found - Packages audited: 42
Severity: 5 Moderate
Done in 0.63s.

or

npm init --yes
npm install sass-graph --save
npm audit
NPM output
Wrote to package.json:

{
  "name": "sass-graph-vulnerability",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "keywords": [],
  "author": "",
  "license": "ISC"
}


npm notice created a lockfile as package-lock.json. You should commit this file.
npm WARN sass-graph-vulnerability@1.0.0 No description
npm WARNsass-graph-vulnerability@1.0.0 No repository field.

+ sass-graph@3.0.5
added 42 packages from 51 contributors and audited 42 packages in 5.325s

2 packages are looking for funding
  run `npm fund` for details

found 5 moderate severity vulnerabilities
  run `npm audit fix` to fix them, or `npm audit` for details
                                                                                
                       === npm audit security report ===                        
                                                                                
┌──────────────────────────────────────────────────────────────────────────────┐
│                                Manual Review                                 │
│            Some vulnerabilities require your attention to resolve            │
│                                                                              │
│         Visit https://go.npm.me/audit-guide for additional guidance          │
└──────────────────────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Moderate      │  Inefficient Regular Expression Complexity in                │
│               │ chalk/ansi-regex                                             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ ansi-regex                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=5.0.1                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ sass-graph                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ sass-graph > yargs > cliui > string-width > strip-ansi >     │
│               │ ansi-regex                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://github.com/advisories/GHSA-93q8-gq69-wqmw            │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Moderate      │  Inefficient Regular Expression Complexity in                │
│               │ chalk/ansi-regex                                             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ ansi-regex                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=5.0.1                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ sass-graph                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ sass-graph > yargs > cliui > wrap-ansi > string-width >      │
│               │ strip-ansi > ansi-regex                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://github.com/advisories/GHSA-93q8-gq69-wqmw            │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Moderate      │  Inefficient Regular Expression Complexity in                │
│               │ chalk/ansi-regex                                             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ ansi-regex                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=5.0.1                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ sass-graph                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ sass-graph > yargs > string-width > strip-ansi > ansi-regex  │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://github.com/advisories/GHSA-93q8-gq69-wqmw            │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Moderate      │  Inefficient Regular Expression Complexity in                │
│               │ chalk/ansi-regex                                             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ ansi-regex                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=5.0.1                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ sass-graph                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ sass-graph > yargs > cliui > strip-ansi > ansi-regex         │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://github.com/advisories/GHSA-93q8-gq69-wqmw            │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Moderate      │  Inefficient Regular Expression Complexity in                │
│               │ chalk/ansi-regex                                             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ ansi-regex                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=5.0.1                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ sass-graph                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ sass-graph > yargs > cliui > wrap-ansi > strip-ansi >        │
│               │ ansi-regex                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://github.com/advisories/GHSA-93q8-gq69-wqmw            │
└───────────────┴──────────────────────────────────────────────────────────────┘
found 5 moderate severity vulnerabilities in 42 scanned packages
  5 vulnerabilities require manual review. See the full report for details.

The fix that needs to be applied

  • Update yargs dependency to ^17.0.0 (preferably ^17.2.1)
  • Go through the changelog of yargs looking for breaking changes
  • Update minimum node version to 12 (yargs requires >=12 so this package should too)
  • Remove old versions from .travis.yml file and add newer node versions (15 & 16)

Testing the changes

I ran the tests on these node versions and both npm test and ./bin/sassgraph descendents test/fixtures test/fixtures/simple/index.scss looked good

  • 12.22.6
  • 13.14.0
  • 14.18.0
  • 15.14.0
  • 16.10.0

There are more issues / pull requests trying to solve similar issues: #112, #114 & #115. Some of them are using lower versions, some of them are incomplete.

@PrinsFrank
Copy link

@xzyfer do you still actively maintain this package? If so, can this PR be merged so sass/node-sass#3190 upstream can be fixed too?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants