v3: implement query params whitelisting to avoid argument injection attacks

lib/travis/api/v3/router.rb

@@ -17,7 +17,7 @@ module Travis::API::V3
 
       raise NotFound unless factory
 
-      service         = factory.new(access_control, env_params.merge(params))
+      service         = factory.new(access_control, factory.filter_params(env_params).merge(params))
       result          = service.run
       render(result, env_params, env)
     rescue Error => error

lib/travis/api/v3/service.rb

@@ -1,5 +1,8 @@
 module Travis::API::V3
   class Service
+    DEFAULT_PARAMS = [ "include".freeze, "@type".freeze ]
+    private_constant :DEFAULT_PARAMS
+
     def self.result_type(type = nil)
       @result_type   = type if type
       @result_type ||= parent.result_type if parent and parent.respond_to? :result_type
@@ -7,6 +10,20 @@ module Travis::API::V3
       @result_type
     end
 
+    def self.filter_params(params)
+      wanted = self.params
+      params.select { |key| wanted.include? key }
+    end
+
+    def self.params(*list, prefix: nil)
+      @params ||= superclass.respond_to?(:params) ? superclass.params.dup : DEFAULT_PARAMS
+      list.each do |entry|
+        @params << entry.to_s
+        @params << "#{prefix || result_type}.#{entry}" if entry.is_a? Symbol
+      end
+      @params
+    end
+
     attr_accessor :access_control, :params
 
     def initialize(access_control, params)

lib/travis/api/v3/services/organizations/for_current_user.rb

@@ -1,5 +1,5 @@
 module Travis::API::V3
-  class Services::Repositories::ForCurrentUser < Service
+  class Services::Organizations::ForCurrentUser < Service
     def run!
       raise LoginRequired unless access_control.logged_in?
       query.for_member(access_control.user)

lib/travis/api/v3/services/repositories/for_current_user.rb

@@ -1,5 +1,7 @@
 module Travis::API::V3
-  class Services::Organizations::ForCurrentUser < Service
+  class Services::Repositories::ForCurrentUser < Service
+    params :active, :private, prefix: :repository
+
     def run!
       raise LoginRequired unless access_control.logged_in?
       query.for_member(access_control.user)

lib/travis/api/v3/services/requests/create.rb

@@ -5,6 +5,7 @@ module Travis::API::V3
     private_constant :TIME_FRAME, :LIMIT
 
     result_type :request
+    params "request", "user", :config, :message, :branch
 
     def run
       raise LoginRequired                              unless access_control.logged_in? or access_control.full_access?

spec/v3/services/account/find_spec.rb

@@ -26,17 +26,14 @@ describe Travis::API::V3::Services::Account::Find do
 
       before  { get("/v3/account/example-org?organization.id=#{other.id}") }
       example { expect(last_response).to be_ok   }
-
-      pending "param whitelisting not yet implemented" do
-        example { expect(JSON.load(body)).to be == {
-          "@type"     => "organization",
-          "@href"     => "/v3/org/#{org.id}",
-          "id"        => org.id,
-          "login"     => "example-org",
-          "name"      => nil,
-          "github_id" => nil
-        }}
-      end
+      example { expect(JSON.load(body)).to be == {
+        "@type"     => "organization",
+        "@href"     => "/v3/org/#{org.id}",
+        "id"        => org.id,
+        "login"     => "example-org",
+        "name"      => nil,
+        "github_id" => nil
+      }}
     end
   end
 
@@ -65,21 +62,18 @@ describe Travis::API::V3::Services::Account::Find do
       before      { other.save!                   }
       after       { other.delete                  }
 
-      before  { get("/v3/account/example-org?user.id=#{other.id}") }
+      before  { get("/v3/account/example-user?user.id=#{other.id}") }
       example { expect(last_response).to be_ok   }
-
-      pending "param whitelisting not yet implemented" do
-        example { expect(JSON.load(body)).to be == {
-          "@type"     => "user",
-          "@href"     => "/v3/user/#{user.id}",
-          "id"        => user.id,
-          "login"     => "example-user",
-          "name"      => nil,
-          "github_id" => nil,
-          "is_syncing"=> nil,
-          "synced_at" => nil
-        }}
-      end
+      example { expect(JSON.load(body)).to be == {
+        "@type"     => "user",
+        "@href"     => "/v3/user/#{user.id}",
+        "id"        => user.id,
+        "login"     => "example-user",
+        "name"      => nil,
+        "github_id" => nil,
+        "is_syncing"=> nil,
+        "synced_at" => nil
+      }}
     end
   end
 end