Add support for Phoenix 1.8's parameter filtering

.changesets/fix-compatability-with-phoenix-1-18-parameter-filtering.md

@@ -0,0 +1,6 @@
+---
+bump: patch
+type: fix
+---
+
+Fix compatibility with Phoenix 1.18's parameter filtering compiled patterns. This will errors on boot about the AppSignal handlers not attaching.

lib/appsignal/utils/map_filter.ex

@@ -2,14 +2,58 @@ defmodule Appsignal.Utils.MapFilter do
   @moduledoc false
   require Logger
 
-  # Phoenix parameter filtering copied from to ensure we support the same filtering options:
-  # https://github.com/phoenixframework/phoenix/blob/dfb0c00d2077e10f8df6cc6e334e04924c4c2bcd/lib/phoenix/logger.ex#L157-L199
+  # Phoenix parameter filtering adapted from the Phoenix Framework.
+  # Copyright (c) 2014 Chris McCord - Licensed under MIT
+  #
+  # Phoenix < 1.18: https://github.com/phoenixframework/phoenix/blob/dfb0c00d2077e10f8df6cc6e334e04924c4c2bcd/lib/phoenix/logger.ex#L157-L199
+  # Phoenix >= 1.18: https://github.com/phoenixframework/phoenix/blob/8a6baa5e2ddc9cf7a2fc797ac907c40389139122/lib/phoenix/logger.ex#L180-L228
 
-  def filter(values, params \\ Application.get_env(:phoenix, :filter_parameters, []))
-  def filter(values, {:discard, params}), do: discard_values(values, params)
-  def filter(values, {:keep, params}), do: keep_values(values, params)
-  def filter(values, params), do: discard_values(values, params)
+  def filter(values, filter \\ Application.get_env(:phoenix, :filter_parameters, [])) do
+    case filter do
+      # Phoenix >= 1.18
+      {:compiled, key_match, value_match} ->
+        discard_values(values, key_match, value_match)
 
+      # Phoenix < 1.18
+      {:discard, params} ->
+        discard_values(values, params)
+
+      {:keep, match} ->
+        keep_values(values, match)
+
+      # Phoenix < 1.18
+      params ->
+        discard_values(values, params)
+    end
+  end
+
+  # Phoenix >= 1.18
+  defp discard_values(%{__struct__: mod} = struct, _key_match, _value_match) when is_atom(mod) do
+    struct
+  end
+
+  defp discard_values(%{} = map, key_match, value_match) do
+    Enum.into(map, %{}, fn {k, v} ->
+      cond do
+        is_binary(k) and String.contains?(k, key_match) ->
+          {k, "[FILTERED]"}
+
+        is_binary(v) and String.contains?(v, value_match) ->
+          {k, "[FILTERED]"}
+
+        true ->
+          {k, discard_values(v, key_match, value_match)}
+      end
+    end)
+  end
+
+  defp discard_values([_ | _] = list, key_match, value_match) do
+    Enum.map(list, &discard_values(&1, key_match, value_match))
+  end
+
+  defp discard_values(other, _key_match, _value_match), do: other
+
+  # Phoenix < 1.18
   defp discard_values(%{__struct__: mod} = struct, _params) when is_atom(mod) do
     struct
   end
@@ -30,21 +74,21 @@ defmodule Appsignal.Utils.MapFilter do
 
   defp discard_values(other, _params), do: other
 
-  defp keep_values(%{__struct__: mod}, _params) when is_atom(mod), do: "[FILTERED]"
+  defp keep_values(%{__struct__: mod}, _match) when is_atom(mod), do: "[FILTERED]"
 
-  defp keep_values(%{} = map, params) do
+  defp keep_values(%{} = map, match) do
     Enum.into(map, %{}, fn {k, v} ->
-      if is_binary(k) and k in params do
-        {k, discard_values(v, [])}
+      if is_binary(k) and k in match do
+        {k, v}
       else
-        {k, keep_values(v, params)}
+        {k, keep_values(v, match)}
       end
     end)
   end
 
-  defp keep_values([_ | _] = list, params) do
-    Enum.map(list, &keep_values(&1, params))
+  defp keep_values([_ | _] = list, match) do
+    Enum.map(list, &keep_values(&1, match))
   end
 
-  defp keep_values(_other, _params), do: "[FILTERED]"
+  defp keep_values(_other, _match), do: "[FILTERED]"
 end

test/appsignal/utils/map_filter_test.exs

@@ -2,6 +2,23 @@ defmodule Appsignal.Utils.MapFilterTest do
   alias Appsignal.Utils.MapFilter
   use ExUnit.Case
 
+  # Phoenix Logger helpers for compiling filter patterns
+  # Copyright (c) 2014 Chris McCord - Licensed under MIT
+  def compile_filter({:compiled, _key, _value} = filter), do: filter
+  def compile_filter({:discard, params}), do: compile_discard(params)
+  def compile_filter({:keep, params}), do: {:keep, params}
+  def compile_filter(params), do: compile_discard(params)
+
+  defp compile_discard([]) do
+    {:compiled, [], []}
+  end
+
+  defp compile_discard(params) when is_list(params) or is_binary(params) do
+    key_match = :binary.compile_pattern(params)
+    value_match = params |> List.wrap() |> Enum.map(&(&1 <> "=")) |> :binary.compile_pattern()
+    {:compiled, key_match, value_match}
+  end
+
   describe "filter/1, without filters" do
     test "returns the map as-is" do
       assert %{id: 4, name: "David"} = MapFilter.filter(%{id: 4, name: "David"})
