diff --git a/Gemfile b/Gemfile index 3495fe34a..85e864950 100644 --- a/Gemfile +++ b/Gemfile @@ -48,6 +48,7 @@ platforms :ruby do end group :lint do + gem "brakeman" gem "easy_translate" gem "erb_lint" gem "i18n-tasks" diff --git a/Gemfile.lock b/Gemfile.lock index 8b01954af..9a9aec2b9 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -134,6 +134,8 @@ GEM bigdecimal (3.1.6-java) bootsnap (1.18.3) msgpack (~> 1.2) + brakeman (6.1.2) + racc builder (3.2.4) capybara (3.40.0) addressable @@ -498,6 +500,7 @@ DEPENDENCIES appraisal benchmark-ips bootsnap + brakeman capybara debug dotenv-rails diff --git a/app/controllers/good_job/frontends_controller.rb b/app/controllers/good_job/frontends_controller.rb index 14a79ffba..2d353fa7f 100644 --- a/app/controllers/good_job/frontends_controller.rb +++ b/app/controllers/good_job/frontends_controller.rb @@ -2,6 +2,7 @@ module GoodJob class FrontendsController < ActionController::Base # rubocop:disable Rails/ApplicationController + protect_from_forgery with: :exception skip_after_action :verify_same_origin_request, raise: false STATIC_ASSETS = { diff --git a/app/models/good_job/job.rb b/app/models/good_job/job.rb index 3325b723a..a903b55e8 100644 --- a/app/models/good_job/job.rb +++ b/app/models/good_job/job.rb @@ -135,12 +135,9 @@ class Job < BaseRecord # @return [ActiveRecord::Relation] scope :queue_ordered, (lambda do |queues| clauses = queues.map.with_index do |queue_name, index| - "WHEN queue_name = '#{queue_name}' THEN #{index}" + sanitize_sql_array(["WHEN queue_name = ? THEN ?", queue_name, index]) end - - order( - Arel.sql("(CASE #{clauses.join(' ')} ELSE #{queues.length} END)") - ) + order(Arel.sql("(CASE #{clauses.join(' ')} ELSE #{queues.size} END)")) end) # Order jobs by scheduled or created (oldest first). diff --git a/bin/lint b/bin/lint index b1c3550f5..c34721061 100755 --- a/bin/lint +++ b/bin/lint @@ -62,4 +62,7 @@ FileUtils.chdir GEM_ROOT do puts "\n== i18n-tasks health ==" system! "bin/i18n-tasks health " + + puts "\n== Brakeman ==" + system! "bundle exec brakeman" end diff --git a/config/brakeman.ignore b/config/brakeman.ignore new file mode 100644 index 000000000..2a33e32c1 --- /dev/null +++ b/config/brakeman.ignore @@ -0,0 +1,75 @@ +{ + "ignored_warnings": [ + { + "warning_type": "Dynamic Render Path", + "warning_code": 15, + "fingerprint": "520db6b6cd19ef42def0ca6c3031065e3a1f485e47d20db1f4122153785437e3", + "check_name": "Render", + "message": "Render path contains parameter value", + "file": "app/controllers/good_job/frontends_controller.rb", + "line": 44, + "link": "https://brakemanscanner.org/docs/warning_types/dynamic_render_path/", + "code": "render(file => (self.class.js_modules[params[:name].to_sym] or raise(ActionController::RoutingError, \"Not Found\")), {})", + "render_path": null, + "location": { + "type": "method", + "class": "GoodJob::FrontendsController", + "method": "module" + }, + "user_input": "params[:name].to_sym", + "confidence": "Weak", + "cwe_id": [ + 22 + ], + "note": "Files are explicitly enumerated in the array" + }, + { + "warning_type": "SQL Injection", + "warning_code": 0, + "fingerprint": "c837568c590d9608a8bb9927b31b9597aaacc72053b6482e1a54bd02aa0dd2d7", + "check_name": "SQL", + "message": "Possible SQL injection", + "file": "app/models/good_job/job.rb", + "line": 140, + "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", + "code": "Arel.sql(\"(CASE #{queues.map.with_index do\n sanitize_sql_array([\"WHEN queue_name = ? THEN ?\", queue_name, index])\n end.join(\" \")} ELSE #{queues.size} END)\")", + "render_path": null, + "location": { + "type": "method", + "class": "Job", + "method": null + }, + "user_input": "queues.map.with_index do\n sanitize_sql_array([\"WHEN queue_name = ? THEN ?\", queue_name, index])\n end.join(\" \")", + "confidence": "Medium", + "cwe_id": [ + 89 + ], + "note": "Developer provided value, queue_name, is sanitized." + }, + { + "warning_type": "Dynamic Render Path", + "warning_code": 15, + "fingerprint": "dd597dcd0c7443af75784ab306b35936be999bfe8b44e744ad0c6f9012262c6e", + "check_name": "Render", + "message": "Render path contains parameter value", + "file": "app/controllers/good_job/frontends_controller.rb", + "line": 38, + "link": "https://brakemanscanner.org/docs/warning_types/dynamic_render_path/", + "code": "render(file => ({ :css => ({ :bootstrap => GoodJob::Engine.root.join(\"app\", \"frontend\", \"good_job\", \"vendor\", \"bootstrap\", \"bootstrap.min.css\"), :style => GoodJob::Engine.root.join(\"app\", \"frontend\", \"good_job\", \"style.css\") }), :js => ({ :bootstrap => GoodJob::Engine.root.join(\"app\", \"frontend\", \"good_job\", \"vendor\", \"bootstrap\", \"bootstrap.bundle.min.js\"), :chartjs => GoodJob::Engine.root.join(\"app\", \"frontend\", \"good_job\", \"vendor\", \"chartjs\", \"chart.min.js\"), :es_module_shims => GoodJob::Engine.root.join(\"app\", \"frontend\", \"good_job\", \"vendor\", \"es_module_shims.js\"), :rails_ujs => GoodJob::Engine.root.join(\"app\", \"frontend\", \"good_job\", \"vendor\", \"rails_ujs.js\") }) }.dig(params[:format].to_sym, params[:name].to_sym) or raise(ActionController::RoutingError, \"Not Found\")), {})", + "render_path": null, + "location": { + "type": "method", + "class": "GoodJob::FrontendsController", + "method": "static" + }, + "user_input": "params[:name].to_sym", + "confidence": "Weak", + "cwe_id": [ + 22 + ], + "note": "Files are explicitly enumerated in the array" + } + ], + "updated": "2024-07-16 11:28:03 -0700", + "brakeman_version": "6.1.2" +} diff --git a/demo/app/controllers/application_controller.rb b/demo/app/controllers/application_controller.rb index cd1f6b755..e0775e4da 100644 --- a/demo/app/controllers/application_controller.rb +++ b/demo/app/controllers/application_controller.rb @@ -1,4 +1,6 @@ class ApplicationController < ActionController::Base + protect_from_forgery with: :exception + def create_job if params[:wait] wait = params.fetch(:wait, 2).to_i