fix: chown helper socket to staff and verify readiness via socket ping

homebrew-cask/Casks/fanguard.rb

@@ -1,6 +1,6 @@
 cask "fanguard" do
-  version "0.1.0-beta.2"
-  sha256 "8014795b7edbcd0a56639f4292b5564c7403668b4da1e5da5491db1ce5a7d338"
+  version "0.1.0-beta.3"
+  sha256 "b6af7f31344a7de6cf4d2abc9a605de96d1ea63149e42bd78ff21c181894847c"
 
   url "https://github.com/naufaldi/mac-fan-ctrl/releases/download/v#{version}/FanGuard_#{version}_universal.dmg"
   name "FanGuard"

src-tauri/src/bin/fanguard-helper.rs

@@ -1,3 +1,4 @@
+use std::ffi::CString;
 use std::fs;
 use std::io::{BufRead, BufReader, Read, Write};
 use std::os::unix::fs::PermissionsExt;
@@ -8,6 +9,9 @@ use std::sync::{Arc, Mutex};
 use fanguard_lib::smc_protocol::{HelperRequest, HelperResponse, SOCKET_PATH};
 use fanguard_lib::smc_writer::{SmcWriteApi, SmcWriter};
 
+const HELPER_SOCKET_MODE: u32 = 0o660;
+const ROOT_UID: libc::uid_t = 0;
+
 fn main() {
     eprintln!("[fanguard-helper] Starting privileged helper daemon");
 
@@ -40,8 +44,9 @@ fn main() {
     };
 
     // Restrict socket to owner (root) and group (staff) only — no world access
-    if let Err(e) = fs::set_permissions(SOCKET_PATH, fs::Permissions::from_mode(0o660)) {
-        eprintln!("[fanguard-helper] Failed to set socket permissions: {e}");
+    if let Err(e) = configure_socket_access(SOCKET_PATH) {
+        eprintln!("[fanguard-helper] Failed to configure socket access: {e}");
+        std::process::exit(1);
     }
 
     eprintln!("[fanguard-helper] Listening on {SOCKET_PATH}");
@@ -62,11 +67,9 @@ fn main() {
     while running.load(Ordering::SeqCst) {
         match listener.accept() {
             Ok((stream, _addr)) => {
-                stream
-                    .set_nonblocking(false)
-                    .unwrap_or_else(|e| {
-                        eprintln!("[fanguard-helper] Failed to set blocking on client: {e}")
-                    });
+                stream.set_nonblocking(false).unwrap_or_else(|e| {
+                    eprintln!("[fanguard-helper] Failed to set blocking on client: {e}")
+                });
                 let writer_ref = Arc::clone(&writer);
                 if let Err(e) = handle_client(stream, &writer_ref) {
                     eprintln!("[fanguard-helper] Client error: {e}");
@@ -86,7 +89,9 @@ fn main() {
 }
 
 fn handle_client(stream: UnixStream, writer: &Mutex<SmcWriter>) -> std::io::Result<()> {
-    stream.set_read_timeout(Some(std::time::Duration::from_secs(5))).unwrap_or(());
+    stream
+        .set_read_timeout(Some(std::time::Duration::from_secs(5)))
+        .unwrap_or(());
     let limited = (&stream).take(8192);
     let mut reader = BufReader::new(limited);
     let mut line = String::new();
@@ -118,6 +123,41 @@ fn send_response(stream: &UnixStream, response: &HelperResponse) -> std::io::Res
     writer.write_all(resp_line.as_bytes())
 }
 
+fn helper_socket_group_name() -> &'static str {
+    "staff"
+}
+
+fn configure_socket_access(path: &str) -> std::io::Result<()> {
+    let staff_gid = resolve_group_id(helper_socket_group_name())?;
+    chown_socket(path, ROOT_UID, staff_gid)?;
+    fs::set_permissions(path, fs::Permissions::from_mode(HELPER_SOCKET_MODE))
+}
+
+fn resolve_group_id(group_name: &str) -> std::io::Result<libc::gid_t> {
+    let c_group_name = CString::new(group_name)
+        .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidInput, e))?;
+    let group = unsafe { libc::getgrnam(c_group_name.as_ptr()) };
+    if group.is_null() {
+        return Err(std::io::Error::new(
+            std::io::ErrorKind::NotFound,
+            format!("group '{group_name}' not found"),
+        ));
+    }
+
+    Ok(unsafe { (*group).gr_gid })
+}
+
+fn chown_socket(path: &str, uid: libc::uid_t, gid: libc::gid_t) -> std::io::Result<()> {
+    let c_path =
+        CString::new(path).map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidInput, e))?;
+    let result = unsafe { libc::chown(c_path.as_ptr(), uid, gid) };
+    if result == 0 {
+        Ok(())
+    } else {
+        Err(std::io::Error::last_os_error())
+    }
+}
+
 fn dispatch_request(request: HelperRequest, writer: &Mutex<SmcWriter>) -> HelperResponse {
     let guard = match writer.lock() {
         Ok(g) => g,
@@ -164,3 +204,13 @@ fn dispatch_request(request: HelperRequest, writer: &Mutex<SmcWriter>) -> Helper
         },
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::helper_socket_group_name;
+
+    #[test]
+    fn helper_socket_uses_staff_group_for_gui_user_access() {
+        assert_eq!(helper_socket_group_name(), "staff");
+    }
+}

src-tauri/src/commands.rs

@@ -16,10 +16,11 @@ use crate::smc_writer::{SmcWriteApi, SmcWriter};
 
 pub const SENSOR_UPDATE_EVENT: &str = "sensor_update";
 
-const HELPER_SOCKET: &str = "/var/run/fanguard.sock";
 const HELPER_INSTALL_DIR: &str = "/Library/PrivilegedHelperTools";
 const LAUNCHDAEMON_DIR: &str = "/Library/LaunchDaemons";
 const DAEMON_LABEL: &str = "io.github.naufaldi.fanguard.helper";
+const HELPER_READY_RETRY_COUNT: u8 = 20;
+const HELPER_READY_RETRY_DELAY: std::time::Duration = std::time::Duration::from_millis(250);
 
 // ── Tray handle wrapper ─────────────────────────────────────────────────────
 
@@ -673,15 +674,47 @@ pub fn install_helper() -> Result<String, String> {
         return Err(format!("Installation failed: {stderr}"));
     }
 
-    // Wait for socket to appear
-    for _ in 0..20 {
-        if std::path::Path::new(HELPER_SOCKET).exists() {
-            return Ok("Helper installed and running".to_string());
+    wait_for_helper_ready()?;
+    Ok("Helper installed and running".to_string())
+}
+
+fn wait_for_helper_ready() -> Result<(), String> {
+    wait_for_helper_ready_with(
+        HELPER_READY_RETRY_COUNT,
+        || {
+            SmcSocketClient::new()
+                .map(|_| ())
+                .map_err(|e| e.to_string())
+        },
+        || std::thread::sleep(HELPER_READY_RETRY_DELAY),
+    )
+}
+
+fn wait_for_helper_ready_with<FConnect, FSleep>(
+    attempts: u8,
+    mut connect: FConnect,
+    mut sleep: FSleep,
+) -> Result<(), String>
+where
+    FConnect: FnMut() -> Result<(), String>,
+    FSleep: FnMut(),
+{
+    let mut last_error = "helper did not respond".to_string();
+    for attempt in 0..attempts {
+        match connect() {
+            Ok(()) => return Ok(()),
+            Err(error) => {
+                last_error = error;
+                if attempt + 1 < attempts {
+                    sleep();
+                }
+            }
         }
-        std::thread::sleep(std::time::Duration::from_millis(250));
     }
 
-    Err("Helper installed but socket not found after 5 seconds".to_string())
+    Err(format!(
+        "Helper installed but socket was not reachable after 5 seconds: {last_error}"
+    ))
 }
 
 fn build_helper_install_shell_commands(
@@ -738,7 +771,7 @@ pub fn reconnect_writer(state: State<'_, AppState>) -> Result<bool, String> {
 mod tests {
     use super::{
         build_helper_install_shell_commands, ping_backend, should_try_direct_smc_writer,
-        DAEMON_LABEL, HELPER_INSTALL_DIR, LAUNCHDAEMON_DIR,
+        wait_for_helper_ready_with, DAEMON_LABEL, HELPER_INSTALL_DIR, LAUNCHDAEMON_DIR,
     };
     use std::path::Path;
 
@@ -781,4 +814,32 @@ mod tests {
 
         assert!(commands.contains(&format!("launchctl enable system/{DAEMON_LABEL}")));
     }
+
+    #[test]
+    fn helper_ready_wait_requires_successful_socket_ping() {
+        let mut attempts = 0u8;
+        let result = wait_for_helper_ready_with(
+            3,
+            || {
+                attempts += 1;
+                if attempts < 2 {
+                    Err("Permission denied".to_string())
+                } else {
+                    Ok(())
+                }
+            },
+            || {},
+        );
+
+        assert!(result.is_ok());
+        assert_eq!(attempts, 2);
+    }
+
+    #[test]
+    fn helper_ready_wait_reports_last_socket_ping_error() {
+        let result = wait_for_helper_ready_with(2, || Err("Permission denied".to_string()), || {});
+
+        assert!(result.is_err());
+        assert!(result.unwrap_err().contains("Permission denied"));
+    }
 }