From b8e672f7509e9e427f531d8848e705bd2d32f2f9 Mon Sep 17 00:00:00 2001 From: tiennm99 Date: Wed, 21 Oct 2026 12:52:22 +0700 Subject: [PATCH] fix: give the refresh CLI the search PATH and reject non-HTTP system proxies --- src/core/proxy.rs | 31 +++++++++++++++++++---- src/net/mod.rs | 23 ++++++++++++++++- src/usage/refresh.rs | 60 ++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 106 insertions(+), 8 deletions(-) diff --git a/src/core/proxy.rs b/src/core/proxy.rs index dfc952a..a0c2218 100644 --- a/src/core/proxy.rs +++ b/src/core/proxy.rs @@ -41,7 +41,8 @@ pub fn from_registry_values( /// /// A per-scheme value (`http=h:p;https=h:p;…`) uses the `https=` entry, else /// the `http=` entry. A bare `h:p` applies to every scheme. The result always -/// carries a scheme, defaulting to `http://` (a CONNECT proxy). +/// carries a scheme, defaulting to `http://` (a CONNECT proxy). Only `http` +/// and `https` proxies are accepted; any other scheme yields `None`. pub fn parse_proxy_server(value: &str) -> Option { let value = value.trim(); if value.is_empty() { @@ -64,10 +65,15 @@ pub fn parse_proxy_server(value: &str) -> Option { } else { value }; - if target.contains("://") { - Some(target.to_string()) - } else { - Some(format!("http://{target}")) + match target.split_once("://") { + None => Some(format!("http://{target}")), + Some((scheme, _)) + if scheme.eq_ignore_ascii_case("http") || scheme.eq_ignore_ascii_case("https") => + { + Some(target.to_string()) + } + // SOCKS and any other scheme are not supported as a system proxy. + Some(_) => None, } } @@ -162,6 +168,21 @@ mod tests { assert_eq!(parse_proxy_server(" "), None); } + #[test] + fn non_http_proxy_schemes_are_rejected() { + assert_eq!(parse_proxy_server("socks5://h:1"), None); + assert_eq!(parse_proxy_server("https=socks5://h:1"), None); + assert_eq!(parse_proxy_server("SOCKS4://h:1"), None); + assert_eq!( + parse_proxy_server("HTTPS://h:1").as_deref(), + Some("HTTPS://h:1") + ); + assert_eq!( + from_registry_values(Some(1), Some("socks5://h:1"), None), + None + ); + } + #[test] fn registry_values_need_enable_flag_and_server() { assert_eq!( diff --git a/src/net/mod.rs b/src/net/mod.rs index 754eb79..349b6f5 100644 --- a/src/net/mod.rs +++ b/src/net/mod.rs @@ -11,7 +11,7 @@ use serde::de::DeserializeOwned; use serde::Serialize; use ureq::http::{self, Uri}; use ureq::tls::{RootCerts, TlsConfig}; -use ureq::{Agent, Proxy}; +use ureq::{Agent, Proxy, ProxyProtocol}; use crate::core::proxy::SystemProxy; @@ -99,6 +99,13 @@ impl Client { let direct = make_agent(user_agent, env_proxy); let proxied = match system { Some(proxy) => match Proxy::new(&proxy.url) { + // Without ureq's `socks-proxy` feature a manually configured + // SOCKS proxy panics at connect time, so only CONNECT proxies + // are accepted here. + Ok(p) if !matches!(p.protocol(), ProxyProtocol::Http | ProxyProtocol::Https) => { + log::warn!("system proxy ignored, only http and https proxies are supported"); + None + } Ok(p) => Some(Proxied { agent: make_agent(user_agent, Some(p)), proxy, @@ -425,6 +432,20 @@ mod tests { .expect("client"); assert_eq!(client.system_proxy(), None); } + + #[test] + fn socks_system_proxy_falls_back_to_direct() { + let client = Client::build( + USER_AGENT, + None, + Some(SystemProxy { + url: "socks5://127.0.0.1:1080".into(), + bypass: Vec::new(), + }), + ) + .expect("client"); + assert_eq!(client.system_proxy(), None); + } } #[cfg(test)] diff --git a/src/usage/refresh.rs b/src/usage/refresh.rs index 010a471..b13c871 100644 --- a/src/usage/refresh.rs +++ b/src/usage/refresh.rs @@ -146,9 +146,25 @@ fn command_for(name: &str, args: &[&str]) -> Option { let path_var = std::env::var_os("PATH"); let home = dirs::home_dir(); let dirs = search_dirs(path_var.as_deref(), home.as_deref()); - let program = find_executable(name, &dirs)?; + command_in(name, args, &dirs) +} + +/// Resolve `name` in `dirs` and run it with `PATH` set to the same list. +/// npm-installed CLIs are `#!/usr/bin/env node` scripts, so the child has to +/// see the extra install directories too: a GUI session's short `PATH` +/// would let the script start and then exit at once with "node not found". +#[cfg(unix)] +fn command_in(name: &str, args: &[&str], dirs: &[std::path::PathBuf]) -> Option { + let program = find_executable(name, dirs)?; + let path = match std::env::join_paths(dirs) { + Ok(path) => path, + Err(err) => { + log::warn!("refresh CLI search path cannot be joined: {err}"); + return None; + } + }; let mut cmd = Command::new(program); - cmd.args(args); + cmd.args(args).env("PATH", path); Some(cmd) } @@ -291,4 +307,44 @@ mod tests { assert!(!spawn_first(&["definitely-not-a-cli-name"], &[])); let _ = std::fs::remove_dir_all(dir); } + + #[test] + fn child_path_resolves_env_interpreter_from_extra_dirs() { + let tools = temp_dir("tools"); + let bin = temp_dir("bin"); + let marker = bin.join("ran"); + let helper = "ccub-fake-interpreter"; + std::fs::write( + tools.join(helper), + format!("#!/bin/sh\necho interpreted > '{}'\n", marker.display()), + ) + .expect("write helper"); + std::fs::set_permissions(tools.join(helper), std::fs::Permissions::from_mode(0o755)) + .expect("chmod helper"); + std::fs::write(bin.join("fake-cli"), format!("#!/usr/bin/env {helper}\n")) + .expect("write script"); + std::fs::set_permissions(bin.join("fake-cli"), std::fs::Permissions::from_mode(0o755)) + .expect("chmod script"); + let dirs = vec![ + PathBuf::from("/usr/bin"), + PathBuf::from("/bin"), + bin.clone(), + tools.clone(), + ]; + let mut cmd = command_in("fake-cli", &[], &dirs).expect("script is found"); + let status = cmd + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status() + .expect("script runs"); + assert!(status.success(), "child exited with {status}"); + assert_eq!( + std::fs::read_to_string(&marker).unwrap_or_default().trim(), + "interpreted" + ); + assert!(command_in("absent-cli", &[], &dirs).is_none()); + let _ = std::fs::remove_dir_all(tools); + let _ = std::fs::remove_dir_all(bin); + } }