fix(scrubbing): scrub conn path params (#1195)

lib/sentry/plug_capture.ex

@@ -85,7 +85,7 @@ defmodule Sentry.PlugCapture do
       * scrubs `params` and `body_params` through the configured `body_scrubber`
         (defaulting to the sensitive params `password`, `passwd`, `secret`; a
         `nil` `body_scrubber` empties both), and scrubs the same sensitive params
-        in `query_params`
+        in `query_params` and `path_params`
       * derives `request_path`, `path_info` and `query_string` from the URL the
         configured `url_scrubber` returns, so a scrubber that redacts a path
         segment redacts it here too; `query_string` is scrubbed against the

lib/sentry/scrubber.ex

@@ -60,7 +60,7 @@ defmodule Sentry.Scrubber do
   `body_params` (the configurable fields — `body_params` shares the
   `:body_scrubber` with `params`, so it honors the same registered scrubber and
   is emptied when `body_scrubber` is `nil`), clears `req_cookies` and `assigns`
-  to `%{}`, scrubs `query_params` as a params-shaped map, derives `request_path`,
+  to `%{}`, scrubs `query_params` and `path_params` as params-shaped maps, derives `request_path`,
   `path_info` and `query_string` from the scrubbed URL, and reduces `private` to
   its allow-listed keys (`default_private_allow_list/0`). `assigns` is cleared
   wholesale because auth libraries (Guardian, Pow, Coherence) routinely store
@@ -130,6 +130,7 @@ defmodule Sentry.Scrubber do
     params: :body_scrubber,
     body_params: :body_scrubber,
     query_params: :params,
+    path_params: :params,
     query_string: :url_scrubbed,
     request_path: :url_scrubbed,
     path_info: :url_scrubbed,

test/plug_capture_test.exs

@@ -25,6 +25,7 @@ defmodule Sentry.PlugCaptureTest do
     get "/action_clause_error", PhoenixController, :action_clause_error
     get "/assigns_route", PhoenixController, :assigns
     get "/reset_password/:token", PhoenixController, :action_clause_error
+    get "/verify/:secret", PhoenixController, :action_clause_error
   end
 
   defmodule PhoenixEndpoint do
@@ -387,8 +388,13 @@ defmodule Sentry.PlugCaptureTest do
         render_errors: [view: Sentry.ErrorView, accepts: ~w(html)]
       )
 
-      pid = start_supervised!(PhoenixEndpointWithUrlScrubber)
-      Process.link(pid)
+      Application.put_env(:sentry, PhoenixEndpoint,
+        render_errors: [view: Sentry.ErrorView, accepts: ~w(html)]
+      )
+
+      for endpoint <- [PhoenixEndpointWithUrlScrubber, PhoenixEndpoint] do
+        endpoint |> start_supervised!() |> Process.link()
+      end
 
       %{ref: SentryTest.setup_bypass_envelope_collector(bypass, type: "event")}
     end
@@ -415,6 +421,16 @@ defmodule Sentry.PlugCaptureTest do
 
       assert value =~ ~s(query_string: "token=#{@encoded_redacted}")
     end
+
+    test "redacts a route parameter named like a credential", %{ref: ref} do
+      assert_raise Phoenix.ActionClauseError, fn ->
+        conn(:get, "/verify/#{@token}") |> call_phoenix_endpoint()
+      end
+
+      assert [%{"exception" => [%{"value" => value}]}] = SentryTest.collect_sentry_events(ref, 1)
+
+      assert value =~ ~s(path_params: %{"secret" => "#{@redacted}"})
+    end
   end
 
   defp call_plug_app(conn), do: Plug.run(conn, [{Sentry.ExamplePlugApplication, []}])

test/sentry/scrubber_test.exs

@@ -208,6 +208,7 @@ defmodule Sentry.ScrubberTest do
         port: 443,
         request_path: "/users",
         path_info: ["users"],
+        path_params: %{"id" => "42", "secret" => "leak"},
         query_string: "page=2&secret=leak",
         method: "POST"
       }
@@ -271,6 +272,10 @@ defmodule Sentry.ScrubberTest do
       assert scrubbed.path_info == ["users"]
     end
 
+    test "scrubs path_params with default sensitive keys", %{scrubbed: scrubbed} do
+      assert scrubbed.path_params == %{"id" => "42", "secret" => "*********"}
+    end
+
     test "scrubs sensitive params out of query_string", %{scrubbed: scrubbed} do
       refute scrubbed.query_string =~ "leak"
       assert scrubbed.query_string =~ "page=2"