-
Notifications
You must be signed in to change notification settings - Fork 40
fix: Redact SDK in gen_server descriptions and network errors. #166
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,101 @@ | ||
| %%------------------------------------------------------------------- | ||
| %% @doc SDK key redaction utilities | ||
| %% @private | ||
| %% Provides functions to safely format error reasons and other data | ||
| %% to prevent SDK keys from appearing in logs. | ||
| %% @end | ||
| %%------------------------------------------------------------------- | ||
|
|
||
| -module(ldclient_key_redaction). | ||
|
|
||
| %% API | ||
| -export([format_httpc_error/1]). | ||
| -export([format_shotgun_error/1]). | ||
|
|
||
| %%=================================================================== | ||
| %% API | ||
| %%=================================================================== | ||
|
|
||
| %% @doc Format httpc error for safe logging | ||
| %% | ||
| %% This function provides an abstract representation of httpc error reasons | ||
| %% that is safe to log. It only formats known error types. Unknown | ||
| %% error types are represented as "unknown error" to prevent accidental | ||
| %% exposure of SDK keys, especially in cases where the SDK key might | ||
| %% contain special characters like newlines. | ||
| %% @end | ||
| -spec format_httpc_error(Reason :: term()) -> string(). | ||
| %% HTTP status codes or other integers are safe to log | ||
| format_httpc_error(StatusCode) when is_integer(StatusCode) -> | ||
| lists:flatten(io_lib:format("~b", [StatusCode])); | ||
| %% Common httpc connection errors | ||
| format_httpc_error({failed_connect, _}) -> | ||
| "failed to connect"; | ||
| format_httpc_error(timeout) -> | ||
| "timeout opening connection"; | ||
| format_httpc_error(etimedout) -> | ||
| "connection timeout"; | ||
| format_httpc_error(econnrefused) -> | ||
| "connection refused"; | ||
| format_httpc_error(enetunreach) -> | ||
| "network unreachable"; | ||
| format_httpc_error(ehostunreach) -> | ||
| "host unreachable"; | ||
| format_httpc_error(nxdomain) -> | ||
| "domain name not found"; | ||
| format_httpc_error({tls_alert, {_, Description}}) when is_atom(Description) -> | ||
| lists:flatten(io_lib:format("tls_alert: ~p", [Description])); | ||
| format_httpc_error({tls_alert, Alert}) -> | ||
| lists:flatten(io_lib:format("tls_alert: ~p", [Alert])); | ||
| %% Socket errors | ||
| format_httpc_error({socket_closed_remotely, _, _}) -> | ||
| "socket closed remotely"; | ||
| format_httpc_error(closed) -> | ||
| "connection closed"; | ||
| format_httpc_error(enotconn) -> | ||
| "socket not connected"; | ||
| %% Known atom errors | ||
| format_httpc_error(Reason) when is_atom(Reason) -> | ||
| atom_to_list(Reason); | ||
| %% For any unknown error type, do not expose details | ||
| format_httpc_error(_Reason) -> | ||
| "unknown error". | ||
|
|
||
| %% @doc Format shotgun/gun error for safe logging | ||
| %% | ||
| %% This function provides an abstract representation of shotgun error reasons | ||
| %% that is safe to log. Shotgun uses gun underneath, so errors can come from gun. | ||
| %% Unknown error types are represented as "unknown error". | ||
| %% @end | ||
| -spec format_shotgun_error(Reason :: term()) -> string(). | ||
| %% HTTP status codes or other integers are safe to log | ||
| format_shotgun_error(StatusCode) when is_integer(StatusCode) -> | ||
| lists:flatten(io_lib:format("~b", [StatusCode])); | ||
| %% Known shotgun errors from open | ||
| format_shotgun_error(gun_open_failed) -> | ||
| "connection failed to open"; | ||
| format_shotgun_error(gun_open_timeout) -> | ||
| "timeout opening connection"; | ||
| %% Gun/socket errors | ||
| format_shotgun_error(timeout) -> | ||
| "connection timeout"; | ||
| format_shotgun_error(econnrefused) -> | ||
| "connection refused"; | ||
| format_shotgun_error(enetunreach) -> | ||
| "network unreachable"; | ||
| format_shotgun_error(ehostunreach) -> | ||
| "host unreachable"; | ||
| format_shotgun_error(nxdomain) -> | ||
| "domain name not found"; | ||
| format_shotgun_error({shutdown, _}) -> | ||
| "connection shutdown"; | ||
| format_shotgun_error(normal) -> | ||
| "connection closed normally"; | ||
| format_shotgun_error(closed) -> | ||
| "connection closed"; | ||
| %% Known atom errors | ||
| format_shotgun_error(Reason) when is_atom(Reason) -> | ||
| atom_to_list(Reason); | ||
| %% For any unknown error type, do not expose details | ||
| format_shotgun_error(_Reason) -> | ||
| "unknown error". |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| -module(ldclient_key_redaction_SUITE). | ||
|
|
||
| -include_lib("common_test/include/ct.hrl"). | ||
|
|
||
| %% ct functions | ||
| -export([all/0]). | ||
| -export([init_per_suite/1]). | ||
| -export([end_per_suite/1]). | ||
|
|
||
| %% Tests | ||
| -export([ | ||
| format_httpc_error_atom/1, | ||
| format_httpc_error_integer/1, | ||
| format_httpc_error_tuple/1, | ||
| format_httpc_error_unknown/1, | ||
| format_shotgun_error_atom/1, | ||
| format_shotgun_error_integer/1, | ||
| format_shotgun_error_unknown/1 | ||
| ]). | ||
|
|
||
| %%==================================================================== | ||
| %% ct functions | ||
| %%==================================================================== | ||
|
|
||
| all() -> | ||
| [ | ||
| format_httpc_error_atom, | ||
| format_httpc_error_integer, | ||
| format_httpc_error_tuple, | ||
| format_httpc_error_unknown, | ||
| format_shotgun_error_atom, | ||
| format_shotgun_error_integer, | ||
| format_shotgun_error_unknown | ||
| ]. | ||
|
|
||
| init_per_suite(Config) -> | ||
| Config. | ||
|
|
||
| end_per_suite(_) -> | ||
| ok. | ||
|
|
||
| %%==================================================================== | ||
| %% Tests | ||
| %%==================================================================== | ||
|
|
||
| format_httpc_error_atom(_) -> | ||
| % Known atom errors should be formatted safely | ||
| "timeout opening connection" = ldclient_key_redaction:format_httpc_error(timeout), | ||
| "connection refused" = ldclient_key_redaction:format_httpc_error(econnrefused), | ||
| "connection timeout" = ldclient_key_redaction:format_httpc_error(etimedout). | ||
|
|
||
| format_httpc_error_integer(_) -> | ||
| % HTTP status codes should be formatted as integers | ||
| "404" = ldclient_key_redaction:format_httpc_error(404), | ||
| "500" = ldclient_key_redaction:format_httpc_error(500), | ||
| "200" = ldclient_key_redaction:format_httpc_error(200). | ||
|
|
||
| format_httpc_error_tuple(_) -> | ||
| % Known tuple errors should be formatted safely | ||
| "failed to connect" = ldclient_key_redaction:format_httpc_error({failed_connect, []}), | ||
| "connection closed" = ldclient_key_redaction:format_httpc_error(closed). | ||
|
|
||
| format_httpc_error_unknown(_) -> | ||
| % Unknown error types, including those that might contain SDK keys, should be redacted | ||
| "unknown error" = ldclient_key_redaction:format_httpc_error({http_error, "sdk-12345"}), | ||
| "unknown error" = ldclient_key_redaction:format_httpc_error(["Authorization: sdk-test\n"]), | ||
| "unknown error" = ldclient_key_redaction:format_httpc_error({request, [{headers, [{"Authorization", "sdk-key-with-newline\n"}]}]}). | ||
|
|
||
| format_shotgun_error_atom(_) -> | ||
| % Known atom errors should be formatted safely | ||
| "connection timeout" = ldclient_key_redaction:format_shotgun_error(timeout), | ||
| "connection failed to open" = ldclient_key_redaction:format_shotgun_error(gun_open_failed), | ||
| "timeout opening connection" = ldclient_key_redaction:format_shotgun_error(gun_open_timeout), | ||
| "connection refused" = ldclient_key_redaction:format_shotgun_error(econnrefused). | ||
|
|
||
| format_shotgun_error_integer(_) -> | ||
| % HTTP status codes should be formatted as integers | ||
| "401" = ldclient_key_redaction:format_shotgun_error(401), | ||
| "500" = ldclient_key_redaction:format_shotgun_error(500). | ||
|
|
||
| format_shotgun_error_unknown(_) -> | ||
| % Unknown error types, including those that might contain SDK keys, should be redacted | ||
| "unknown error" = ldclient_key_redaction:format_shotgun_error({gun_error, "sdk-12345"}), | ||
| "unknown error" = ldclient_key_redaction:format_shotgun_error(["Headers: sdk-test\n"]), | ||
| "unknown error" = ldclient_key_redaction:format_shotgun_error({connection_error, [{auth, "sdk-malformed\nkey"}]}). |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
State#{sdk_key => "[REDACTED]"}would be like{...State, sdk_key: "[REDACTED]"]}in JS.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively, should we log the auth suffix? That might make it more useful?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For erlang each SDK instance has a
tagand that will be included in the state. Sodefaultfor the default instance, and then subsequent instances you have to provide atagfor.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which provides similar functionality to the SDK key suffix, without any potential issues from short keys, newlines, or other malformed nonsense.