VEN-99 Add brakeman

.circleci/config.yml

@@ -145,6 +145,27 @@ jobs:
             - vendor/bundle
           key: spree-dashboard-bundle-v10-ruby-3-0-{{ checksum "Gemfile.lock" }}
 
+  brakeman:
+    <<: *defaults_3_0
+    steps:
+      - checkout
+      - restore_cache:
+          keys:
+            - spree-dashboard-brakeman-{{ .Branch }}
+            - spree-dashboard-brakeman
+      - run:
+          name: Ensure Bundle Install
+          command: |
+            bundle check || bundle install
+      - run:
+          name: Run Brakeman
+          command: |
+            bundle exec brakeman --ignore-config brakeman.ignore --exit-on-warn --exit-on-error
+      - save_cache:
+          key: spree-dashboard-brakeman-{{ .Branch }}-{{ checksum "Gemfile.lock" }}
+          paths:
+            - ~/vendor/bundle
+
   tests_postgres: &tests_postgres
     <<: *run_tests
     environment: &postgres_environment
@@ -188,6 +209,9 @@ workflows:
     jobs:
       - bundle
       - bundle_ruby_3_0
+      - brakeman:
+          requires:
+            - bundle_ruby_3_0
       - tests_postgres:
           requires:
             - bundle

Gemfile

@@ -43,6 +43,7 @@ end
 
 group :test, :development do
   gem 'awesome_print'
+  gem 'brakeman', '~> 5.0'
   gem 'gem-release'
   gem 'redis'
   gem 'rubocop', '~> 1.22.3', require: false # bumped

app/controllers/spree/admin/payment_methods_controller.rb

@@ -8,7 +8,8 @@ class PaymentMethodsController < ResourceController
       respond_to :html
 
       def create
-        @payment_method = params[:payment_method].delete(:type).constantize.new(payment_method_params)
+        provider_name = @providers.map(&:to_s).find { |provider| provider == params[:payment_method][:type] }
+        @payment_method = provider_name.constantize.new(payment_method_params)
         @object = @payment_method
         set_current_store
         invoke_callbacks(:create, :before)
@@ -80,7 +81,7 @@ def validate_payment_method_provider
       end
 
       def payment_method_params
-        params.require(:payment_method).permit!
+        params.require(:payment_method).permit(:type, :name, :description, :display_on, :auto_capture, :active, store_ids: [])
       end
 
       def preferences_params

app/controllers/spree/admin/payments_controller.rb

@@ -64,7 +64,7 @@ def create
       end
 
       def fire
-        return unless event = params[:e] and @payment.payment_source
+        return unless (event = find_payment_event) && @payment.payment_source
 
         # Because we have a transition method also called void, we do this to avoid conflicts.
         event = 'void_transaction' if event == 'void'
@@ -107,6 +107,10 @@ def load_payment
       def model_class
         Spree::Payment
       end
+
+      def find_payment_event
+        %w[process authorize purchase capture void cancel].find { |e| e == params[:e] }
+      end
     end
   end
 end

app/controllers/spree/admin/promotion_rules_controller.rb

@@ -49,6 +49,6 @@ def validate_promotion_rule_type
   end
 
   def promotion_rule_params
-    params[:promotion_rule].permit!
+    params[:promotion_rule].permit(:type)
   end
 end

app/views/spree/admin/return_index/return_authorizations.html.erb

@@ -10,12 +10,12 @@
   </li>
   <li class="nav-item">
     <%= link_to Spree.t(:authorized),
-      params.merge({q: {state_eq: :authorized}}).permit!,
+      params.permit(q: {}).merge({q: {state_eq: :authorized}}),
       class: "nav-link #{'active' if params[:q][:state_eq] == 'authorized'}" %>
   </li>
   <li class="nav-item">
     <%= link_to Spree.t(:canceled),
-      params.merge({q: {state_eq: :canceled}}).permit!,
+      params.permit(q: {}).merge({q: {state_eq: :canceled}}),
       class: "nav-link #{'active' if params[:q][:state_eq] == 'canceled'}" %>
   </li>
 <% end %>

brakeman.ignore

