From 6d405b234e7ff98c6e3dc57d53e23603ba84d4a3 Mon Sep 17 00:00:00 2001 From: Matthew Lefevre <83986509+zendesk-mattlefevre@users.noreply.github.com> Date: Wed, 31 May 2023 08:59:28 -0600 Subject: [PATCH] faraday upgrade * bump faraday and dependent gems * update auth tests; connections no longer have auth headers built in * set default branch to master and CVE-2022-39253 fix * update parent class due to faraday bump * fix broken helper test * ensure error raised if redirected in octokit * rubocops * fixing bundler version * faraday 2.0 update * account for changes in how 301s are handled in Octokit * account for faraday error changes * rubocops --- Gemfile | 3 +- Gemfile.lock | 68 +++++++++++-------- app/models/outbound_webhook.rb | 4 +- config/initializers/octokit.rb | 4 +- .../lib/samson_assertible/samson_plugin.rb | 2 +- .../test/samson_github/samson_plugin_test.rb | 12 +++- .../slack_webhooks_service.rb | 2 +- .../app/models/zendesk_notification.rb | 1 + test/helpers/dashboards_helper_test.rb | 10 +-- test/models/outbound_webhook_test.rb | 11 +-- test/support/git_repo_test_support.rb | 3 +- test/support/webmock.rb | 4 +- 12 files changed, 75 insertions(+), 49 deletions(-) diff --git a/Gemfile b/Gemfile index c9dd23d861..4e8ae3f7b3 100644 --- a/Gemfile +++ b/Gemfile @@ -33,7 +33,8 @@ gem 'omniauth-gitlab' gem 'omniauth-bitbucket' gem 'omniauth-rails_csrf_protection' # remove once https://github.com/omniauth/omniauth/pull/809 is resolved gem 'octokit' -gem 'faraday' +gem 'faraday', '~> 2.7' +gem 'faraday-net_http_persistent', '~> 2.0' gem 'faraday-http-cache' gem 'warden' gem 'active_hash' diff --git a/Gemfile.lock b/Gemfile.lock index 284c9a4cc2..d0d2874e63 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -255,7 +255,7 @@ GEM minitest (>= 5.1) tzinfo (~> 2.0) zeitwerk (~> 2.3) - addressable (2.8.1) + addressable (2.8.4) public_suffix (>= 2.0.2, < 6.0) airbrake (11.0.1) airbrake-ruby (~> 5.1) @@ -330,10 +330,17 @@ GEM erubi (1.11.0) erubis (2.7.0) execjs (2.7.0) - faraday (0.17.3) - multipart-post (>= 1.2, < 3) - faraday-http-cache (2.0.0) - faraday (~> 0.8) + faraday (2.7.4) + faraday-net_http (>= 2.0, < 3.1) + ruby2_keywords (>= 0.0.4) + faraday-http-cache (2.5.0) + faraday (>= 0.8) + faraday-multipart (1.0.4) + multipart-post (~> 2) + faraday-net_http (3.0.2) + faraday-net_http_persistent (2.1.0) + faraday (~> 2.5) + net-http-persistent (~> 4.0) ffi (1.15.5) ffi-compiler (1.0.1) ffi (>= 1.0.0) @@ -358,7 +365,7 @@ GEM activerecord (>= 4.2, < 6.3) activesupport (>= 4.2, < 6.3) hashdiff (0.3.4) - hashie (3.6.0) + hashie (5.0.0) http (5.1.1) addressable (~> 2.8) http-cookie (~> 1.0) @@ -387,7 +394,7 @@ GEM json (2.6.3) jsonpath (1.1.2) multi_json - jwt (2.2.1) + jwt (2.7.0) kubeclient (4.11.0) http (>= 3.0, < 6.0) jsonpath (~> 1.0) @@ -433,9 +440,10 @@ GEM msgpack (1.4.2) multi_json (1.15.0) multi_xml (0.6.0) - multipart-post (2.1.1) + multipart-post (2.3.0) mysql2 (0.5.3) - net-http-persistent (2.9.4) + net-http-persistent (4.0.2) + connection_pool (~> 2.2) net-ldap (0.16.1) netrc (0.11.0) newrelic_rpm (6.7.0.359) @@ -448,15 +456,15 @@ GEM nokogiri (1.14.3-x86_64-linux) racc (~> 1.4) oauth (0.5.6) - oauth2 (1.4.4) - faraday (>= 0.8, < 2.0) + oauth2 (1.4.11) + faraday (>= 0.17.3, < 3.0) jwt (>= 1.0, < 3.0) multi_json (~> 1.3) multi_xml (~> 0.5) - rack (>= 1.2, < 3) - octokit (4.18.0) - faraday (>= 0.9) - sawyer (~> 0.8.0, >= 0.5.3) + rack (>= 1.2, < 4) + octokit (6.1.1) + faraday (>= 1, < 3) + sawyer (~> 0.9) omniauth (1.9.2) hashie (>= 3.4.6) rack (>= 1.6.2, < 3) @@ -467,10 +475,11 @@ GEM omniauth-gitlab (1.0.2) omniauth (~> 1.0) omniauth-oauth2 (~> 1.0) - omniauth-google-oauth2 (0.6.0) + omniauth-google-oauth2 (0.8.2) jwt (>= 2.0) - omniauth (>= 1.1.1) - omniauth-oauth2 (>= 1.5) + oauth2 (~> 1.1) + omniauth (~> 1.1) + omniauth-oauth2 (>= 1.6) omniauth-ldap (1.0.5) net-ldap (~> 0.12) omniauth (~> 1.0) @@ -479,9 +488,9 @@ GEM omniauth-oauth (1.1.0) oauth omniauth (~> 1.0) - omniauth-oauth2 (1.7.0) - oauth2 (~> 1.4) - omniauth (~> 1.9) + omniauth-oauth2 (1.7.3) + oauth2 (>= 1.4, < 3) + omniauth (>= 1.9, < 3) omniauth-rails_csrf_protection (0.1.2) actionpack (>= 4.2) omniauth (>= 1.3.1) @@ -580,6 +589,7 @@ GEM rack (>= 1.1) rubocop (>= 0.72.0) ruby-progressbar (1.10.1) + ruby2_keywords (0.0.5) ruby_parser (3.13.1) sexp_processor (~> 4.9) rubyntlm (0.3.4) @@ -594,9 +604,9 @@ GEM sprockets (> 3.0) sprockets-rails tilt - sawyer (0.8.2) + sawyer (0.9.2) addressable (>= 2.3.5) - faraday (> 0.8, < 2.0) + faraday (>= 0.17.3, < 3) sentry-rails (5.4.2) railties (>= 5.0) sentry-ruby (~> 5.4.2) @@ -645,11 +655,12 @@ GEM websocket-extensions (>= 0.1.0) websocket-extensions (0.1.5) zeitwerk (2.6.8) - zendesk_api (1.16.0) - faraday (~> 0.9) - hashie (>= 3.5.2, < 4.0.0) + zendesk_api (2.0.1) + faraday (> 2.0.0) + faraday-multipart + hashie (>= 3.5.2, < 6.0.0) inflection - mime-types + mini_mime multipart-post (~> 2.0) PLATFORMS @@ -678,8 +689,9 @@ DEPENDENCIES dogstatsd-ruby doorkeeper (~> 5.4.0) dotenv - faraday + faraday (~> 2.7) faraday-http-cache + faraday-net_http_persistent (~> 2.0) flay forking_test_runner gcloud_image_tagger! diff --git a/app/models/outbound_webhook.rb b/app/models/outbound_webhook.rb index d3578a6c3d..054e65059c 100644 --- a/app/models/outbound_webhook.rb +++ b/app/models/outbound_webhook.rb @@ -100,8 +100,8 @@ def connection case auth_type when "None" # rubocop:disable Lint/EmptyWhen noop - when "Basic" then connection.basic_auth(username, password) - when "Bearer", "Token" then connection.authorization auth_type, password + when "Basic" then connection.request :authorization, :basic, username, password + when "Bearer", "Token" then connection.request :authorization, auth_type, password else raise ArgumentError, "Unsupported auth_type #{auth_type.inspect}" end end diff --git a/config/initializers/octokit.rb b/config/initializers/octokit.rb index a2ed6efc77..75aaa8f325 100644 --- a/config/initializers/octokit.rb +++ b/config/initializers/octokit.rb @@ -8,7 +8,7 @@ # to reproduce/remove: rename a repository and try to create a diff with the old name # it should return a NullComparison and not a broken Changeset with nil commits # tested via test/models/changeset_test.rb -class Octokit::RedirectAsError < Faraday::Response::Middleware +class Octokit::RedirectAsError < Faraday::Middleware private def on_complete(response) @@ -23,6 +23,8 @@ def on_complete(response) builder.response :logger, Rails.logger builder.use Octokit::Response::RaiseError builder.use Octokit::RedirectAsError + # Raise a RedirectLimitReached error if a request is redirected. + builder.use Octokit::Middleware::FollowRedirects, limit: 0 builder.adapter Faraday.default_adapter end diff --git a/plugins/assertible/lib/samson_assertible/samson_plugin.rb b/plugins/assertible/lib/samson_assertible/samson_plugin.rb index da1413588e..79c5638a0b 100644 --- a/plugins/assertible/lib/samson_assertible/samson_plugin.rb +++ b/plugins/assertible/lib/samson_assertible/samson_plugin.rb @@ -10,7 +10,7 @@ def deliver(deploy) return unless deploy.stage.notify_assertible? && deploy.succeeded? conn = Faraday.new(url: 'https://assertible.com') - conn.basic_auth(deploy_token, '') + conn.request :authorization, :basic, deploy_token, '' conn.post( '/deployments', { diff --git a/plugins/github/test/samson_github/samson_plugin_test.rb b/plugins/github/test/samson_github/samson_plugin_test.rb index 48a984163f..26010603bf 100644 --- a/plugins/github/test/samson_github/samson_plugin_test.rb +++ b/plugins/github/test/samson_github/samson_plugin_test.rb @@ -143,8 +143,16 @@ def fire(a, b) # tests config/initializers/octokit.rb patch it "catches exception and returns NullComparison" do - stub_github_api("repos/foo/bar/compare/a...b", {}, 301) - assert_raises(Octokit::RepositoryUnavailable) { fire("a", "b") }.message.must_include "GET https://api.github" + stub_github_api( + "repos/foo/bar/compare/a...b", + {}, + 301, + {'location': 'https://api.github.com/repos/foobar/bar/compare'} + ) + + assert_raises(Octokit::Middleware::RedirectLimitReached) { fire("a", "b") }.message.must_include( + "too many redirects; last one to: https://api.github" + ) end end end diff --git a/plugins/slack_webhooks/lib/samson_slack_webhooks/slack_webhooks_service.rb b/plugins/slack_webhooks/lib/samson_slack_webhooks/slack_webhooks_service.rb index 44485fac0d..45988a83d2 100644 --- a/plugins/slack_webhooks/lib/samson_slack_webhooks/slack_webhooks_service.rb +++ b/plugins/slack_webhooks/lib/samson_slack_webhooks/slack_webhooks_service.rb @@ -45,7 +45,7 @@ def deliver_message_via_webhook(webhook:, message:, attachments:) if response.status >= 300 raise "Error: channel #{webhook.channel.inspect} #{response.status} #{response.body.to_s[0..100]}" end - rescue Faraday::ClientError, RuntimeError => e + rescue Faraday::ClientError, Faraday::ConnectionFailed, RuntimeError => e Samson::ErrorNotifier.notify( e, webhook_id: webhook.id, diff --git a/plugins/zendesk/app/models/zendesk_notification.rb b/plugins/zendesk/app/models/zendesk_notification.rb index fd4ecdac4c..df20da31a9 100644 --- a/plugins/zendesk/app/models/zendesk_notification.rb +++ b/plugins/zendesk/app/models/zendesk_notification.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'zendesk_api' require 'net/http/persistent' +require 'faraday/net_http_persistent' class ZendeskNotification cattr_accessor(:zendesk_url) { ENV['ZENDESK_URL'] } diff --git a/test/helpers/dashboards_helper_test.rb b/test/helpers/dashboards_helper_test.rb index e203355006..0d321e1734 100644 --- a/test/helpers/dashboards_helper_test.rb +++ b/test/helpers/dashboards_helper_test.rb @@ -7,12 +7,12 @@ describe '#project_multi_deploys?' do it 'has no warnings for same deploy across all deploy groups' do prep_versions('v1.0') - project_has_different_deploys?(@versions['1']).must_equal false + project_has_different_deploys?(@versions[1]).must_equal false end it 'has warnings for different deploy across all deploy groups' do prep_versions('v1.1') - project_has_different_deploys?(@versions['1']).must_equal true + project_has_different_deploys?(@versions[1]).must_equal true end end @@ -53,9 +53,9 @@ def prep_versions(ref) @versions = Hashie::Mash.new( 0 => {}, 1 => { - '1' => {reference: ref, id: 1}, - '2' => {reference: 'v1.0'}, - '3' => {reference: 'v1.0'} + 1 => {reference: ref, id: 1}, + 2 => {reference: 'v1.0'}, + 3 => {reference: 'v1.0'} } ) end diff --git a/test/models/outbound_webhook_test.rb b/test/models/outbound_webhook_test.rb index bcc6444d22..6dce5b2662 100644 --- a/test/models/outbound_webhook_test.rb +++ b/test/models/outbound_webhook_test.rb @@ -91,21 +91,22 @@ let(:connection) { webhook.send(:connection) } it "does not add auth when not configured" do - refute_includes connection.headers, 'Authorization' + webhook.auth_type = "None" + refute_includes connection.builder.handlers, Faraday::Request::Authorization end - it "adds basic auth" do + it "adds auth with Basic authentication" do webhook.auth_type = "Basic" webhook.username = "adminuser" webhook.password = "abc123" - assert_equal connection.headers['Authorization'], 'Basic YWRtaW51c2VyOmFiYzEyMw==' + assert_includes connection.builder.handlers, Faraday::Request::Authorization end - it "adds token auth" do + it "adds auth with token authentication" do webhook.auth_type = "Token" webhook.username = "adminuser" webhook.password = "abc123" - assert_equal connection.headers['Authorization'], 'Token abc123' + assert_includes connection.builder.handlers, Faraday::Request::Authorization end it "fails on unsupported type" do diff --git a/test/support/git_repo_test_support.rb b/test/support/git_repo_test_support.rb index 26e7b6a75e..f883d2e8dc 100644 --- a/test/support/git_repo_test_support.rb +++ b/test/support/git_repo_test_support.rb @@ -70,7 +70,8 @@ def create_submodule_repo def init_repo_commands <<-SHELL - git init + git init --initial-branch=master + git config protocol.file.allow always git config user.email "test@example.com" git config user.name "Test User" git config commit.gpgsign false diff --git a/test/support/webmock.rb b/test/support/webmock.rb index fc298634c7..4e2f4ecb17 100644 --- a/test/support/webmock.rb +++ b/test/support/webmock.rb @@ -39,12 +39,12 @@ def self.assert_requests after { @assert_requests.each { |assert_args| assert_requested(*assert_args) } } end - def stub_github_api(path, response = {}, status = 200) + def stub_github_api(path, response = {}, status = 200, headers = {}) url = "https://api.github.com/#{path}" stub_request(:get, url).to_return( status: status, body: JSON.dump(response), - headers: {'Content-Type' => 'application/json'} + headers: {'Content-Type' => 'application/json'}.merge(headers) ) end end