From 1b8c114a4210fc1c0fa7229c6b4ad779c749a137 Mon Sep 17 00:00:00 2001 From: Finn Bacall Date: Mon, 7 Sep 2026 15:28:17 +0100 Subject: [PATCH 1/4] Update filter parameter list. Add test. --- .../initializers/filter_parameter_logging.rb | 2 +- lib/seek/config.rb | 2 +- lib/seek/errors/controller_error_handling.rb | 5 +-- lib/seek/errors/exception_forwarder.rb | 4 +- .../exception_notification_test.rb | 42 +++++++++++++++++++ 5 files changed, 48 insertions(+), 7 deletions(-) create mode 100644 test/integration/exception_notification_test.rb diff --git a/config/initializers/filter_parameter_logging.rb b/config/initializers/filter_parameter_logging.rb index c010b83ddd..d67c20371b 100644 --- a/config/initializers/filter_parameter_logging.rb +++ b/config/initializers/filter_parameter_logging.rb @@ -4,5 +4,5 @@ # Use this to limit dissemination of sensitive information. # See the ActiveSupport::ParameterFilter documentation for supported notations and behaviors. Rails.application.config.filter_parameters += [ - :passw, :email, :secret, :token, :_key, :crypt, :salt, :certificate, :otp, :ssn + :passw, :email, :secret, :token, :_key, :crypt, :salt, :certificate, :otp, :ssn, :auth ] diff --git a/lib/seek/config.rb b/lib/seek/config.rb index 7159b2ba61..38ce9f60c7 100644 --- a/lib/seek/config.rb +++ b/lib/seek/config.rb @@ -110,7 +110,7 @@ def configure_recaptcha_keys end def configure_exception_notification - if exception_notification_enabled && Rails.env.production? + if exception_notification_enabled ExceptionNotification.configure do |config| config.ignored_exceptions = ['ActionDispatch::Http::Parameters::ParseError', 'ActionController::InvalidAuthenticityToken', diff --git a/lib/seek/errors/controller_error_handling.rb b/lib/seek/errors/controller_error_handling.rb index e5af939926..4c63801977 100644 --- a/lib/seek/errors/controller_error_handling.rb +++ b/lib/seek/errors/controller_error_handling.rb @@ -13,12 +13,11 @@ module ControllerErrorHandling }.freeze def self.included(base) - unless Rails.application.config.consider_all_requests_local - base.rescue_from Exception, with: :render_application_error - end + base.rescue_from Exception, with: :render_application_error end def render_application_error(exception) + raise exception if Rails.application.config.consider_all_requests_local logger.error "ERROR - #{exception.class.name} (#{exception.message})" status = error_response_code(exception) exception_notification(status, exception) diff --git a/lib/seek/errors/exception_forwarder.rb b/lib/seek/errors/exception_forwarder.rb index e9dd221de8..39ed69ca27 100644 --- a/lib/seek/errors/exception_forwarder.rb +++ b/lib/seek/errors/exception_forwarder.rb @@ -7,13 +7,13 @@ class ExceptionForwarder # the option :data will get merged with some default info that reports the configured site host, and current user # information def self.send_notification(exception, options = {}, user = User.current_user) - Rails.logger.error "Sending execption ERROR - #{exception.class.name} (#{exception.message})" + Rails.logger.error "Sending exception ERROR - #{exception.class.name} (#{exception.message})" return unless Seek::Config.exception_notification_enabled env = options[:env] data = default_data(user).merge(options[:data] || {}) begin ExceptionNotifier.notify_exception(exception, env: env, data: data) - rescue StandardError => deliver_exception + rescue StandardError => deliver_exception Rails.logger.error 'Error delivering exception email - ' \ "#{deliver_exception.class.name} (#{deliver_exception.message})" end diff --git a/test/integration/exception_notification_test.rb b/test/integration/exception_notification_test.rb new file mode 100644 index 0000000000..6fdb240173 --- /dev/null +++ b/test/integration/exception_notification_test.rb @@ -0,0 +1,42 @@ +require 'test_helper' +require 'minitest/mock' + +class ExceptionNotificationTest < ActionDispatch::IntegrationTest + test 'filters sensitive parameters out of exception notifications' do + assert_equal 0, ActionMailer::Base.deliveries.length + + User.stub(:admin_logged_in?, true) do + with_config_value(:email_enabled, true) do + with_config_value(:exception_notification_enabled, true) do + with_config_value(:exception_notification_recipients, 'no-reply@sysmo-db.org') do + with_config_value(:auth_lookup_enabled, true) do + headers = { + 'Accept' => 'application/vnd.api+json', + 'Authorization' => 'Token unique_string_1' + } + + Rails.application.config.stub(:consider_all_requests_local, false) do + get fail_path, params: { http_code: '500', + password: 'unique_string_2', + email: 'unique_string_3', + unfiltered_param: 'unique_string_4' }, as: :json, headers: headers + end + + assert_equal 1, ActionMailer::Base.deliveries.length # Exception notification + + email = ActionMailer::Base.deliveries.last + body = email.body.to_s + + assert_includes body, 'A NoMethodError occurred in fail' + assert_not_includes body, 'unique_string_1' + assert_not_includes body, 'unique_string_2' + assert_not_includes body, 'unique_string_3' + assert_includes body, 'unique_string_4' + assert_match /HTTP_AUTHORIZATION\s+: \[FILTERED\]/, body + end + end + end + end + end + end +end \ No newline at end of file From 567c967caa6735aa4264de5322512bca22666392 Mon Sep 17 00:00:00 2001 From: Finn Bacall Date: Mon, 7 Sep 2026 18:14:12 +0100 Subject: [PATCH 2/4] Stricter parameter name match --- config/initializers/filter_parameter_logging.rb | 2 +- lib/seek/errors/exception_forwarder.rb | 4 ++-- test/integration/exception_notification_test.rb | 5 ++++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/config/initializers/filter_parameter_logging.rb b/config/initializers/filter_parameter_logging.rb index d67c20371b..89d5ec6d24 100644 --- a/config/initializers/filter_parameter_logging.rb +++ b/config/initializers/filter_parameter_logging.rb @@ -4,5 +4,5 @@ # Use this to limit dissemination of sensitive information. # See the ActiveSupport::ParameterFilter documentation for supported notations and behaviors. Rails.application.config.filter_parameters += [ - :passw, :email, :secret, :token, :_key, :crypt, :salt, :certificate, :otp, :ssn, :auth + :passw, :email, :secret, :token, :_key, :crypt, :salt, :certificate, :otp, :ssn, :authoriz ] diff --git a/lib/seek/errors/exception_forwarder.rb b/lib/seek/errors/exception_forwarder.rb index 39ed69ca27..df60aef562 100644 --- a/lib/seek/errors/exception_forwarder.rb +++ b/lib/seek/errors/exception_forwarder.rb @@ -14,8 +14,8 @@ def self.send_notification(exception, options = {}, user = User.current_user) begin ExceptionNotifier.notify_exception(exception, env: env, data: data) rescue StandardError => deliver_exception - Rails.logger.error 'Error delivering exception email - ' \ - "#{deliver_exception.class.name} (#{deliver_exception.message})" + Rails.logger.error 'Error delivering exception email - ' \ + "#{deliver_exception.class.name} (#{deliver_exception.message})" end end diff --git a/test/integration/exception_notification_test.rb b/test/integration/exception_notification_test.rb index 6fdb240173..2fce3a8854 100644 --- a/test/integration/exception_notification_test.rb +++ b/test/integration/exception_notification_test.rb @@ -19,7 +19,9 @@ class ExceptionNotificationTest < ActionDispatch::IntegrationTest get fail_path, params: { http_code: '500', password: 'unique_string_2', email: 'unique_string_3', - unfiltered_param: 'unique_string_4' }, as: :json, headers: headers + unfiltered_param: 'unique_string_4', + author: 'unique_string_5' + }, as: :json, headers: headers end assert_equal 1, ActionMailer::Base.deliveries.length # Exception notification @@ -32,6 +34,7 @@ class ExceptionNotificationTest < ActionDispatch::IntegrationTest assert_not_includes body, 'unique_string_2' assert_not_includes body, 'unique_string_3' assert_includes body, 'unique_string_4' + assert_includes body, 'unique_string_5' assert_match /HTTP_AUTHORIZATION\s+: \[FILTERED\]/, body end end From 3d9427a3f544ea183470e10fbe36975408b728eb Mon Sep 17 00:00:00 2001 From: Finn Bacall Date: Mon, 7 Sep 2026 18:29:00 +0100 Subject: [PATCH 3/4] Clear deliveries --- test/integration/exception_notification_test.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/integration/exception_notification_test.rb b/test/integration/exception_notification_test.rb index 2fce3a8854..ca26bc2c26 100644 --- a/test/integration/exception_notification_test.rb +++ b/test/integration/exception_notification_test.rb @@ -3,7 +3,8 @@ class ExceptionNotificationTest < ActionDispatch::IntegrationTest test 'filters sensitive parameters out of exception notifications' do - assert_equal 0, ActionMailer::Base.deliveries.length + ActionMailer::Base.deliveries.clear + assert_empty ActionMailer::Base.deliveries User.stub(:admin_logged_in?, true) do with_config_value(:email_enabled, true) do From 4a3cb25be7145c85a11a669a2cf1b84cfbdf0345 Mon Sep 17 00:00:00 2001 From: Finn Bacall Date: Tue, 8 Sep 2026 14:01:38 +0100 Subject: [PATCH 4/4] Tidy up test --- .../exception_notification_test.rb | 62 ++++++++----------- 1 file changed, 27 insertions(+), 35 deletions(-) diff --git a/test/integration/exception_notification_test.rb b/test/integration/exception_notification_test.rb index ca26bc2c26..0734fa6043 100644 --- a/test/integration/exception_notification_test.rb +++ b/test/integration/exception_notification_test.rb @@ -3,44 +3,36 @@ class ExceptionNotificationTest < ActionDispatch::IntegrationTest test 'filters sensitive parameters out of exception notifications' do - ActionMailer::Base.deliveries.clear - assert_empty ActionMailer::Base.deliveries - - User.stub(:admin_logged_in?, true) do - with_config_value(:email_enabled, true) do - with_config_value(:exception_notification_enabled, true) do - with_config_value(:exception_notification_recipients, 'no-reply@sysmo-db.org') do - with_config_value(:auth_lookup_enabled, true) do - headers = { - 'Accept' => 'application/vnd.api+json', - 'Authorization' => 'Token unique_string_1' - } - - Rails.application.config.stub(:consider_all_requests_local, false) do - get fail_path, params: { http_code: '500', - password: 'unique_string_2', - email: 'unique_string_3', - unfiltered_param: 'unique_string_4', - author: 'unique_string_5' - }, as: :json, headers: headers - end - - assert_equal 1, ActionMailer::Base.deliveries.length # Exception notification - - email = ActionMailer::Base.deliveries.last - body = email.body.to_s - - assert_includes body, 'A NoMethodError occurred in fail' - assert_not_includes body, 'unique_string_1' - assert_not_includes body, 'unique_string_2' - assert_not_includes body, 'unique_string_3' - assert_includes body, 'unique_string_4' - assert_includes body, 'unique_string_5' - assert_match /HTTP_AUTHORIZATION\s+: \[FILTERED\]/, body - end + with_config_values(email_enabled: true, + exception_notification_enabled: true, + exception_notification_recipients: 'no-reply@sysmo-db.org') do + emails = capture_emails do + User.stub(:admin_logged_in?, true) do + Rails.application.config.stub(:consider_all_requests_local, false) do + get fail_path, params: { http_code: '500', + password: 'unique_string_2', + email: 'unique_string_3', + unfiltered_param: 'unique_string_4', + author: 'unique_string_5' + }, as: :json, headers: { + 'Accept' => 'application/vnd.api+json', + 'Authorization' => 'Token unique_string_1' + } end end end + + assert_equal 1, emails.length + email = emails.last + body = email.body.to_s + + assert_includes body, 'A NoMethodError occurred in fail' + assert_not_includes body, 'unique_string_1' + assert_not_includes body, 'unique_string_2' + assert_not_includes body, 'unique_string_3' + assert_includes body, 'unique_string_4' + assert_includes body, 'unique_string_5' + assert_match /HTTP_AUTHORIZATION\s+: \[FILTERED\]/, body end end end \ No newline at end of file