Preserve Rails filter matchers and harden ParamFilters

ndbroadbent · ndbroadbent · commit 0651e9e4ec3d · 2025-09-22T01:31:50.000+12:00
diff --git a/lib/log_struct/concerns/configuration.rb b/lib/log_struct/concerns/configuration.rb
@@ -72,20 +72,133 @@ def merge_rails_filter_parameters!
 
           rails_filter_params = ::Rails.application.config.filter_parameters
           return unless rails_filter_params.is_a?(Array)
+          return if rails_filter_params.empty?
+
+          symbol_filters = T.let([], T::Array[Symbol])
+          matchers = T.let([], T::Array[ConfigStruct::FilterMatcher])
+          leftovers = T.let([], T::Array[T.untyped])
+
+          rails_filter_params.each do |entry|
+            matcher = build_filter_matcher(entry)
+
+            if matcher
+              matchers << matcher
+              next
+            end
+
+            normalized_symbol = normalize_filter_symbol(entry)
+            if normalized_symbol
+              symbol_filters << normalized_symbol
+            else
+              leftovers << entry
+            end
+          end
+
+          if symbol_filters.any?
+            config.filters.filter_keys |= symbol_filters
+          end
 
-          # Convert all Rails filter parameters to symbols and merge with our filter keys
-          converted_params = rails_filter_params.map do |param|
-            param.respond_to?(:to_sym) ? param.to_sym : param
+          if matchers.any?
+            matchers.each do |matcher|
+              existing = config.filters.filter_matchers.any? do |registered|
+                registered.label == matcher.label
+              end
+              config.filters.filter_matchers << matcher unless existing
+            end
           end
 
-          # Add Rails filter parameters to our filter keys
-          config.filters.filter_keys += converted_params
+          replace_filter_parameters(rails_filter_params, leftovers)
+        end
+
+        private
+
+        sig { params(filter: T.untyped).returns(T.nilable(Symbol)) }
+        def normalize_filter_symbol(filter)
+          return filter if filter.is_a?(Symbol)
+          return filter.downcase.to_sym if filter.is_a?(String)
 
-          # Ensure no duplicates
-          config.filters.filter_keys.uniq!
+          return nil unless filter.respond_to?(:to_sym)
+
+          begin
+            sym = filter.to_sym
+            sym.is_a?(Symbol) ? sym : nil
+          rescue
+            nil
+          end
+        end
+
+        sig { params(filter: T.untyped).returns(T.nilable(ConfigStruct::FilterMatcher)) }
+        def build_filter_matcher(filter)
+          case filter
+          when ::Regexp
+            callable = Kernel.lambda do |key, _value|
+              filter.match?(key)
+            end
+            return ConfigStruct::FilterMatcher.new(callable: callable, label: filter.inspect)
+          else
+            return build_callable_filter_matcher(filter) if callable_filter?(filter)
+          end
+
+          nil
+        end
+
+        sig { params(filter: T.untyped).returns(T::Boolean) }
+        def callable_filter?(filter)
+          filter.respond_to?(:call)
+        end
+
+        sig { params(filter: T.untyped).returns(T.nilable(ConfigStruct::FilterMatcher)) }
+        def build_callable_filter_matcher(filter)
+          callable = Kernel.lambda do |key, value|
+            call_args = case arity_for_filter(filter)
+            when 0
+              []
+            when 1
+              [key]
+            else
+              [key, value]
+            end
+
+            result = filter.call(*call_args)
+            !!result
+          rescue ArgumentError
+            begin
+              !!filter.call(key)
+            rescue => e
+              handle_filter_error(e, filter, key)
+              false
+            end
+          rescue => e
+            handle_filter_error(e, filter, key)
+            false
+          end
+          ConfigStruct::FilterMatcher.new(callable: callable, label: filter.inspect)
+        end
+
+        sig { params(filter: T.untyped).returns(Integer) }
+        def arity_for_filter(filter)
+          filter.respond_to?(:arity) ? filter.arity : 2
+        end
+
+        sig { params(filter_params: T::Array[T.untyped], leftovers: T::Array[T.untyped]).void }
+        def replace_filter_parameters(filter_params, leftovers)
+          filter_params.clear
+          filter_params.concat(leftovers)
+        end
 
-          # Clear Rails filter parameters since we've incorporated them
-          ::Rails.application.config.filter_parameters.clear
+        sig { params(error: StandardError, filter: T.untyped, key: String).void }
+        def handle_filter_error(error, filter, key)
+          context = {
+            filter: filter.class.name,
+            key: key,
+            filter_label: begin
+              filter.inspect
+            rescue
+              "unknown"
+            end
+          }
+
+          LogStruct.handle_exception(error, source: Source::Internal, context: context)
         end
       end
     end
