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

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

Merged
merged 1 commit into from
Nov 28, 2018
Merged

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

merged 1 commit into from
Nov 28, 2018

Conversation

Ardakilic
Copy link
Contributor

@Ardakilic Ardakilic commented Nov 28, 2018

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
Copy link
Owner

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

@filipedeschamps filipedeschamps merged commit f5b5055 into filipedeschamps:master Nov 28, 2018
@filipedeschamps
Copy link
Owner

Done! New release rss-feed-emitter@2.0.1

@Ardakilic
Copy link
Contributor Author

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
Copy link
Owner

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
Copy link
Contributor Author

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

Thanks again for the quick patch version 👍

@filipedeschamps
Copy link
Owner

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

Thanks for your PR and congratulations on your project 👍

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