search

filipedeschamps / rss-feed-emitter

Super RSS News Feed aggregator written in Node.js and ES6
MIT License
1.06k stars 77 forks source link

Updated dependency "request", due to CVE-2018-1000620 #173

Closed Ardakilic closed 5 years ago

Ardakilic commented 5 years ago

Hello,

My tool, AlertHub depends on this package. First of all, thank you for making this available for us all!

I've realized there's been a security issue with one of the dependencies, request:

https://nvd.nist.gov/vuln/detail/CVE-2018-1000620

alerthub@1.1.0 /Users/arda/projects/alerthub git:(master) ✗ npm ls cryptiles
└─┬ rss-feed-emitter@2.0.0
  └─┬ request@2.83.0
    └─┬ hawk@6.0.2
      └── cryptiles@3.1.4

This pull request aims to resolve this issue by updating the dependency.

Also, there are other vulnerabilities when you run npm audit.

I'd appreciate if this'd be considered.

Thanks in advance,

filipedeschamps commented 5 years ago

That's awesome, thanks for pointing it out. Merging and releasing a patch version right after.

filipedeschamps commented 5 years ago

Done! New release rss-feed-emitter@2.0.1

Ardakilic commented 5 years ago

Thank you so much for the quick merge and fix! 🎉

But this may possibly break older node versions, so I'd appreciate if this could be investigated a bit further.

filipedeschamps commented 5 years ago

There's only one thing that it's breaking, but it's not related to this patch:

  1) RssFeedEmitter ( integration ) #on should emit items from "The Huffington Post":
     Error: timeout of 30000ms exceeded. Ensure the done() callback is being called in this test.

It's an end-to-end test and The Huffington Post isn't responding accordingly.

Besides that, every test is passing, including 4.x Node.js versions 👍

Ardakilic commented 5 years ago

Oh, so these Travis fails are due to Huffington Post 😄

Thanks again for the quick patch version 👍

filipedeschamps commented 5 years ago

Yep, that's why I forced the merge :)

Thanks for your PR and congratulations on your project 👍