search

SassDoc / sassdoc

Release the docs!
http://sassdoc.com
MIT License
1.41k stars 56 forks source link

Update vinyl-fs depedency to version ^3.0.3 #547

Closed joekrump closed 4 years ago

joekrump commented 4 years ago

This update was made because this newer version of vinyl-fs does not have a dependency on a version of "braces" that is vulnerable to a ReDoS attack. This addresses this issue: https://github.com/SassDoc/sassdoc/issues/537

vinyl-fs is used in just a few places in this project and its methods that are used have not changed between version 2.4.4 (the previous version of this dependency that was specified in package.json and version 3.0.3.

Details about the braces vulnerability can be found here: https://snyk.io/vuln/npm:braces:20180219

coveralls commented 4 years ago

Coverage Status

Coverage remained the same at 94.752% when pulling 369bf40cd55511639c840174cec9e46e32629363 on joekrump:update-vinyl-fs-dependency-to-latest-version into 21bf5916a3e624f5c420a6e1e049395dc0efe9ea on SassDoc:master.

joekrump commented 4 years ago

@pascalduez, are you able to review this pull request or recommend another maintainer or contributor who can review and merge + publish a new version of this package once this is merged?

joekrump commented 4 years ago

@HugoGiraudel are you able to assist with reviewing and perhaps merging and deploying a new version with the security related update?

KittyGiraudel commented 4 years ago

Hello. :)

That change is a breaking change and we would have to make sure everything works exactly the same before merging. Unfortunately I do not have the capacity to conduct these tests right now.

joekrump commented 4 years ago

@HugoGiraudel is there some way I can help with the testing? As I note in the PR description, although there are breaking changes, they do not impact sassdoc because the methods that are used are not impacted by the breaking changes.

Alternatively, if you are too busy, is there another maintainer of this project that you'd recommend I reach out to who can help with this?

Here are the 3 instances of vinyl-fs being used in source code:

  1. https://github.com/SassDoc/sassdoc/blob/master/develop/index.js#L39
  2. https://github.com/SassDoc/sassdoc/blob/master/src/recurse.js#L32
  3. https://github.com/SassDoc/sassdoc/blob/master/src/sassdoc.js#L255

3 additional usages of vinyl-fs appear in unit test files and also are not impacted by the breaking changes as far as I've been able to investigate. These tests should also fail if the breaking changes impact sassdoc.

joekrump commented 4 years ago

@valeriangalliat or @FWeinb are you able to please help here or advise on the next actionable steps I can take? I'm trying to help make this library more secure but feel I'm hitting a brick wall here.

KittyGiraudel commented 4 years ago

Our tests seem to be passing fine after the dependency update. I will merge this, but I don’t know if I’ll be able to publish a patch. I will get in touch with Pascal to see how we can have it published. :)

Thank you for your help @joekrump!

joekrump commented 4 years ago

Hey @HugoGiraudel, how's it going? Were you able to get in touch with Pascal?

KittyGiraudel commented 4 years ago

I think it should be published in 2.7.2. ;)

joekrump commented 4 years ago

Thank you for your help @HugoGiraudel!