-
-
Notifications
You must be signed in to change notification settings - Fork 228
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
fix: Redact auth tokens when logging CLI args #2115
Merged
Merged
+41
−8
Conversation
This file contains 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
Redact anything that might be an auth token when logging the command line arguments to console. This occurs only when the log level is set to `info` or `debug`; the default is `warn`.
d594ae1
to
2665d8a
Compare
Swatinem
approved these changes
Aug 1, 2024
mdtro
approved these changes
Aug 1, 2024
b55b354
to
35547f7
Compare
Base automatically changed from
szokeasaurusrex/delete-unnecessary-test
to
master
August 1, 2024 15:52
The test ensures that when the CLI args are echoed back, anything which might reasonably be an auth token is redacted.
35547f7
to
3d05326
Compare
szokeasaurusrex
added a commit
that referenced
this pull request
Aug 2, 2024
#2115 aimed to redact auth tokens when logging the arguments to the CLI. Although that change addressed some cases where auth tokens were passed as a CLI argument, not all cases were addressed. For example, the following was redacted properly with #2115: ```sh sentry-cli --auth-token this-gets-redacted --log-level=info info ``` But, the following was not: ```sh sentry-cli --auth-token=this-does-not-get-redacted --log-level=info info ``` The difference is that in the second example, the auth token is passed with `--auth-token=token` rather than separated by whitespace `--auth-token token`. This change improves the redacting so that auth tokens passed like `--auth-token=token` are also redacted. The change also redacts any non-whitespace-containing substrings starting with `sntrys_` or `sntryu_` (prefixes that all auth tokens generated in the latest version of Sentry should start with), so that if an auth token appears where it is not expected, we redact it. For example, the following would be redacted with this change: ```sh sentry-cli --auth=sntrys_my-token-passed-as-non-existing-auth-argument --log-level=info info ``` Note that as in #2115, this change is only relevant in the case where the log level is set to `info` or `debug` (the default is `warn`) – command line arguments are logged at the `info` level.
szokeasaurusrex
added a commit
that referenced
this pull request
Aug 2, 2024
#2115 aimed to redact auth tokens when logging the arguments to the CLI. Although that change addressed some cases where auth tokens were passed as a CLI argument, not all cases were addressed. For example, the following was redacted properly with #2115: ```sh sentry-cli --auth-token this-gets-redacted --log-level=info info ``` But, the following was not: ```sh sentry-cli --auth-token=this-does-not-get-redacted --log-level=info info ``` The difference is that in the second example, the auth token is passed with `--auth-token=token` rather than separated by whitespace `--auth-token token`. This change improves the redacting so that auth tokens passed like `--auth-token=token` are also redacted. The change also redacts any non-whitespace-containing substrings starting with `sntrys_` or `sntryu_` (prefixes that all auth tokens generated in the latest version of Sentry should start with), so that if an auth token appears where it is not expected, we redact it. For example, the following would be redacted with this change: ```sh sentry-cli --auth=sntrys_my-token-passed-as-non-existing-auth-argument --log-level=info info ``` Note that as in #2115, this change is only relevant in the case where the log level is set to `info` or `debug` (the default is `warn`) – command line arguments are logged at the `info` level.
This was referenced Sep 10, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Redact anything that might be an auth token when logging the command line arguments to console. This occurs only when the log level is set to
info
ordebug
; the default iswarn
.