Skip to content

Commit e3a4c91

Browse files
solnicclaude
andauthored
fix(plug): broader scrubbing (#1070)
* fix(plug): broaden default %Plug.Conn{} scrubbing Expand the default @scrubbable_conn_fields so scrub/1 also clears req_cookies and assigns, scrubs body_params and query_params with the default sensitive keys, and reduces private to an allow-list of framework metadata — in addition to today's cookies/req_headers/params. assigns is cleared wholesale because auth libraries (Guardian, Pow, Coherence) routinely store decoded tokens, full user structs, and password hashes there, where no key-based heuristic redacts safely. private mixes sensitive data (the decoded session under :plug_session, Guardian's raw JWT) with high-signal framework routing metadata, so it uses the :private_allow_list strategy: Phoenix routing/render keys are retained (configurable via :scrub_conn_private_allow_list) and everything else is dropped. This keeps the most useful triage breadcrumb — which controller/action failed — without leaking secrets. Expressed entirely through the strategy-aware attribute and the allow-list mechanism from earlier commits — no changes to scrub/1 dispatch or to the consumers (Sentry.PlugContext and Sentry.PlugCapture both funnel through Sentry.Scrubber.scrub/1). The PlugContext request-interface payload is unchanged (it reads back only params/cookies/req_headers); the broadening manifests on the PlugCapture ActionClauseError conn path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(plug): cover default params, private allow-list, assigns, and req_cookies scrubbing Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refa(scrubber): handle `body_params` the same as `params` * test(plug): assert cookie value is scrubbed from stacktrace frame vars Broadened conn scrubbing now clears req_cookies, so the session cookie value no longer appears in the inspected conn captured into frame vars. Restore the cookie-value assertion alongside the auth-header one. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent b4e55c2 commit e3a4c91

11 files changed

Lines changed: 498 additions & 30 deletions

File tree

lib/sentry/config.ex

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -514,6 +514,37 @@ defmodule Sentry.Config do
514514
`{Sentry.Test.Config, :namespace}` as the resolver to enable per-test
515515
configuration isolation via `Sentry.Test.Config.put/1`.
516516
"""
517+
],
518+
scrubber: [
519+
type: :keyword_list,
520+
default: [],
521+
type_doc: "`t:keyword/0`",
522+
doc: """
523+
Configuration for how the SDK scrubs sensitive data out of captured events.
524+
525+
*Available since v13.1.1*.
526+
""",
527+
keys: [
528+
conn_private_allow_list: [
529+
type: {:list, :atom},
530+
default: Sentry.Scrubber.default_private_allow_list(),
531+
type_doc: "list of `t:atom/0`",
532+
doc: """
533+
A list of keys that are retained in a `%Plug.Conn{}`'s `:private` map when
534+
a captured error embeds a connection (for example a `Phoenix.ActionClauseError`
535+
reported via `Sentry.PlugCapture`). All other `:private` keys are dropped.
536+
537+
The `:private` map can hold framework internals and sensitive data (such as
538+
the decoded session under `:plug_session`), so it is not safe to report
539+
wholesale. By default the SDK keeps only Phoenix's routing and render
540+
metadata — see `Sentry.Scrubber.default_private_allow_list/0` — which is
541+
high-signal for triaging which controller/action failed. Set this option to
542+
extend or replace that list.
543+
544+
*Available since v13.1.1*.
545+
"""
546+
]
547+
]
517548
]
518549
]
519550

@@ -914,6 +945,9 @@ defmodule Sentry.Config do
914945
@spec in_app_module_allow_list() :: [atom()]
915946
def in_app_module_allow_list, do: fetch!(:in_app_module_allow_list)
916947

948+
@spec scrubber() :: keyword()
949+
def scrubber, do: fetch!(:scrubber)
950+
917951
@spec send_result() :: :none | :sync
918952
def send_result, do: fetch!(:send_result)
919953

lib/sentry/plug_capture.ex

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,16 @@ defmodule Sentry.PlugCapture do
8080
request. When no `Sentry.PlugContext` has run, falls back to the
8181
defaults defined by `Sentry.Scrubber.scrub/2`:
8282
83-
* scrubs *all* cookies
83+
* scrubs *all* cookies (`cookies` and `req_cookies`)
8484
* drops sensitive request headers (`authorization`, `authentication`, `cookie`)
85-
* scrubs sensitive body params (`password`, `passwd`, `secret`)
85+
* scrubs `params` and `body_params` through the configured `body_scrubber`
86+
(defaulting to the sensitive params `password`, `passwd`, `secret`; a
87+
`nil` `body_scrubber` empties both), and scrubs the same sensitive params
88+
in `query_params`
89+
* clears `assigns` (where auth libraries store user structs and tokens)
90+
* reduces `private` to an allow-list of framework metadata, dropping
91+
everything else (notably the decoded session under `:plug_session`);
92+
configurable via the `scrubber: [conn_private_allow_list: ...]` option
8693
8794
"""
8895
defmacro __using__(opts) do

