From 4a5993bb9577936b85e446e325bd21c433db03ce Mon Sep 17 00:00:00 2001 From: Alex Lewontin Date: Thu, 28 May 2026 18:34:22 -0400 Subject: [PATCH 1/8] feat(bootstrap): add system gateway registry for installer defaults Adds a read-only installer-seeded gateway registry that the CLI consults after per-user gateway config. The registry uses the same layout as per-user config with `active_gateway` at the root and `gateways//metadata.json` beneath it. By default the system config root is `/etc/openshell`, while `OPENSHELL_SYSTEM_GATEWAY_DIR` remains available as an override for packages that need a different location. User-managed gateways continue to shadow installer entries on name collision. Originally-authored-by: Mark Shuttleworth Signed-off-by: Alex Lewontin --- architecture/gateway.md | 15 + crates/openshell-bootstrap/src/edge_token.rs | 6 +- crates/openshell-bootstrap/src/lib.rs | 7 +- crates/openshell-bootstrap/src/metadata.rs | 397 ++++++++++++++++--- crates/openshell-bootstrap/src/oidc_token.rs | 6 +- crates/openshell-bootstrap/src/paths.rs | 156 +++++++- docs/sandboxes/manage-gateways.mdx | 2 + 7 files changed, 523 insertions(+), 66 deletions(-) diff --git a/architecture/gateway.md b/architecture/gateway.md index a13d58f0a..7afec0767 100644 --- a/architecture/gateway.md +++ b/architecture/gateway.md @@ -446,6 +446,21 @@ Driver-specific values that are not part of the inheritance allowlist (e.g. Podman `socket_path`, VM `vcpus`) only come from the driver's own table. +### Package-managed gateway registry + +The CLI reads its active-gateway and per-gateway metadata from +`$XDG_CONFIG_HOME/openshell/`. It also looks for a package-manager owned +system config root at `/etc/openshell`, using the same layout as the per-user +config root: `active_gateway` plus `gateways//metadata.json`. Packages +or runtimes that need a different location can override that root with a +non-empty absolute `OPENSHELL_SYSTEM_GATEWAY_DIR`; empty or relative values +fall back to `/etc/openshell` and emit a warning. The CLI falls back to this +system config when no per-user `metadata.json` exists; malformed user metadata +still shadows the system entry, but stray empty directories do not. + +System entries are read-only from the CLI, so `gateway remove` rejects a pure +system entry instead of pretending to delete package-manager owned state. + ## Operational Constraints - Gateway TLS and client certificate distribution are deployment concerns owned diff --git a/crates/openshell-bootstrap/src/edge_token.rs b/crates/openshell-bootstrap/src/edge_token.rs index 7b1892d14..f647cc5e0 100644 --- 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//edge_token`. //! The token is a plain-text JWT string with `0600` permissions. -use crate::paths::gateways_dir; +use crate::paths::user_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. pub fn edge_token_path(gateway_name: &str) -> Result { - Ok(gateways_dir()?.join(gateway_name).join("edge_token")) + Ok(user_gateways_dir()?.join(gateway_name).join("edge_token")) } /// Legacy path used before the rename to `edge_token`. fn legacy_token_path(gateway_name: &str) -> Result { - Ok(gateways_dir()?.join(gateway_name).join("cf_token")) + Ok(user_gateways_dir()?.join(gateway_name).join("cf_token")) } /// Store an edge authentication token for a gateway. diff --git a/crates/openshell-bootstrap/src/lib.rs b/crates/openshell-bootstrap/src/lib.rs index 8845f0392..0dd1b54d2 100644 --- a/crates/openshell-bootstrap/src/lib.rs +++ b/crates/openshell-bootstrap/src/lib.rs @@ -8,7 +8,7 @@ pub mod oidc_token; mod metadata; pub mod mtls; -pub mod paths; +mod paths; pub mod pki; #[cfg(test)] @@ -21,8 +21,9 @@ use std::sync::Mutex; pub(crate) static XDG_TEST_LOCK: Mutex<()> = Mutex::new(()); pub use crate::metadata::{ - GatewayMetadata, clear_active_gateway, clear_last_sandbox_if_matches, - extract_host_from_ssh_destination, get_gateway_metadata, list_gateways, load_active_gateway, + GatewayMetadata, GatewayMetadataSource, ListedGateway, clear_active_gateway, + clear_last_sandbox_if_matches, extract_host_from_ssh_destination, gateway_metadata_source, + get_gateway_metadata, list_gateways, list_gateways_with_source, load_active_gateway, load_gateway_metadata, load_last_sandbox, remove_gateway_metadata, resolve_ssh_hostname, save_active_gateway, save_last_sandbox, store_gateway_metadata, }; diff --git a/crates/openshell-bootstrap/src/metadata.rs b/crates/openshell-bootstrap/src/metadata.rs index 108a99b8a..87ea9204d 100644 --- a/crates/openshell-bootstrap/src/metadata.rs +++ b/crates/openshell-bootstrap/src/metadata.rs @@ -1,11 +1,14 @@ // SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -use crate::paths::{active_gateway_path, gateways_dir, last_sandbox_path}; +use crate::paths::{ + last_sandbox_path, system_active_gateway_path, system_gateways_dir, user_active_gateway_path, + user_gateways_dir, +}; use miette::{IntoDiagnostic, Result, WrapErr}; use openshell_core::paths::ensure_parent_dir_restricted; use serde::{Deserialize, Serialize}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; /// Gateway metadata stored for CLI endpoint resolution and authentication. #[derive(Debug, Clone, Default, Serialize, Deserialize)] @@ -70,10 +73,60 @@ pub struct GatewayMetadata { pub vm_driver_state_dir: Option, } -fn stored_metadata_path(name: &str) -> Result { - Ok(gateways_dir()?.join(name).join("metadata.json")) +/// Storage layer that provides a gateway metadata record. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum GatewayMetadataSource { + /// Per-user metadata under `$XDG_CONFIG_HOME/openshell/gateways`. + User, + /// Installer-provided metadata under the system gateway registry. + System, } +impl GatewayMetadataSource { + pub const fn label(self) -> &'static str { + match self { + Self::User => "user", + Self::System => "system", + } + } +} + +#[derive(Debug, Clone)] +pub struct ListedGateway { + pub metadata: GatewayMetadata, + pub source: GatewayMetadataSource, +} + +fn user_gateway_metadata_path(name: &str) -> Result { + Ok(user_gateways_dir()?.join(name).join("metadata.json")) +} + +fn system_gateway_metadata_path(name: &str) -> PathBuf { + system_gateways_dir().join(name).join("metadata.json") +} + +fn resolve_gateway_metadata_path(name: &str) -> Result> { + let user = user_gateway_metadata_path(name)?; + if user.exists() { + return Ok(Some((user, GatewayMetadataSource::User))); + } + + let system = system_gateway_metadata_path(name); + if system.exists() { + return Ok(Some((system, GatewayMetadataSource::System))); + } + + Ok(None) +} + +fn parse_gateway_metadata(path: &Path) -> Result { + let contents = std::fs::read_to_string(path) + .into_diagnostic() + .wrap_err_with(|| format!("failed to read metadata from {}", path.display()))?; + serde_json::from_str(&contents) + .into_diagnostic() + .wrap_err("failed to parse gateway metadata") +} /// Extract the hostname from an SSH destination string. /// /// Handles formats like: @@ -137,7 +190,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)?; + let path = user_gateway_metadata_path(name)?; ensure_parent_dir_restricted(&path)?; let contents = serde_json::to_string_pretty(metadata) .into_diagnostic() @@ -148,14 +201,22 @@ pub fn store_gateway_metadata(name: &str, metadata: &GatewayMetadata) -> Result< Ok(()) } +/// Return whether a gateway metadata record would resolve from user or system config. +pub fn gateway_metadata_source(name: &str) -> Result> { + Ok(resolve_gateway_metadata_path(name)?.map(|(_, source)| source)) +} + pub fn load_gateway_metadata(name: &str) -> Result { - let path = stored_metadata_path(name)?; - let contents = std::fs::read_to_string(&path) - .into_diagnostic() - .wrap_err_with(|| format!("failed to read metadata from {}", path.display()))?; - serde_json::from_str(&contents) - .into_diagnostic() - .wrap_err("failed to parse gateway metadata") + let primary = user_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 {})", + primary.display(), + system.display(), + )); + }; + parse_gateway_metadata(&path) } /// Load gateway metadata if available. @@ -165,22 +226,30 @@ pub fn get_gateway_metadata(name: &str) -> Option { /// Save the active gateway name to persistent storage. pub fn save_active_gateway(name: &str) -> Result<()> { - let path = active_gateway_path()?; + 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_nonempty_trimmed(path: &Path) -> Option { + let contents = std::fs::read_to_string(path).ok()?; + let value = contents.trim(); + (!value.is_empty()).then(|| value.to_string()) +} /// Load the active gateway name from persistent storage. /// -/// Returns `None` if no active gateway has been set. +/// Returns `None` if no active gateway has been set. Falls back to the +/// 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 { - let path = active_gateway_path().ok()?; - let contents = std::fs::read_to_string(&path).ok()?; - let name = contents.trim().to_string(); - if name.is_empty() { None } else { Some(name) } + user_active_gateway_path() + .ok() + .as_deref() + .and_then(read_nonempty_trimmed) + .or_else(|| read_nonempty_trimmed(&system_active_gateway_path())) } /// Save the last-used sandbox name for a gateway to persistent storage. @@ -197,10 +266,10 @@ pub fn save_last_sandbox(gateway: &str, sandbox: &str) -> Result<()> { /// /// Returns `None` if no last sandbox has been set. pub fn load_last_sandbox(gateway: &str) -> Option { - let path = last_sandbox_path(gateway).ok()?; - let contents = std::fs::read_to_string(&path).ok()?; - let name = contents.trim().to_string(); - if name.is_empty() { None } else { Some(name) } + last_sandbox_path(gateway) + .ok() + .as_deref() + .and_then(read_nonempty_trimmed) } /// Clear the last-used sandbox record for a gateway if it matches the given name. @@ -216,41 +285,60 @@ pub fn clear_last_sandbox_if_matches(gateway: &str, sandbox: &str) { } } -/// List all gateways that have stored metadata. -/// -/// Scans `$XDG_CONFIG_HOME/openshell/gateways/` for subdirectories containing -/// `metadata.json` and returns the parsed metadata for each. -pub fn list_gateways() -> Result> { - let dir = gateways_dir()?; - if !dir.exists() { - return Ok(Vec::new()); - } - +/// List all gateways that have stored metadata, along with the config layer +/// that supplied each record. +pub fn list_gateways_with_source() -> Result> { let mut gateways = Vec::new(); - let entries = std::fs::read_dir(&dir) - .into_diagnostic() - .wrap_err_with(|| format!("failed to read directory {}", dir.display()))?; - - for entry in entries { - let entry = entry.into_diagnostic()?; - let path = entry.path(); - // Only consider directories that contain a metadata.json file - if path.is_dir() { - let gateway_name = entry.file_name().to_string_lossy().to_string(); - if let Ok(metadata) = load_gateway_metadata(&gateway_name) { - gateways.push(metadata); + let mut seen: std::collections::HashSet = std::collections::HashSet::new(); + + let mut scan = |dir: PathBuf, source: GatewayMetadataSource| -> Result<()> { + if !dir.exists() { + return Ok(()); + } + let entries = std::fs::read_dir(&dir) + .into_diagnostic() + .wrap_err_with(|| format!("failed to read directory {}", dir.display()))?; + for entry in entries { + let entry = entry.into_diagnostic()?; + let path = entry.path(); + if !path.is_dir() { + continue; + } + let name = entry.file_name().to_string_lossy().to_string(); + if seen.contains(&name) { + continue; + } + let metadata_path = path.join("metadata.json"); + if let Ok(metadata) = parse_gateway_metadata(&metadata_path) { + seen.insert(name); + gateways.push(ListedGateway { metadata, source }); } } - } + Ok(()) + }; + + scan(user_gateways_dir()?, GatewayMetadataSource::User)?; + scan(system_gateways_dir(), GatewayMetadataSource::System)?; - // Sort by name for stable output - gateways.sort_by(|a, b| a.name.cmp(&b.name)); + gateways.sort_by(|a, b| a.metadata.name.cmp(&b.metadata.name)); Ok(gateways) } +/// List all gateways that have stored metadata. +/// +/// Scans `$XDG_CONFIG_HOME/openshell/gateways/` and the system registry under +/// `/etc/openshell/gateways/` (or `OPENSHELL_SYSTEM_GATEWAY_DIR/gateways/` +/// when set). Per-user entries shadow system entries on name collision. +pub fn list_gateways() -> Result> { + Ok(list_gateways_with_source()? + .into_iter() + .map(|gateway| gateway.metadata) + .collect()) +} + /// Remove the active gateway file (used when destroying the active gateway). pub fn clear_active_gateway() -> Result<()> { - let path = active_gateway_path()?; + let path = user_active_gateway_path()?; if path.exists() { std::fs::remove_file(&path) .into_diagnostic() @@ -261,7 +349,7 @@ pub fn clear_active_gateway() -> Result<()> { /// Remove gateway metadata file. pub fn remove_gateway_metadata(name: &str) -> Result<()> { - let path = stored_metadata_path(name)?; + let path = user_gateway_metadata_path(name)?; if path.exists() { std::fs::remove_file(&path) .into_diagnostic() @@ -341,20 +429,26 @@ mod tests { /// Helper: hold the shared XDG test lock, set `XDG_CONFIG_HOME` to a /// tempdir, run `f`, then restore the original value. #[allow(unsafe_code)] - fn with_tmp_xdg(tmp: &std::path::Path, f: F) { + fn with_tmp_xdg(tmp: &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(); + let orig_xdg = std::env::var("XDG_CONFIG_HOME").ok(); + let orig_sys = std::env::var(crate::paths::SYSTEM_GATEWAY_DIR_ENV).ok(); unsafe { std::env::set_var("XDG_CONFIG_HOME", tmp); + std::env::remove_var(crate::paths::SYSTEM_GATEWAY_DIR_ENV); } f(); unsafe { - match orig { + match orig_xdg { Some(v) => std::env::set_var("XDG_CONFIG_HOME", v), None => std::env::remove_var("XDG_CONFIG_HOME"), } + match orig_sys { + Some(v) => std::env::set_var(crate::paths::SYSTEM_GATEWAY_DIR_ENV, v), + None => std::env::remove_var(crate::paths::SYSTEM_GATEWAY_DIR_ENV), + } } } @@ -437,4 +531,203 @@ mod tests { ); }); } + + // ── system gateway dir fallback ─────────────────────────────────── + + /// Helper: hold the shared XDG test lock, point `XDG_CONFIG_HOME` at + /// `user` and `OPENSHELL_SYSTEM_GATEWAY_DIR` at the system config root, + /// run `f`, then restore both env vars. + #[allow(unsafe_code)] + fn with_tmp_xdg_and_system(user: &Path, system: &Path, f: F) { + let _guard = crate::XDG_TEST_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let orig_xdg = std::env::var("XDG_CONFIG_HOME").ok(); + let orig_sys = std::env::var(crate::paths::SYSTEM_GATEWAY_DIR_ENV).ok(); + unsafe { + std::env::set_var("XDG_CONFIG_HOME", user); + std::env::set_var(crate::paths::SYSTEM_GATEWAY_DIR_ENV, system); + } + f(); + unsafe { + match orig_xdg { + Some(v) => std::env::set_var("XDG_CONFIG_HOME", v), + None => std::env::remove_var("XDG_CONFIG_HOME"), + } + match orig_sys { + Some(v) => std::env::set_var(crate::paths::SYSTEM_GATEWAY_DIR_ENV, v), + None => std::env::remove_var(crate::paths::SYSTEM_GATEWAY_DIR_ENV), + } + } + } + + /// Write a `//metadata.json` file for the given endpoint. + fn write_system_metadata(dir: &Path, name: &str, endpoint: &str) { + let gw_dir = dir.join(name); + std::fs::create_dir_all(&gw_dir).unwrap(); + let meta = GatewayMetadata { + name: name.to_string(), + gateway_endpoint: endpoint.to_string(), + ..Default::default() + }; + std::fs::write( + gw_dir.join("metadata.json"), + serde_json::to_string(&meta).unwrap(), + ) + .unwrap(); + } + + #[test] + fn load_active_gateway_falls_back_to_system_dir() { + let user = tempfile::tempdir().unwrap(); + let system = tempfile::tempdir().unwrap(); + with_tmp_xdg_and_system(user.path(), system.path(), || { + std::fs::write(system.path().join("active_gateway"), "from-system").unwrap(); + assert_eq!(load_active_gateway(), Some("from-system".to_string())); + }); + } + + #[test] + fn load_active_gateway_prefers_user_over_system() { + let user = tempfile::tempdir().unwrap(); + let system = tempfile::tempdir().unwrap(); + with_tmp_xdg_and_system(user.path(), system.path(), || { + save_active_gateway("from-user").unwrap(); + std::fs::write(system.path().join("active_gateway"), "from-system").unwrap(); + assert_eq!(load_active_gateway(), Some("from-user".to_string())); + }); + } + + #[test] + fn load_gateway_metadata_falls_back_to_system_dir() { + let user = tempfile::tempdir().unwrap(); + let system = tempfile::tempdir().unwrap(); + with_tmp_xdg_and_system(user.path(), system.path(), || { + write_system_metadata( + &system.path().join("gateways"), + "sys-gw", + "unix:///tmp/sys.sock", + ); + let meta = load_gateway_metadata("sys-gw").unwrap(); + assert_eq!(meta.name, "sys-gw"); + assert_eq!(meta.gateway_endpoint, "unix:///tmp/sys.sock"); + }); + } + + #[test] + fn gateway_metadata_source_reports_user_system_and_missing() { + let user = tempfile::tempdir().unwrap(); + let system = tempfile::tempdir().unwrap(); + with_tmp_xdg_and_system(user.path(), system.path(), || { + write_system_metadata( + &system.path().join("gateways"), + "sys-gw", + "unix:///tmp/sys.sock", + ); + assert_eq!( + gateway_metadata_source("sys-gw").unwrap(), + Some(GatewayMetadataSource::System) + ); + + let user_meta = GatewayMetadata { + name: "user-gw".to_string(), + gateway_endpoint: "https://user-endpoint".to_string(), + ..Default::default() + }; + store_gateway_metadata("user-gw", &user_meta).unwrap(); + assert_eq!( + gateway_metadata_source("user-gw").unwrap(), + Some(GatewayMetadataSource::User) + ); + + assert_eq!(gateway_metadata_source("missing").unwrap(), None); + }); + } + + #[test] + fn load_gateway_metadata_error_mentions_both_search_paths() { + let user = tempfile::tempdir().unwrap(); + let system = tempfile::tempdir().unwrap(); + with_tmp_xdg_and_system(user.path(), system.path(), || { + let err = load_gateway_metadata("missing").unwrap_err(); + let msg = err.to_string(); + assert!(msg.contains("missing"), "expected name in error: {msg}"); + assert!( + msg.contains(user.path().to_str().unwrap()), + "expected user path in error: {msg}" + ); + assert!( + msg.contains(system.path().to_str().unwrap()), + "expected system path in error: {msg}" + ); + }); + } + + #[test] + fn load_gateway_metadata_prefers_user_over_system() { + let user = tempfile::tempdir().unwrap(); + let system = tempfile::tempdir().unwrap(); + with_tmp_xdg_and_system(user.path(), system.path(), || { + let user_meta = GatewayMetadata { + name: "shared".to_string(), + gateway_endpoint: "https://user-endpoint".to_string(), + ..Default::default() + }; + store_gateway_metadata("shared", &user_meta).unwrap(); + write_system_metadata( + &system.path().join("gateways"), + "shared", + "https://system-endpoint", + ); + let meta = load_gateway_metadata("shared").unwrap(); + assert_eq!(meta.gateway_endpoint, "https://user-endpoint"); + }); + } + + #[test] + fn list_gateways_merges_user_and_system() { + let user = tempfile::tempdir().unwrap(); + let system = tempfile::tempdir().unwrap(); + with_tmp_xdg_and_system(user.path(), system.path(), || { + let user_meta = GatewayMetadata { + name: "alpha".to_string(), + gateway_endpoint: "https://alpha".to_string(), + ..Default::default() + }; + store_gateway_metadata("alpha", &user_meta).unwrap(); + write_system_metadata(&system.path().join("gateways"), "beta", "https://beta"); + let gateways = list_gateways_with_source().unwrap(); + assert_eq!(gateways.len(), 2); + assert_eq!(gateways[0].metadata.name, "alpha"); + assert_eq!(gateways[0].source, GatewayMetadataSource::User); + assert_eq!(gateways[1].metadata.name, "beta"); + assert_eq!(gateways[1].source, GatewayMetadataSource::System); + }); + } + + #[test] + fn list_gateways_user_shadows_system_on_collision() { + let user = tempfile::tempdir().unwrap(); + let system = tempfile::tempdir().unwrap(); + with_tmp_xdg_and_system(user.path(), system.path(), || { + let user_meta = GatewayMetadata { + name: "local-vm".to_string(), + gateway_endpoint: "https://user-override".to_string(), + ..Default::default() + }; + store_gateway_metadata("local-vm", &user_meta).unwrap(); + write_system_metadata( + &system.path().join("gateways"), + "local-vm", + "unix:///tmp/sys.sock", + ); + let gateways = list_gateways_with_source().unwrap(); + assert_eq!(gateways.len(), 1); + assert_eq!( + gateways[0].metadata.gateway_endpoint, + "https://user-override" + ); + assert_eq!(gateways[0].source, GatewayMetadataSource::User); + }); + } } diff --git a/crates/openshell-bootstrap/src/oidc_token.rs b/crates/openshell-bootstrap/src/oidc_token.rs index 19c6cabaa..987ac9111 100644 --- 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//oidc_token.json`. //! File permissions are `0600` (owner-only). -use crate::paths::gateways_dir; +use crate::paths::user_gateways_dir; use miette::{IntoDiagnostic, Result, WrapErr}; use openshell_core::paths::{ensure_parent_dir_restricted, set_file_owner_only}; use serde::{Deserialize, Serialize}; @@ -36,7 +36,9 @@ pub struct OidcTokenBundle { /// Path to the stored OIDC token bundle for a gateway. pub fn oidc_token_path(gateway_name: &str) -> Result { - Ok(gateways_dir()?.join(gateway_name).join("oidc_token.json")) + Ok(user_gateways_dir()? + .join(gateway_name) + .join("oidc_token.json")) } /// Store an OIDC token bundle for a gateway. diff --git a/crates/openshell-bootstrap/src/paths.rs b/crates/openshell-bootstrap/src/paths.rs index cd3cb7693..7d0074d0f 100644 --- a/crates/openshell-bootstrap/src/paths.rs +++ b/crates/openshell-bootstrap/src/paths.rs @@ -2,34 +2,178 @@ // SPDX-License-Identifier: Apache-2.0 use miette::Result; -use openshell_core::paths::xdg_config_dir; +use openshell_core::paths::openshell_config_dir; use std::path::PathBuf; +/// Env var pointing at a system-level `OpenShell` config root override. +/// +/// Set by installers (snap, deb, systemd unit, dev wrappers) that want +/// to surface deployment-provided gateways without requiring the user to +/// register them. The directory uses the same layout as the per-user config +/// root: `active_gateway` plus `gateways//metadata.json`. CLI behaviour +/// treats it as read-only; all writes go to the per-user XDG location, which +/// shadows system entries on name collision. When unset, `OpenShell` falls +/// back to `/etc/openshell`. +pub const SYSTEM_GATEWAY_DIR_ENV: &str = "OPENSHELL_SYSTEM_GATEWAY_DIR"; + +const DEFAULT_SYSTEM_CONFIG_DIR: &str = "/etc/openshell"; +fn system_config_dir_override() -> Option { + let path = PathBuf::from(std::env::var_os(SYSTEM_GATEWAY_DIR_ENV)?); + if path.as_os_str().is_empty() { + tracing::warn!( + env = SYSTEM_GATEWAY_DIR_ENV, + "ignoring empty system gateway dir override" + ); + return None; + } + if !path.is_absolute() { + tracing::warn!( + env = SYSTEM_GATEWAY_DIR_ENV, + path = %path.display(), + "ignoring relative system gateway dir override" + ); + return None; + } + Some(path) +} + /// Path to the file that stores the active gateway name. /// /// Location: `$XDG_CONFIG_HOME/openshell/active_gateway` -pub fn active_gateway_path() -> Result { - Ok(xdg_config_dir()?.join("openshell").join("active_gateway")) +pub fn user_active_gateway_path() -> Result { + Ok(openshell_config_dir()?.join("active_gateway")) } /// Base directory for all gateway metadata files. /// /// Location: `$XDG_CONFIG_HOME/openshell/gateways/` -pub fn gateways_dir() -> Result { - Ok(xdg_config_dir()?.join("openshell").join("gateways")) +pub fn user_gateways_dir() -> Result { + Ok(openshell_config_dir()?.join("gateways")) +} + +/// Read-only system-level `OpenShell` config root. +/// +/// Uses `OPENSHELL_SYSTEM_GATEWAY_DIR` when set; otherwise falls back to +/// `/etc/openshell`. +pub fn system_config_dir() -> PathBuf { + system_config_dir_override().unwrap_or_else(|| PathBuf::from(DEFAULT_SYSTEM_CONFIG_DIR)) +} + +/// Read-only system-level gateway metadata directory. +pub fn system_gateways_dir() -> PathBuf { + system_config_dir().join("gateways") +} + +/// Optional system-level active gateway file within the system config root. +pub fn system_active_gateway_path() -> PathBuf { + system_config_dir().join("active_gateway") } /// Path to the file that stores the last-used sandbox name for a gateway. /// /// Location: `$XDG_CONFIG_HOME/openshell/gateways//last_sandbox` pub fn last_sandbox_path(gateway: &str) -> Result { - Ok(gateways_dir()?.join(gateway).join("last_sandbox")) + Ok(user_gateways_dir()?.join(gateway).join("last_sandbox")) } #[cfg(test)] mod tests { use super::*; + #[test] + #[allow(unsafe_code)] + fn system_config_dir_defaults_to_etc_openshell() { + let _guard = crate::XDG_TEST_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let orig_sys = std::env::var(SYSTEM_GATEWAY_DIR_ENV).ok(); + unsafe { + std::env::remove_var(SYSTEM_GATEWAY_DIR_ENV); + } + assert_eq!(system_config_dir(), PathBuf::from("/etc/openshell")); + assert_eq!( + system_gateways_dir(), + PathBuf::from("/etc/openshell/gateways") + ); + assert_eq!( + system_active_gateway_path(), + PathBuf::from("/etc/openshell/active_gateway") + ); + unsafe { + match orig_sys { + Some(v) => std::env::set_var(SYSTEM_GATEWAY_DIR_ENV, v), + None => std::env::remove_var(SYSTEM_GATEWAY_DIR_ENV), + } + } + } + + #[test] + #[allow(unsafe_code)] + fn system_config_dir_prefers_env_override() { + let _guard = crate::XDG_TEST_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let tmp = tempfile::tempdir().unwrap(); + let override_dir = tmp.path().join("openshell-system"); + let orig_sys = std::env::var(SYSTEM_GATEWAY_DIR_ENV).ok(); + unsafe { + std::env::set_var(SYSTEM_GATEWAY_DIR_ENV, &override_dir); + } + assert_eq!(system_config_dir(), override_dir); + assert_eq!( + system_gateways_dir(), + tmp.path().join("openshell-system/gateways") + ); + assert_eq!( + system_active_gateway_path(), + tmp.path().join("openshell-system/active_gateway") + ); + unsafe { + match orig_sys { + Some(v) => std::env::set_var(SYSTEM_GATEWAY_DIR_ENV, v), + None => std::env::remove_var(SYSTEM_GATEWAY_DIR_ENV), + } + } + } + + #[test] + #[allow(unsafe_code)] + fn system_config_dir_ignores_empty_env_override() { + let _guard = crate::XDG_TEST_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let orig_sys = std::env::var(SYSTEM_GATEWAY_DIR_ENV).ok(); + unsafe { + std::env::set_var(SYSTEM_GATEWAY_DIR_ENV, ""); + } + assert_eq!(system_config_dir(), PathBuf::from("/etc/openshell")); + unsafe { + match orig_sys { + Some(v) => std::env::set_var(SYSTEM_GATEWAY_DIR_ENV, v), + None => std::env::remove_var(SYSTEM_GATEWAY_DIR_ENV), + } + } + } + + #[test] + #[allow(unsafe_code)] + fn system_config_dir_ignores_relative_env_override() { + let _guard = crate::XDG_TEST_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let orig_sys = std::env::var(SYSTEM_GATEWAY_DIR_ENV).ok(); + unsafe { + std::env::set_var(SYSTEM_GATEWAY_DIR_ENV, "relative/openshell-system"); + } + assert_eq!(system_config_dir(), PathBuf::from("/etc/openshell")); + unsafe { + match orig_sys { + Some(v) => std::env::set_var(SYSTEM_GATEWAY_DIR_ENV, v), + None => std::env::remove_var(SYSTEM_GATEWAY_DIR_ENV), + } + } + } + #[test] #[allow(unsafe_code)] fn last_sandbox_path_layout() { diff --git a/docs/sandboxes/manage-gateways.mdx b/docs/sandboxes/manage-gateways.mdx index c92b86958..ce4496e8d 100644 --- a/docs/sandboxes/manage-gateways.mdx +++ b/docs/sandboxes/manage-gateways.mdx @@ -82,6 +82,8 @@ One gateway is always the active gateway. All CLI commands target it by default. The active gateway is the persisted default. The `-g` flag and the `OPENSHELL_GATEWAY` environment variable override it when commands resolve a gateway. If `OPENSHELL_GATEWAY` is set to a different gateway, `openshell gateway select ` still saves the new default and warns that the current shell continues to use the environment value until you unset or update it. +Installers can seed read-only gateway entries for package-managed local services. By default the CLI reads these from `/etc/openshell`, using the same `active_gateway` plus `gateways//metadata.json` layout as per-user config. Packages can override that system config root with a non-empty absolute `OPENSHELL_SYSTEM_GATEWAY_DIR` when needed; empty or relative values fall back to `/etc/openshell` and log a warning. These entries appear in `openshell gateway list` and can be selected like user registrations. `openshell gateway list` and `openshell term` label each gateway as `user` or `system` so you can see which config layer owns it. `openshell gateway remove` removes only per-user registrations. Register a per-user gateway with the same name when you need to shadow an installer-provided default. + List all registered gateways: ```shell From 19c8d8f80df1692d4a73ef90ff38cdbd0e067870 Mon Sep 17 00:00:00 2001 From: Alex Lewontin Date: Thu, 28 May 2026 18:34:43 -0400 Subject: [PATCH 2/8] feat(cli): show gateway config source in list and term Expose whether a gateway registration comes from user or system config in `openshell gateway list`, the TUI gateway pane, and list JSON output. The CLI also refuses to remove system-managed registrations and the smoke tests cover the new list output. Signed-off-by: Alex Lewontin --- crates/openshell-cli/src/run.rs | 119 +++++++---- crates/openshell-tui/src/app.rs | 44 ++++ crates/openshell-tui/src/lib.rs | 10 +- crates/openshell-tui/src/ui/dashboard.rs | 7 +- crates/openshell-tui/src/ui/mod.rs | 10 +- e2e/rust/tests/cli_smoke.rs | 244 ++++++++++++++++++++++- 6 files changed, 385 insertions(+), 49 deletions(-) diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index dbd240238..59dd7e9c3 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -24,6 +24,9 @@ use openshell_bootstrap::{ remove_gateway_metadata, resolve_ssh_hostname, save_active_gateway, save_last_sandbox, store_gateway_metadata, }; +use openshell_bootstrap::{ + GatewayMetadataSource, ListedGateway, gateway_metadata_source, list_gateways_with_source, +}; use openshell_core::progress::{ PROGRESS_ACTIVE_DETAIL_KEY, PROGRESS_ACTIVE_STEP_KEY, PROGRESS_COMPLETE_LABEL_KEY, PROGRESS_COMPLETE_STEP_KEY, PROGRESS_STEP_PULLING_IMAGE, PROGRESS_STEP_REQUESTING_SANDBOX, @@ -923,15 +926,17 @@ pub async fn gateway_add( &derived_name }; - // Fail if a gateway with this name already exists. - if get_gateway_metadata(name).is_some() { - return Err(miette::miette!( - "Gateway '{}' already exists.\n\ - Remove it first with: openshell gateway remove {}\n\ - Or choose a different name with: --name ", - name, - name, - )); + match gateway_metadata_source(name)? { + Some(GatewayMetadataSource::User) => { + return Err(miette::miette!( + "Gateway '{}' already exists.\n\ + Remove it first with: openshell gateway remove {}\n\ + Or choose a different name with: --name ", + name, + name, + )); + } + Some(GatewayMetadataSource::System) | None => {} } // OIDC takes precedence over plaintext/mTLS/edge detection — the user @@ -1284,7 +1289,7 @@ pub fn gateway_logout(name: &str) -> Result<()> { /// List all registered gateways. pub fn gateway_list(gateway_flag: &Option, output: &str) -> Result<()> { - let gateways = list_gateways()?; + let gateways = list_gateways_with_source()?; let active = gateway_flag.clone().or_else(load_active_gateway); if crate::output::print_output_collection(output, &gateways, |g| gateway_to_json(g, &active))? { @@ -1304,41 +1309,52 @@ pub fn gateway_list(gateway_flag: &Option, output: &str) -> Result<()> { // Calculate column widths let name_width = gateways .iter() - .map(|g| g.name.len()) + .map(|g| g.metadata.name.len()) .max() .unwrap_or(4) .max(4); let endpoint_width = gateways .iter() - .map(|g| g.gateway_endpoint.len()) + .map(|g| g.metadata.gateway_endpoint.len()) .max() .unwrap_or(8) .max(8); let type_width = gateways .iter() - .map(|g| gateway_type_label(g).len()) + .map(|g| gateway_type_label(&g.metadata).len()) .max() .unwrap_or(4) .max(4); + let source_width = gateways + .iter() + .map(|g| g.source.label().len()) + .max() + .unwrap_or(6) + .max(6); // Print header println!( - " {:, output: &str) -> Result<()> { Ok(()) } -fn gateway_to_json(gateway: &GatewayMetadata, active: &Option) -> serde_json::Value { +fn gateway_to_json(gateway: &ListedGateway, active: &Option) -> serde_json::Value { + let metadata = &gateway.metadata; serde_json::json!({ - "name": gateway.name, - "endpoint": gateway.gateway_endpoint, - "type": gateway_type_label(gateway), - "auth": gateway_auth_label(gateway), - "active": active.as_deref() == Some(&gateway.name), + "name": metadata.name, + "endpoint": metadata.gateway_endpoint, + "type": gateway_type_label(metadata), + "source": gateway.source.label(), + "auth": gateway_auth_label(metadata), + "active": active.as_deref() == Some(&metadata.name), }) } @@ -1455,11 +1473,20 @@ fn remove_gateway_registration(name: &str) { /// Remove a local gateway registration without touching the gateway service. pub fn gateway_remove(name: &str) -> Result<()> { - if get_gateway_metadata(name).is_none() { - return Err(miette::miette!( - "No gateway metadata found for '{name}'.\n\ - List available gateways: openshell gateway select" - )); + match gateway_metadata_source(name)? { + Some(GatewayMetadataSource::User) => {} + Some(GatewayMetadataSource::System) => { + return Err(miette::miette!( + "Gateway registration '{name}' is installed by the system and cannot be removed from user config.\n\ + Register a per-user gateway with the same name to override it, or select another gateway." + )); + } + None => { + return Err(miette::miette!( + "No gateway metadata found for '{name}'.\n\ + List available gateways: openshell gateway select" + )); + } } remove_gateway_registration(name); @@ -7568,8 +7595,8 @@ mod tests { ProvisioningStep, TlsOptions, build_sandbox_resource_limits, dockerfile_sources_supported_for_gateway, format_endpoint, format_gateway_select_header, format_gateway_select_items, format_provider_attachment_table, gateway_add, - gateway_auth_label, gateway_env_override_warning, gateway_select_with, gateway_type_label, - git_sync_files, http_health_check, import_local_package_mtls_bundle, + gateway_auth_label, gateway_env_override_warning, gateway_select_with, gateway_to_json, + gateway_type_label, git_sync_files, http_health_check, import_local_package_mtls_bundle, inferred_provider_type, mtls_certs_exist_for_gateway, package_managed_tls_dirs, parse_cli_setting_value, parse_credential_expiry_cli_value, parse_credential_expiry_pairs, parse_credential_pairs, parse_driver_config_json, plaintext_gateway_is_remote, @@ -7580,7 +7607,6 @@ mod tests { }; use crate::TEST_ENV_LOCK; use hyper::StatusCode; - use openshell_bootstrap::{load_active_gateway, load_gateway_metadata, store_gateway_metadata}; use std::fs; use std::io::{Read, Write}; use std::net::TcpListener; @@ -7589,7 +7615,10 @@ mod tests { use std::thread; use tonic::Status; - use openshell_bootstrap::GatewayMetadata; + use openshell_bootstrap::{ + GatewayMetadata, GatewayMetadataSource, ListedGateway, load_active_gateway, + load_gateway_metadata, store_gateway_metadata, + }; use openshell_core::progress::{ PROGRESS_STEP_PULLING_IMAGE, PROGRESS_STEP_REQUESTING_SANDBOX, PROGRESS_STEP_STARTING_SANDBOX, @@ -8578,6 +8607,26 @@ mod tests { assert!(items[1].contains("http://127.0.0.1:8080")); } + #[test] + fn gateway_to_json_includes_config_source() { + let gateway = ListedGateway { + metadata: GatewayMetadata { + name: "local-vm".to_string(), + gateway_endpoint: "http://127.0.0.1:17670".to_string(), + auth_mode: Some("plaintext".to_string()), + ..Default::default() + }, + source: GatewayMetadataSource::System, + }; + + let json = gateway_to_json(&gateway, &Some("local-vm".to_string())); + + assert_eq!(json["source"], "system"); + assert_eq!(json["type"], "local"); + assert_eq!(json["auth"], "plaintext"); + assert_eq!(json["active"], true); + } + #[test] fn gateway_auth_label_defaults_https_gateways_to_mtls() { let gateway = GatewayMetadata { diff --git a/crates/openshell-tui/src/app.rs b/crates/openshell-tui/src/app.rs index cb02c8c24..4538b207d 100644 --- a/crates/openshell-tui/src/app.rs +++ b/crates/openshell-tui/src/app.rs @@ -5,6 +5,7 @@ use std::collections::HashMap; use std::time::{Duration, Instant}; use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; +use openshell_bootstrap::GatewayMetadataSource; use openshell_core::auth::EdgeAuthInterceptor; use openshell_core::proto::open_shell_client::OpenShellClient; use openshell_core::proto::setting_value; @@ -214,10 +215,21 @@ pub fn display_setting_value(value: &Option) -> String { // Gateway entry // --------------------------------------------------------------------------- +#[derive(Debug, Clone, PartialEq, Eq)] pub struct GatewayEntry { pub name: String, pub endpoint: String, pub is_remote: bool, + pub source: Option, +} + +impl GatewayEntry { + pub const fn source_label(&self) -> &'static str { + match self.source { + Some(source) => source.label(), + None => "unknown", + } + } } // --------------------------------------------------------------------------- @@ -2819,6 +2831,7 @@ fn clamped_scroll(current: usize, delta: isize, total: usize, viewport: usize) - #[cfg(test)] mod tests { use super::*; + use openshell_bootstrap::GatewayMetadataSource; // -- clamped_scroll ------------------------------------------------- @@ -2887,4 +2900,35 @@ mod tests { fn scroll_up_one() { assert_eq!(clamped_scroll(10, -1, 100, 20), 9); } + + #[test] + fn gateway_entry_source_label_formats_known_sources() { + let user_gateway = GatewayEntry { + name: "user-gw".to_string(), + endpoint: "https://user.example.com".to_string(), + is_remote: true, + source: Some(GatewayMetadataSource::User), + }; + let system_gateway = GatewayEntry { + name: "system-gw".to_string(), + endpoint: "http://127.0.0.1:17670".to_string(), + is_remote: false, + source: Some(GatewayMetadataSource::System), + }; + + assert_eq!(user_gateway.source_label(), "user"); + assert_eq!(system_gateway.source_label(), "system"); + } + + #[test] + fn gateway_entry_source_label_handles_unknown_source() { + let gateway = GatewayEntry { + name: "mystery".to_string(), + endpoint: "https://mystery.example.com".to_string(), + is_remote: true, + source: None, + }; + + assert_eq!(gateway.source_label(), "unknown"); + } } diff --git a/crates/openshell-tui/src/lib.rs b/crates/openshell-tui/src/lib.rs index b7e03c6ac..7992666d3 100644 --- a/crates/openshell-tui/src/lib.rs +++ b/crates/openshell-tui/src/lib.rs @@ -18,6 +18,7 @@ use crossterm::terminal::{ EnterAlternateScreen, LeaveAlternateScreen, disable_raw_mode, enable_raw_mode, }; use miette::{IntoDiagnostic, Result}; +use openshell_bootstrap::list_gateways_with_source; use openshell_core::auth::EdgeAuthInterceptor; use openshell_core::metadata::{ObjectId, ObjectLabels, ObjectName}; use openshell_core::proto::SandboxPhase; @@ -440,13 +441,14 @@ pub async fn run( /// Refresh the list of known gateways from disk. fn refresh_gateway_list(app: &mut App) { - if let Ok(gateways) = openshell_bootstrap::list_gateways() { + if let Ok(gateways) = list_gateways_with_source() { app.gateways = gateways .into_iter() .map(|m| GatewayEntry { - name: m.name, - endpoint: m.gateway_endpoint, - is_remote: m.is_remote, + source: Some(m.source), + name: m.metadata.name, + endpoint: m.metadata.gateway_endpoint, + is_remote: m.metadata.is_remote, }) .collect(); diff --git a/crates/openshell-tui/src/ui/dashboard.rs b/crates/openshell-tui/src/ui/dashboard.rs index 1de2578fb..c0e2c573c 100644 --- a/crates/openshell-tui/src/ui/dashboard.rs +++ b/crates/openshell-tui/src/ui/dashboard.rs @@ -41,6 +41,7 @@ fn draw_gateway_list(frame: &mut Frame<'_>, app: &App, area: Rect) { let header = Row::new(vec![ Cell::from(Span::styled(" NAME", t.muted)), Cell::from(Span::styled("TYPE", t.muted)), + Cell::from(Span::styled("SOURCE", t.muted)), Cell::from(Span::styled("STATUS", t.muted)), Cell::from(Span::styled("", t.muted)), ]) @@ -93,6 +94,7 @@ fn draw_gateway_list(frame: &mut Frame<'_>, app: &App, area: Rect) { Row::new(vec![ name_cell, Cell::from(Span::styled(type_label, t.muted)), + Cell::from(Span::styled(entry.source_label(), t.muted)), status_cell, policy_cell, ]) @@ -108,9 +110,10 @@ fn draw_gateway_list(frame: &mut Frame<'_>, app: &App, area: Rect) { .padding(Padding::horizontal(1)); let widths = [ - Constraint::Percentage(30), + Constraint::Percentage(24), Constraint::Percentage(10), - Constraint::Percentage(25), + Constraint::Percentage(10), + Constraint::Percentage(21), Constraint::Percentage(35), ]; diff --git a/crates/openshell-tui/src/ui/mod.rs b/crates/openshell-tui/src/ui/mod.rs index f44b9eef1..9200e8f76 100644 --- a/crates/openshell-tui/src/ui/mod.rs +++ b/crates/openshell-tui/src/ui/mod.rs @@ -132,13 +132,21 @@ fn draw_title_bar(frame: &mut Frame<'_>, app: &App, area: Rect) { _ => Span::styled(&app.status_text, t.muted), }; + let active_gateway_source = app + .gateways + .iter() + .find(|gateway| gateway.name == app.gateway_name) + .map_or("unknown", app::GatewayEntry::source_label); + let mut parts: Vec> = vec![ Span::styled(" >_ OpenShell ", t.accent_bold), Span::styled(" ALPHA ", t.badge), Span::styled(" | ", t.muted), Span::styled("Current Gateway: ", t.text), Span::styled(&app.gateway_name, t.heading), - Span::styled(" (", t.muted), + Span::styled(" [", t.muted), + Span::styled(active_gateway_source, t.muted), + Span::styled("] (", t.muted), status_span, Span::styled(")", t.muted), Span::styled(" | ", t.muted), diff --git a/e2e/rust/tests/cli_smoke.rs b/e2e/rust/tests/cli_smoke.rs index 265b236c4..a68823e80 100644 --- a/e2e/rust/tests/cli_smoke.rs +++ b/e2e/rust/tests/cli_smoke.rs @@ -7,24 +7,33 @@ //! directly, validating that the restructured command tree parses correctly and //! handles edge cases like missing gateway configuration. +use std::fs; +use std::path::Path; use std::process::Stdio; use openshell_e2e::harness::binary::openshell_cmd; use openshell_e2e::harness::output::strip_ansi; -/// Run `openshell ` with an isolated (empty) config directory so it -/// cannot discover any real gateway. -async fn run_isolated(args: &[&str]) -> (String, i32) { - let tmpdir = tempfile::tempdir().expect("create isolated config dir"); +async fn run_with_config( + config_dir: &Path, + system_dir: Option<&Path>, + args: &[&str], +) -> (String, i32) { let mut cmd = openshell_cmd(); cmd.args(args) - .env("XDG_CONFIG_HOME", tmpdir.path()) - .env("HOME", tmpdir.path()) + .env("XDG_CONFIG_HOME", config_dir) + .env("HOME", config_dir) .env_remove("OPENSHELL_GATEWAY") .env_remove("OPENSHELL_GATEWAY_ENDPOINT") .stdout(Stdio::piped()) .stderr(Stdio::piped()); + if let Some(system_dir) = system_dir { + cmd.env("OPENSHELL_SYSTEM_GATEWAY_DIR", system_dir); + } else { + cmd.env_remove("OPENSHELL_SYSTEM_GATEWAY_DIR"); + } + let output = cmd.output().await.expect("spawn openshell"); let stdout = String::from_utf8_lossy(&output.stdout).to_string(); let stderr = String::from_utf8_lossy(&output.stderr).to_string(); @@ -33,6 +42,100 @@ async fn run_isolated(args: &[&str]) -> (String, i32) { (combined, code) } +/// Run `openshell ` with an isolated (empty) config directory so it +/// cannot discover any real gateway. +async fn run_isolated(args: &[&str]) -> (String, i32) { + let tmpdir = tempfile::tempdir().expect("create isolated config dir"); + let system_dir = tempfile::tempdir().expect("create isolated system config dir"); + run_with_config(tmpdir.path(), Some(system_dir.path()), args).await +} + +fn write_gateway_metadata( + root: &Path, + name: &str, + endpoint: &str, + gateway_port: u16, + is_remote: bool, + auth_mode: &str, +) { + let gateway_dir = root.join("gateways").join(name); + fs::create_dir_all(&gateway_dir).expect("create gateway dir"); + let metadata = serde_json::json!({ + "name": name, + "gateway_endpoint": endpoint, + "gateway_port": gateway_port, + "is_remote": is_remote, + "auth_mode": auth_mode, + }); + fs::write( + gateway_dir.join("metadata.json"), + serde_json::to_vec_pretty(&metadata).expect("serialize gateway metadata"), + ) + .expect("write gateway metadata"); +} + +fn write_user_gateway_metadata( + config_dir: &Path, + name: &str, + endpoint: &str, + gateway_port: u16, + is_remote: bool, + auth_mode: &str, +) { + write_gateway_metadata( + &config_dir.join("openshell"), + name, + endpoint, + gateway_port, + is_remote, + auth_mode, + ); +} + +fn write_system_gateway_metadata( + system_dir: &Path, + name: &str, + endpoint: &str, + gateway_port: u16, + is_remote: bool, + auth_mode: &str, +) { + write_gateway_metadata( + system_dir, + name, + endpoint, + gateway_port, + is_remote, + auth_mode, + ); +} + +fn write_active_gateway(config_dir: &Path, name: &str) { + let active_path = config_dir.join("openshell").join("active_gateway"); + fs::create_dir_all(active_path.parent().expect("active gateway parent")) + .expect("create active gateway parent"); + fs::write(active_path, format!("{name}\n")).expect("write active gateway"); +} + +fn seed_gateway_sources(config_dir: &Path, system_dir: &Path) { + write_user_gateway_metadata( + config_dir, + "alpha", + "https://alpha.example.com", + 443, + true, + "cloudflare_jwt", + ); + write_system_gateway_metadata( + system_dir, + "beta", + "http://127.0.0.1:17670", + 17670, + false, + "plaintext", + ); +} + // ------------------------------------------------------------------- // Top-level --help shows the restructured command tree // ------------------------------------------------------------------- @@ -84,7 +187,9 @@ async fn sandbox_help_shows_upload_download() { assert_eq!(code, 0, "openshell sandbox --help should exit 0"); let clean = strip_ansi(&output); - for sub in ["upload", "download", "create", "get", "list", "delete", "connect"] { + for sub in [ + "upload", "download", "create", "get", "list", "delete", "connect", + ] { assert!( clean.contains(sub), "expected '{sub}' in sandbox --help output:\n{clean}" @@ -170,3 +275,128 @@ async fn status_without_gateway_prints_friendly_message() { "expected hint to register a gateway:\n{clean}" ); } + +// ------------------------------------------------------------------- +// Gateway list source indicators +// ------------------------------------------------------------------- + +#[tokio::test] +async fn gateway_list_table_shows_user_and_system_sources() { + let config_dir = tempfile::tempdir().expect("create config dir"); + let system_dir = tempfile::tempdir().expect("create system dir"); + seed_gateway_sources(config_dir.path(), system_dir.path()); + write_active_gateway(config_dir.path(), "alpha"); + + let (output, code) = run_with_config( + config_dir.path(), + Some(system_dir.path()), + &["gateway", "list"], + ) + .await; + assert_eq!(code, 0, "gateway list should exit 0:\n{output}"); + + let clean = strip_ansi(&output); + assert!(clean.contains("SOURCE"), "expected SOURCE column:\n{clean}"); + + let alpha_line = clean + .lines() + .find(|line| line.contains("alpha")) + .expect("find alpha row"); + assert!( + alpha_line.contains("user"), + "expected alpha row to show user source:\n{clean}" + ); + + let beta_line = clean + .lines() + .find(|line| line.contains("beta")) + .expect("find beta row"); + assert!( + beta_line.contains("system"), + "expected beta row to show system source:\n{clean}" + ); +} + +#[tokio::test] +async fn gateway_list_json_includes_user_and_system_sources() { + let config_dir = tempfile::tempdir().expect("create config dir"); + let system_dir = tempfile::tempdir().expect("create system dir"); + seed_gateway_sources(config_dir.path(), system_dir.path()); + + let (output, code) = run_with_config( + config_dir.path(), + Some(system_dir.path()), + &["gateway", "list", "-o", "json"], + ) + .await; + assert_eq!(code, 0, "gateway list -o json should exit 0:\n{output}"); + + let items: serde_json::Value = serde_json::from_str(&output).expect("parse gateway list json"); + let items = items.as_array().expect("gateway list json array"); + assert_eq!(items.len(), 2, "expected two gateways in json output"); + + let alpha = items + .iter() + .find(|item| item["name"] == "alpha") + .expect("find alpha entry"); + assert_eq!(alpha["source"], "user"); + + let beta = items + .iter() + .find(|item| item["name"] == "beta") + .expect("find beta entry"); + assert_eq!(beta["source"], "system"); +} + +#[tokio::test] +async fn gateway_add_can_shadow_system_gateway_with_user_registration() { + let config_dir = tempfile::tempdir().expect("create config dir"); + let system_dir = tempfile::tempdir().expect("create system dir"); + write_system_gateway_metadata( + system_dir.path(), + "beta", + "http://127.0.0.1:17670", + 17670, + false, + "plaintext", + ); + + let (add_output, add_code) = run_with_config( + config_dir.path(), + Some(system_dir.path()), + &[ + "gateway", + "add", + "http://127.0.0.1:17671", + "--name", + "beta", + ], + ) + .await; + assert_eq!( + add_code, 0, + "gateway add should allow a user registration to shadow a system gateway:\n{add_output}" + ); + + let (list_output, list_code) = run_with_config( + config_dir.path(), + Some(system_dir.path()), + &["gateway", "list", "-o", "json"], + ) + .await; + assert_eq!( + list_code, 0, + "gateway list -o json should exit 0:\n{list_output}" + ); + + let items: serde_json::Value = + serde_json::from_str(&list_output).expect("parse gateway list json"); + let beta = items + .as_array() + .expect("gateway list json array") + .iter() + .find(|item| item["name"] == "beta") + .expect("find beta entry"); + assert_eq!(beta["source"], "user"); + assert_eq!(beta["endpoint"], "http://127.0.0.1:17671"); +} From 91264b97f12c9a488674956ae5f6bf5e0031f9d6 Mon Sep 17 00:00:00 2001 From: Alex Lewontin Date: Thu, 4 Jun 2026 12:08:07 -0400 Subject: [PATCH 3/8] fix(bootstrap): preserve user shadowing on invalid metadata Signed-off-by: Alex Lewontin --- crates/openshell-bootstrap/src/metadata.rs | 104 ++++++++++++++++++--- 1 file changed, 91 insertions(+), 13 deletions(-) diff --git a/crates/openshell-bootstrap/src/metadata.rs b/crates/openshell-bootstrap/src/metadata.rs index 87ea9204d..fea2ecd18 100644 --- a/crates/openshell-bootstrap/src/metadata.rs +++ b/crates/openshell-bootstrap/src/metadata.rs @@ -107,7 +107,7 @@ fn system_gateway_metadata_path(name: &str) -> PathBuf { fn resolve_gateway_metadata_path(name: &str) -> Result> { let user = user_gateway_metadata_path(name)?; - if user.exists() { + if user_entry_shadows_system(&user) { return Ok(Some((user, GatewayMetadataSource::User))); } @@ -127,6 +127,10 @@ fn parse_gateway_metadata(path: &Path) -> Result { .into_diagnostic() .wrap_err("failed to parse gateway metadata") } +fn user_entry_shadows_system(metadata_path: &Path) -> bool { + metadata_path.try_exists().unwrap_or(true) +} + /// Extract the hostname from an SSH destination string. /// /// Handles formats like: @@ -291,34 +295,63 @@ pub fn list_gateways_with_source() -> Result> { let mut gateways = Vec::new(); let mut seen: std::collections::HashSet = std::collections::HashSet::new(); - let mut scan = |dir: PathBuf, source: GatewayMetadataSource| -> Result<()> { - if !dir.exists() { - return Ok(()); + let user_dir = user_gateways_dir()?; + if user_dir.exists() { + let entries = std::fs::read_dir(&user_dir) + .into_diagnostic() + .wrap_err_with(|| format!("failed to read directory {}", user_dir.display()))?; + for entry in entries { + let entry = entry.into_diagnostic()?; + let path = entry.path(); + if !path.is_dir() { + continue; + } + + let metadata_path = path.join("metadata.json"); + if !user_entry_shadows_system(&metadata_path) { + continue; + } + + let name = entry.file_name().to_string_lossy().to_string(); + if !seen.insert(name) { + continue; + } + + if let Ok(metadata) = parse_gateway_metadata(&metadata_path) { + gateways.push(ListedGateway { + metadata, + source: GatewayMetadataSource::User, + }); + } } - let entries = std::fs::read_dir(&dir) + } + + let system_dir = system_gateways_dir(); + if system_dir.exists() { + let entries = std::fs::read_dir(&system_dir) .into_diagnostic() - .wrap_err_with(|| format!("failed to read directory {}", dir.display()))?; + .wrap_err_with(|| format!("failed to read directory {}", system_dir.display()))?; for entry in entries { let entry = entry.into_diagnostic()?; let path = entry.path(); if !path.is_dir() { continue; } + let name = entry.file_name().to_string_lossy().to_string(); if seen.contains(&name) { continue; } + let metadata_path = path.join("metadata.json"); if let Ok(metadata) = parse_gateway_metadata(&metadata_path) { - seen.insert(name); - gateways.push(ListedGateway { metadata, source }); + gateways.push(ListedGateway { + metadata, + source: GatewayMetadataSource::System, + }); } } - Ok(()) - }; - - scan(user_gateways_dir()?, GatewayMetadataSource::User)?; - scan(system_gateways_dir(), GatewayMetadataSource::System)?; + } gateways.sort_by(|a, b| a.metadata.name.cmp(&b.metadata.name)); Ok(gateways) @@ -730,4 +763,49 @@ mod tests { assert_eq!(gateways[0].source, GatewayMetadataSource::User); }); } + + #[test] + fn list_gateways_invalid_user_entry_still_shadows_system() { + let user = tempfile::tempdir().unwrap(); + let system = tempfile::tempdir().unwrap(); + with_tmp_xdg_and_system(user.path(), system.path(), || { + let user_metadata_path = user_gateway_metadata_path("shared").unwrap(); + std::fs::create_dir_all(user_metadata_path.parent().unwrap()).unwrap(); + std::fs::write(&user_metadata_path, "{not-json").unwrap(); + + write_system_metadata(&system.path().join("gateways"), "shared", "https://system"); + + let gateways = list_gateways_with_source().unwrap(); + assert!(gateways.is_empty()); + }); + } + #[test] + fn list_gateways_empty_user_dir_does_not_hide_system() { + let user = tempfile::tempdir().unwrap(); + let system = tempfile::tempdir().unwrap(); + with_tmp_xdg_and_system(user.path(), system.path(), || { + let user_meta = GatewayMetadata { + name: "shared".to_string(), + gateway_endpoint: "https://user".to_string(), + ..Default::default() + }; + store_gateway_metadata("shared", &user_meta).unwrap(); + remove_gateway_metadata("shared").unwrap(); + + let user_gateway_dir = user_gateway_metadata_path("shared") + .unwrap() + .parent() + .unwrap() + .to_path_buf(); + assert!(user_gateway_dir.is_dir()); + assert!(!user_gateway_dir.join("metadata.json").exists()); + + write_system_metadata(&system.path().join("gateways"), "shared", "https://system"); + + let gateways = list_gateways_with_source().unwrap(); + assert_eq!(gateways.len(), 1); + assert_eq!(gateways[0].metadata.gateway_endpoint, "https://system"); + assert_eq!(gateways[0].source, GatewayMetadataSource::System); + }); + } } From 571ed2b886df2ec8bbd80deeabeadf83b9b5ed8b Mon Sep 17 00:00:00 2001 From: Alex Lewontin Date: Thu, 11 Jun 2026 10:49:33 -0400 Subject: [PATCH 4/8] fix(tui): honor plaintext gateway auth mode --- crates/openshell-tui/src/lib.rs | 112 ++++++++++++++++++++++++++------ 1 file changed, 93 insertions(+), 19 deletions(-) diff --git a/crates/openshell-tui/src/lib.rs b/crates/openshell-tui/src/lib.rs index 7992666d3..c7c57d405 100644 --- a/crates/openshell-tui/src/lib.rs +++ b/crates/openshell-tui/src/lib.rs @@ -18,7 +18,8 @@ use crossterm::terminal::{ EnterAlternateScreen, LeaveAlternateScreen, disable_raw_mode, enable_raw_mode, }; use miette::{IntoDiagnostic, Result}; -use openshell_bootstrap::list_gateways_with_source; +use openshell_bootstrap::{GatewayMetadata, list_gateways_with_source}; + use openshell_core::auth::EdgeAuthInterceptor; use openshell_core::metadata::{ObjectId, ObjectLabels, ObjectName}; use openshell_core::proto::SandboxPhase; @@ -494,6 +495,22 @@ async fn handle_gateway_switch(app: &mut App) { } } +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum GatewayChannelMode { + Oidc, + Plaintext, + Mtls, +} + +fn gateway_channel_mode(meta: Option<&GatewayMetadata>, endpoint: &str) -> GatewayChannelMode { + match meta.and_then(|m| m.auth_mode.as_deref()) { + Some("oidc") => GatewayChannelMode::Oidc, + Some("plaintext") => GatewayChannelMode::Plaintext, + _ if endpoint.starts_with("http://") => GatewayChannelMode::Plaintext, + _ => GatewayChannelMode::Mtls, + } +} + /// Build a gRPC channel and auth interceptor for a gateway. /// /// Checks gateway metadata for the auth mode and loads the appropriate @@ -501,28 +518,47 @@ async fn handle_gateway_switch(app: &mut App) { async fn connect_to_gateway(name: &str, endpoint: &str) -> Result<(Channel, EdgeAuthInterceptor)> { let meta = openshell_bootstrap::get_gateway_metadata(name); - if meta.as_ref().and_then(|m| m.auth_mode.as_deref()) == Some("oidc") { - let bundle = openshell_bootstrap::oidc_token::load_oidc_token(name).ok_or_else(|| { - miette::miette!( - "No OIDC token for gateway '{name}'.\n\ + match gateway_channel_mode(meta.as_ref(), endpoint) { + GatewayChannelMode::Oidc => { + let bundle = + openshell_bootstrap::oidc_token::load_oidc_token(name).ok_or_else(|| { + miette::miette!( + "No OIDC token for gateway '{name}'.\n\ Authenticate with: openshell gateway login" - ) - })?; - if openshell_bootstrap::oidc_token::is_token_expired(&bundle) { - miette::bail!( - "OIDC token for gateway '{name}' has expired.\n\ - Re-authenticate with: openshell gateway login" - ); - } - let interceptor = EdgeAuthInterceptor::new(Some(&bundle.access_token), None)?; - let channel = build_oidc_channel(name, endpoint).await?; - Ok((channel, interceptor)) - } else { - let channel = build_mtls_channel(name, endpoint).await?; - Ok((channel, EdgeAuthInterceptor::noop())) + ) + })?; + if openshell_bootstrap::oidc_token::is_token_expired(&bundle) { + miette::bail!( + "OIDC token for gateway '{name}' has expired.\n\ + Re-authenticate with: openshell gateway login" + ); + } + let interceptor = EdgeAuthInterceptor::new(Some(&bundle.access_token), None)?; + let channel = build_oidc_channel(name, endpoint).await?; + Ok((channel, interceptor)) + } + GatewayChannelMode::Plaintext => { + let channel = build_plaintext_channel(endpoint).await?; + Ok((channel, EdgeAuthInterceptor::noop())) + } + GatewayChannelMode::Mtls => { + let channel = build_mtls_channel(name, endpoint).await?; + Ok((channel, EdgeAuthInterceptor::noop())) + } } } +async fn build_plaintext_channel(endpoint: &str) -> Result { + Endpoint::from_shared(endpoint.to_string()) + .into_diagnostic()? + .connect_timeout(Duration::from_secs(10)) + .http2_keep_alive_interval(Duration::from_secs(10)) + .keep_alive_while_idle(true) + .connect() + .await + .into_diagnostic() +} + /// Build an HTTPS channel for OIDC-authenticated gateways. /// /// Tries mTLS client certs for the transport layer when available (the server @@ -2510,3 +2546,41 @@ fn days_to_ymd(days: i64) -> (i64, i64, i64) { let y = if m <= 2 { y + 1 } else { y }; (y, m, d) } + +#[cfg(test)] +mod tests { + use super::{GatewayChannelMode, gateway_channel_mode}; + use openshell_bootstrap::GatewayMetadata; + + #[test] + fn gateway_channel_mode_uses_plaintext_auth_mode() { + let meta = GatewayMetadata { + auth_mode: Some("plaintext".to_string()), + ..Default::default() + }; + assert_eq!( + gateway_channel_mode(Some(&meta), "https://gateway.example.com"), + GatewayChannelMode::Plaintext + ); + } + + #[test] + fn gateway_channel_mode_uses_http_endpoint_fallback() { + assert_eq!( + gateway_channel_mode(None, "http://127.0.0.1:17670"), + GatewayChannelMode::Plaintext + ); + } + + #[test] + fn gateway_channel_mode_prefers_oidc_metadata() { + let meta = GatewayMetadata { + auth_mode: Some("oidc".to_string()), + ..Default::default() + }; + assert_eq!( + gateway_channel_mode(Some(&meta), "https://gateway.example.com"), + GatewayChannelMode::Oidc + ); + } +} From d0363b48f9c7ab9bbed514d431844b6e75fb41b4 Mon Sep 17 00:00:00 2001 From: Alex Lewontin Date: Thu, 11 Jun 2026 10:49:33 -0400 Subject: [PATCH 5/8] fix(cli): keep system fallback out of rollback state --- crates/openshell-bootstrap/src/lib.rs | 4 +- crates/openshell-bootstrap/src/metadata.rs | 25 ++++- crates/openshell-cli/src/run.rs | 110 ++++++++++++++++++++- 3 files changed, 127 insertions(+), 12 deletions(-) diff --git a/crates/openshell-bootstrap/src/lib.rs b/crates/openshell-bootstrap/src/lib.rs index 0dd1b54d2..aa0532260 100644 --- a/crates/openshell-bootstrap/src/lib.rs +++ b/crates/openshell-bootstrap/src/lib.rs @@ -24,6 +24,6 @@ pub use crate::metadata::{ GatewayMetadata, GatewayMetadataSource, ListedGateway, clear_active_gateway, clear_last_sandbox_if_matches, extract_host_from_ssh_destination, gateway_metadata_source, get_gateway_metadata, list_gateways, list_gateways_with_source, load_active_gateway, - load_gateway_metadata, load_last_sandbox, remove_gateway_metadata, resolve_ssh_hostname, - save_active_gateway, save_last_sandbox, store_gateway_metadata, + load_gateway_metadata, load_last_sandbox, load_user_active_gateway, remove_gateway_metadata, + resolve_ssh_hostname, save_active_gateway, save_last_sandbox, store_gateway_metadata, }; diff --git a/crates/openshell-bootstrap/src/metadata.rs b/crates/openshell-bootstrap/src/metadata.rs index fea2ecd18..d5246aad3 100644 --- a/crates/openshell-bootstrap/src/metadata.rs +++ b/crates/openshell-bootstrap/src/metadata.rs @@ -243,17 +243,23 @@ fn read_nonempty_trimmed(path: &Path) -> Option { (!value.is_empty()).then(|| value.to_string()) } +/// Load the per-user active gateway name from persistent storage. +/// +/// Returns `None` if no user-scoped active gateway has been set. +pub fn load_user_active_gateway() -> Option { + user_active_gateway_path() + .ok() + .as_deref() + .and_then(read_nonempty_trimmed) +} + /// Load the active gateway name from persistent storage. /// /// Returns `None` if no active gateway has been set. Falls back to the /// 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 { - user_active_gateway_path() - .ok() - .as_deref() - .and_then(read_nonempty_trimmed) - .or_else(|| read_nonempty_trimmed(&system_active_gateway_path())) + load_user_active_gateway().or_else(|| read_nonempty_trimmed(&system_active_gateway_path())) } /// Save the last-used sandbox name for a gateway to persistent storage. @@ -610,6 +616,15 @@ mod tests { .unwrap(); } + #[test] + fn load_user_active_gateway_does_not_fall_back_to_system_dir() { + let user = tempfile::tempdir().unwrap(); + let system = tempfile::tempdir().unwrap(); + with_tmp_xdg_and_system(user.path(), system.path(), || { + std::fs::write(system.path().join("active_gateway"), "from-system").unwrap(); + assert_eq!(load_user_active_gateway(), None); + }); + } #[test] fn load_active_gateway_falls_back_to_system_dir() { let user = tempfile::tempdir().unwrap(); diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 59dd7e9c3..85cfaf01c 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -21,8 +21,8 @@ use miette::{IntoDiagnostic, Result, WrapErr, miette}; use openshell_bootstrap::{ GatewayMetadata, clear_active_gateway, clear_last_sandbox_if_matches, extract_host_from_ssh_destination, get_gateway_metadata, list_gateways, load_active_gateway, - remove_gateway_metadata, resolve_ssh_hostname, save_active_gateway, save_last_sandbox, - store_gateway_metadata, + load_user_active_gateway, remove_gateway_metadata, resolve_ssh_hostname, save_active_gateway, + save_last_sandbox, store_gateway_metadata, }; use openshell_bootstrap::{ GatewayMetadataSource, ListedGateway, gateway_metadata_source, list_gateways_with_source, @@ -942,7 +942,8 @@ pub async fn gateway_add( // OIDC takes precedence over plaintext/mTLS/edge detection — the user // explicitly opted in with --oidc-issuer regardless of scheme. if let Some(issuer) = oidc_issuer { - let previous_active = load_active_gateway(); + let previous_active = load_user_active_gateway(); + let metadata = GatewayMetadata { name: name.to_string(), gateway_endpoint: endpoint.clone(), @@ -1133,7 +1134,8 @@ pub async fn gateway_add( eprintln!("{} TLS certificates present", "✓".green().bold()); } else { // Cloud (edge-authenticated) gateway. - let previous_active = load_active_gateway(); + let previous_active = load_user_active_gateway(); + let metadata = GatewayMetadata { name: name.to_string(), gateway_endpoint: endpoint.clone(), @@ -7617,7 +7619,7 @@ mod tests { use openshell_bootstrap::{ GatewayMetadata, GatewayMetadataSource, ListedGateway, load_active_gateway, - load_gateway_metadata, store_gateway_metadata, + load_gateway_metadata, load_user_active_gateway, store_gateway_metadata, }; use openshell_core::progress::{ PROGRESS_STEP_PULLING_IMAGE, PROGRESS_STEP_REQUESTING_SANDBOX, @@ -7679,6 +7681,22 @@ mod tests { f(); drop(guard); } + fn with_tmp_xdg_and_system(tmp: &Path, system: &Path, f: F) { + let _guard = TEST_ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let xdg_guard = EnvVarGuard::set( + "XDG_CONFIG_HOME", + tmp.to_str().expect("temp path should be utf-8"), + ); + let system_guard = EnvVarGuard::set( + "OPENSHELL_SYSTEM_GATEWAY_DIR", + system.to_str().expect("system path should be utf-8"), + ); + f(); + drop(system_guard); + drop(xdg_guard); + } fn edge_registration(name: &str, endpoint: &str) -> GatewayMetadata { GatewayMetadata { @@ -9073,6 +9091,46 @@ mod tests { ); }); } + #[test] + fn gateway_add_oidc_rollback_keeps_system_active_fallback_userless() { + let _ = rustls::crypto::ring::default_provider().install_default(); + let user = tempfile::tempdir().expect("create user tmpdir"); + let system = tempfile::tempdir().expect("create system tmpdir"); + with_tmp_xdg_and_system(user.path(), system.path(), || { + fs::write(system.path().join("active_gateway"), "system-default") + .expect("write system active gateway"); + assert_eq!(load_user_active_gateway(), None); + assert_eq!(load_active_gateway().as_deref(), Some("system-default")); + + let runtime = tokio::runtime::Runtime::new().expect("create runtime"); + runtime.block_on(async { + gateway_add( + "https://gateway.example.com", + Some("oidc-fail"), + None, + false, + Some("http://127.0.0.1:1/realms/nonexistent"), + "openshell-cli", + None, + None, + false, + ) + .await + .expect("gateway_add should not return Err on auth failure"); + }); + + assert!( + load_gateway_metadata("oidc-fail").is_err(), + "failed OIDC gateway should be removed after auth failure" + ); + assert_eq!( + load_user_active_gateway(), + None, + "rollback should not persist the system fallback into user config" + ); + assert_eq!(load_active_gateway().as_deref(), Some("system-default")); + }); + } #[test] fn gateway_add_cloud_rolls_back_on_auth_failure() { @@ -9132,4 +9190,46 @@ mod tests { ); }); } + #[test] + fn gateway_add_cloud_rollback_keeps_system_active_fallback_userless() { + let _ = rustls::crypto::ring::default_provider().install_default(); + let user = tempfile::tempdir().expect("create user tmpdir"); + let system = tempfile::tempdir().expect("create system tmpdir"); + with_tmp_xdg_and_system(user.path(), system.path(), || { + let _no_browser = EnvVarGuard::set("OPENSHELL_NO_BROWSER", "0"); + let _browser_auth_failure = EnvVarGuard::set("OPENSHELL_TEST_BROWSER_AUTH_FAIL", "1"); + fs::write(system.path().join("active_gateway"), "system-default") + .expect("write system active gateway"); + assert_eq!(load_user_active_gateway(), None); + assert_eq!(load_active_gateway().as_deref(), Some("system-default")); + + let runtime = tokio::runtime::Runtime::new().expect("create runtime"); + runtime.block_on(async { + gateway_add( + "https://127.0.0.1:1", + Some("cloud-fail"), + None, + false, + None, + "openshell-cli", + None, + None, + false, + ) + .await + .expect("gateway_add should not return Err on auth failure"); + }); + + assert!( + load_gateway_metadata("cloud-fail").is_err(), + "failed cloud gateway should be removed after auth failure" + ); + assert_eq!( + load_user_active_gateway(), + None, + "rollback should not persist the system fallback into user config" + ); + assert_eq!(load_active_gateway().as_deref(), Some("system-default")); + }); + } } From 82aee2d5331d6dcfbe923864303f548160d2fe0b Mon Sep 17 00:00:00 2001 From: Alex Lewontin Date: Thu, 11 Jun 2026 10:49:33 -0400 Subject: [PATCH 6/8] docs(gateway): describe system config fallback layout --- docs/reference/gateway-auth.mdx | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/docs/reference/gateway-auth.mdx b/docs/reference/gateway-auth.mdx index 90f1dd668..94aca659e 100644 --- a/docs/reference/gateway-auth.mdx +++ b/docs/reference/gateway-auth.mdx @@ -16,9 +16,9 @@ When any CLI command needs to talk to the gateway, it resolves the target throug 1. `--gateway-endpoint ` flag (direct URL). 2. `-g ` flag. 3. `OPENSHELL_GATEWAY` environment variable. -4. Active gateway from `~/.config/openshell/active_gateway`. +4. Active gateway from `~/.config/openshell/active_gateway`, falling back to `/etc/openshell/active_gateway` when no user selection exists. -The CLI loads gateway metadata from disk to determine the endpoint URL and authentication mode. +The CLI loads gateway metadata from `~/.config/openshell/gateways//metadata.json` first and falls back to `/etc/openshell/gateways//metadata.json` when no user entry exists. ## Authentication Modes @@ -175,7 +175,7 @@ This stores the gateway with `auth_mode = plaintext`, skips mTLS client certific ## File Layout -All gateway credentials and metadata are stored under `~/.config/openshell/`: +User-managed gateway credentials and metadata are stored under `~/.config/openshell/`: ```text openshell/ @@ -192,4 +192,16 @@ openshell/ last_sandbox # Last-used sandbox for this gateway ``` +Installers can also seed read-only defaults under `/etc/openshell/` (or a non-empty absolute `OPENSHELL_SYSTEM_GATEWAY_DIR` override) using the same active-gateway and metadata layout: + +```text +/etc/openshell/ + active_gateway + gateways/ + / + metadata.json +``` + +Only `active_gateway` and `metadata.json` fall back to the system layout. mTLS bundles, OIDC tokens, edge tokens, and `last_sandbox` remain per-user state. + For OIDC gateways, `metadata.json` also stores the issuer, CLI client ID, optional audience, and requested scopes. Treat `oidc_token.json` as a credential. OpenShell writes it with owner-only file permissions. From 104f5a5b65361c6bcff6e60006bf6a852aef31b3 Mon Sep 17 00:00:00 2001 From: Alex Lewontin Date: Thu, 11 Jun 2026 11:24:41 -0400 Subject: [PATCH 7/8] test(bootstrap): cover system gateway last_sandbox persistence --- crates/openshell-bootstrap/src/metadata.rs | 76 ++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/crates/openshell-bootstrap/src/metadata.rs b/crates/openshell-bootstrap/src/metadata.rs index d5246aad3..74cb62403 100644 --- a/crates/openshell-bootstrap/src/metadata.rs +++ b/crates/openshell-bootstrap/src/metadata.rs @@ -616,6 +616,82 @@ mod tests { .unwrap(); } + #[test] + fn system_gateway_last_sandbox_persists_in_user_config_without_shadowing() { + let user = tempfile::tempdir().unwrap(); + let system = tempfile::tempdir().unwrap(); + with_tmp_xdg_and_system(user.path(), system.path(), || { + write_system_metadata(&system.path().join("gateways"), "shared", "https://system"); + + save_last_sandbox("shared", "sb-123").unwrap(); + + assert_eq!(load_last_sandbox("shared"), Some("sb-123".to_string())); + assert_eq!( + gateway_metadata_source("shared").unwrap(), + Some(GatewayMetadataSource::System) + ); + assert_eq!( + load_gateway_metadata("shared").unwrap().gateway_endpoint, + "https://system" + ); + + let user_gateway_dir = user_gateway_metadata_path("shared") + .unwrap() + .parent() + .unwrap() + .to_path_buf(); + assert!(user_gateway_dir.join("last_sandbox").exists()); + assert!(!user_gateway_dir.join("metadata.json").exists()); + }); + } + + #[test] + fn system_gateway_last_sandbox_creates_user_parent_dir_without_metadata() { + let user = tempfile::tempdir().unwrap(); + let system = tempfile::tempdir().unwrap(); + with_tmp_xdg_and_system(user.path(), system.path(), || { + write_system_metadata(&system.path().join("gateways"), "shared", "https://system"); + + let user_gateway_dir = user_gateway_metadata_path("shared") + .unwrap() + .parent() + .unwrap() + .to_path_buf(); + assert!(!user_gateway_dir.exists()); + + save_last_sandbox("shared", "sb-123").unwrap(); + + assert!(user_gateway_dir.is_dir()); + assert_eq!( + std::fs::read_to_string(user_gateway_dir.join("last_sandbox")).unwrap(), + "sb-123" + ); + assert!(!user_gateway_dir.join("metadata.json").exists()); + assert_eq!( + gateway_metadata_source("shared").unwrap(), + Some(GatewayMetadataSource::System) + ); + }); + } + + #[test] + fn clearing_system_gateway_last_sandbox_keeps_system_metadata_visible() { + let user = tempfile::tempdir().unwrap(); + let system = tempfile::tempdir().unwrap(); + with_tmp_xdg_and_system(user.path(), system.path(), || { + write_system_metadata(&system.path().join("gateways"), "shared", "https://system"); + save_last_sandbox("shared", "sb-123").unwrap(); + + clear_last_sandbox_if_matches("shared", "sb-123"); + + assert_eq!(load_last_sandbox("shared"), None); + let gateways = list_gateways_with_source().unwrap(); + assert_eq!(gateways.len(), 1); + assert_eq!(gateways[0].metadata.name, "shared"); + assert_eq!(gateways[0].source, GatewayMetadataSource::System); + assert_eq!(gateways[0].metadata.gateway_endpoint, "https://system"); + }); + } #[test] fn load_user_active_gateway_does_not_fall_back_to_system_dir() { let user = tempfile::tempdir().unwrap(); From 033ee36fa9ea74e17f461996a0820c63c392861a Mon Sep 17 00:00:00 2001 From: Alex Lewontin Date: Thu, 11 Jun 2026 11:24:41 -0400 Subject: [PATCH 8/8] test(gateway): cover system-only removal rejection --- e2e/rust/tests/cli_smoke.rs | 58 +++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/e2e/rust/tests/cli_smoke.rs b/e2e/rust/tests/cli_smoke.rs index a68823e80..b88f59af2 100644 --- a/e2e/rust/tests/cli_smoke.rs +++ b/e2e/rust/tests/cli_smoke.rs @@ -400,3 +400,61 @@ async fn gateway_add_can_shadow_system_gateway_with_user_registration() { assert_eq!(beta["source"], "user"); assert_eq!(beta["endpoint"], "http://127.0.0.1:17671"); } + +#[tokio::test] +async fn gateway_remove_rejects_system_only_registration_and_preserves_entry() { + let config_dir = tempfile::tempdir().expect("create config dir"); + let system_dir = tempfile::tempdir().expect("create system dir"); + write_system_gateway_metadata( + system_dir.path(), + "beta", + "http://127.0.0.1:17670", + 17670, + false, + "plaintext", + ); + + let (remove_output, remove_code) = run_with_config( + config_dir.path(), + Some(system_dir.path()), + &["gateway", "remove", "beta"], + ) + .await; + assert_ne!( + remove_code, 0, + "gateway remove should reject system-only registrations:\n{remove_output}" + ); + let clean_remove = strip_ansi(&remove_output); + let normalized_remove = clean_remove + .replace(['│', '×'], " ") + .split_whitespace() + .collect::>() + .join(" "); + assert!( + normalized_remove + .contains("installed by the system and cannot be removed from user config"), + "expected system-only removal guidance:\n{clean_remove}" + ); + + let (list_output, list_code) = run_with_config( + config_dir.path(), + Some(system_dir.path()), + &["gateway", "list", "-o", "json"], + ) + .await; + assert_eq!( + list_code, 0, + "gateway list -o json should still succeed:\n{list_output}" + ); + + let items: serde_json::Value = + serde_json::from_str(&list_output).expect("parse gateway list json"); + let beta = items + .as_array() + .expect("gateway list json array") + .iter() + .find(|item| item["name"] == "beta") + .expect("find beta entry after failed remove"); + assert_eq!(beta["source"], "system"); + assert_eq!(beta["endpoint"], "http://127.0.0.1:17670"); +}