Skip to content

Commit a9b42e7

Browse files
kinyoklionkinyoklion
committed
fix: Redact SDK in gen_server descriptions and network errors.
1 parent 5838c2b commit a9b42e7

7 files changed

+437
-11
lines changed

‎src/ldclient_event_dispatch_httpc.erl‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,10 @@ send(State, JsonEvents, PayloadId, Uri) ->
6060

6161
-type http_request() :: {ok, {{string(), integer(), string()}, [{string(), string()}], string() | binary()}}.
6262

63-
-spec process_request({error, term()} | http_request())
63+
-spec process_request({error, term()} | http_request())
6464
-> {ok, integer()} | {error, temporary, string()} | {error, permanent, string()}.
6565
process_request({error, Reason}) ->
66-
{error, temporary, Reason};
66+
{error, temporary, ldclient_key_redaction:format_httpc_error(Reason)};
6767
process_request({ok, {{_Version, StatusCode, _ReasonPhrase}, Headers, _Body}}) when StatusCode < 400 ->
6868
{ok, get_server_time(Headers)};
6969
process_request({ok, {{Version, StatusCode, ReasonPhrase}, _Headers, _Body}}) ->

‎src/ldclient_event_process_server.erl‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
-export([start_link/1, init/1]).
1313

1414
%% Behavior callbacks
15-
-export([code_change/3, handle_call/3, handle_cast/2, handle_info/2, terminate/2]).
15+
-export([code_change/3, handle_call/3, handle_cast/2, handle_info/2, terminate/2, format_status/1]).
1616

