From 5c1ac1ff76c65648ace6798516297af11a2e9bdd Mon Sep 17 00:00:00 2001 From: Daniel Szoke Date: Fri, 2 Aug 2024 11:47:44 +0200 Subject: [PATCH] fix: Improve token redaction in CLI arg logging MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #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. --- src/commands/mod.rs | 40 +++++++---- src/utils/auth_token/auth_token_impl.rs | 11 +--- src/utils/auth_token/mod.rs | 4 +- src/utils/auth_token/redacting.rs | 66 +++++++++++++++++++ .../_cases/token-redacted-2.trycmd | 9 +++ .../integration/_cases/token-redacted.trycmd | 2 +- tests/integration/mod.rs | 5 ++ 7 files changed, 111 insertions(+), 26 deletions(-) create mode 100644 src/utils/auth_token/redacting.rs create mode 100644 tests/integration/_cases/token-redacted-2.trycmd diff --git a/src/commands/mod.rs b/src/commands/mod.rs index e221d9b82c..b224d3e126 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -1,19 +1,18 @@ //! This module implements the root command of the CLI tool. -use std::io; -use std::process; -use std::{env, iter}; - use anyhow::{bail, Result}; use clap::{value_parser, Arg, ArgAction, ArgMatches, Command}; use clap_complete::{generate, Generator, Shell}; use log::{debug, info, set_logger, set_max_level, LevelFilter}; +use std::borrow::Cow; +use std::io; +use std::process; +use std::{env, iter}; use crate::api::Api; use crate::config::{Auth, Config}; use crate::constants::{ARCH, PLATFORM, VERSION}; -use crate::utils::auth_token; -use crate::utils::auth_token::AuthToken; +use crate::utils::auth_token::{redact_token_from_string, AuthToken}; use crate::utils::logging::set_quiet_mode; use crate::utils::logging::Logger; use crate::utils::system::{init_backtrace, load_dotenv, print_error, QuietExit}; @@ -83,6 +82,9 @@ const UPDATE_NAGGER_CMDS: &[&str] = &[ "sourcemaps", ]; +/// The long auth token argument (--auth-token). +const AUTH_TOKEN_ARG: &str = "auth-token"; + fn print_completions(gen: G, cmd: &mut Command) { generate(gen, cmd, cmd.get_name().to_string(), &mut io::stdout()); } @@ -165,7 +167,7 @@ fn app() -> Command { .arg( Arg::new("auth_token") .value_name("AUTH_TOKEN") - .long("auth-token") + .long(AUTH_TOKEN_ARG) .global(true) .value_parser(auth_token_parser) .help("Use the given Sentry auth token."), @@ -283,15 +285,25 @@ pub fn execute() -> Result<()> { "sentry-cli was invoked with the following command line: {}", env::args() // Check whether the previous argument is "--auth-token" - .zip(iter::once(false).chain(env::args().map(|arg| arg == "--auth-token"))) + .zip( + iter::once(false) + .chain(env::args().map(|arg| arg == format!("--{AUTH_TOKEN_ARG}"))) + ) .map(|(a, is_auth_token_arg)| { - // Redact anything that comes after --auth-token or looks like a token - if is_auth_token_arg || auth_token::looks_like_auth_token(&a) { - return String::from("(redacted)"); - } - format!("\"{a}\"") + let redact_replacement = "[REDACTED]"; + + // Redact anything that comes after --auth-token + let redacted = if is_auth_token_arg { + Cow::Borrowed(redact_replacement) + } else if a.starts_with(&format!("--{AUTH_TOKEN_ARG}=")) { + Cow::Owned(format!("--{AUTH_TOKEN_ARG}={redact_replacement}")) + } else { + redact_token_from_string(&a, redact_replacement) + }; + + format!("\"{redacted}\"") }) - .collect::>() + .collect::>() .join(" ") ); diff --git a/src/utils/auth_token/auth_token_impl.rs b/src/utils/auth_token/auth_token_impl.rs index 44dea852f3..a4be00478a 100644 --- a/src/utils/auth_token/auth_token_impl.rs +++ b/src/utils/auth_token/auth_token_impl.rs @@ -1,6 +1,6 @@ //! Defines the AuthToken type, which stores a Sentry auth token. -use super::{AuthTokenPayload, ORG_AUTH_TOKEN_PREFIX, USER_TOKEN_PREFIX}; +use super::AuthTokenPayload; use super::{OrgAuthToken, UserAuthToken}; use secrecy::SecretString; @@ -92,12 +92,3 @@ impl AuthTokenInner { } } } - -/// Returns whether a given string looks like it might be an auth token. -/// Specifically, we say a string looks like an auth token when it starts with one of the auth -/// token prefixes (sntrys_ or sntryu_) or passes the auth token soft validation. -pub fn looks_like_auth_token(s: &str) -> bool { - s.starts_with(ORG_AUTH_TOKEN_PREFIX) - || s.starts_with(USER_TOKEN_PREFIX) - || AuthToken::from(s).format_recognized() -} diff --git a/src/utils/auth_token/mod.rs b/src/utils/auth_token/mod.rs index 6624cbb613..71d977a2fb 100644 --- a/src/utils/auth_token/mod.rs +++ b/src/utils/auth_token/mod.rs @@ -3,10 +3,12 @@ mod auth_token_impl; mod error; mod org_auth_token; +mod redacting; mod user_auth_token; -pub use auth_token_impl::{looks_like_auth_token, AuthToken}; +pub use auth_token_impl::AuthToken; pub use org_auth_token::AuthTokenPayload; +pub use redacting::redact_token_from_string; use error::{AuthTokenParseError, Result}; use org_auth_token::OrgAuthToken; diff --git a/src/utils/auth_token/redacting.rs b/src/utils/auth_token/redacting.rs new file mode 100644 index 0000000000..acdafae0b1 --- /dev/null +++ b/src/utils/auth_token/redacting.rs @@ -0,0 +1,66 @@ +use crate::utils::auth_token::{AuthToken, ORG_AUTH_TOKEN_PREFIX, USER_TOKEN_PREFIX}; +use lazy_static::lazy_static; +use regex::Regex; +use std::borrow::Cow; + +pub fn redact_token_from_string<'r>(to_redact: &'r str, replacement: &'r str) -> Cow<'r, str> { + if AuthToken::from(to_redact).format_recognized() { + // The string is itself an auth token, redact the whole thing + Cow::Borrowed(replacement) + } else { + // Redact any substrings consisting of non-whitespace characters starting with the org or + // user auth token prefixes, as these are likely to be auth tokens. Note that this will + // miss old-style user auth tokens that do not contain the prefix. + lazy_static! { + static ref AUTH_TOKEN_REGEX: Regex = Regex::new(&format!( + "(({ORG_AUTH_TOKEN_PREFIX})|({USER_TOKEN_PREFIX}))\\S+" + )) + .unwrap(); + } + + AUTH_TOKEN_REGEX.replace_all(to_redact, replacement) + } +} + +#[cfg(test)] +mod tests { + use crate::utils::auth_token::redacting::redact_token_from_string; + + #[test] + fn test_no_redaction() { + let input = "This string should remain unchanged."; + + let output = redact_token_from_string(input, "[REDACTED]"); + assert_eq!(input, output); + } + + #[test] + fn test_redaction() { + let input = "Here we have a usersntryu_user/auth@#tok3n\\which_should.be3redacted and a sntrys_org_auth_token,too."; + let expected_output = "Here we have a user[REDACTED] and a [REDACTED]"; + + let output = redact_token_from_string(input, "[REDACTED]"); + assert_eq!(expected_output, output); + } + + #[test] + fn test_redaction_org_auth_token() { + let input = "sntrys_\ + eyJpYXQiOjE3MDQyMDU4MDIuMTk5NzQzLCJ1cmwiOiJodHRwOi8vbG9jYWxob3N0OjgwMDAiLCJyZ\ + Wdpb25fdXJsIjoiaHR0cDovL2xvY2FsaG9zdDo4MDAwIiwib3JnIjoic2VudHJ5In0=_\ + lQ5ETt61cHhvJa35fxvxARsDXeVrd0pu4/smF4sRieA"; + let expected_output = "[REDACTED]"; + + let output = redact_token_from_string(input, "[REDACTED]"); + assert_eq!(expected_output, output); + } + + #[test] + fn test_redaction_old_user_token() { + let input = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; + let expected_output = "[REDACTED]"; + + let output = redact_token_from_string(input, "[REDACTED]"); + assert_eq!(expected_output, output); + } +} diff --git a/tests/integration/_cases/token-redacted-2.trycmd b/tests/integration/_cases/token-redacted-2.trycmd new file mode 100644 index 0000000000..63d7554ed7 --- /dev/null +++ b/tests/integration/_cases/token-redacted-2.trycmd @@ -0,0 +1,9 @@ +``` +$ sentry-cli sourcemaps upload --auth-token=not-following-token-format -o asdf --project=sntrys_project_looks_like_token ./file-sntryu_looks-like-token --log-level=info +? failed +[..] +[..] +[..]INFO[..] sentry-cli was invoked with the following command line: "[..]" "sourcemaps" "upload" "--auth-token=[REDACTED]" "-o" "asdf" "--project=[REDACTED]" "./file-[REDACTED]" "--log-level=info" +... + +``` diff --git a/tests/integration/_cases/token-redacted.trycmd b/tests/integration/_cases/token-redacted.trycmd index f7cacc3d80..90b18e7e6b 100644 --- a/tests/integration/_cases/token-redacted.trycmd +++ b/tests/integration/_cases/token-redacted.trycmd @@ -3,7 +3,7 @@ $ sentry-cli sourcemaps upload --auth-token not-following-token-format -o asdf - ? failed [..] [..] -[..]INFO[..] sentry-cli was invoked with the following command line: "[..]" "sourcemaps" "upload" "--auth-token" (redacted) "-o" "asdf" "-p" (redacted) "./" "--log-level=info" +[..]INFO[..] sentry-cli was invoked with the following command line: "[..]" "sourcemaps" "upload" "--auth-token" "[REDACTED]" "-o" "asdf" "-p" "[REDACTED]" "./" "--log-level=info" ... ``` diff --git a/tests/integration/mod.rs b/tests/integration/mod.rs index 5f73a6ffab..095fa88b7f 100644 --- a/tests/integration/mod.rs +++ b/tests/integration/mod.rs @@ -240,3 +240,8 @@ pub fn assert_endpoints(mocks: &[Mock]) { pub fn token_redacted() { register_test("token-redacted.trycmd"); } + +#[test] +pub fn token_redacted_2() { + register_test("token-redacted-2.trycmd"); +}