From 72b9418c05f8d03647707e327824d1e5b3c64e67 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 24 Oct 2026 22:18:27 -0400 Subject: [PATCH] fix(security): address review comments on proxy and logging - Redact Bearer tokens with multiple spaces and add regression test. - Explicitly disable proxies on direct and direct_download ureq agents. - Add direct_download agent with download timeout profile for loopback downloads. - Preserve proxy username, password, and NO_PROXY exclusions in proxy::pick(). --- apps/artcraft-launcher/src/logging.rs | 11 +++++++++-- crates/native/src/http.rs | 16 ++++++++++++++-- crates/native/src/proxy.rs | 14 ++++++++++++++ 3 files changed, 37 insertions(+), 4 deletions(-) diff --git a/apps/artcraft-launcher/src/logging.rs b/apps/artcraft-launcher/src/logging.rs index 9dee228c..f06b8ea5 100644 --- a/apps/artcraft-launcher/src/logging.rs +++ b/apps/artcraft-launcher/src/logging.rs @@ -124,8 +124,14 @@ pub(crate) fn redact_credentials(text: &str) -> String { let mut result = String::with_capacity(text.len()); let mut remaining = text; while let Some(idx) = remaining.to_ascii_lowercase().find("bearer ") { - result.push_str(&remaining[..idx + 7]); - let token_rest = &remaining[idx + 7..]; + let prefix_len = idx + 7; + result.push_str(&remaining[..prefix_len]); + let mut token_rest = &remaining[prefix_len..]; + let spaces = token_rest.chars().take_while(|c| c.is_whitespace() && *c != '\r' && *c != '\n').count(); + if spaces > 0 { + result.push_str(&token_rest[..spaces]); + token_rest = &token_rest[spaces..]; + } let end_token = token_rest.find(|c: char| c.is_whitespace() || c == '"' || c == '\'' || c == '\r' || c == '\n').unwrap_or(token_rest.len()); result.push_str("[REDACTED]"); remaining = &token_rest[end_token..]; @@ -352,6 +358,7 @@ mod tests { #[test] fn redact_credentials_replaces_bearer_tokens() { assert_eq!(redact_credentials("Authorization: Bearer ghp_secretToken1234567890"), "Authorization: Bearer [REDACTED]"); + assert_eq!(redact_credentials("Authorization: Bearer secret_double_space"), "Authorization: Bearer [REDACTED]"); assert_eq!(redact_credentials("header \"authorization: bearer my_secret\" and more"), "header \"authorization: bearer [REDACTED]\" and more"); assert_eq!(redact_credentials("normal log line without tokens"), "normal log line without tokens"); } diff --git a/crates/native/src/http.rs b/crates/native/src/http.rs index 982f585e..4bdc49c7 100644 --- a/crates/native/src/http.rs +++ b/crates/native/src/http.rs @@ -37,6 +37,7 @@ pub struct Http { no_redirect: ureq::Agent, download: ureq::Agent, direct: ureq::Agent, + direct_download: ureq::Agent, token: Option, } @@ -106,6 +107,7 @@ impl Http { .build() .new_agent(); let direct = ureq::Agent::config_builder() + .proxy(None) .tls_config(tls()) .timeout_connect(Some(Duration::from_secs(5))) .timeout_global(Some(Duration::from_secs(20))) @@ -113,10 +115,20 @@ impl Http { .user_agent(USER_AGENT) .build() .new_agent(); + let direct_download = ureq::Agent::config_builder() + .proxy(None) + .tls_config(tls()) + .timeout_connect(Some(Duration::from_secs(20))) + .timeout_recv_response(Some(Duration::from_secs(60))) + .timeout_recv_body(Some(ATTEMPT_BODY_TIME)) + .http_status_as_error(false) + .user_agent(USER_AGENT) + .build() + .new_agent(); // An optional token raises GitHub's limit from 60 to 5000 requests an hour. Only ever sent // to api.github.com. let token = std::env::var("ARTCRAFT_LAUNCHER_GITHUB_TOKEN").ok().map(|t| t.trim().to_string()).filter(|t| !t.is_empty()); - Ok(Self { small, feed, no_redirect, download, direct, token }) + Ok(Self { small, feed, no_redirect, download, direct, direct_download, token }) } /// GET a GitHub API resource, revalidating with `etag` when given. @@ -306,7 +318,7 @@ impl Http { fn attempt(&self, url: &str, part: &Path, have: u64, size: u64, stop: &Stop, progress: &mut dyn FnMut(Transfer)) -> Result<(), AttemptError> { let retry = |error: String, progressed: bool| AttemptError::Retry { error, progressed }; // Binaries are already compressed; ask for the bytes exactly as published. - let agent = if is_loopback(url) { &self.direct } else { &self.download }; + let agent = if is_loopback(url) { &self.direct_download } else { &self.download }; let mut req = agent.get(url).header("Accept-Encoding", "identity"); if have > 0 { req = req.header("Range", &format!("bytes={have}-")); diff --git a/crates/native/src/proxy.rs b/crates/native/src/proxy.rs index 5b4bd8a7..87aa3524 100644 --- a/crates/native/src/proxy.rs +++ b/crates/native/src/proxy.rs @@ -20,6 +20,20 @@ pub struct SystemProxy { pub fn pick() -> Option { if let Some(proxy) = Proxy::try_from_env() { let mut builder = Proxy::builder(proxy.protocol()).host(proxy.host()).port(proxy.port()); + if let Some(user) = proxy.username() { + builder = builder.username(user); + } + if let Some(pass) = proxy.password() { + builder = builder.password(pass); + } + if let Ok(no_proxy_env) = std::env::var("NO_PROXY").or_else(|_| std::env::var("no_proxy")) { + for entry in no_proxy_env.split(',') { + let entry = entry.trim(); + if !entry.is_empty() { + builder = builder.no_proxy(entry); + } + } + } for entry in ["localhost", "127.0.0.1", "[::1]"] { builder = builder.no_proxy(entry); }