lib/sentry/plug_context.ex

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ defmodule Sentry.PlugContext do
155155
# when no :url_scrubber is configured, fall back to the no-op
156156
# default_url_scrubber/1 rather than Sentry.Scrubber's scrubbing default.
157157
|> Keyword.put_new(:url_scrubber, {__MODULE__, :default_url_scrubber, []})
158+
|> Keyword.put(:private_allow_list, Sentry.Config.scrubber()[:conn_private_allow_list])
158159

159160
Sentry.Scrubber.put_conn_scrubber(conn_scrubber_opts)
160161

lib/sentry/scrubber.ex

Lines changed: 79 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,22 @@ defmodule Sentry.Scrubber do
4949
(typically from `Sentry.PlugContext.call/2`), falling back to the SDK
5050
default `scrub(conn, field)` clause when none is registered, or
5151
* a fixed tag — `:clear` replaces the field with `%{}`, `:params` scrubs the
52-
field as a params-shaped map, and `:query_string` redacts sensitive params
53-
from a raw query string.
52+
field as a params-shaped map, `:query_string` redacts sensitive params from
53+
a raw query string, and `:private_allow_list` keeps only the registered
54+
allow-listed keys of the field (see `default_private_allow_list/0` and the
55+
`:private_allow_list` option of `put_conn_scrubber/1`), dropping everything
56+
else.
57+
58+
By default `scrub/1` redacts `cookies`, `req_headers`, `params`, and
59+
`body_params` (the configurable fields — `body_params` shares the
60+
`:body_scrubber` with `params`, so it honors the same registered scrubber and
61+
is emptied when `body_scrubber` is `nil`), clears `req_cookies` and `assigns`
62+
to `%{}`, scrubs `query_params` as a params-shaped map, and reduces `private`
63+
to its allow-listed keys (`default_private_allow_list/0`). `assigns` is cleared
64+
wholesale because auth libraries (Guardian, Pow, Coherence) routinely store
65+
decoded tokens, full user structs, and session data there, where no key-based
66+
heuristic redacts safely. `private` keeps only the allow-listed framework
67+
metadata and drops everything else (notably `:plug_session`).
5468
5569
The defaults can be overridden per call with `scrub(conn, overrides)`, where
5670
`overrides` is a `field: strategy` keyword list merged over the attribute —
@@ -67,35 +81,70 @@ defmodule Sentry.Scrubber do
6781
@scrubber_pdict_key {__MODULE__, :scrubber}
6882
@scrubber_names [:body_scrubber, :header_scrubber, :cookie_scrubber, :url_scrubber]
6983

84+
# Keys retained when a `%Plug.Conn{}`'s `:private` map is scrubbed with the
85+
# `:private_allow_list` strategy. These are Phoenix's routing/render metadata
86+
# — safe, high-signal breadcrumbs for triaging which controller/action failed.
87+
# Anything not listed (e.g. `:plug_session`, which holds decoded session data)
88+
# is dropped. This is the SDK default; the `scrubber: [conn_private_allow_list: ...]`
89+
# config option exposes it as a user-configurable option.
90+
@default_private_allow_list [
91+
:phoenix_controller,
92+
:phoenix_action,
93+
:phoenix_endpoint,
94+
:phoenix_router,
95+
:phoenix_view,
96+
:phoenix_layout,
97+
:phoenix_format,
98+
:phoenix_template,
99+
:phoenix_router_url,
100+
:phoenix_static_url
101+
]
102+
70103
# Default `field -> strategy` mapping applied by `scrub/1` (overridable per
71104
# call via `scrub(conn, overrides)`). A strategy is either a configurable
72105
# scrubber struct-key (resolved per process via `get/1`) or a fixed tag:
73106
# `:clear` -> `%{}`, `:params` -> params-shaped scrub (Unfetched-safe),
74-
# `:query_string` -> redact sensitive params from the raw query string.
107+
# `:query_string` -> redact sensitive params from the raw query string,
108+
# `:private_allow_list` -> keep only the registered allow-listed keys.
75109
# Add an entry to make a new conn field scrubbed by default.
110+
#
111+
# `assigns` is cleared wholesale because auth libraries (Guardian, Pow,
112+
# Coherence) routinely store decoded tokens, full user structs, and session
113+
# data there — there is no reliable key-based heuristic to redact it safely.
114+
# `private` mixes sensitive data (e.g. `:plug_session`) with high-signal
115+
# framework metadata (Phoenix routing), so it uses an allow-list instead of
116+
# clearing wholesale — see `@default_private_allow_list`.
76117
@scrubbable_conn_fields [
77118
cookies: :cookie_scrubber,
119+
req_cookies: :clear,
78120
req_headers: :header_scrubber,
79121
params: :body_scrubber,
80-
query_string: :query_string
122+
body_params: :body_scrubber,
123+
query_params: :params,
124+
query_string: :query_string,
125+
assigns: :clear,
126+
private: :private_allow_list
81127
]
82128

