fix(bootstrap): validate gateway names before path joins

alexclewontin · alexclewontin · commit acefaadae288 · 2026-06-11T13:47:48.000-04:00
diff --git a/crates/openshell-bootstrap/src/edge_token.rs b/crates/openshell-bootstrap/src/edge_token.rs
@@ -7,19 +7,19 @@
 //! `$XDG_CONFIG_HOME/openshell/gateways/<name>/edge_token`.
 //! The token is a plain-text JWT string with `0600` permissions.
 
-use crate::paths::user_gateways_dir;
+use crate::paths::user_gateway_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.
 pub fn edge_token_path(gateway_name: &str) -> Result<PathBuf> {
-    Ok(user_gateways_dir()?.join(gateway_name).join("edge_token"))
+    Ok(user_gateway_dir(gateway_name)?.join("edge_token"))
 }
 
 /// Legacy path used before the rename to `edge_token`.
 fn legacy_token_path(gateway_name: &str) -> Result<PathBuf> {
-    Ok(user_gateways_dir()?.join(gateway_name).join("cf_token"))
+    Ok(user_gateway_dir(gateway_name)?.join("cf_token"))
 }
 
 /// Store an edge authentication token for a gateway.
@@ -158,6 +158,15 @@ mod tests {
         });
     }
 
