From 2ad446f342f27762d38b27298c3f6c89cb00109e Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Mon, 7 Sep 2026 13:50:42 +0000 Subject: [PATCH] fix(scrubbing): scrub conn path params (#1195) --- lib/sentry/plug_capture.ex | 2 +- lib/sentry/scrubber.ex | 3 ++- test/plug_capture_test.exs | 20 ++++++++++++++++++-- test/sentry/scrubber_test.exs | 5 +++++ 4 files changed, 26 insertions(+), 4 deletions(-) diff --git a/lib/sentry/plug_capture.ex b/lib/sentry/plug_capture.ex index f49b45c9..15bd73c1 100644 --- a/lib/sentry/plug_capture.ex +++ b/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 diff --git a/lib/sentry/scrubber.ex b/lib/sentry/scrubber.ex index c4a9a3b7..6e672fdd 100644 --- a/lib/sentry/scrubber.ex +++ b/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, diff --git a/test/plug_capture_test.exs b/test/plug_capture_test.exs index 67b45885..a40e5741 100644 --- a/test/plug_capture_test.exs +++ b/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, []}]) diff --git a/test/sentry/scrubber_test.exs b/test/sentry/scrubber_test.exs index d63db206..87b4cddd 100644 --- a/test/sentry/scrubber_test.exs +++ b/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"