1717
%% API
1818
-export([
@@ -152,6 +152,15 @@ terminate(_Reason, _State) ->
152152
code_change(_OldVsn, State, _Extra) ->
153153
{ok, State}.
154154

155+
%% @doc Redact SDK key from state for logging
156+
%% @private
157+
%%
158+
%% @end
159+
format_status(#{state := State}) ->
160+
#{state => State#{sdk_key => "[REDACTED]"}};
161+
format_status(Other) ->
162+
Other.
163+
155164
%%===================================================================
156165
%% Internal functions
157166
%%===================================================================

‎src/ldclient_key_redaction.erl‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
%%-------------------------------------------------------------------
2+
%% @doc SDK key redaction utilities
3+
%% @private
4+
%% Provides functions to safely format error reasons and other data
5+
%% to prevent SDK keys from appearing in logs.
6+
%% @end
7+
%%-------------------------------------------------------------------
8+
9+
-module(ldclient_key_redaction).
10+
11+
%% API
12+
-export([format_httpc_error/1]).
13+
-export([format_shotgun_error/1]).
14+
15+
%%===================================================================
16+
%% API
17+
%%===================================================================
18+
19+
%% @doc Format httpc error for safe logging
20+
%%
21+
%% This function provides an abstract representation of httpc error reasons
22+
%% that is safe to log. It only formats known error types. Unknown
23+
%% error types are represented as "unknown error" to prevent accidental
24+
%% exposure of SDK keys, especially in cases where the SDK key might
25+
%% contain special characters like newlines.
26+
%% @end
27+
-spec format_httpc_error(Reason :: term()) -> string().
28+
%% HTTP status codes or other integers are safe to log
29+
format_httpc_error(StatusCode) when is_integer(StatusCode) ->
30+
lists:flatten(io_lib:format("~b", [StatusCode]));
31+
%% Common httpc connection errors
32+
format_httpc_error({failed_connect, _}) ->
33+
"failed to connect";
34+
format_httpc_error(timeout) ->
35+
"timeout opening connection";
36+
format_httpc_error(etimedout) ->
37+
"connection timeout";
38+
format_httpc_error(econnrefused) ->
39+
"connection refused";
40+
format_httpc_error(enetunreach) ->
41+
"network unreachable";
42+
format_httpc_error(ehostunreach) ->
43+
"host unreachable";
44+
format_httpc_error(nxdomain) ->
45+
"domain name not found";
46+
format_httpc_error({tls_alert, {_, Description}}) when is_atom(Description) ->
47+
lists:flatten(io_lib:format("tls_alert: ~p", [Description]));
48+
format_httpc_error({tls_alert, Alert}) ->
49+
lists:flatten(io_lib:format("tls_alert: ~p", [Alert]));
50+
%% Socket errors
51+
format_httpc_error({socket_closed_remotely, _, _}) ->
52+
"socket closed remotely";
53+
format_httpc_error(closed) ->
54+
"connection closed";
55+
format_httpc_error(enotconn) ->
56+
"socket not connected";
57+
%% Known atom errors
58+
format_httpc_error(Reason) when is_atom(Reason) ->
59+
atom_to_list(Reason);
60+
%% For any unknown error type, do not expose details
61+
format_httpc_error(_Reason) ->
62+
"unknown error".
63+
64+
%% @doc Format shotgun/gun error for safe logging
65+
%%
66+
%% This function provides an abstract representation of shotgun error reasons
67+
%% that is safe to log. Shotgun uses gun underneath, so errors can come from gun.
68+
%% Unknown error types are represented as "unknown error".
69+
%% @end
70+
-spec format_shotgun_error(Reason :: term()) -> string().
71+
%% HTTP status codes or other integers are safe to log
72+
format_shotgun_error(StatusCode) when is_integer(StatusCode) ->
73+
lists:flatten(io_lib:format("~b", [StatusCode]));
74+
%% Known shotgun errors from open
75+
format_shotgun_error(gun_open_failed) ->
76+
"connection failed to open";
77+
format_shotgun_error(gun_open_timeout) ->
78+
"timeout opening connection";
79+
%% Gun/socket errors
80+
format_shotgun_error(timeout) ->
81+
"connection timeout";
82+
format_shotgun_error(econnrefused) ->
83+
"connection refused";
84+
format_shotgun_error(enetunreach) ->
85+
"network unreachable";
86+
format_shotgun_error(ehostunreach) ->
87+
"host unreachable";
88+
format_shotgun_error(nxdomain) ->
89+
"domain name not found";
90+
format_shotgun_error({shutdown, _}) ->
91+
"connection shutdown";
92+
format_shotgun_error(normal) ->
93+
"connection closed normally";
94+
format_shotgun_error(closed) ->
95+
"connection closed";
96+
%% Known atom errors
97+
format_shotgun_error(Reason) when is_atom(Reason) ->
98+
atom_to_list(Reason);
99+
%% For any unknown error type, do not expose details
100+
format_shotgun_error(_Reason) ->
101+
"unknown error".

‎src/ldclient_update_poll_server.erl‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
-export([start_link/1, init/1]).
1313

1414
%% Behavior callbacks
15-
-export([code_change/3, handle_call/3, handle_cast/2, handle_info/2, terminate/2]).
15+
-export([code_change/3, handle_call/3, handle_cast/2, handle_info/2, terminate/2, format_status/1]).
1616

1717
-type state() :: #{
1818
sdk_key := string(),
@@ -107,6 +107,15 @@ terminate(_Reason, _State) ->
107107
code_change(_OldVsn, State, _Extra) ->
108108
{ok, State}.
109109

110+
%% @doc Redact SDK key from state for logging
111+
%% @private
112+
%%
113+
%% @end
114+
format_status(#{state := State}) ->
115+
#{state => State#{sdk_key => "[REDACTED]"}};
116+
format_status(Other) ->
117+
Other.
118+
110119
%%===================================================================
111120
%% Internal functions
112121
%%===================================================================

‎src/ldclient_update_stream_server.erl‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,9 @@ handle_info({listen}, #{stream_uri := Uri} = State) ->
9292
handle_info({'DOWN', _Mref, process, ShotgunPid, Reason}, #{conn := ShotgunPid, backoff := Backoff} = State) ->
9393
NewBackoff = ldclient_backoff:fail(Backoff),
9494
_ = ldclient_backoff:fire(NewBackoff),
95-
error_logger:warning_msg("Got DOWN message from shotgun pid with reason: ~p, will retry in ~p ms~n", [Reason, maps:get(current, NewBackoff)]),
95+
% Reason from DOWN message could contain connection details with headers/SDK keys
96+
SafeReason = ldclient_key_redaction:format_shotgun_error(Reason),
97+
error_logger:warning_msg("Got DOWN message from shotgun pid with reason: ~s, will retry in ~p ms~n", [SafeReason, maps:get(current, NewBackoff)]),
9698
{noreply, State#{conn := undefined, backoff := NewBackoff}};
9799
handle_info({timeout, _TimerRef, listen}, State) ->
98100
error_logger:info_msg("Reconnecting streaming connection...~n"),
@@ -132,13 +134,16 @@ do_listen(#{
132134
NewBackoff = do_listen_fail_backoff(Backoff, temporary, Reason),
133135
State#{backoff := NewBackoff};
134136
{error, permanent, Reason} ->
137+
% Reason here is already safe: either a sanitized string from format_shotgun_error
138+
% or an integer status code from the do_listen/5 method.
135139
error_logger:error_msg("Stream encountered permanent error ~p, giving up~n", [Reason]),
136140
State;
137141
{ok, Pid} ->
138142
NewBackoff = ldclient_backoff:succeed(Backoff),
139143
State#{conn := Pid, backoff := NewBackoff}
140-
catch Code:Reason ->
141-
NewBackoff = do_listen_fail_backoff(Backoff, Code, Reason),
144+
catch Code:_Reason ->
145+
% Don't pass raw exception reason as it could contain unsafe data
146+
NewBackoff = do_listen_fail_backoff(Backoff, Code, "unexpected exception"),
142147
State#{backoff := NewBackoff}
143148
end.
144149

@@ -171,9 +176,10 @@ do_listen(Uri, FeatureStore, Tag, GunOpts, Headers) ->
171176
F = fun(nofin, _Ref, Bin) ->
172177
try
173178
process_event(parse_shotgun_event(Bin), FeatureStore, Tag)
174-
catch Code:Reason ->
175-
% Exception when processing event, log error, close connection
176-
error_logger:warning_msg("Invalid SSE event error (~p): ~p", [Code, Reason]),
179+
catch Code:_Reason ->
180+
% Exception when processing event - don't log exception details
181+
% as they could theoretically contain sensitive data
182+
error_logger:warning_msg("Invalid SSE event error (~p)", [Code]),
177183
shotgun:close(Pid)
178184
end;
179185
(fin, _Ref, _Bin) ->
@@ -185,7 +191,8 @@ do_listen(Uri, FeatureStore, Tag, GunOpts, Headers) ->
185191
case shotgun:get(Pid, Path ++ Query, Headers, Options) of
186192
{error, Reason} ->
187193
shotgun:close(Pid),
188-
{error, temporary, Reason};
194+
SafeReason = ldclient_key_redaction:format_shotgun_error(Reason),
195+
{error, temporary, SafeReason};
189196
{ok, #{status_code := StatusCode}} when StatusCode >= 400 ->
190197
{error, ldclient_http:is_http_error_code_recoverable(StatusCode), StatusCode};
191198
{ok, _Ref} ->

‎test/ldclient_key_redaction_SUITE.erl‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
-module(ldclient_key_redaction_SUITE).
2+
3+
-include_lib("common_test/include/ct.hrl").
4+
5+
%% ct functions
6+
-export([all/0]).
7+
-export([init_per_suite/1]).
8+
-export([end_per_suite/1]).
9+
10+
%% Tests
11+
-export([
12+
format_httpc_error_atom/1,
13+
format_httpc_error_integer/1,
14+
format_httpc_error_tuple/1,
15+
format_httpc_error_unknown/1,
16+
format_shotgun_error_atom/1,
17+
format_shotgun_error_integer/1,
18+
format_shotgun_error_unknown/1
19+
]).
20+
21+
%%====================================================================
22+
%% ct functions
23+
%%====================================================================
24+
25+
all() ->
26+
[
27+
format_httpc_error_atom,
28+
format_httpc_error_integer,
29+
format_httpc_error_tuple,
30+
format_httpc_error_unknown,
31+
format_shotgun_error_atom,
32+
format_shotgun_error_integer,
33+
format_shotgun_error_unknown
34+
].
35+
36+
init_per_suite(Config) ->
37+
Config.
38+
39+
end_per_suite(_) ->
40+
ok.
41+
42+
%%====================================================================
43+
%% Tests
44+
%%====================================================================
45+
46+
format_httpc_error_atom(_) ->
47+
% Known atom errors should be formatted safely
48+
"timeout opening connection" = ldclient_key_redaction:format_httpc_error(timeout),
49+
"connection refused" = ldclient_key_redaction:format_httpc_error(econnrefused),
50+
"connection timeout" = ldclient_key_redaction:format_httpc_error(etimedout).
51+
52+
format_httpc_error_integer(_) ->
53+
% HTTP status codes should be formatted as integers
54+
"404" = ldclient_key_redaction:format_httpc_error(404),
55+
"500" = ldclient_key_redaction:format_httpc_error(500),
56+
"200" = ldclient_key_redaction:format_httpc_error(200).
57+
58+
format_httpc_error_tuple(_) ->
59+
% Known tuple errors should be formatted safely
60+
"failed to connect" = ldclient_key_redaction:format_httpc_error({failed_connect, []}),
61+
"connection closed" = ldclient_key_redaction:format_httpc_error(closed).
62+
63+
format_httpc_error_unknown(_) ->
64+
% Unknown error types, including those that might contain SDK keys, should be redacted
65+
"unknown error" = ldclient_key_redaction:format_httpc_error({http_error, "sdk-12345"}),
66+
"unknown error" = ldclient_key_redaction:format_httpc_error(["Authorization: sdk-test\n"]),
67+
"unknown error" = ldclient_key_redaction:format_httpc_error({request, [{headers, [{"Authorization", "sdk-key-with-newline\n"}]}]}).
68+
69+
format_shotgun_error_atom(_) ->
70+
% Known atom errors should be formatted safely
71+
"connection timeout" = ldclient_key_redaction:format_shotgun_error(timeout),
72+
"connection failed to open" = ldclient_key_redaction:format_shotgun_error(gun_open_failed),
73+
"timeout opening connection" = ldclient_key_redaction:format_shotgun_error(gun_open_timeout),
74+
"connection refused" = ldclient_key_redaction:format_shotgun_error(econnrefused).
75+
76+
format_shotgun_error_integer(_) ->
77+
% HTTP status codes should be formatted as integers
78+
"401" = ldclient_key_redaction:format_shotgun_error(401),
79+
"500" = ldclient_key_redaction:format_shotgun_error(500).
80+
81+
format_shotgun_error_unknown(_) ->
82+
% Unknown error types, including those that might contain SDK keys, should be redacted
83+
"unknown error" = ldclient_key_redaction:format_shotgun_error({gun_error, "sdk-12345"}),
84+
"unknown error" = ldclient_key_redaction:format_shotgun_error(["Headers: sdk-test\n"]),
85+
"unknown error" = ldclient_key_redaction:format_shotgun_error({connection_error, [{auth, "sdk-malformed\nkey"}]}).

0 commit comments

Comments
 (0)