Skip to content

Commit 27a41d6

Browse files
committed
Auto merge of #12095 - ehuss:fix-token-redact, r=weihanglo
Fix redacting tokens in http debug. Unfortunately it seems like #8222 didn't properly redact tokens when connecting to an http2 server. There were multiple problems: * For some reason, curl changes the authorization header to be lowercase when using http2. * Curl also logs the h2h3 lines separately with a different syntax. This fixes it by checking for these additional cases. This also adds a test, but it doesn't actually detect this problem because we don't have an http2 server handy. You can test this yourself by running `CARGO_LOG=trace CARGO_HTTP_DEBUG=true cargo publish --token a-unique-token --allow-dirty --no-verify`, and verifying the output does not contain the given token text.
2 parents 5409b7f + d6c20cf commit 27a41d6

File tree

2 files changed

+79
-3
lines changed

2 files changed

+79
-3
lines changed

src/cargo/ops/registry.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -704,12 +704,17 @@ pub fn configure_http_handle(config: &Config, handle: &mut Easy) -> CargoResult<
704704
InfoType::SslDataIn | InfoType::SslDataOut => return,
705705
_ => return,
706706
};
707+
let starts_with_ignore_case = |line: &str, text: &str| -> bool {
708+
line[..line.len().min(text.len())].eq_ignore_ascii_case(text)
709+
};
707710
match str::from_utf8(data) {
708711
Ok(s) => {
709712
for mut line in s.lines() {
710-
if line.starts_with("Authorization:") {
713+
if starts_with_ignore_case(line, "authorization:") {
711714
line = "Authorization: [REDACTED]";
712-
} else if line[..line.len().min(10)].eq_ignore_ascii_case("set-cookie") {
715+
} else if starts_with_ignore_case(line, "h2h3 [authorization:") {
716+
line = "h2h3 [Authorization: [REDACTED]]";
717+
} else if starts_with_ignore_case(line, "set-cookie") {
713718
line = "set-cookie: [REDACTED]";
714719
}
715720
log!(level, "http-debug: {} {}", prefix, line);

tests/testsuite/registry_auth.rs

Lines changed: 72 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
//! Tests for registry authentication.
22
3-
use cargo_test_support::registry::{Package, RegistryBuilder};
3+
use cargo_test_support::compare::match_contains;
4+
use cargo_test_support::registry::{Package, RegistryBuilder, Token};
45
use cargo_test_support::{project, Execs, Project};
56

67
fn cargo(p: &Project, s: &str) -> Execs {
@@ -517,3 +518,73 @@ Caused by:
517518
)
518519
.run();
519520
}
521+
522+
#[cargo_test]
523+
fn token_not_logged() {
524+
// Checks that the token isn't displayed in debug output (for both HTTP
525+
// index and registry API). Note that this doesn't fully verify the
526+
// correct behavior since we don't have an HTTP2 server, and curl behaves
527+
// significantly differently when using HTTP2.
528+
let crates_io = RegistryBuilder::new()
529+
.http_api()
530+
.http_index()
531+
.auth_required()
532+
.token(Token::Plaintext("a-unique_token".to_string()))
533+
.build();
534+
Package::new("bar", "1.0.0").publish();
535+
let p = project()
536+
.file(
537+
"Cargo.toml",
538+
r#"
539+
[package]
540+
name = "foo"
541+
version = "0.1.0"
542+
543+
[dependencies]
544+
bar = "1.0"
545+
"#,
546+
)
547+
.file("src/lib.rs", "")
548+
.build();
549+
let output = cargo(&p, "publish")
550+
.replace_crates_io(crates_io.index_url())
551+
.env("CARGO_HTTP_DEBUG", "true")
552+
.env("CARGO_LOG", "trace")
553+
.exec_with_output()
554+
.unwrap();
555+
let log = String::from_utf8(output.stderr).unwrap();
556+
let lines = "\
557+
[UPDATING] crates.io index
558+
[PACKAGING] foo v0.1.0 [..]
559+
[VERIFYING] foo v0.1.0 [..]
560+
[DOWNLOADING] crates ...
561+
[DOWNLOADED] bar v1.0.0
562+
[COMPILING] bar v1.0.0
563+
[COMPILING] foo v0.1.0 [..]
564+
[FINISHED] [..]
565+
[PACKAGED] 3 files[..]
566+
[UPLOADING] foo v0.1.0[..]
567+
[UPLOADED] foo v0.1.0 to registry `crates-io`
568+
note: Waiting [..]
569+
";
570+
for line in lines.lines() {
571+
match_contains(line, &log, None).unwrap();
572+
}
573+
let authorizations: Vec<_> = log
574+
.lines()
575+
.filter(|line| {
576+
line.contains("http-debug:") && line.to_lowercase().contains("authorization")
577+
})
578+
.collect();
579+
assert!(authorizations.iter().all(|line| line.contains("REDACTED")));
580+
// Total authorizations:
581+
// 1. Initial config.json
582+
// 2. config.json again for verification
583+
// 3. /index/3/b/bar
584+
// 4. /dl/bar/1.0.0/download
585+
// 5. /api/v1/crates/new
586+
// 6. config.json for the "wait for publish"
587+
// 7. /index/3/f/foo for the "wait for publish"
588+
assert_eq!(authorizations.len(), 7);
589+
assert!(!log.contains("a-unique_token"));
590+
}

0 commit comments

Comments
 (0)