diff --git a/rust/gui-client/src-common/src/controller.rs b/rust/gui-client/src-common/src/controller.rs index 85ad28c91..bee570ce9 100644 --- a/rust/gui-client/src-common/src/controller.rs +++ b/rust/gui-client/src-common/src/controller.rs @@ -13,7 +13,7 @@ use firezone_headless_client::{ IpcClientMsg::{self, SetDisabledResources}, IpcServerMsg, IpcServiceError, LogFilterReloader, }; -use firezone_telemetry::Telemetry; +use firezone_telemetry::{self as telemetry, Telemetry}; use secrecy::{ExposeSecret as _, SecretString}; use std::{collections::BTreeSet, ops::ControlFlow, path::PathBuf, time::Instant}; use tokio::sync::{mpsc, oneshot}; @@ -70,11 +70,16 @@ impl Builder { let (ipc_tx, ipc_rx) = mpsc::channel(1); let ipc_client = ipc::Client::new(ipc_tx).await?; // Get the device ID after connecting to the IPC service, this creates a happens-before relationship where we know the IPC service has written a device ID to disk. - let firezone_id = firezone_headless_client::device_id::get() - .context("Failed to read / create device ID")? - .id; - firezone_telemetry::Hub::main() - .configure_scope(|scope| scope.set_tag("firezone_id", &firezone_id)); + match firezone_headless_client::device_id::get() { + Ok(id) => { + telemetry::Hub::main() + .configure_scope(|scope| scope.set_tag("firezone_id", &id.id)); + } + Err(error) => { + telemetry::capture_anyhow(&error.context("Failed to read device ID")); + } + } + Ok(Controller { advanced_settings, auth: auth::Auth::new()?, diff --git a/rust/headless-client/src/device_id.rs b/rust/headless-client/src/device_id.rs index 3f17a8b8d..ff1851cc4 100644 --- a/rust/headless-client/src/device_id.rs +++ b/rust/headless-client/src/device_id.rs @@ -55,13 +55,13 @@ pub fn device_info() -> phoenix_channel::DeviceInfo { /// Returns the device ID without generating it pub fn get() -> Result { let path = path()?; - let Some(j) = fs::read_to_string(&path) - .ok() - .and_then(|s| serde_json::from_str::(&s).ok()) - else { - anyhow::bail!("Couldn't read device ID from disk"); - }; - Ok(DeviceId { id: j.device_id() }) + let content = fs::read_to_string(&path).context("Failed to read file")?; + let device_id_json = serde_json::from_str::(&content) + .context("Failed to deserialize content as JSON")?; + + Ok(DeviceId { + id: device_id_json.device_id(), + }) } /// Returns the device ID, generating it and saving it to disk if needed. @@ -80,7 +80,7 @@ pub fn get_or_create() -> Result { // Make sure the dir exists, and fix its permissions so the GUI can write the // log filter file fs::create_dir_all(dir).context("Failed to create dir for firezone-id")?; - set_permissions(dir).with_context(|| { + set_dir_permissions(dir).with_context(|| { format!( "Couldn't set permissions on IPC service config dir `{}`", dir.display() @@ -94,6 +94,8 @@ pub fn get_or_create() -> Result { { let id = j.device_id(); tracing::debug!(?id, "Loaded device ID from disk"); + // Correct permissions for #6989 + set_id_permissions(&path).context("Couldn't set permissions on Firezone ID file")?; return Ok(DeviceId { id }); } @@ -110,13 +112,14 @@ pub fn get_or_create() -> Result { let id = j.device_id(); tracing::debug!(?id, "Saved device ID to disk"); + set_id_permissions(&path).context("Couldn't set permissions on Firezone ID file")?; Ok(DeviceId { id }) } #[cfg(target_os = "linux")] -fn set_permissions(dir: &Path) -> Result<()> { +fn set_dir_permissions(dir: &Path) -> Result<()> { use std::os::unix::fs::PermissionsExt; - // user read/write, group read-write, others nothing + // user read/write, group read/write, others nothing // directories need `+x` to work of course let perms = fs::Permissions::from_mode(0o770); std::fs::set_permissions(dir, perms)?; @@ -126,7 +129,23 @@ fn set_permissions(dir: &Path) -> Result<()> { /// Does nothing on non-Linux systems #[cfg(not(target_os = "linux"))] #[expect(clippy::unnecessary_wraps)] -fn set_permissions(_: &Path) -> Result<()> { +fn set_dir_permissions(_: &Path) -> Result<()> { + Ok(()) +} + +#[cfg(target_os = "linux")] +fn set_id_permissions(path: &Path) -> Result<()> { + use std::os::unix::fs::PermissionsExt; + // user read/write, group read, others nothing + let perms = fs::Permissions::from_mode(0o640); + std::fs::set_permissions(path, perms)?; + Ok(()) +} + +/// Does nothing on non-Linux systems +#[cfg(not(target_os = "linux"))] +#[expect(clippy::unnecessary_wraps)] +fn set_id_permissions(_: &Path) -> Result<()> { Ok(()) } diff --git a/rust/tests/gui-smoke-test/src/main.rs b/rust/tests/gui-smoke-test/src/main.rs index 1d19ec9b4..135b90851 100644 --- a/rust/tests/gui-smoke-test/src/main.rs +++ b/rust/tests/gui-smoke-test/src/main.rs @@ -46,6 +46,7 @@ fn main() -> Result<()> { .popen()?; gui.wait()?.fz_exit_ok().context("GUI process")?; + ipc_service.wait()?.fz_exit_ok().context("IPC service")?; // Force the GUI to crash diff --git a/website/src/components/Changelog/GUI.tsx b/website/src/components/Changelog/GUI.tsx index f45caeb46..dd9134d27 100644 --- a/website/src/components/Changelog/GUI.tsx +++ b/website/src/components/Changelog/GUI.tsx @@ -14,7 +14,11 @@ export default function GUI({ title }: { title: string }) { return ( {/* When you cut a release, remove any solved issues from the "known issues" lists over in `client-apps`. This must not be done when the issue's PR merges. */} - + + + Fixes a crash on startup caused by incorrect permissions on the ID file. + + Fixes the GUI shutting down slowly.