83129
@typedoc """
84130
A resolved set of per-field scrubbers for a `%Plug.Conn{}`.
85131
86132
Each scrubber field holds a 1-arity function that takes the conn and returns
87-
the scrubbed value for the corresponding field. Built by `put_conn_scrubber/1`
88-
from `t:conn_scrubber_opts/0` and stored in the process dictionary.
133+
the scrubbed value for the corresponding field. `private_allow_list` holds the
134+
keys retained by the `:private_allow_list` strategy. Built by
135+
`put_conn_scrubber/1` from `t:conn_scrubber_opts/0` and stored in the process
136+
dictionary.
89137
"""
90138
@type t :: %__MODULE__{
91139
body_scrubber: (Plug.Conn.t() -> term()),
92140
header_scrubber: (Plug.Conn.t() -> term()),
93141
cookie_scrubber: (Plug.Conn.t() -> term()),
94-
url_scrubber: (Plug.Conn.t() -> String.t())
142+
url_scrubber: (Plug.Conn.t() -> String.t()),
143+
private_allow_list: [atom()]
95144
}
96145

97146
@enforce_keys @scrubber_names
98-
defstruct @scrubber_names
147+
defstruct @scrubber_names ++ [private_allow_list: @default_private_allow_list]
99148

100149
@doc false
101150
@spec scrubber_names() :: [atom()]
@@ -123,12 +172,14 @@ defmodule Sentry.Scrubber do
123172
124173
Each `*_scrubber` key, when omitted, falls back to the field's default
125174
scrubber — the matching `scrub(conn, field)` clause of `scrub/2`.
175+
`:private_allow_list` defaults to `default_private_allow_list/0`.
126176
"""
127177
@type conn_scrubber_opts :: [
128178
body_scrubber: field_scrubber(),
129179
header_scrubber: field_scrubber(),
130180
cookie_scrubber: field_scrubber(),
131-
url_scrubber: field_scrubber()
181+
url_scrubber: field_scrubber(),
182+
private_allow_list: [atom()]
132183
]
133184

134185
@doc """
@@ -152,6 +203,18 @@ defmodule Sentry.Scrubber do
152203
@spec default_header_keys() :: [String.t()]
153204
def default_header_keys, do: @default_scrubbed_header_keys
154205

