Skip to content

Commit 012f3ef

Browse files
authored
Avoid leaking Authorization request headers in cleartext (#160)
1 parent 72b30ab commit 012f3ef

File tree

2 files changed

+60
-1
lines changed

2 files changed

+60
-1
lines changed

lib/error_tracker/integrations/plug.ex

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,14 +111,16 @@ defmodule ErrorTracker.Integrations.Plug do
111111
conn |> build_context |> ErrorTracker.set_context()
112112
end
113113

114+
@sensitive_headers ["cookie", "authorization"]
115+
114116
defp build_context(conn = %Plug.Conn{}) do
115117
%{
116118
"request.host" => conn.host,
117119
"request.path" => conn.request_path,
118120
"request.query" => conn.query_string,
119121
"request.method" => conn.method,
120122
"request.ip" => remote_ip(conn),
121-
"request.headers" => conn.req_headers |> Map.new() |> Map.drop(["cookie"]),
123+
"request.headers" => conn.req_headers |> Map.new() |> Map.drop(@sensitive_headers),
122124
# Depending on the error source, the request params may have not been fetched yet
123125
"request.params" => unless(is_struct(conn.params, Plug.Conn.Unfetched), do: conn.params)
124126
}

test/integrations/plug_test.exs

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
defmodule ErrorTracker.Integrations.PlugTest do
2+
use ErrorTracker.Test.Case
3+
4+
alias ErrorTracker.Integrations.Plug, as: IntegrationPlug
5+
6+
@fake_callstack []
7+
8+
setup do
9+
[conn: Phoenix.ConnTest.build_conn()]
10+
end
11+
12+
test "it reports errors, including the request headers", %{conn: conn} do
13+
conn = conn |> Plug.Conn.put_req_header("accept", "application/json")
14+
15+
IntegrationPlug.report_error(
16+
conn,
17+
{"an error from Phoenix", "something bad happened"},
18+
@fake_callstack
19+
)
20+
21+
[error] = repo().all(ErrorTracker.Error)
22+
23+
assert error.kind == "an error from Phoenix"
24+
assert error.reason == "something bad happened"
25+
26+
[occurrence] = repo().all(ErrorTracker.Occurrence)
27+
assert occurrence.error_id == error.id
28+
29+
%{"request.headers" => request_headers} = occurrence.context
30+
assert request_headers == %{"accept" => "application/json"}
31+
end
32+
33+
test "it does not save sensitive request headers, to avoid storing them in cleartext", %{
34+
conn: conn
35+
} do
36+
conn =
37+
conn
38+
|> Plug.Conn.put_req_header("cookie", "who stole the cookie from the cookie jar ?")
39+
|> Plug.Conn.put_req_header("authorization", "Bearer plz-dont-leak-my-secrets")
40+
|> Plug.Conn.put_req_header("safe", "this can be safely stored in cleartext")
41+
42+
IntegrationPlug.report_error(
43+
conn,
44+
{"an error from Phoenix", "something bad happened"},
45+
@fake_callstack
46+
)
47+
48+
[occurrence] = repo().all(ErrorTracker.Occurrence)
49+
50+
header_names = occurrence.context |> Map.get("request.headers") |> Map.keys()
51+
52+
assert "cookie" not in header_names
53+
assert "authorization" not in header_names
54+
55+
assert "safe" in header_names
56+
end
57+
end

0 commit comments

Comments
 (0)