Skip to content

Commit

Permalink
fix: Improve token redaction in CLI arg logging
Browse files Browse the repository at this point in the history
#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.
  • Loading branch information
szokeasaurusrex committed Aug 2, 2024
1 parent 8b89630 commit 5c1ac1f
Show file tree
Hide file tree
Showing 7 changed files with 111 additions and 26 deletions.
40 changes: 26 additions & 14 deletions src/commands/mod.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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<G: Generator>(gen: G, cmd: &mut Command) {
generate(gen, cmd, cmd.get_name().to_string(), &mut io::stdout());
}
Expand Down Expand Up @@ -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."),
Expand Down Expand Up @@ -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::<Vec<String>>()
.collect::<Vec<_>>()
.join(" ")
);

Expand Down
11 changes: 1 addition & 10 deletions src/utils/auth_token/auth_token_impl.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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()
}
4 changes: 3 additions & 1 deletion src/utils/auth_token/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
66 changes: 66 additions & 0 deletions src/utils/auth_token/redacting.rs
Original file line number Diff line number Diff line change
@@ -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);
}
}
9 changes: 9 additions & 0 deletions tests/integration/_cases/token-redacted-2.trycmd
Original file line number Diff line number Diff line change
@@ -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"
...

```
2 changes: 1 addition & 1 deletion tests/integration/_cases/token-redacted.trycmd
Original file line number Diff line number Diff line change
Expand Up @@ -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"
...

```
5 changes: 5 additions & 0 deletions tests/integration/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}

0 comments on commit 5c1ac1f

Please sign in to comment.