Merge pull request #16 from DocSpring/nathan/filtering-security-fixes

Keep Rails' parameter filtering working, and filter every value LogStruct logs

Author: ndbroadbent

.cspell/project-words.txt

@@ -15,6 +15,7 @@ defn
 demodulize
 docspring
 dotenv
+duplicable
 Dotenv
 ESRCH
 Fanout
@@ -47,6 +48,7 @@ nonblock
 nonblocking
 optparse
 parseable
+passw
 pentester
 pentesters
 popen

README.md

@@ -55,7 +55,7 @@ Once initialized (and enabled), the gem automatically includes its modules into
 - Lograge request logs include request metadata (request_id, source_ip, user_agent, referer, host, content_type, accept) and custom fields from `lograge_custom_options`
 - A Rack middleware is inserted to catch and log errors, including security violations (IP spoofing, CSRF, blocked hosts, etc.)
 - Structured logging is set up for ActiveJob, Sidekiq, Shrine, etc.
-- Rails `config.filter_parameters` are merged into LogStruct's filters and then cleared (to avoid double filtering). Configure sensitive keys via `LogStruct.config.filters`.
+- LogStruct also filters the keys in Rails' `config.filter_parameters`, matched the way Rails matches them (`:passw` filters `user_password`), so the same keys are filtered in every log line. Rails' own list is left in place, so `request.filtered_parameters`, `ActiveRecord::Base.filter_attributes` and other Rails features keep filtering, and values Rails has already replaced are logged as `[FILTERED]`. Configure extra sensitive keys via `LogStruct.config.filters`.
 - When `RAILS_LOG_TO_STDOUT` is set, we log to STDOUT only. Otherwise, we log to STDOUT by default without adding a file appender to avoid duplicate logs.
 
 ### Default behavior by process type

docs/app/docs/filtering-sensitive-data/page.tsx

