foundation / inky

Convert a simple HTML syntax into tables compatible with Foundation for Emails.
http://foundation.zurb.com/emails
673 stars 107 forks source link

Fixes #120 - Updates vinyl-fs to latest version 3.0.3 #121

Closed jwmunn closed 4 years ago

jwmunn commented 4 years ago

Fixes #120

Dependabot has recently been natively pulled into Github and is flagging braces package as a security vulnerability. This is a dependency of vinyl-fs and has been flagged in issue #120

Proposal

Update vinyl-fs to 3.0.3. This updates the glob-stream dependency to 6.1.0, which removes the dependency on micromatch and braces which has the vulnerability.

image

Other uses of braces are using newer versions 👍

image

Context

Issue #120

Hello Inky maintaining team,

I started using Inky in a project and running npm audit I realised that there is a vulnerability in a dependency chain. Here is the audit result of project using inky@^1.3.7:

┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Low           │ Regular Expression Denial of Service                         │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ braces                                                       │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=2.3.1                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ inky                                                         │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ inky > vinyl-fs > glob-stream > micromatch > braces          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://npmjs.com/advisories/786                             │
└───────────────┴──────────────────────────────────────────────────────────────┘

I was looking for the best approach to fix that and seems that updating vinyl-fs to 3.0. will do the trick: every other 2. versions use a vulnerable version of braces at the end, and 3 doesn't. Since it's a major update I suppose the team need to check if there's some change to do to make it still working.

If possible, I can contribute with code too, since I'm interested on see this working in a project that have a vulnerabilities check at their pipeline.

Additional context:

Security vulnerability

image

Packages affected

image |

joeworkman commented 4 years ago

This pull request has been mentioned on Foundation Open Source Community. There might be relevant details there:

https://foundation.discourse.group/t/security-vulnerability-in-inky/2774/1

jwmunn commented 4 years ago

@DanielRuf since the issue was closed, do you know if there is a timeline on testing and implementing this update? Is there a way I could help with that?

DanielRuf commented 4 years ago

We will update this separately in the future as described at https://github.com/foundation/inky/issues/120#issuecomment-642795421.

joeworkman commented 4 years ago

@jwmunn I spent most of the day updating dependencies, fixing vulnerabilities and tests. Please give v1.4.1 a shot.

jwmunn commented 4 years ago

@joeworkman Awesome! Thanks for the update, I'll upgrade and report back.