Update rest-client gem because security vulnerabilities

heroku / legacy-cli

Heroku CLI

https://cli.heroku.com

MIT License

1.37k stars 381 forks source link

Update rest-client gem because security vulnerabilities #1984

Closed balazslaszlo closed 7 years ago

balazslaszlo commented 8 years ago

rest-client 1.6.8 has security vulnerabilities https://www.versioneye.com/ruby/rest-client/1.6.8

jdx commented 8 years ago

haven't been able to figure out why yet, but this fails locally for me

bluta commented 8 years ago

@dickeyxxx maybe only bcs restclient switched on netrc auth on 1.7.0 so your stubbed requests do not match if you have a .netrc file.

bluta commented 8 years ago

@dickeyxxx could you take a look if - on top of the additional commit kind of taking care of the stubbed requests - there's anything more we can do to have this merged?

eschersgirl commented 8 years ago

:+1:

storhet commented 7 years ago

👍

Hawerrr commented 7 years ago

:+1:

bluta commented 7 years ago

@atmos -- anything we can do get this rechecked/merged?

atmos commented 7 years ago

@bluta I don't understand the security vulnerabilities well enough to comment. 1.6 -> 1.8 is an API breaking change in a lot of places and I don't have enough things running constantly to speak authoritatively.

bluta commented 7 years ago

@atmos - you mean although the coverage for the first look seems promising for the API, still feel it'll fail in real applications? by any chance you have a rough direction for such problems? sorry to bother, but our application works fine with the changes, and we also need the new version of rest-client for our own oauth - and we usually try to avoid forking if possible.

ransombriggs commented 7 years ago

@balazslaszlo I merged this into https://github.com/heroku/heroku/pull/2011 and released, I just removed the regexes in favor of a host / netrc existence check. I was going to mock out Netrc completely so that it never did user / pass in the url, but we depend on it to verify things in other tests, so it was not as straightforward as I had hoped. Thanks for putting this together and sorry it took so long to get merged.

ransombriggs commented 7 years ago

@balazslaszlo I had to roll back https://github.com/heroku/heroku/pull/2011 because this did not work on windows due to an ffi dependency.

from C:/Program Files (x86)/Heroku/ruby-2.1.7/lib/ruby/2.1.0/rubygems/core_ext/kernel_require.rb:55:in require': cannot load such file -- ffi (LoadError)
from C:/Program Files (x86)/Heroku/ruby-2.1.7/lib/ruby/2.1.0/rubygems/core_ext/kernel_require.rb:55:inrequire'
from C:/Program Files (x86)/Heroku/vendor/gems/rest-client-1.8.0/lib/restclient/windows/root_certs.rb:2:in <top (required)>'
from C:/Program Files (x86)/Heroku/vendor/gems/rest-client-1.8.0/lib/restclient/windows.rb:7:inrequire_relative'
from C:/Program Files (x86)/Heroku/vendor/gems/rest-client-1.8.0/lib/restclient/windows.rb:7