fix(core): harden file permissions for user config directory (#328)

Author: drew

Cargo.lock

@@ -10,6 +10,7 @@ repository.workspace = true
 rust-version.workspace = true
 
 [dependencies]
+openshell-core = { path = "../openshell-core" }
 base64 = "0.22"
 bollard = { version = "0.20", features = ["ssh"] }
 bytes = { workspace = true }

crates/openshell-bootstrap/Cargo.toml

@@ -9,6 +9,7 @@
 
 use crate::paths::gateways_dir;
 use miette::{IntoDiagnostic, Result, WrapErr};
+use openshell_core::paths::{ensure_parent_dir_restricted, set_file_owner_only};
 use std::path::PathBuf;
 
 /// Path to the stored edge auth token for a gateway.
@@ -24,38 +25,47 @@ fn legacy_token_path(gateway_name: &str) -> Result<PathBuf> {
 /// Store an edge authentication token for a gateway.
 pub fn store_edge_token(gateway_name: &str, token: &str) -> Result<()> {
     let path = edge_token_path(gateway_name)?;
-    if let Some(parent) = path.parent() {
-        std::fs::create_dir_all(parent)
-            .into_diagnostic()
-            .wrap_err_with(|| format!("failed to create {}", parent.display()))?;
-    }
+    ensure_parent_dir_restricted(&path)?;
     std::fs::write(&path, token)
         .into_diagnostic()
         .wrap_err_with(|| format!("failed to write edge token to {}", path.display()))?;
     // Restrict permissions to owner-only (0600).
-    #[cfg(unix)]
-    {
-        use std::os::unix::fs::PermissionsExt;
-        std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o600))
-            .into_diagnostic()
-            .wrap_err("failed to set token file permissions")?;
-    }
+    set_file_owner_only(&path)?;
     Ok(())
 }
 
 /// Load a stored edge authentication token for a gateway.
 ///
 /// Returns `None` if no token file exists or the file is empty.
 /// Falls back to the legacy `cf_token` path for backwards compatibility.
+/// When loading from the legacy path, migrates the token to the new path
+/// with proper permissions.
 pub fn load_edge_token(gateway_name: &str) -> Option<String> {
-    // Try the new path first, then fall back to legacy.
-    let path = edge_token_path(gateway_name)
+    // Try the new path first.
+    if let Some(path) = edge_token_path(gateway_name).ok().filter(|p| p.exists()) {
+        let contents = std::fs::read_to_string(&path).ok()?;
+        let token = contents.trim().to_string();
+        if !token.is_empty() {
+            return Some(token);
+        }
+    }
+
+    // Fall back to the legacy cf_token path.
+    let legacy_path = legacy_token_path(gateway_name)
         .ok()
-        .filter(|p| p.exists())
-        .or_else(|| legacy_token_path(gateway_name).ok().filter(|p| p.exists()))?;
-    let contents = std::fs::read_to_string(&path).ok()?;
+        .filter(|p| p.exists())?;
+    let contents = std::fs::read_to_string(&legacy_path).ok()?;
     let token = contents.trim().to_string();
-    if token.is_empty() { None } else { Some(token) }
+    if token.is_empty() {
+        return None;
+    }
+
+    // Migrate: write to new path with proper permissions, then remove legacy.
+    if store_edge_token(gateway_name, &token).is_ok() {
+        let _ = std::fs::remove_file(&legacy_path);
+    }
+
+    Some(token)
 }
 
 /// Remove a stored edge authentication token.

crates/openshell-bootstrap/src/edge_token.rs

@@ -4,6 +4,7 @@
 use crate::RemoteOptions;
 use crate::paths::{active_gateway_path, gateways_dir, last_sandbox_path};
 use miette::{IntoDiagnostic, Result, WrapErr};
+use openshell_core::paths::ensure_parent_dir_restricted;
 use serde::{Deserialize, Serialize};
 use std::path::PathBuf;
 
