Skip to content
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: Improve token redaction in CLI arg logging #2118

Merged
merged 1 commit into from
Aug 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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");
}
Loading