Add filter_metadata config option (#973)

We had a report of an app that contains sensitive information in the
request path and the desire to filter this out. We have no system in
place to filter metadata like path and request method, as set by the
Sinatra middleware.

This change allow apps to filter out some metadata that's set by
default, like `path`, to avoid sending PII or other sensitive data,
using the `filter_metadata` config option.

Filtering is done with String based keys, like all the other `filter_*`
config options are, so the keys need to be transformed to keys
beforehand to make sure they're filtered out.

I didn't merge how we set the metadata, now it's set using
`Transaction#set_metadata` and through `sample_data` when the
Transaction is being sampled as sample data. I've left the behavior the
same as much as possible to avoid breaking things.

See also this internal discussion:
https://appsignal.slack.com/archives/CNPP953E2/p1687785270464119

.changesets/add-filter_metadata-config-option.md

@@ -0,0 +1,6 @@
+---
+bump: "patch"
+type: "add"
+---
+
+Add `filter_metadata` config option to filter metadata set on Transactions set by default. Metadata like `path`, (request)  `method`, `request_id`, `hostname`, etc. This can be useful if there's PII or other sensitive data in any of the app's metadata.

lib/appsignal/config.rb

@@ -24,6 +24,7 @@ class Config
       :enable_rails_error_reporter => true,
       :endpoint => "https://push.appsignal.com",
       :files_world_accessible => true,
+      :filter_metadata => [],
       :filter_parameters => [],
       :filter_session_data => [],
       :ignore_actions => [],
@@ -77,6 +78,7 @@ class Config
       "APPSIGNAL_ENABLE_GVL_WAITING_THREADS" => :enable_gvl_waiting_threads,
       "APPSIGNAL_ENABLE_RAILS_ERROR_REPORTER" => :enable_rails_error_reporter,
       "APPSIGNAL_FILES_WORLD_ACCESSIBLE" => :files_world_accessible,
+      "APPSIGNAL_FILTER_METADATA" => :filter_metadata,
       "APPSIGNAL_FILTER_PARAMETERS" => :filter_parameters,
       "APPSIGNAL_FILTER_SESSION_DATA" => :filter_session_data,
       "APPSIGNAL_HOSTNAME" => :hostname,
@@ -150,6 +152,7 @@ class Config
     # @api private
     ENV_ARRAY_KEYS = %w[
       APPSIGNAL_DNS_SERVERS
+      APPSIGNAL_FILTER_METADATA
       APPSIGNAL_FILTER_PARAMETERS
       APPSIGNAL_FILTER_SESSION_DATA
       APPSIGNAL_IGNORE_ACTIONS

lib/appsignal/transaction.rb

@@ -308,6 +308,7 @@ def set_http_or_background_queue_start
 
     def set_metadata(key, value)
       return unless key && value
+      return if Appsignal.config[:filter_metadata].include?(key.to_s)
 
       @ext.set_metadata(key, value)
     end
@@ -337,7 +338,7 @@ def sample_data
         :params => sanitized_params,
         :environment => sanitized_environment,
         :session_data => sanitized_session_data,
-        :metadata => metadata,
+        :metadata => sanitized_metadata,
         :tags => sanitized_tags,
         :breadcrumbs => breadcrumbs
       }.each do |key, data|
@@ -522,12 +523,17 @@ def sanitized_session_data
       )
     end
 
-    # Returns metadata from the environment.
+    # Returns sanitized metadata set by {#set_metadata} and from the
+    # {#environment}.
     #
-    # @return [nil] if no `:metadata` key is present in the {#environment}.
     # @return [Hash<String, Object>]
-    def metadata
-      environment[:metadata]
+    def sanitized_metadata
+      metadata = environment[:metadata]
+      return unless metadata
+
+      metadata
+        .transform_keys(&:to_s)
+        .except(*Appsignal.config[:filter_metadata])
     end
 
     # Returns the environment for a transaction.

spec/lib/appsignal/config_spec.rb

@@ -165,6 +165,7 @@
         :enable_rails_error_reporter    => true,
         :endpoint                       => "https://push.appsignal.com",
         :files_world_accessible         => true,
+        :filter_metadata                => [],
         :filter_parameters              => [],
         :filter_session_data            => [],
         :ignore_actions                 => [],

spec/lib/appsignal/rack/streaming_listener_spec.rb

@@ -1,6 +1,7 @@
 require "appsignal/rack/streaming_listener"
 
 describe Appsignal::Rack::StreamingListener do
+  before(:context) { start_agent }
   let(:headers) { {} }
   let(:env) do
     {

spec/lib/appsignal/transaction_spec.rb

@@ -623,6 +623,18 @@ def current_transaction
         expect(transaction.to_h["metadata"]).to eq("request_method" => "GET")
       end
 
+      context "when filter_metadata includes metadata key" do
+        before { Appsignal.config[:filter_metadata] = ["filter_key"] }
+        after { Appsignal.config[:filter_metadata] = [] }
+
+        it "does not set the metadata on the transaction" do
+          transaction.set_metadata(:filter_key, "filtered value")
+          transaction.set_metadata("filter_key", "filtered value")
+
+          expect(transaction.to_h["metadata"].keys).to_not include("filter_key")
+        end
+      end
+
       context "when the key is nil" do
         it "does not update the metadata on the transaction" do
           transaction.set_metadata(nil, "GET")
@@ -1275,8 +1287,8 @@ def session_exists?(_env)
       end
     end
 
-    describe "#metadata" do
-      subject { transaction.send(:metadata) }
+    describe "#sanitized_metadata" do
+      subject { transaction.send(:sanitized_metadata) }
 
       context "when request is nil" do
         let(:request) { nil }
@@ -1291,9 +1303,18 @@ def session_exists?(_env)
       end
 
       context "when env is present" do
-        let(:env) { { :metadata => { :key => "value" } } }
+        let(:env) { { "key" => "value" } }
 
-        it { is_expected.to eq env[:metadata] }
+        it { is_expected.to eq("key" => "value") }
+
+        context "with filter_metadata option set" do
+          before { Appsignal.config[:filter_metadata] = ["key"] }
+          after { Appsignal.config[:filter_metadata] = [] }
+
+          it "filters out keys listed in the filter_metadata option" do
+            expect(subject.keys).to_not include("key")
+          end
+        end
       end
     end