diff --git a/lib/log_struct/config_struct/filters.rb b/lib/log_struct/config_struct/filters.rb
@@ -3,6 +3,18 @@
 
 module LogStruct
   module ConfigStruct
+    class FilterMatcher < T::Struct
+      extend T::Sig
+
+      const :callable, T.proc.params(key: String, value: T.untyped).returns(T::Boolean)
+      const :label, String
+
+      sig { params(key: String, value: T.untyped).returns(T::Boolean) }
+      def matches?(key, value)
+        callable.call(key, value)
+      end
+    end
+
     class Filters < T::Struct
       include Sorbet::SerializeSymbolKeys
 
@@ -75,6 +87,12 @@ class Filters < T::Struct
       # Filter MAC addresses
       # Default: false
       prop :mac_addresses, T::Boolean, default: false
+
+      # Additional filter matchers built from Rails filter_parameters entries that aren't simple symbols.
+      # Each matcher receives the key (String) and optional value, returning true when the pair should be filtered.
+      prop :filter_matchers,
+        T::Array[FilterMatcher],
+        factory: -> { [] }
     end
   end
 end
diff --git a/lib/log_struct/formatter.rb b/lib/log_struct/formatter.rb
@@ -62,7 +62,7 @@ def process_values(arg, recursion_depth: 0)
         # Process each key-value pair
         arg.each do |key, value|
           # Check if this key should be filtered at any depth
-          result[key] = if ParamFilters.should_filter_key?(key)
+          result[key] = if ParamFilters.should_filter_key?(key, value)
             # Filter the value
             {_filtered: ParamFilters.summarize_json_attribute(key, value)}
           else
diff --git a/lib/log_struct/param_filters.rb b/lib/log_struct/param_filters.rb
@@ -3,6 +3,8 @@
 
 require "digest"
 require_relative "hash_utils"
+require_relative "config_struct/filters"
+require_relative "enums/source"
 
 module LogStruct
   # This class contains methods for filtering sensitive data in logs
@@ -12,19 +14,30 @@ class << self
       extend T::Sig
 
       # Check if a key should be filtered based on our defined sensitive keys
-      sig { params(key: T.any(String, Symbol)).returns(T::Boolean) }
-      def should_filter_key?(key)
-        LogStruct.config.filters.filter_keys.include?(key.to_s.downcase.to_sym)
+      sig { params(key: T.untyped, value: T.untyped).returns(T::Boolean) }
+      def should_filter_key?(key, value = nil)
+        filters = LogStruct.config.filters
+        normalized_key = key.to_s
+        normalized_symbol = normalized_key.downcase.to_sym
+
+        return true if filters.filter_keys.include?(normalized_symbol)
+
+        filters.filter_matchers.any? do |matcher|
+          matcher.matches?(normalized_key, value)
+        rescue => e
+          handle_filter_matcher_error(e, matcher, normalized_key)
+          false
+        end
       end
 
       # Check if a key should be hashed rather than completely filtered
-      sig { params(key: T.any(String, Symbol)).returns(T::Boolean) }
+      sig { params(key: T.untyped).returns(T::Boolean) }
       def should_include_string_hash?(key)
         LogStruct.config.filters.filter_keys_with_hashes.include?(key.to_s.downcase.to_sym)
       end
 
       # Convert a value to a filtered summary hash (e.g. { _filtered: { class: "String", ... }})
-      sig { params(key: T.any(String, Symbol), data: T.untyped).returns(T::Hash[Symbol, T.untyped]) }
+      sig { params(key: T.untyped, data: T.untyped).returns(T::Hash[Symbol, T.untyped]) }
       def summarize_json_attribute(key, data)
         case data
         when Hash
@@ -59,12 +72,18 @@ def summarize_hash(hash)
         return {_class: "Hash", _empty: true} if hash.empty?
 
         # Don't include byte size if hash contains any filtered keys
-        has_sensitive_keys = hash.keys.any? { |key| should_filter_key?(key) }
+        has_sensitive_keys = T.let(false, T::Boolean)
+        normalized_keys = []
+
+        hash.each do |key, value|
+          has_sensitive_keys ||= should_filter_key?(key, value)
+          normalized_keys << normalize_summary_key(key)
+        end
 
         summary = {
           _class: Hash,
           _keys_count: hash.keys.size,
-          _keys: hash.keys.map(&:to_sym).take(10)
+          _keys: normalized_keys.take(10)
         }
 
         # Only add byte size if no sensitive keys are present