@@ -205,11 +206,7 @@ pub fn resolve_ssh_hostname(host: &str) -> String {
 
 pub fn store_gateway_metadata(name: &str, metadata: &GatewayMetadata) -> Result<()> {
     let path = stored_metadata_path(name)?;
-    if let Some(parent) = path.parent() {
-        std::fs::create_dir_all(parent)
-            .into_diagnostic()
-            .wrap_err_with(|| format!("failed to create {}", parent.display()))?;
-    }
+    ensure_parent_dir_restricted(&path)?;
     let contents = serde_json::to_string_pretty(metadata)
         .into_diagnostic()
         .wrap_err("failed to serialize gateway metadata")?;
@@ -237,11 +234,7 @@ pub fn get_gateway_metadata(name: &str) -> Option<GatewayMetadata> {
 /// Save the active gateway name to persistent storage.
 pub fn save_active_gateway(name: &str) -> Result<()> {
     let path = active_gateway_path()?;
-    if let Some(parent) = path.parent() {
-        std::fs::create_dir_all(parent)
-            .into_diagnostic()
-            .wrap_err_with(|| format!("failed to create {}", parent.display()))?;
-    }
+    ensure_parent_dir_restricted(&path)?;
     std::fs::write(&path, name)
         .into_diagnostic()
         .wrap_err_with(|| format!("failed to write active gateway to {}", path.display()))?;
@@ -261,11 +254,7 @@ pub fn load_active_gateway() -> Option<String> {
 /// Save the last-used sandbox name for a gateway to persistent storage.
 pub fn save_last_sandbox(gateway: &str, sandbox: &str) -> Result<()> {
     let path = last_sandbox_path(gateway)?;
-    if let Some(parent) = path.parent() {
-        std::fs::create_dir_all(parent)
-            .into_diagnostic()
-            .wrap_err_with(|| format!("failed to create {}", parent.display()))?;
-    }
+    ensure_parent_dir_restricted(&path)?;
     std::fs::write(&path, sandbox)
         .into_diagnostic()
         .wrap_err_with(|| format!("failed to write last sandbox to {}", path.display()))?;

crates/openshell-bootstrap/src/metadata.rs

@@ -1,15 +1,16 @@
 // SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-use crate::paths::xdg_config_dir;
 use crate::pki::PkiBundle;
 use miette::{IntoDiagnostic, Result};
+use openshell_core::paths::{create_dir_restricted, set_file_owner_only, xdg_config_dir};
 use std::path::PathBuf;
 
 /// Store the PKI bundle's client materials (ca.crt, tls.crt, tls.key) to the
 /// local filesystem so the CLI can use them for mTLS connections.
 ///
 /// Files are written atomically: temp dir -> validate -> rename over target.
+/// Directories are created with `0o700` and `tls.key` is set to `0o600`.
 pub fn store_pki_bundle(name: &str, bundle: &PkiBundle) -> Result<()> {
     let dir = cli_mtls_dir(name)?;
     let temp_dir = cli_mtls_temp_dir(name)?;
@@ -21,9 +22,9 @@ pub fn store_pki_bundle(name: &str, bundle: &PkiBundle) -> Result<()> {
             .map_err(|e| e.wrap_err(format!("failed to remove {}", temp_dir.display())))?;
     }
 
-    std::fs::create_dir_all(&temp_dir)
-        .into_diagnostic()
-        .map_err(|e| e.wrap_err(format!("failed to create {}", temp_dir.display())))?;
+    // Create the temp dir with restricted permissions so the private key
+    // is never world-readable, even momentarily.
+    create_dir_restricted(&temp_dir)?;
 
     std::fs::write(temp_dir.join("ca.crt"), &bundle.ca_cert_pem)
         .into_diagnostic()
@@ -35,6 +36,9 @@ pub fn store_pki_bundle(name: &str, bundle: &PkiBundle) -> Result<()> {
         .into_diagnostic()
         .map_err(|e| e.wrap_err("failed to write tls.key"))?;
 
+    // Restrict the private key to owner-only.
+    set_file_owner_only(&temp_dir.join("tls.key"))?;
+
     validate_cli_mtls_bundle_dir(&temp_dir)?;
 
     let had_backup = if dir.exists() {
@@ -61,6 +65,9 @@ pub fn store_pki_bundle(name: &str, bundle: &PkiBundle) -> Result<()> {
         return Err(err);
     }
 
+    // Ensure the final directory also has restricted permissions after rename.
+    create_dir_restricted(&dir)?;
+
     if had_backup {
         std::fs::remove_dir_all(&backup_dir)
             .into_diagnostic()

crates/openshell-bootstrap/src/mtls.rs

@@ -1,19 +1,10 @@
 // SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-use miette::{IntoDiagnostic, Result, WrapErr};
+use miette::Result;
+use openshell_core::paths::xdg_config_dir;
 use std::path::PathBuf;
 
-pub fn xdg_config_dir() -> Result<PathBuf> {
-    if let Ok(path) = std::env::var("XDG_CONFIG_HOME") {
-        return Ok(PathBuf::from(path));
-    }
-    let home = std::env::var("HOME")
-        .into_diagnostic()
-        .wrap_err("HOME is not set")?;
-    Ok(PathBuf::from(home).join(".config"))
-}
-
 /// Path to the file that stores the active gateway name.
 ///
 /// Location: `$XDG_CONFIG_HOME/openshell/active_gateway`

crates/openshell-bootstrap/src/paths.rs

@@ -773,15 +773,9 @@ fn render_ssh_config(gateway: &str, name: &str) -> String {
 }
 
 fn openshell_ssh_config_path() -> Result<PathBuf> {
-    let base = if let Ok(path) = std::env::var("XDG_CONFIG_HOME") {
-        PathBuf::from(path)
-    } else {
-        let home = std::env::var("HOME")
-            .into_diagnostic()
-            .wrap_err("HOME is not set")?;
-        PathBuf::from(home).join(".config")
-    };
-    Ok(base.join("openshell").join("ssh_config"))
+    Ok(openshell_core::paths::xdg_config_dir()?
+        .join("openshell")
+        .join("ssh_config"))
 }
 
 fn user_ssh_config_path() -> Result<PathBuf> {
@@ -905,9 +899,7 @@ pub fn install_ssh_config(gateway: &str, name: &str) -> Result<PathBuf> {
     ensure_openshell_include(&main_config, &managed_config)?;
 
     if let Some(parent) = managed_config.parent() {
-        fs::create_dir_all(parent)
-            .into_diagnostic()
-            .wrap_err("failed to create OpenShell config directory")?;
+        openshell_core::paths::create_dir_restricted(parent)?;
     }
 
     let alias = host_alias(name);

crates/openshell-cli/src/ssh.rs

@@ -150,13 +150,7 @@ fn sanitize_name(value: &str) -> String {
 }
 
 fn xdg_config_dir() -> Result<PathBuf> {
-    if let Ok(path) = std::env::var("XDG_CONFIG_HOME") {
-        return Ok(PathBuf::from(path));
-    }
-    let home = std::env::var("HOME")
-        .into_diagnostic()
-        .wrap_err("HOME is not set")?;
-    Ok(PathBuf::from(home).join(".config"))
+    openshell_core::paths::xdg_config_dir()
 }
 
 pub fn require_tls_materials(server: &str, tls: &TlsOptions) -> Result<TlsMaterials> {

crates/openshell-cli/src/tls.rs

@@ -24,5 +24,8 @@ url = { workspace = true }
 tonic-build = { workspace = true }
 protobuf-src = { workspace = true }
 
+[dev-dependencies]
+tempfile = "3"
+
 [lints]
 workspace = true

crates/openshell-core/Cargo.toml

@@ -6,6 +6,7 @@
 //! Used by both the CLI (`openshell-cli`) and the TUI (`openshell-tui`) to
 //! start, stop, list, and track background SSH port forwards.
 
+use crate::paths::{create_dir_restricted, xdg_config_dir};
 use miette::{IntoDiagnostic, Result, WrapErr};
 use std::net::TcpListener;
 use std::path::PathBuf;
@@ -17,15 +18,7 @@ use std::process::Command;
 
 /// Base directory for forward PID files.
 pub fn forward_pid_dir() -> Result<PathBuf> {
-    let base = if let Ok(path) = std::env::var("XDG_CONFIG_HOME") {
-        PathBuf::from(path)
-    } else {
-        let home = std::env::var("HOME")
-            .into_diagnostic()
-            .wrap_err("HOME is not set")?;
-        PathBuf::from(home).join(".config")
-    };
-    Ok(base.join("openshell").join("forwards"))
+    Ok(xdg_config_dir()?.join("openshell").join("forwards"))
 }
 
 /// PID file path for a specific sandbox + port forward.
@@ -44,9 +37,7 @@ pub fn write_forward_pid(
     bind_addr: &str,
 ) -> Result<()> {
     let dir = forward_pid_dir()?;
-    std::fs::create_dir_all(&dir)
-        .into_diagnostic()
-        .wrap_err("failed to create forwards directory")?;
+    create_dir_restricted(&dir)?;
     let path = forward_pid_path(name, port)?;
     std::fs::write(&path, format!("{pid}\t{sandbox_id}\t{bind_addr}"))
         .into_diagnostic()