@@ -0,0 +1,104 @@
+{
+  "ignored_warnings": [
+    {
+      "warning_type": "Mass Assignment",
+      "warning_code": 70,
+      "fingerprint": "2096b83195b858fd16fa7b7139bc84400d108b5f85a8c64ae91da708b5fe21f9",
+      "check_name": "MassAssignment",
+      "message": "Specify exact keys allowed for mass assignment instead of using `permit!` which allows any keys",
+      "file": "app/controllers/spree/admin/resource_controller.rb",
+      "line": 268,
+      "link": "https://brakemanscanner.org/docs/warning_types/mass_assignment/",
+      "code": "params.require(resource.object_name).permit!",
+      "render_path": null,
+      "location": {
+        "type": "method",
+        "class": "Spree::Admin::ResourceController",
+        "method": "permitted_resource_params"
+      },
+      "user_input": null,
+      "confidence": "Medium",
+      "note": ""
+    },
+    {
+      "warning_type": "Mass Assignment",
+      "warning_code": 70,
+      "fingerprint": "934f23be465f0d2c5f7278aac4f085771ba22d5e8b56645813a2364313df9561",
+      "check_name": "MassAssignment",
+      "message": "Specify exact keys allowed for mass assignment instead of using `permit!` which allows any keys",
+      "file": "app/helpers/spree/admin/navigation_helper.rb",
+      "line": 116,
+      "link": "https://brakemanscanner.org/docs/warning_types/mass_assignment/",
+      "code": "params.permit!",
+      "render_path": null,
+      "location": {
+        "type": "method",
+        "class": "Spree::Admin::NavigationHelper",
+        "method": "per_page_dropdown_params"
+      },
+      "user_input": null,
+      "confidence": "Medium",
+      "note": ""
+    },
+    {
+      "warning_type": "Mass Assignment",
+      "warning_code": 70,
+      "fingerprint": "d45f35a46d0896dcd87099cbae9ce1c77b79ac3c935c0a6317e1a2462186e486",
+      "check_name": "MassAssignment",
+      "message": "Specify exact keys allowed for mass assignment instead of using `permit!` which allows any keys",
+      "file": "app/controllers/spree/admin/prices_controller.rb",
+      "line": 7,
+      "link": "https://brakemanscanner.org/docs/warning_types/mass_assignment/",
+      "code": "params.require(:vp).permit!",
+      "render_path": null,
+      "location": {
+        "type": "method",
+        "class": "Spree::Admin::PricesController",
+        "method": "create"
+      },
+      "user_input": null,
+      "confidence": "Medium",
+      "note": ""
+    },
+    {
+      "warning_type": "Remote Code Execution",
+      "warning_code": 24,
+      "fingerprint": "f46c38504b6d7300a6152298cbcd4f65afde1f0b5fd1c642d79908d56ff1aa8c",
+      "check_name": "UnsafeReflection",
+      "message": "Unsafe reflection method `constantize` called with parameter value",
+      "file": "app/controllers/spree/admin/promotion_actions_controller.rb",
+      "line": 7,
+      "link": "https://brakemanscanner.org/docs/warning_types/remote_code_execution/",
+      "code": "params[:action_type].constantize",
+      "render_path": null,
+      "location": {
+        "type": "method",
+        "class": "Spree::Admin::PromotionActionsController",
+        "method": "create"
+      },
+      "user_input": "params[:action_type]",
+      "confidence": "High",
+      "note": ""
+    },
+    {
+      "warning_type": "Mass Assignment",
+      "warning_code": 70,
+      "fingerprint": "f5df7c2ded2d794ea955c7ac98e0b5868e4787e56bec481b5e794742f7800308",
+      "check_name": "MassAssignment",
+      "message": "Specify exact keys allowed for mass assignment instead of using `permit!` which allows any keys",
+      "file": "app/controllers/spree/admin/payment_methods_controller.rb",
+      "line": 91,
+      "link": "https://brakemanscanner.org/docs/warning_types/mass_assignment/",
+      "code": "params.require(ActiveModel::Naming.param_key(@payment_method)).permit!",
+      "render_path": null,
+      "location": {
+        "type": "method",
+        "class": "Spree::Admin::PaymentMethodsController",
+        "method": "preferences_params"
+      },
+      "user_input": null,
+      "confidence": "Medium",
+      "note": ""
+    }
+  ]
+}