@@ -89,6 +89,30 @@ export default function FilteringSensitiveDataPage() {
 :credit_card, :ssn, :social_security`}
       </CodeBlock>
 
+      <HeadingWithAnchor id="rails-filter-parameters" level={3}>
+        Rails filter_parameters
+      </HeadingWithAnchor>
+      <p className="text-neutral-600 dark:text-neutral-300 mb-4">
+        LogStruct also filters the keys in your app&apos;s <code>config.filter_parameters</code>,
+        and matches them the way Rails does: a Symbol or String matches any key that contains it,
+        ignoring case (<code>:passw</code> filters <code>user_password</code>), and a Regexp is
+        matched as it is. Rails matches entries with a dot (
+        <code>&quot;credit_card.code&quot;</code>) against the path of a nested request parameter.
+        LogStruct checks one key at a time, so it only filters a key that contains the whole entry.
+        Partial matches don&apos;t apply to the names of LogStruct&apos;s own fields at the top of a
+        line, so Rails&apos; default <code>:_key</code> doesn&apos;t filter <code>cron_key</code>.
+        Exact names and Regexps do. Rails&apos; list is left in place, so{' '}
+        <code>request.filtered_parameters</code> and{' '}
+        <code>ActiveRecord::Base.filter_attributes</code> keep working, and values Rails has already
+        replaced are logged as <code>[FILTERED]</code>.
+      </p>
+      <p className="text-neutral-600 dark:text-neutral-300 mb-4">
+        Filter blocks (<code>{'->(key, value) { ... }'}</code>) get copies of the key and value, as
+        they do in Rails. LogStruct filters a key when its block returns a truthy value or changes
+        the String it was given (Rails&apos; blocks mask a value by changing it in place). A block
+        that takes Rails&apos; third argument, the request&apos;s parameters, gets an empty Hash.
+      </p>
+
       <HeadingWithAnchor id="email-hashing" level={3}>
         Email Hashing for Request Tracing
       </HeadingWithAnchor>

lib/log_struct/concerns/configuration.rb

@@ -1,6 +1,8 @@
 # typed: strict
 # frozen_string_literal: true
 
+require "active_support/parameter_filter"
+require "active_support/core_ext/object/duplicable"
 require_relative "../configuration"
 
 module LogStruct
@@ -13,6 +15,11 @@ module ClassMethods
         CONSOLE_COMMAND_ARGS = T.let(["console", "c"].freeze, T::Array[String])
         EMPTY_ARGV = T.let([].freeze, T::Array[String])
         CI_FALSE_VALUES = T.let(["false", "0", "no"].freeze, T::Array[String])
+        RAILS_PATTERNS_LABEL = "Rails filter_parameters"
+        RAILS_REGEXPS_LABEL = "Rails filter_parameters (Regexps)"
+        RAILS_PATTERN_LABELS = T.let([RAILS_PATTERNS_LABEL, RAILS_REGEXPS_LABEL].freeze, T::Array[String])
+        RAILS_FILTER_MASK = "[FILTERED]"
+        RAILS_FILTER_PROBE = "value"
 
         sig { params(block: T.proc.params(config: LogStruct::Configuration).void).void }
         def configure(&block)
@@ -84,40 +91,26 @@ def merge_rails_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
+          blocks, patterns = rails_filter_params.partition { |entry| callable_filter?(entry) }
+          regexps, partial_names = patterns.partition { |entry| entry.is_a?(Regexp) }
+          matchers = blocks.map { |block| build_callable_filter_matcher(block) }
+          if partial_names.any?
+            matchers << build_rails_pattern_matcher(partial_names, label: RAILS_PATTERNS_LABEL, matches_logstruct_fields: false)
           end
-
-          if symbol_filters.any?
-            config.filters.filter_keys |= symbol_filters
+          if regexps.any?
+            matchers << build_rails_pattern_matcher(regexps, label: RAILS_REGEXPS_LABEL, matches_logstruct_fields: true)
           end
 
-          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
+          # Exact names also filter LogStruct's own fields, which the partial matches skip
+          names = patterns.filter_map { |entry| entry.to_s.downcase.to_sym if entry.is_a?(Symbol) || entry.is_a?(String) }
+          config.filters.filter_keys |= names
 
-          replace_filter_parameters(rails_filter_params, leftovers)
+          registered = config.filters.filter_matchers
+          # The latest list replaces the patterns merged before
+          registered.reject! { |matcher| RAILS_PATTERN_LABELS.include?(matcher.label) }
+          matchers.each do |matcher|
+            registered << matcher unless registered.any? { |existing| existing.label == matcher.label }
+          end
         end
 
         private
@@ -183,55 +176,52 @@ def ci_build?
           !CI_FALSE_VALUES.include?(normalized)
         end
 
-        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)
-
-          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)
+        # Rails matches Symbols and Strings as case-insensitive substrings (:passw filters
+        # "user_password") and Regexps as they are. Entries with a dot match nested request
+        # parameters by their path in Rails. LogStruct checks one key at a time, so they only
+        # match a key that contains the whole path.
+        #
+        # The substring matches skip LogStruct's own fields (Rails' :_key would hide GoodJob's
+        # cron_key); their exact names are added to filter_keys instead. A Regexp says exactly
+        # what it matches, so Regexps apply to every key, LogStruct's fields included.
+        sig { params(patterns: T::Array[T.untyped], label: String, matches_logstruct_fields: T::Boolean).returns(ConfigStruct::FilterMatcher) }
+        def build_rails_pattern_matcher(patterns, label:, matches_logstruct_fields:)
+          parameter_filter = ::ActiveSupport::ParameterFilter.new(patterns, mask: RAILS_FILTER_MASK)
+          callable = Kernel.lambda do |key, _value|
+            parameter_filter.filter_param(key, RAILS_FILTER_PROBE) == RAILS_FILTER_MASK
           end
-
-          nil
+          ConfigStruct::FilterMatcher.new(callable: callable, label: label, matches_logstruct_fields: matches_logstruct_fields)
         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)) }
+        # Rails gives filter blocks copies of the key and value, ignores what they return, and
+        # logs the value as the block left it: the Rails guides' example reverses it in place,
+        # and a block can replace it with "[FILTERED]". LogStruct filters a key when its block
+        # returns a truthy value or changes a String it's given, and never changes the logged
+        # value itself. Rails passes blocks with three arguments the request's parameters as
+        # well; LogStruct checks one key at a time, so they get an empty Hash.
+        sig { params(filter: T.untyped).returns(ConfigStruct::FilterMatcher) }
         def build_callable_filter_matcher(filter)
-          callable = Kernel.lambda do |key, value|
+          callable = Kernel.lambda do |original_key, original_value|
+            key = original_key.dup
+            value = original_value.duplicable? ? original_value.dup : original_value
             call_args = case arity_for_filter(filter)
             when 0
               []
             when 1
               [key]
+            when 3
+              [key, value, {}]
             else
               [key, value]
             end
 
             result = filter.call(*call_args)
-            !!result
+            !!result || (value.is_a?(String) && value != original_value)
           rescue ArgumentError
             begin
               !!filter.call(key)
@@ -251,12 +241,6 @@ 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
-
         sig { params(error: StandardError, filter: T.untyped, key: String).void }
         def handle_filter_error(error, filter, key)
           context = {

lib/log_struct/config_struct/filters.rb

@@ -8,6 +8,9 @@ class FilterMatcher < T::Struct
 
       const :callable, T.proc.params(key: String, value: T.untyped).returns(T::Boolean)
       const :label, String
+      # Whether the matcher also applies to the names of LogStruct's own fields at the top of a line.
+      # LogStruct's matcher for Rails' partial matches doesn't: Rails' default :_key would filter cron_key.
+      const :matches_logstruct_fields, T::Boolean, default: true
 
       sig { params(key: String, value: T.untyped).returns(T::Boolean) }
       def matches?(key, value)
@@ -101,9 +104,9 @@ class Filters < T::Struct
         Regexp
       )
 
-      # 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.
-      # By default, includes a regex matcher for common sensitive key patterns.
+      # Matchers for keys that aren't listed by name in filter_keys. Each matcher receives the key (String)
+      # and optional value, returning true when the pair should be filtered. Includes a regex matcher for
+      # common sensitive key patterns, and LogStruct adds matchers for Rails' filter_parameters at boot.
       prop :filter_matchers,
         T::Array[FilterMatcher],
         factory: -> {