fix: give the refresh CLI the search PATH and reject non-HTTP system proxies

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<String> {
     let value = value.trim();
     if value.is_empty() {
@@ -64,10 +65,15 @@ pub fn parse_proxy_server(value: &str) -> Option<String> {
     } 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!(

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)]

src/usage/refresh.rs

@@ -146,9 +146,25 @@ fn command_for(name: &str, args: &[&str]) -> Option<Command> {
     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<Command> {
+    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);
+    }
 }