@@ -84,6 +103,30 @@ def summarize_array(array)
           _bytes: array.to_json.bytesize
         }
       end
+
+      private
+
+      sig { params(key: T.any(String, Symbol, Integer, T.untyped)).returns(T.any(Symbol, String)) }
+      def normalize_summary_key(key)
+        if key.is_a?(Symbol)
+          key
+        elsif key.respond_to?(:to_sym)
+          key.to_sym
+        else
+          key.to_s
+        end
+      rescue
+        key.to_s
+      end
+
+      sig { params(error: StandardError, matcher: ConfigStruct::FilterMatcher, key: String).void }
+      def handle_filter_matcher_error(error, matcher, key)
+        context = {
+          matcher: matcher.label,
+          key: key
+        }
+        LogStruct.handle_exception(error, source: Source::Internal, context: context)
+      end
     end
   end
 end
diff --git a/test/code_examples/configuration_test.rb b/test/code_examples/configuration_test.rb
@@ -136,6 +136,14 @@ def test_filter_configuration
           config.filters.ssns = true                 # Filter social security numbers
           config.filters.url_passwords = true        # Filter passwords in URLs
 
+          # Configure additional matchers for custom filtering rules
+          config.filters.filter_matchers = [
+            LogStruct::ConfigStruct::FilterMatcher.new(
+              callable: ->(key, _value) { key.start_with?("secret_") },
+              label: "secret prefix matcher"
+            )
+          ]
+
           # Configure the salt used for hashing filtered email addresses
           config.filters.hash_salt = ENV.fetch("EMAIL_HASH_SALT", "test_salt")
 
@@ -156,6 +164,7 @@ def test_filter_configuration
         assert LogStruct.configuration.filters.url_passwords
         refute LogStruct.configuration.filters.ip_addresses
         refute LogStruct.configuration.filters.mac_addresses
+        assert_equal 1, LogStruct.configuration.filters.filter_matchers.size
         assert_equal LogStruct.configuration.filters.hash_salt, LogStruct.configuration.filters.hash_salt
         assert_equal 12, LogStruct.configuration.filters.hash_length
       end
diff --git a/test/log_struct/configuration_test.rb b/test/log_struct/configuration_test.rb
@@ -159,5 +159,41 @@ def test_merge_rails_filter_parameters
       # Restore original filter keys
       LogStruct.config.filters.filter_keys = original_filter_keys
     end
+
+    def test_merge_rails_filter_parameters_preserves_regex_filters
+      original_filter_keys = LogStruct.config.filters.filter_keys.dup
+      original_matchers = LogStruct.config.filters.filter_matchers.dup
+
+      regex_filter = /\Aapi_/i
+      filter_params = [regex_filter]
+
+      filter_params.define_singleton_method(:clear) do
+        replace([])
+      end
+
+      mock_config = T.unsafe(Object.new)
+      mock_config.define_singleton_method(:filter_parameters) { filter_params }
+      mock_config.define_singleton_method(:respond_to?) do |method_name, include_private = false|
+        if method_name.to_sym == :filter_parameters
+          true
+        else
+          super(method_name, include_private)
+        end
+      end
+
+      mock_app = T.unsafe(Object.new)
+      mock_app.define_singleton_method(:config) { mock_config }
+
+      LogStruct.config.filters.filter_keys = []
+
+      Rails.stub(:application, mock_app) do
+        LogStruct.merge_rails_filter_parameters!
+      end
+
+      assert ParamFilters.should_filter_key?("api_token"), "regex-based Rails filters should continue filtering keys"
+    ensure
+      LogStruct.config.filters.filter_keys = original_filter_keys
+      LogStruct.config.filters.filter_matchers = original_matchers
+    end
   end
 end
diff --git a/test/log_struct/param_filters_test.rb b/test/log_struct/param_filters_test.rb
@@ -56,6 +56,14 @@ class ParamFiltersTest < ActiveSupport::TestCase
       refute summary2.key?(:_bytes)
     end
 
+    test "summarize_hash handles non symbol keys" do
+      summary = ParamFilters.summarize_hash({1 => "value"})
+
+      assert_equal Hash, summary[:_class]
+      assert_equal 1, summary[:_keys_count]
+      assert_equal ["1"], summary[:_keys]
+    end
+
     test "summarize_array reports count/bytes and handles empty" do
       s = ParamFilters.summarize_array([1, 2, 3])
 