206+
@doc """
207+
Returns the default list of `%Plug.Conn{}` `:private` keys retained by the
208+
`:private_allow_list` scrubbing strategy.
209+
210+
These are Phoenix's routing/render metadata keys, kept because they are
211+
high-signal, non-sensitive breadcrumbs for triaging errors. This is the
212+
default for the `scrubber: [conn_private_allow_list: ...]` configuration option.
213+
"""
214+
@doc since: "13.1.1"
215+
@spec default_private_allow_list() :: [atom()]
216+
def default_private_allow_list, do: @default_private_allow_list
217+
155218
@doc """
156219
Drops sensitive keys from a flat map.
157220
@@ -261,7 +324,8 @@ defmodule Sentry.Scrubber do
261324
body_scrubber: resolve_scrubber(opts, :body_scrubber, :body),
262325
header_scrubber: resolve_scrubber(opts, :header_scrubber, :headers),
263326
cookie_scrubber: resolve_scrubber(opts, :cookie_scrubber, :cookies),
264-
url_scrubber: resolve_scrubber(opts, :url_scrubber, :url)
327+
url_scrubber: resolve_scrubber(opts, :url_scrubber, :url),
328+
private_allow_list: Keyword.get(opts, :private_allow_list, @default_private_allow_list)
265329
}
266330
end
267331

@@ -468,6 +532,8 @@ defmodule Sentry.Scrubber do
468532
# params-shaped map, leaving `%Plug.Conn.Unfetched{}` untouched
469533
# * `:query_string` — redacts sensitive params from THIS field (a raw query
470534
# string) via `scrub_query_string/1`
535+
# * `:private_allow_list` — keeps only the registered allow-listed keys of
536+
# THIS field (a map), dropping everything else
471537
defp scrub_conn_field(conn, _field, scrubber_key) when scrubber_key in @scrubber_names,
472538
do: get(scrubber_key).(conn)
473539

@@ -479,6 +545,9 @@ defmodule Sentry.Scrubber do
479545
defp scrub_conn_field(conn, field, :query_string),
480546
do: scrub_query_string(Map.fetch!(conn, field))
481547

548+
defp scrub_conn_field(conn, field, :private_allow_list),
549+
do: Map.take(Map.fetch!(conn, field), scrubber().private_allow_list)
550+
482551
# Scrubs a params-shaped value with the default sensitive keys, leaving
483552
# `%Plug.Conn.Unfetched{}` (and any non-plain-map) untouched. Shared by the
484553
# `:body` default clause and the `:params` strategy.

test/plug_capture_test.exs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,37 @@ defmodule Sentry.PlugCaptureTest do
213213
refute arg2 =~ "secret", "non-conn params arg leaked into exception value: #{arg2}"
214214
end
215215

216+
test "retains allow-listed conn.private metadata when scrubbing ActionClauseError" do
217+
# The embedded conn's :private map is reduced to the allow-list: Phoenix
218+
# routing metadata is high-signal for triage and must survive, while
219+
# non-allow-listed entries (e.g. the per-router pipeline key) are dropped.
220+
assert_raise Phoenix.ActionClauseError, fn ->
221+
conn(:get, "/action_clause_error?password=secret")
222+
|> call_phoenix_endpoint()
223+
end
224+
225+
event =
226+
assert_sentry_report(:event,
227+
culprit: "Sentry.PlugCaptureTest.PhoenixController.action_clause_error/2"
228+
)
229+
230+
assert [exception] = event.exception
231+
232+
# Sensitive data is still scrubbed (proves the scrubber ran)...
233+
assert exception.value =~ ~s(params: %{"password" => "*********"})
234+
235+
# ...routing metadata in the allow-list survives...
236+
assert exception.value =~ "phoenix_controller"
237+
assert exception.value =~ "Sentry.PlugCaptureTest.PhoenixController"
238+
assert exception.value =~ "phoenix_action"
239+
assert exception.value =~ ":action_clause_error"
240+
241+
# ...but non-allow-listed private entries are dropped. Phoenix stores a
242+
# per-router pipeline list under the router module key; it is not in the
243+
# allow-list, so it must not appear in the reported conn.
244+
refute exception.value =~ "Sentry.PlugCaptureTest.PhoenixRouter => []"
245+
end
246+
216247
test "scrubs Phoenix.ActionClauseError using PlugContext-configured body_scrubber" do
217248
Application.put_env(:sentry, PhoenixEndpointWithCustomPlugContext,
218249
render_errors: [view: Sentry.ErrorView, accepts: ~w(html)]

test/sentry/config_test.exs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,20 @@ defmodule Sentry.ConfigTest do
122122
end
123123
end
124124

125+
test ":scrubber :conn_private_allow_list" do
126+
assert Config.validate!([])[:scrubber][:conn_private_allow_list] ==
127+
Sentry.Scrubber.default_private_allow_list()
128+
129+
config = [scrubber: [conn_private_allow_list: [:phoenix_action, :my_custom_key]]]
130+
131+
assert Config.validate!(config)[:scrubber][:conn_private_allow_list] ==
132+
[:phoenix_action, :my_custom_key]
133+
134+
assert_raise ArgumentError, ~r/invalid list in :conn_private_allow_list/, fn ->
135+
Config.validate!(scrubber: [conn_private_allow_list: ["not_an_atom"]])
136+
end
137+
end
138+
125139
# TODO: remove me on v11.0.0. :included_environments has been deprecated in v10.0.0.
126140
test ":included_environments" do
127141
output =

test/sentry/plug_context_test.exs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,24 @@ defmodule Sentry.PlugContextTest do
225225

226226
assert scrubbed.params == %{"foo" => "kept"}
227227
end
228+
229+
test "registers the configured scrubber :conn_private_allow_list", %{conn: conn} do
230+
Sentry.Test.Config.put(scrubber: [conn_private_allow_list: [:phoenix_action]])
231+
232+
call(conn, [])
233+
234+
# call/2 read the config value and registered it, so the :private_allow_list
235+
# strategy now retains only the configured key.
236+
scrubbed =
237+
Sentry.Scrubber.scrub(
238+
%Plug.Conn{
239+
private: %{phoenix_action: :show, phoenix_controller: SomeApp.PageController}
240+
},
241+
private: :private_allow_list
242+
)
243+
244+
assert scrubbed.private == %{phoenix_action: :show}
245+
end
228246
end
229247

230248
defp call(conn, opts) do

0 commit comments

Comments
 (0)