Merge branch 'master' into jc-v3-env-vars

Gemfile

@@ -22,7 +22,7 @@ gem 'sentry-raven'
 gem 'yard-sinatra',    github: 'rkh/yard-sinatra'
 gem 'rack-contrib'
 gem 'rack-cache',      github: 'rtomayko/rack-cache'
-gem 'rack-attack'
+gem 'rack-attack', '5.0.0.beta1'
 gem 'gh'
 gem 'bunny',           '~> 0.8.0'
 gem 'dalli'
@@ -33,6 +33,7 @@ gem 'micro_migrations'
 gem 'simplecov'
 gem 'skylight', '~> 0.6.0.beta.1'
 gem 'stackprof'
+gem 'netaddr'
 
 gem 'jemalloc'
 gem 'customerio'

Gemfile.lock

@@ -260,6 +260,7 @@ GEM
     multipart-post (2.0.0)
     net-http-persistent (2.9.4)
     net-http-pipeline (1.0.1)
+    netaddr (1.5.1)
     os (0.9.6)
     pg (0.18.4)
     proxies (0.2.1)
@@ -273,7 +274,7 @@ GEM
       pusher-signature (~> 0.1.8)
     pusher-signature (0.1.8)
     rack (1.4.7)
-    rack-attack (4.4.1)
+    rack-attack (5.0.0.beta1)
       rack
     rack-contrib (1.4.0)
       git-version-bump (~> 0.15)
@@ -397,8 +398,9 @@ DEPENDENCIES
   micro_migrations
   mocha (~> 0.12)
   mustermann!
+  netaddr
   pry
-  rack-attack
+  rack-attack (= 5.0.0.beta1)
   rack-cache!
   rack-contrib
   rake (~> 0.9.2)

lib/travis/api/app.rb

@@ -132,7 +132,7 @@ module Travis::Api
         use Travis::Api::App::Middleware::UserAgentTracker
 
         # make sure this is below ScopeCheck so we have the token
-        use Rack::Attack if Endpoint.production?
+        use Rack::Attack if Endpoint.production? and not Travis.config.enterprise
 
         # if this is a v3 API request, ignore everything after
         use Travis::API::V3::OptIn

lib/travis/api/app/endpoint/authorization.rb

@@ -114,7 +114,7 @@ class Travis::Api::App
       # access token and user payload to the parent window via postMessage.
       #
       # However, the endpoint to send the payload to has to be explicitely
-      # whitelisted in production, as this is endpoint is only meant to be used
+      # safelisted in production, as this is endpoint is only meant to be used
       # with the official Travis CI client at the moment.
       #
       # Example usage:
@@ -408,7 +408,7 @@ __END__
 
 @@ invalid_target
 <script>
-console.log('refusing to send a token to <%= target_origin.inspect %>, not whitelisted!');
+console.log('refusing to send a token to <%= target_origin.inspect %>, not safelisted!');
 </script>
 
 @@ common

lib/travis/api/attack.rb

@@ -1,4 +1,5 @@
 require 'rack/attack'
+require 'netaddr'
 
 class Rack::Attack
   class Request
@@ -31,29 +32,38 @@ class Rack::Attack
     "/auth/post_message/iframe"
   ]
 
-  whitelist('safelist build status images') do |request|
+  GITHUB_CIDR = NetAddr::CIDR.create('192.30.252.0/22')
+
+  safelist('safelist build status images') do |request|
     /\.(png|svg)$/.match(request.path)
   end
 
+  # https://help.github.com/articles/what-ip-addresses-does-github-use-that-i-should-safelist/
+  safelist('safelist anything coming from github') do |request|
+    request.ip && GITHUB_CIDR.contains?(request.ip)
+  end
+
   ####
   # Whitelisted IP addresses
-  whitelist('whitelist client requesting from redis') do |request|
+  safelist('safelist client requesting from redis') do |request|
-    Travis.redis.sismember(:api_whitelisted_ips, request.ip)
+    # TODO: deprecate :api_whitelisted_ips in favour of api_safelisted_ips
+    Travis.redis.sismember(:api_whitelisted_ips, request.ip) || Travis.redis.sismember(:api_safelisted_ips, request.ip)
   end
 
   ####
   # Ban based on: IP address
   # Ban time:     indefinite
   # Ban after:    manually banned
-  blacklist('block client requesting from redis') do |request|
+  blocklist('block client requesting from redis') do |request|
-    Travis.redis.sismember(:api_blacklisted_ips, request.ip)
+    # TODO: deprecate :api_blacklisted_ips in favour of api_blocklisted_ips
+    Travis.redis.sismember(:api_blacklisted_ips, request.ip) || Travis.redis.sismember(:api_blocklisted_ips, request.ip)
   end
 
   ####
   # Ban based on: IP address or access token
   # Ban time:     5 hours
   # Ban after:    10 POST requests within five minutes to /auth/github
