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

fix: removed watch dependency and bumped cssnano version #16

Merged
merged 3 commits into from
Nov 13, 2021

Conversation

PeculiarGoat
Copy link
Contributor

  • Removed watch (because we have chokidar-cli).
  • Updated cssnano.

There is one more issue with ansi-regex which is used by chokidar-cli. Npm audit suggests dropping the chokidar-cli version from 3.0.0 to 1.2.0 which I think is a bit lame.

@stajs I'm wondering if it's worth dropping chokidar-cli completely and writing a pure node script to do our file watching... I don't think it would be too big of a time investment but is it worth it?

@PeculiarGoat PeculiarGoat requested a review from stajs November 3, 2021 07:03
@stajs
Copy link
Contributor

stajs commented Nov 4, 2021

I doubt it's worth it, the underlying chokidar is pretty battletested and handles a number of weird crossplat watch issues. chokidar-cli doesn't directly depent on ansi-regex, I wonder where it is transitively introduced from. What level warning is it? Maybe we just live with it if it's low.

@PeculiarGoat
Copy link
Contributor Author

@stajs it's moderate:

ansi-regex  >2.1.1 <5.0.1
Severity: moderate
 Inefficient Regular Expression Complexity in chalk/ansi-regex - https://github.com/advisories/GHSA-93q8-gq69-wqmw
fix available via `npm audit fix --force`
Will install [email protected], which is a breaking change
node_modules/chokidar-cli/node_modules/ansi-regex
  strip-ansi  4.0.0 - 5.2.0
  Depends on vulnerable versions of ansi-regex
  node_modules/chokidar-cli/node_modules/strip-ansi
    cliui  4.0.0 - 5.0.0
    Depends on vulnerable versions of strip-ansi
    Depends on vulnerable versions of wrap-ansi
    node_modules/chokidar-cli/node_modules/cliui
      yargs  10.1.0 - 15.0.0
      Depends on vulnerable versions of cliui
      Depends on vulnerable versions of string-width
      node_modules/chokidar-cli/node_modules/yargs
        chokidar-cli  >=1.2.1
        Depends on vulnerable versions of yargs
        node_modules/chokidar-cli
    string-width  2.1.0 - 4.1.0
    Depends on vulnerable versions of strip-ansi
    node_modules/chokidar-cli/node_modules/string-width
      wrap-ansi  3.0.0 - 6.1.0
      Depends on vulnerable versions of string-width
      Depends on vulnerable versions of strip-ansi
      node_modules/chokidar-cli/node_modules/wrap-ansi

I'll see what happens if I force fix it.

@PeculiarGoat
Copy link
Contributor Author

Force fixing to drop down to chokidar-cli 1.2.0 add 16 high severity warnings... force fixing those brings us back to where we are with chokidar-cli 3.0.0 lol

@stajs
Copy link
Contributor

stajs commented Nov 4, 2021

Yeah, the vulnerability doesn't sound critical: https://app.snyk.io/vuln/SNYK-JS-ANSIREGEX-1583908

I suggest we use better npm audit and exclude the warning with a reason: there isn't a patched version available. If we decide it is critical we could potentially fork and upgrade yargs ourselves. We should probably drop an issue in https://github.com/open-cli-tools/chokidar-cli and request they fix it.

@PeculiarGoat
Copy link
Contributor Author

A week with no response: open-cli-tools/chokidar-cli#105

Is it worth looking at alternatives?

@stajs
Copy link
Contributor

stajs commented Nov 12, 2021

I don't think so, I suggest better npm audit and move on.

@PeculiarGoat PeculiarGoat marked this pull request as ready for review November 12, 2021 03:12
Copy link
Contributor

@stajs stajs left a comment

Choose a reason for hiding this comment

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

👍

@PeculiarGoat PeculiarGoat merged commit 7c91827 into main Nov 13, 2021
@PeculiarGoat PeculiarGoat deleted the npm-vulnerabilities branch November 13, 2021 01:04
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.

2 participants