refactor: extract scrubber

lib/sentry/plug_context.ex

@@ -153,9 +153,6 @@ defmodule Sentry.PlugContext do
     end
   end
 
-  @default_scrubbed_param_keys ["password", "passwd", "secret"]
-  @default_scrubbed_header_keys ["authorization", "authentication", "cookie"]
-  @scrubbed_value "*********"
   @default_plug_request_id_header "x-request-id"
 
   @doc false
@@ -179,7 +176,7 @@ defmodule Sentry.PlugContext do
       url: apply_fun_with_conn(conn, url_scrubber, Plug.Conn.request_url(conn)),
       method: conn.method,
       data: apply_fun_with_conn(conn, body_scrubber, %{}),
-      query_string: conn.query_string,
+      query_string: default_query_string_scrubber(conn),
       cookies: apply_fun_with_conn(conn, cookie_scrubber, %{}),
       headers: apply_fun_with_conn(conn, header_scrubber, %{}),
       env: %{
@@ -230,11 +227,20 @@ defmodule Sentry.PlugContext do
   defp apply_fun_with_conn(conn, fun, _default) when is_function(fun, 1), do: fun.(conn)
 
   @doc """
-  Scrubs **all** cookies off of the request.
+  Scrubs cookie values while preserving cookie names.
+
+  Every cookie value is replaced with `"[Filtered]"`. Cookie names are
+  preserved. This conforms to the Sentry data collection spec, which
+  requires key/header names to be kept and only values to be redacted.
+  Because cookies frequently carry session identifiers and other
+  credentials, the safe default is to scrub all values.
   """
   @spec default_cookie_scrubber(Plug.Conn.t()) :: map()
-  def default_cookie_scrubber(_conn) do
-    %{}
+  def default_cookie_scrubber(conn) do
+    conn
+    |> Plug.Conn.fetch_cookies()
+    |> Map.fetch!(:req_cookies)
+    |> Map.new(fn {name, _value} -> {name, Sentry.Scrubber.scrubbed_value()} end)
   end
 
   @doc """
@@ -246,57 +252,48 @@ defmodule Sentry.PlugContext do
   end
 
   @doc """
-  Scrubs the headers of a request.
+  Scrubs the query string of a request.
 
-  The default scrubbed headers are:
+  Parses the query string, redacts values whose key matches the
+  `Sentry.Scrubber` denylist, and re-encodes it. The original key order
+  is not preserved (it is not preserved by `Plug.Conn.fetch_query_params/1`
+  either).
+  """
+  @spec default_query_string_scrubber(Plug.Conn.t()) :: String.t()
+  def default_query_string_scrubber(%{query_string: ""}), do: ""
+
+  def default_query_string_scrubber(conn) do
+    conn
+    |> Plug.Conn.fetch_query_params()
+    |> Map.fetch!(:query_params)
+    |> Sentry.Scrubber.scrub_map()
+    |> URI.encode_query()
+  end
 
-  #{Enum.map_join(@default_scrubbed_header_keys, "\n", &"*  `#{&1}`")}
+  @doc """
+  Scrubs the values of sensitive request headers.
+
+  Header **names** are always preserved; values for headers whose name matches
+  the `Sentry.Scrubber` denylist (substring, case-insensitive) are replaced
+  with `"[Filtered]"`. The `cookie` header is also always scrubbed since its
+  raw value typically carries session identifiers.
   """
   @spec default_header_scrubber(Plug.Conn.t()) :: map()
   def default_header_scrubber(conn) do
     conn.req_headers
     |> Map.new()
-    |> Map.drop(@default_scrubbed_header_keys)
+    |> Sentry.Scrubber.scrub_map(["cookie"])
   end
 
   @doc """
   Scrubs the body of a request.
 
   The default scrubbed keys are:
 
-  #{Enum.map_join(@default_scrubbed_param_keys, "\n", &"*  `#{&1}`")}
+  #{Enum.map_join(Sentry.Scrubber.default_scrubbed_param_keys(), "\n", &"*  `#{&1}`")}
   """
   @spec default_body_scrubber(Plug.Conn.t()) :: map()
   def default_body_scrubber(conn) do
-    scrub_map(conn.params, @default_scrubbed_param_keys)
+    Sentry.Scrubber.scrub_map(conn.params)
   end
-
-  defp scrub_map(map, scrubbed_keys) do
-    Map.new(map, fn {key, value} ->
-      value =
-        cond do
-          key in scrubbed_keys -> @scrubbed_value
-          is_binary(value) and value =~ credit_card_regex() -> @scrubbed_value
-          is_struct(value) -> value |> Map.from_struct() |> scrub_map(scrubbed_keys)
-          is_map(value) -> scrub_map(value, scrubbed_keys)
-          is_list(value) -> scrub_list(value, scrubbed_keys)
-          true -> value
-        end
-
-      {key, value}
-    end)
-  end
-
-  defp scrub_list(list, scrubbed_keys) do
-    Enum.map(list, fn value ->
-      cond do
-        is_struct(value) -> value |> Map.from_struct() |> scrub_map(scrubbed_keys)
-        is_map(value) -> scrub_map(value, scrubbed_keys)
-        is_list(value) -> scrub_list(value, scrubbed_keys)
-        true -> value
-      end
-    end)
-  end
-
-  defp credit_card_regex, do: ~r/^(?:\d[ -]*?){13,16}$/
 end

lib/sentry/scrubber.ex

@@ -0,0 +1,135 @@
+defmodule Sentry.Scrubber do
+  @default_scrubbed_param_keys [
+    "auth",
+    "token",
+    "secret",
+    "password",
+    "passwd",
+    "pwd",
+    "key",
+    "jwt",
+    "bearer",
+    "sso",
+    "saml",
+    "csrf",
+    "xsrf",
+    "credentials",
+    "session",
+    "sid",
+    "identity"
+  ]
+
+  @scrubbed_value "[Filtered]"
+
+  @moduledoc """
+  Provides scrubbing of sensitive data from values before they are sent to Sentry.
+
+  This module is used internally to redact sensitive values like passwords, tokens, and credit card
+  numbers before they appear in Sentry events and breadcrumbs.
+
+  Scrubbing follows the [Sentry Data Collection
+  spec](https://develop.sentry.dev/sdk/foundations/client/data-collection/):
+
+    * The placeholder value is `"[Filtered]"`
+    * Keys are matched case-insensitively as substrings — for example, the
+      term `"auth"` matches `"Authorization"`, `"X-Auth-Token"`, and
+      `"oauth_token"`
+    * Key names are always preserved; only values are replaced
+
+  ## Default sensitive denylist
+
+  The following terms are scrubbed by default:
+
+  #{Enum.map_join(@default_scrubbed_param_keys, "\n", &"  * `\"#{&1}\"`")}
+  """
+
+  @moduledoc since: "13.1.0"
+
+  @doc """
+  Returns the list of parameter keys scrubbed by default.
+  """
+  @spec default_scrubbed_param_keys() :: [String.t()]
+  def default_scrubbed_param_keys, do: @default_scrubbed_param_keys
+
+  @doc """
+  Returns the placeholder value used to replace scrubbed data.
+  """
+  @spec scrubbed_value() :: String.t()
+  def scrubbed_value, do: @scrubbed_value
+
+  @doc """
+  Scrubs a map of sensitive parameter values using the default denylist.
+
+  See `scrub_map/2` to extend the denylist with additional terms.
+  """
+  @spec scrub_map(map()) :: map()
+  def scrub_map(map) when is_map(map) do
+    scrub_map(map, [])
+  end
+
+  @doc """
+  Scrubs a map of sensitive parameter values using the default denylist plus
+  any `extra_terms`.
+
+  Keys are matched case-insensitively as substrings. Values are replaced with
+  `"[Filtered]"` when their key matches a denylist term or when the value
+  itself looks like a credit card number. The function recurses into nested
+  maps and lists.
+  """
+  @spec scrub_map(map(), [String.t()]) :: map()
+  def scrub_map(map, extra_terms) when is_map(map) and is_list(extra_terms) do
+    do_scrub_map(map, denylist(extra_terms))
+  end
+
+  @doc """
+  Returns `"[Filtered]"`, used as a fallback when a value (e.g. an unparseable
+  cookie or body string) cannot be inspected for sensitive keys.
+  """
+  @spec scrub_string(String.t()) :: String.t()
+  def scrub_string(value) when is_binary(value), do: @scrubbed_value
+
+  defp do_scrub_map(map, denylist) do
+    Map.new(map, fn {key, value} ->
+      value =
+        cond do
+          sensitive_key?(key, denylist) -> @scrubbed_value
+          is_binary(value) and value =~ credit_card_regex() -> @scrubbed_value
+          is_struct(value) -> value |> Map.from_struct() |> do_scrub_map(denylist)
+          is_map(value) -> do_scrub_map(value, denylist)
+          is_list(value) -> do_scrub_list(value, denylist)
+          true -> value
+        end
+
+      {key, value}
+    end)
+  end
+
+  defp do_scrub_list(list, denylist) do
+    Enum.map(list, fn value ->
+      cond do
+        is_struct(value) -> value |> Map.from_struct() |> do_scrub_map(denylist)
+        is_map(value) -> do_scrub_map(value, denylist)
+        is_list(value) -> do_scrub_list(value, denylist)
+        true -> value
+      end
+    end)
+  end
+
+  defp denylist(extra_terms) do
+    Enum.map(@default_scrubbed_param_keys ++ extra_terms, &String.downcase/1)
+  end
+
+  defp sensitive_key?(key, denylist) when is_binary(key) do
+    downcased = String.downcase(key)
+    Enum.any?(denylist, &String.contains?(downcased, &1))
+  end
+
+  defp sensitive_key?(key, denylist)
+       when is_atom(key) and not is_nil(key) and not is_boolean(key) do
+    sensitive_key?(Atom.to_string(key), denylist)
+  end
+
+  defp sensitive_key?(_key, _denylist), do: false
+
+  defp credit_card_regex, do: ~r/^(?:\d[ -]*?){13,16}$/
+end

test/plug_capture_test.exs

@@ -181,7 +181,7 @@ defmodule Sentry.PlugCaptureTest do
 
       assert [exception] = event.exception
       assert exception.type == "Phoenix.ActionClauseError"
-      assert exception.value =~ ~s(params: %{"password" => "*********"})
+      assert exception.value =~ ~s(params: %{"password" => "[Filtered]"})
     end
 
     test "can render feedback form in Phoenix ErrorView" do

test/sentry/plug_context_test.exs

@@ -117,6 +117,25 @@ defmodule Sentry.PlugContextTest do
     assert "http://www.example.com/secret-token/****" == Sentry.Context.get_all().request.url
   end
 
+  test "default query string scrubbing redacts sensitive params" do
+    conn = conn(:get, "/test?password=hunter2&keep=this&api_token=abc")
+    call(conn, [])
+
+    qs = Sentry.Context.get_all().request.query_string
+    decoded = URI.decode_query(qs)
+
+    assert decoded["password"] == "[Filtered]"
+    assert decoded["api_token"] == "[Filtered]"
+    assert decoded["keep"] == "this"
+  end
+
+  test "default query string scrubbing leaves an empty query string alone" do
+    conn = conn(:get, "/test")
+    call(conn, [])
+
+    assert Sentry.Context.get_all().request.query_string == ""
+  end
+
   test "allows configuring request id header", %{conn: conn} do
     conn
     |> put_resp_header("my-request-id", "abc123")
@@ -148,20 +167,29 @@ defmodule Sentry.PlugContextTest do
 
     request_context = Sentry.Context.get_all().request
 
-    assert request_context.headers == %{"content-type" => "application/json"}
-    assert request_context.cookies == %{}
+    assert request_context.headers == %{
+             "authentication" => "[Filtered]",
+             "authorization" => "[Filtered]",
+             "content-type" => "application/json",
+             "cookie" => "[Filtered]"
+           }
+
+    assert request_context.cookies == %{
+             "regular" => "[Filtered]",
+             "secret" => "[Filtered]"
+           }
 
     assert request_context.data == %{
-             "another_cc" => "*********",
-             "cc" => "*********",
+             "another_cc" => "[Filtered]",
+             "cc" => "[Filtered]",
              "count" => 334,
-             "credit_card" => "*********",
-             "passwd" => "*********",
-             "password" => "*********",
-             "secret" => "*********",
-             "user" => %{"password" => "*********"},
+             "credit_card" => "[Filtered]",
+             "passwd" => "[Filtered]",
+             "password" => "[Filtered]",
+             "secret" => "[Filtered]",
+             "user" => %{"password" => "[Filtered]"},
              "payments" => [
-               %{"yet_another_cc" => "*********"}
+               %{"yet_another_cc" => "[Filtered]"}
              ]
            }
   end
@@ -173,7 +201,7 @@ defmodule Sentry.PlugContextTest do
     call(conn, [])
 
     assert Sentry.Context.get_all().request.data == %{
-             "password" => "*********",
+             "password" => "[Filtered]",
              "image" => %{
                content_type: nil,
                filename: "my_image.png",