-  blacklist('hammering /auth/github') do |request|
+  blocklist('hammering /auth/github') do |request|
     Rack::Attack::Allow2Ban.filter(request.identifier, maxretry: 2, findtime: 5.minutes, bantime: bantime(5.hours)) do
       request.post? and request.path == '/auth/github'
     end
@@ -63,7 +73,7 @@ class Rack::Attack
   # Ban based on: IP address or access token
   # Ban time:     1 hour
   # Ban after:    10 POST requests within 30 seconds
-  blacklist('spamming with POST requests') do |request|
+  blocklist('spamming with POST requests') do |request|
     Rack::Attack::Allow2Ban.filter(request.identifier, maxretry: 10, findtime: 30.seconds, bantime: bantime(1.hour)) do
       request.post? and not POST_SAFELIST.include? request.path
     end

lib/travis/api/v3/router.rb

@@ -29,7 +29,7 @@ module Travis::API::V3
       result   = service.run
       metrics.tick(:service)
 
-      env_params.each_key { |key| result.ignored_param(key, reason: "not whitelisted".freeze) unless filtered.include?(key) }
+      env_params.each_key { |key| result.ignored_param(key, reason: "not safelisted".freeze) unless filtered.include?(key) }
       response = render(result, env_params, env)
 
       metrics.tick(:renderer)

spec/unit/attack_spec.rb

@@ -6,18 +6,31 @@ describe Rack::Attack do
     }
 
     it 'should be safelisted' do
-      expect(Rack::Attack.whitelisted?(request)).to be_truthy
+      expect(Rack::Attack.safelisted?(request)).to be_truthy
     end
   end
 
-  describe 'non-image API request' do
+  describe 'request from GitHub ip' do
+    let(:request) {
+      env = Rack::MockRequest.env_for("https://api-test.travis-ci.org/repos/rails/rails/branches", {
+        'REMOTE_ADDR' => '192.30.252.42',
+      })
+      Rack::Attack::Request.new(env)
+    }
+
+    it 'should be safelisted' do
+      expect(Rack::Attack.safelisted?(request)).to be_truthy
+    end
+  end
+
+  describe 'non-safelisted request' do
     let(:request) {
       env = Rack::MockRequest.env_for("https://api-test.travis-ci.org/repos/rails/rails/branches")
       Rack::Attack::Request.new(env)
     }
 
     it 'should not be safelisted' do
-      expect(Rack::Attack.whitelisted?(request)).to be_falsy
+      expect(Rack::Attack.safelisted?(request)).to be_falsy
     end
   end
 end

spec/v3/services/owner/find_spec.rb

@@ -167,7 +167,7 @@ describe Travis::API::V3::Services::Owner::Find, set_app: true do
         "avatar_url"     => nil,
         "@warnings"      => [{
           "@type"        => "warning",
-          "message"      => "query parameter organization.id not whitelisted, ignored",
+          "message"      => "query parameter organization.id not safelisted, ignored",
           "warning_type" => "ignored_parameter",
           "parameter"    => "organization.id"}]
       }}
@@ -254,7 +254,7 @@ describe Travis::API::V3::Services::Owner::Find, set_app: true do
         "synced_at"        => nil,
         "@warnings"        => [{
           "@type"          => "warning",
-          "message"        => "query parameter user.id not whitelisted, ignored",
+          "message"        => "query parameter user.id not safelisted, ignored",
           "warning_type"   => "ignored_parameter",
           "parameter"      => "user.id"}]
       }}

spec_core/model/job_spec.rb

@@ -129,7 +129,7 @@ describe Job do
       }
     end
 
-    it 'removes addons items which are not whitelisted' do
+    it 'removes addons items which are not safelisted' do
       job = Job.new(repository: repo)
       config = { rvm: '1.8.7',
                  addons: { sauce_connect: true, firefox: '22.0' },
@@ -328,7 +328,7 @@ describe Job do
         }
       end
 
-      it 'removes addons items which are not whitelisted' do
+      it 'removes addons items which are not safelisted' do
         config = { rvm: '1.8.7',
                    addons: {
                      sauce_connect: {

vendor/travis-core/lib/travis/model/job.rb

@@ -14,7 +14,7 @@ class Job < Travis::Model
   require 'travis/model/job/test'
   require 'travis/model/env_helpers'
 
-  WHITELISTED_ADDONS = %w(
+  SAFELISTED_ADDONS = %w(
     apt
     apt_packages
     apt_sources
@@ -167,7 +167,7 @@ class Job < Travis::Model
 
     def delete_addons(config)
       if config[:addons].is_a?(Hash)
-        config[:addons].keep_if { |key, _| WHITELISTED_ADDONS.include? key.to_s }
+        config[:addons].keep_if { |key, _| SAFELISTED_ADDONS.include? key.to_s }
       else
         config.delete(:addons)
       end