+    #[test]
+    fn edge_token_paths_reject_multi_component_gateway_names() {
+        let tmp = tempfile::tempdir().unwrap();
+        with_tmp_xdg(tmp.path(), || {
+            assert!(store_edge_token("../escape", "token").is_err());
+            assert_eq!(load_edge_token("../escape"), None);
+            assert!(remove_edge_token("../escape").is_err());
+        });
+    }
     #[test]
     fn load_edge_token_trims_whitespace() {
         let tmp = tempfile::tempdir().unwrap();
diff --git a/crates/openshell-bootstrap/src/metadata.rs b/crates/openshell-bootstrap/src/metadata.rs
@@ -2,8 +2,8 @@
 // SPDX-License-Identifier: Apache-2.0
 
 use crate::paths::{
-    last_sandbox_path, system_active_gateway_path, system_gateways_dir, user_active_gateway_path,
-    user_gateways_dir,
+    last_sandbox_path, system_active_gateway_path, system_gateway_dir, system_gateways_dir,
+    user_active_gateway_path, user_gateway_dir, user_gateways_dir, validated_gateway_name,
 };
 use miette::{IntoDiagnostic, Result, WrapErr};
 use openshell_core::paths::ensure_parent_dir_restricted;
@@ -98,11 +98,11 @@ pub struct ListedGateway {
 }
 
 fn user_gateway_metadata_path(name: &str) -> Result<PathBuf> {
-    Ok(user_gateways_dir()?.join(name).join("metadata.json"))
+    Ok(user_gateway_dir(name)?.join("metadata.json"))
 }
 
-fn system_gateway_metadata_path(name: &str) -> PathBuf {
-    system_gateways_dir().join(name).join("metadata.json")
+fn system_gateway_metadata_path(name: &str) -> Result<PathBuf> {
+    Ok(system_gateway_dir(name)?.join("metadata.json"))
 }
 
 fn resolve_gateway_metadata_path(name: &str) -> Result<Option<(PathBuf, GatewayMetadataSource)>> {
@@ -111,7 +111,7 @@ fn resolve_gateway_metadata_path(name: &str) -> Result<Option<(PathBuf, GatewayM
         return Ok(Some((user, GatewayMetadataSource::User)));
     }
 
-    let system = system_gateway_metadata_path(name);
+    let system = system_gateway_metadata_path(name)?;
     if system.exists() {
         return Ok(Some((system, GatewayMetadataSource::System)));
     }
@@ -212,7 +212,7 @@ pub fn gateway_metadata_source(name: &str) -> Result<Option<GatewayMetadataSourc
 
 pub fn load_gateway_metadata(name: &str) -> Result<GatewayMetadata> {
     let primary = user_gateway_metadata_path(name)?;
-    let system = system_gateway_metadata_path(name);
+    let system = system_gateway_metadata_path(name)?;
     let Some((path, _source)) = resolve_gateway_metadata_path(name)? else {
         return Err(miette::miette!(
             "no metadata found for gateway '{name}' (looked in {} and {})",
@@ -230,13 +230,25 @@ 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<()> {
+    validated_gateway_name(name)?;
     let path = user_active_gateway_path()?;
     ensure_parent_dir_restricted(&path)?;
     std::fs::write(&path, name)
         .into_diagnostic()
         .wrap_err_with(|| format!("failed to write active gateway to {}", path.display()))?;
     Ok(())
 }
+
+fn read_gateway_name(path: &Path) -> Option<String> {
+    let value = read_nonempty_trimmed(path)?;
+    match validated_gateway_name(&value) {
+        Ok(_) => Some(value),
+        Err(err) => {
+            tracing::warn!(path = %path.display(), error = %err, "ignoring invalid active gateway name");
+            None
+        }
+    }
+}
 fn read_nonempty_trimmed(path: &Path) -> Option<String> {
     let contents = std::fs::read_to_string(path).ok()?;
     let value = contents.trim();
@@ -250,7 +262,7 @@ pub fn load_user_active_gateway() -> Option<String> {
     user_active_gateway_path()
         .ok()
         .as_deref()
-        .and_then(read_nonempty_trimmed)
+        .and_then(read_gateway_name)
 }
 
 /// Load the active gateway name from persistent storage.
@@ -259,7 +271,7 @@ pub fn load_user_active_gateway() -> Option<String> {
 /// system-level active gateway file when no per-user selection exists, so
 /// installer-provided defaults can take effect on a fresh system.
 pub fn load_active_gateway() -> Option<String> {
-    load_user_active_gateway().or_else(|| read_nonempty_trimmed(&system_active_gateway_path()))
+    load_user_active_gateway().or_else(|| read_gateway_name(&system_active_gateway_path()))
 }
 
 /// Save the last-used sandbox name for a gateway to persistent storage.
@@ -899,4 +911,34 @@ mod tests {
             assert_eq!(gateways[0].source, GatewayMetadataSource::System);
         });
     }
+
+    #[test]
+    fn gateway_names_must_be_single_path_components() {
+        let user = tempfile::tempdir().unwrap();
+        let system = tempfile::tempdir().unwrap();
+        with_tmp_xdg_and_system(user.path(), system.path(), || {
+            let meta = GatewayMetadata {
+                name: "shared".to_string(),
+                gateway_endpoint: "https://example.com".to_string(),
+                ..Default::default()
+            };
+            assert!(store_gateway_metadata("../escape", &meta).is_err());
+            assert!(load_gateway_metadata("../escape").is_err());
+            assert!(save_last_sandbox("../escape", "sb-123").is_err());
+            assert!(save_active_gateway("../escape").is_err());
+        });
+    }
+
+    #[test]
+    fn load_active_gateway_ignores_invalid_user_name_and_falls_back_to_system() {
+        let user = tempfile::tempdir().unwrap();
+        let system = tempfile::tempdir().unwrap();
+        with_tmp_xdg_and_system(user.path(), system.path(), || {
+            std::fs::write(user.path().join("active_gateway"), "../escape").unwrap();
+            std::fs::write(system.path().join("active_gateway"), "system-default").unwrap();
+
+            assert_eq!(load_user_active_gateway(), None);
+            assert_eq!(load_active_gateway(), Some("system-default".to_string()));
+        });
+    }
 }
diff --git a/crates/openshell-bootstrap/src/mtls.rs b/crates/openshell-bootstrap/src/mtls.rs
@@ -1,9 +1,10 @@
 // SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
 // SPDX-License-Identifier: Apache-2.0
 
+use crate::paths::user_gateway_dir;
 use crate::pki::PkiBundle;
 use miette::{IntoDiagnostic, Result};
-use openshell_core::paths::{create_dir_restricted, set_file_owner_only, xdg_config_dir};
+use openshell_core::paths::{create_dir_restricted, set_file_owner_only};
 use std::path::PathBuf;
 
 /// Store the PKI bundle's client materials (ca.crt, tls.crt, tls.key) to the
@@ -77,11 +78,7 @@ pub fn store_pki_bundle(name: &str, bundle: &PkiBundle) -> Result<()> {
 }
 
 fn cli_mtls_dir(name: &str) -> Result<PathBuf> {
-    Ok(xdg_config_dir()?
-        .join("openshell")
-        .join("gateways")
-        .join(name)
-        .join("mtls"))
+    Ok(user_gateway_dir(name)?.join("mtls"))
 }
 
 fn cli_mtls_temp_dir(name: &str) -> Result<PathBuf> {
@@ -104,3 +101,45 @@ fn validate_cli_mtls_bundle_dir(dir: &std::path::Path) -> Result<()> {
     }
     Ok(())
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[allow(unsafe_code)]
+    fn with_tmp_xdg<F: FnOnce()>(tmp: &std::path::Path, f: F) {
+        let _guard = crate::XDG_TEST_LOCK
+            .lock()
+            .unwrap_or_else(std::sync::PoisonError::into_inner);
+        let orig = std::env::var("XDG_CONFIG_HOME").ok();
+        unsafe {
+            std::env::set_var("XDG_CONFIG_HOME", tmp);
+        }
+        f();
+        unsafe {
+            match orig {
+                Some(v) => std::env::set_var("XDG_CONFIG_HOME", v),
+                None => std::env::remove_var("XDG_CONFIG_HOME"),
+            }
+        }
+    }
+
+    #[test]
+    fn store_pki_bundle_rejects_multi_component_gateway_names() {
+        let tmp = tempfile::tempdir().unwrap();
+        with_tmp_xdg(tmp.path(), || {
+            let bundle = PkiBundle {
+                ca_cert_pem: "ca".to_string(),
+                ca_key_pem: "cakey".to_string(),
+                server_cert_pem: "server".to_string(),
+                server_key_pem: "serverkey".to_string(),
+                client_cert_pem: "client".to_string(),
+                client_key_pem: "clientkey".to_string(),
+                jwt_signing_key_pem: "signing".to_string(),
+                jwt_public_key_pem: "public".to_string(),
+                jwt_key_id: "kid".to_string(),
+            };
+            assert!(store_pki_bundle("../escape", &bundle).is_err());
+        });
+    }
+}
diff --git a/crates/openshell-bootstrap/src/oidc_token.rs b/crates/openshell-bootstrap/src/oidc_token.rs
@@ -7,7 +7,7 @@
 //! `$XDG_CONFIG_HOME/openshell/gateways/<name>/oidc_token.json`.
 //! File permissions are `0600` (owner-only).
 
-use crate::paths::user_gateways_dir;
+use crate::paths::user_gateway_dir;
 use miette::{IntoDiagnostic, Result, WrapErr};
 use openshell_core::paths::{ensure_parent_dir_restricted, set_file_owner_only};
 use serde::{Deserialize, Serialize};
@@ -36,9 +36,7 @@ pub struct OidcTokenBundle {
 
 /// Path to the stored OIDC token bundle for a gateway.
 pub fn oidc_token_path(gateway_name: &str) -> Result<PathBuf> {
-    Ok(user_gateways_dir()?
-        .join(gateway_name)
-        .join("oidc_token.json"))
+    Ok(user_gateway_dir(gateway_name)?.join("oidc_token.json"))
 }
 
 /// Store an OIDC token bundle for a gateway.
@@ -92,3 +90,43 @@ pub fn is_token_expired(bundle: &OidcTokenBundle) -> bool {
         .as_secs();
     now + 30 >= expires_at
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[allow(unsafe_code)]
+    fn with_tmp_xdg<F: FnOnce()>(tmp: &std::path::Path, f: F) {
+        let _guard = crate::XDG_TEST_LOCK
+            .lock()
+            .unwrap_or_else(std::sync::PoisonError::into_inner);
+        let orig = std::env::var("XDG_CONFIG_HOME").ok();
+        unsafe {
+            std::env::set_var("XDG_CONFIG_HOME", tmp);
+        }
+        f();
+        unsafe {
+            match orig {
+                Some(v) => std::env::set_var("XDG_CONFIG_HOME", v),
+                None => std::env::remove_var("XDG_CONFIG_HOME"),
+            }
+        }
+    }
+
+    #[test]
+    fn oidc_token_paths_reject_multi_component_gateway_names() {
+        let tmp = tempfile::tempdir().unwrap();
+        with_tmp_xdg(tmp.path(), || {
+            let bundle = OidcTokenBundle {
+                access_token: "token".to_string(),
+                refresh_token: None,
+                expires_at: None,
+                issuer: "https://issuer.example.com".to_string(),
+                client_id: "openshell-cli".to_string(),
+            };
+            assert!(store_oidc_token("../escape", &bundle).is_err());
+            assert!(load_oidc_token("../escape").is_none());
+            assert!(remove_oidc_token("../escape").is_err());
+        });
+    }
+}
diff --git a/crates/openshell-bootstrap/src/paths.rs b/crates/openshell-bootstrap/src/paths.rs
