diff --git a/.circleci/config.yml b/.circleci/config.yml
index c964532244..68e53df778 100644
--- a/.circleci/config.yml
+++ b/.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,12 @@ workflows:
jobs:
- bundle
- bundle_ruby_3_0
+ - brakeman:
+ requires:
+ - bundle_ruby_3_0
+ - tests_postgres:
+ requires:
+ - bundle
- tests_postgres_ruby_3_0:
requires:
- bundle_ruby_3_0
diff --git a/Gemfile b/Gemfile
index 5d351b38b5..38ab9cdf1b 100644
--- a/Gemfile
+++ b/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
diff --git a/app/controllers/spree/admin/payment_methods_controller.rb b/app/controllers/spree/admin/payment_methods_controller.rb
index e034fdfd66..bee40a358d 100644
--- a/app/controllers/spree/admin/payment_methods_controller.rb
+++ b/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
diff --git a/app/controllers/spree/admin/payments_controller.rb b/app/controllers/spree/admin/payments_controller.rb
index 59208798da..e7f8713b84 100644
--- a/app/controllers/spree/admin/payments_controller.rb
+++ b/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
diff --git a/app/controllers/spree/admin/promotion_rules_controller.rb b/app/controllers/spree/admin/promotion_rules_controller.rb
index 9dea34e3f1..093b2db7bc 100644
--- a/app/controllers/spree/admin/promotion_rules_controller.rb
+++ b/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
diff --git a/app/views/spree/admin/return_index/return_authorizations.html.erb b/app/views/spree/admin/return_index/return_authorizations.html.erb
index b4f0bbf8da..b1440012ac 100644
--- a/app/views/spree/admin/return_index/return_authorizations.html.erb
+++ b/app/views/spree/admin/return_index/return_authorizations.html.erb
@@ -10,12 +10,12 @@
<%= 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'}" %>
<%= 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'}" %>
<% end %>
diff --git a/brakeman.ignore b/brakeman.ignore
new file mode 100644
index 0000000000..ac45b0a872
--- /dev/null
+++ b/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": ""
+ }
+ ]
+}