@@ -18,9 +35,20 @@ defmodule Appsignal.Utils.MapFilterTest do
 
       Application.delete_env(:phoenix, :filter_parameters)
     end
+
+    test "reads compiled filter config from Phoenix config" do
+      compiled_filter = compile_filter(["password", "secret"])
+      Application.put_env(:phoenix, :filter_parameters, compiled_filter)
+      values = %{"foo" => "bar", "password" => "should_not_show", "token" => "secret=value"}
+
+      assert MapFilter.filter(values) ==
+               %{"foo" => "bar", "password" => "[FILTERED]", "token" => "[FILTERED]"}
+
+      Application.delete_env(:phoenix, :filter_parameters)
+    end
   end
 
-  describe "filter/2 with discard strategy" do
+  describe "filter/2 with discard strategy (Phoenix < 1.18)" do
     test "in top level map" do
       values = %{"foo" => "bar", "password" => "should_not_show"}
 
@@ -76,12 +104,108 @@ defmodule Appsignal.Utils.MapFilterTest do
     end
   end
 
+  describe "filter/2 with compiled strategy (Phoenix >= 1.18)" do
+    test "filters keys that contain key substring" do
+      values = %{"foo" => "bar", "password" => "secret", "user_password" => "secret"}
+      compiled_filter = compile_filter(["password"])
+
+      assert MapFilter.filter(values, compiled_filter) ==
+               %{"foo" => "bar", "password" => "[FILTERED]", "user_password" => "[FILTERED]"}
+    end
+
+    test "filters values that contain value substring" do
+      values = %{"foo" => "bar", "token" => "secret=abc123", "key" => "secret=def456"}
+      compiled_filter = compile_filter(["secret"])
+
+      assert MapFilter.filter(values, compiled_filter) ==
+               %{"foo" => "bar", "token" => "[FILTERED]", "key" => "[FILTERED]"}
+    end
+
+    test "filters both keys and values when they match" do
+      values = %{"password" => "secret", "token" => "secret=value", "foo" => "bar"}
+      compiled_filter = compile_filter(["password", "secret"])
+
+      assert MapFilter.filter(values, compiled_filter) ==
+               %{"password" => "[FILTERED]", "token" => "[FILTERED]", "foo" => "bar"}
+    end
+
+    test "filters nested maps" do
+      values = %{"foo" => "bar", "user" => %{"password" => "secret", "name" => "John"}}
+      compiled_filter = compile_filter(["password", "secret"])
+
+      assert MapFilter.filter(values, compiled_filter) ==
+               %{"foo" => "bar", "user" => %{"password" => "[FILTERED]", "name" => "John"}}
+    end
+
+    test "does not filter bare values in lists (only filters map keys/values)" do
+      values = %{"foo" => "bar", "tokens" => ["secret=value", "password=123"]}
+      compiled_filter = compile_filter(["secret", "password"])
+
+      # Phoenix compiled filters only work on map keys/values, not bare strings in lists
+      assert MapFilter.filter(values, compiled_filter) ==
+               %{"foo" => "bar", "tokens" => ["secret=value", "password=123"]}
+    end
+
+    test "filters nested lists with maps" do
+      values = %{"users" => [%{"password" => "secret", "name" => "John"}]}
+      compiled_filter = compile_filter(["password", "secret"])
+
+      assert MapFilter.filter(values, compiled_filter) ==
+               %{"users" => [%{"password" => "[FILTERED]", "name" => "John"}]}
+    end
+
+    test "does not filter structs" do
+      values = %{"foo" => "bar", "file" => %Plug.Upload{}}
+      compiled_filter = compile_filter(["password", "secret"])
+
+      assert MapFilter.filter(values, compiled_filter) ==
+               %{"foo" => "bar", "file" => %Plug.Upload{}}
+    end
+
+    test "handles atomic keys" do
+      values = %{:foo => "bar", "password" => "secret"}
+
+      # We can't actually filter by atomic key, so we just test if it doesn't raise an error overal
+      compiled_filter = compile_filter(["password", "secret"])
+
+      assert MapFilter.filter(values, compiled_filter) ==
+               %{:foo => "bar", "password" => "[FILTERED]"}
+    end
+
+    test "handles non-binary values" do
+      values = %{"foo" => 123, "password" => "secret", "count" => 456}
+      compiled_filter = compile_filter(["password", "secret"])
+
+      assert MapFilter.filter(values, compiled_filter) ==
+               %{"foo" => 123, "password" => "[FILTERED]", "count" => 456}
+    end
+
+    test "filters when key and value both match different patterns" do
+      values = %{"secret_key" => "user=password"}
+      compiled_filter = compile_filter(["secret", "password"])
+
+      assert MapFilter.filter(values, compiled_filter) ==
+               %{"secret_key" => "[FILTERED]"}
+    end
+
+    test "handles empty patterns like Phoenix" do
+      values = %{"foo" => "bar", "password" => "secret"}
+      compiled_filter = compile_filter([])
+
+      assert MapFilter.filter(values, compiled_filter) ==
+               %{"foo" => "bar", "password" => "secret"}
+    end
+  end
+
   describe "filter/2 with keep strategy" do
     test "discards values not specified in params" do
       values = %{"foo" => "bar", "password" => "abc123", "file" => %Plug.Upload{}}
 
       assert MapFilter.filter(values, {:keep, []}) ==
                %{"foo" => "[FILTERED]", "password" => "[FILTERED]", "file" => "[FILTERED]"}
+
+      assert MapFilter.filter(values, compile_filter({:keep, []})) ==
+               %{"foo" => "[FILTERED]", "password" => "[FILTERED]", "file" => "[FILTERED]"}
     end
 
     test "keeps values that are specified in params" do