blacklane · anruban · Jun 23, 2021 · Jun 18, 2021
diff --git a/lib/kiev/base.rb b/lib/kiev/base.rb
@@ -26,8 +26,20 @@ def logger
       Config.instance.logger
     end
 
+    def filtered_params
+      Config.instance.filtered_params
+    end
+
+    def ignored_params
+      Config.instance.ignored_params
+    end
+
     def event(event_name, data = EMPTY_OBJ)
-      logger.log(::Logger::Severity::INFO, data, event_name)
+      logger.log(
+        ::Logger::Severity::INFO,
+        ParamFilter.filter(data, filtered_params, ignored_params),
+        event_name
+      )
     end
 
     def []=(name, value)

diff --git a/lib/kiev/param_filter.rb b/lib/kiev/param_filter.rb
@@ -1,12 +1,21 @@
 # frozen_string_literal: true
 
 module Kiev
-  module ParamFilter
+  class ParamFilter
     FILTERED = "[FILTERED]"
 
     def self.filter(params, filtered_params, ignored_params)
+      new(filtered_params, ignored_params).call(params)
+    end
+
+    def initialize(filtered_params, ignored_params)
+      @filtered_params = normalize(filtered_params)
+      @ignored_params = normalize(ignored_params)
+    end
+
+    def call(params)
       params.each_with_object({}) do |(
8000
key, value), acc|
-        next if ignored_params.include?(key)
+        next if ignored_params.include?(key.to_s)
 
         if defined?(ActionDispatch) && value.is_a?(ActionDispatch::Http::UploadedFile)
           value = {
@@ -17,14 +26,22 @@ def self.filter(params, filtered_params, ignored_params)
         end
 
         acc[key] =
-          if filtered_params.include?(key) && !value.is_a?(Hash)
+          if filtered_params.include?(key.to_s) && !value.is_a?(Hash)
             FILTERED
           elsif value.is_a?(Hash)
-            filter(value, filtered_params, ignored_params)
+            call(value)
           else
             value
           end
       end
     end
+
+    private
+
+    attr_reader :filtered_params, :ignored_params
+
+    def normalize(params)
+      Set.new(params.map(&:to_s))
+    end
   end
 end
diff --git a/lib/kiev/rack/request_logger.rb b/lib/kiev/rack/request_logger.rb
@@ -85,8 +85,6 @@ def form_data(request:, began_at:, status:, env:, body:, response:, exception:)
             request.params
           end
 
-        params = ParamFilter.filter(params, config.filtered_params, config.ignored_params)
-
         data = {
           host: request.host, # env["HTTP_HOST"] || env["HTTPS_HOST"],
           params: params.empty? ? nil : params, # env[Rack::QUERY_STRING],

diff --git a/lib/kiev/version.rb b/lib/kiev/version.rb
@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module Kiev
-  VERSION = "4.5.0"
+  VERSION = "4.6.0"
 end
diff --git a/spec/lib/kiev/param_filter_spec.rb b/spec/lib/kiev/param_filter_spec.rb
@@ -22,8 +22,13 @@
       expect(described_class.filter(input, filtered, ignored)).to eq(expected)
     end
 
-    it "does not filter symbol param" do
-      expect(described_class.filter({ "password": "password" }, filtered, ignored)).to eq("password": "password")
+    it "filters symbol param" do
+      expect(described_class.filter({ "password": "password" }, filtered, ignored)).to eq("password": "[FILTERED]")
+    end
+
+    it "filters mixed param" do
+      expect(described_class.filter({ "password": "password", "password" => "password" }, filtered, ignored))
+        .to eq("password": "[FILTERED]", "password" => "[FILTERED]")
     end
 
     it "ignores param" do
@@ -34,8 +39,31 @@
       expect(described_class.filter({ "form" => { "action" => "submit" } }, filtered, ignored)).to eq("form" => {})
     end
 
-    it "does not ignore symbol param" do
-      expect(described_class.filter({ "utf8": "utf8" }, filtered, ignored)).to eq("utf8": "utf8")
+    it "ignores symbol param" do
+      expect(described_class.filter({ "utf8": "utf8" }, filtered, ignored)).to eq({})
+    end
+
+    it "ignores mixed params" do
+      expect(described_class.filter({ "utf8": "utf8", "utf8" => "utf8" }, filtered, ignored)).to eq({})
+    end
+
+    context "when configuration params specified as strings and symbols at the same time" do
+      context "when filtered" do
+        let(:filtered) { [:password, "type"] }
+
+        it "filters both" do
+          expect(described_class.filter({ type: "type", "password" => "password"}, filtered, ignored))
+            .to eq(type: "[FILTERED]", "password" => "[FILTERED]")
+        end
+      end
+
+      context "when ignored" do
+        let(:ignored) { [:password, "type"] }
+
+        it "ignores both" do
+          expect(described_class.filter({ type: "type", "password" => "password"}, filtered, ignored)).to eq({})
+        end
+      end
     end
   end
 end
diff --git a/spec/lib/kiev/rack/request_logger_spec.rb b/spec/lib/kiev/rack/request_logger_spec.rb
@@ -8,6 +8,7 @@
     include Rack::Test::Methods
     before do
       allow(Kiev).to receive(:event)
+      allow(logger).to receive(:log)
       allow(Time).to receive(:now).and_return(Time.new(2000))
     end
 
@@ -50,13 +51,22 @@ def request_finished(options = {})
         end
 
         it "filters params" do
+          allow(Kiev).to receive(:event).and_call_original
           get("/", password: "secret")
-          expect(subject).to have_received(:event).with(*request_finished(params: { "password" => "[FILTERED]" }))
+
+          expect(logger).to have_received(:log)
+            .with(
+              1,
+              request_finished(params: { "password" => "[FILTERED]" }).last,
+              :request_finished
+            )
         end
 
         it "ignores params" do
+          allow(Kiev).to receive(:event).and_call_original
           get("/", utf8: "1")
-          expect(subject).to have_received(:event).with(*request_finished)
+
+          expect(logger).to have_received(:log).with(1, request_finished(params: {}).last, :request_finished)
         end
 
         it "ignores request body" do

diff --git a/spec/lib/kiev/shoryuken_spec.rb b/spec/lib/kiev/shoryuken_spec.rb
@@ -358,6 +358,18 @@ def perform(_sqs_msg, _body)
             end
           end
         end
+
+        context "when sensitive data" do
+          let(:message_body) { { "password" => "secret" } }
+
+          subject { Kiev::Test::Log.entries.first }
+
+          before { processor.process }
+
+          it "filters logging data" do
+            is_expected.to include("body" => "{\"password\":\"[FILTERED]\"}")
+          end
+        end
       end
     end
   end

diff --git a/spec/lib/kiev_spec.rb b/spec/lib/kiev_spec.rb
@@ -45,5 +45,17 @@
       Kiev.event(:test_one, data: "hello")
       expect(log_first["data"]).to eq("hello")
     end
+
+    context "when sensitive data" do
+      let(:data) { { data: "hello" } }
+
+      before { allow(Kiev::ParamFilter).to receive(:filter) }
+
+      it "filters logging data" do
+        Kiev.event(:test_one, data)
+        expect(Kiev::ParamFilter).to have_received(:filter)
+          .with(data, Kiev::Config.instance.filtered_params, Kiev::Config.instance.ignored_params)
+      end
+    end
   end
 end