diff --git a/crates/openshell-core/src/paths.rs b/crates/openshell-core/src/paths.rs index 20196eaf4..79124738c 100644 --- a/crates/openshell-core/src/paths.rs +++ b/crates/openshell-core/src/paths.rs @@ -13,12 +13,21 @@ use miette::{IntoDiagnostic, Result, WrapErr}; use std::path::{Path, PathBuf}; +/// Treat an empty XDG base-directory value as unset. +/// +/// The XDG base directory specification says an empty value must be handled +/// like an unset one. Joining onto an empty base would otherwise produce a +/// relative path that depends on the current working directory. +fn non_empty_xdg_base(value: Option) -> Option { + value.filter(|v| !v.is_empty()).map(PathBuf::from) +} + /// Resolve the XDG config base directory. /// -/// Returns `$XDG_CONFIG_HOME` if set, otherwise `$HOME/.config`. +/// Returns `$XDG_CONFIG_HOME` if set and non-empty, otherwise `$HOME/.config`. pub fn xdg_config_dir() -> Result { - if let Ok(path) = std::env::var("XDG_CONFIG_HOME") { - return Ok(PathBuf::from(path)); + if let Some(path) = non_empty_xdg_base(std::env::var("XDG_CONFIG_HOME").ok()) { + return Ok(path); } #[cfg(target_os = "windows")] if let Ok(path) = std::env::var("APPDATA") { @@ -175,6 +184,24 @@ mod tests { assert!(result.is_ok()); } + #[test] + fn non_empty_xdg_base_treats_empty_as_unset() { + assert_eq!(non_empty_xdg_base(None), None); + assert_eq!(non_empty_xdg_base(Some(String::new())), None); + assert_eq!( + non_empty_xdg_base(Some("/tmp/xdg".to_string())), + Some(PathBuf::from("/tmp/xdg")) + ); + } + + #[test] + fn non_empty_xdg_base_keeps_spaces() { + assert_eq!( + non_empty_xdg_base(Some("/tmp/xdg config".to_string())), + Some(PathBuf::from("/tmp/xdg config")) + ); + } + #[test] fn openshell_config_dir_appends_openshell() { let dir = openshell_config_dir().unwrap(); diff --git a/install.sh b/install.sh index f552289c4..6440bff0f 100755 --- a/install.sh +++ b/install.sh @@ -612,6 +612,20 @@ user_home() { echo "/home/${_user}" } +# Config dir the openshell CLI will use, resolved in the same environment the CLI +# runs in. The CLI takes $XDG_CONFIG_HOME when set and $HOME/.config otherwise, +# so asking through as_target_user covers both cases: the same-user branch +# inherits the caller's XDG_CONFIG_HOME, while the sudo and runuser branches +# reset the environment and fall back to the target user's home. +# +# An empty XDG_CONFIG_HOME falls back to $HOME/.config, per the XDG base +# directory spec. openshell_core::paths::xdg_config_dir() does the same, so the +# installer and the CLI agree on every value. +target_openshell_config_dir() { + # shellcheck disable=SC2016 + as_target_user sh -c 'printf "%s\n" "${XDG_CONFIG_HOME:-$HOME/.config}/openshell"' +} + as_target_user() { if [ "${PLATFORM:-}" = "darwin" ]; then if [ "$(id -u)" -eq "$TARGET_UID" ]; then @@ -1043,7 +1057,7 @@ wait_for_local_gateway_listener() { _last_output="" _service_failed=0 _probe_url="$(local_gateway_endpoint)/" - _mtls_dir="${TARGET_HOME}/.config/openshell/gateways/openshell/mtls" + _mtls_dir="$(target_openshell_config_dir)/gateways/openshell/mtls" info "waiting for local gateway listener to become reachable..." while [ "$_elapsed" -lt "$_timeout" ]; do @@ -1119,7 +1133,7 @@ remove_local_gateway_registration_from() { } remove_local_gateway_registration() { - remove_local_gateway_registration_from "${TARGET_HOME}/.config/openshell" + remove_local_gateway_registration_from "$(target_openshell_config_dir)" } remove_snap_gateway_registration() { diff --git a/tasks/scripts/test-install-sh.sh b/tasks/scripts/test-install-sh.sh index 82bfb9427..80ac03d2c 100755 --- a/tasks/scripts/test-install-sh.sh +++ b/tasks/scripts/test-install-sh.sh @@ -1007,4 +1007,93 @@ done unset -f gh uname linux_package_method +# The installer must look for gateway entries and client certs where the CLI +# keeps them, which is $XDG_CONFIG_HOME/openshell when that variable is set. +assert_target_config_dir() { + local name=$1 + local expected=$2 + local actual + actual="$(target_openshell_config_dir)" + if [ "$actual" != "$expected" ]; then + echo "FAIL: ${name}: expected ${expected}, got ${actual}" >&2 + exit 1 + fi +} + +TARGET_HOME="${tmpdir}/target-home" +TARGET_USER="$(id -un)" +TARGET_UID="$(id -u)" +PLATFORM=linux +TARGET_RUNTIME_DIR="${tmpdir}/runtime" + +saved_xdg_config_home="${XDG_CONFIG_HOME-}" +saved_xdg_config_home_set="${XDG_CONFIG_HOME+set}" + +unset XDG_CONFIG_HOME +assert_target_config_dir "unset XDG_CONFIG_HOME uses the target home" \ + "${TARGET_HOME}/.config/openshell" + +export XDG_CONFIG_HOME="${tmpdir}/xdg-config" +assert_target_config_dir "XDG_CONFIG_HOME is honored" \ + "${tmpdir}/xdg-config/openshell" + +export XDG_CONFIG_HOME="" +assert_target_config_dir "empty XDG_CONFIG_HOME falls back per the XDG spec" \ + "${TARGET_HOME}/.config/openshell" + +export XDG_CONFIG_HOME="${tmpdir}/xdg config" +assert_target_config_dir "XDG_CONFIG_HOME with spaces is honored" \ + "${tmpdir}/xdg config/openshell" + +# Registration cleanup must clear the entry the CLI wrote, wherever +# XDG_CONFIG_HOME puts it, and leave the default location alone. +seed_gateway_registration() { + local config_dir=$1 + mkdir -p "${config_dir}/gateways/openshell" "${config_dir}/gateways/local" + printf '{}\n' >"${config_dir}/gateways/openshell/metadata.json" + printf 'openshell\n' >"${config_dir}/active_gateway" +} + +assert_registration_removed() { + local name=$1 + local config_dir=$2 + if [ -e "${config_dir}/gateways/openshell/metadata.json" ] || + [ -e "${config_dir}/gateways/local" ] || + [ -e "${config_dir}/active_gateway" ]; then + echo "FAIL: ${name}: registration left behind in ${config_dir}" >&2 + exit 1 + fi +} + +xdg_spaces_config="${tmpdir}/xdg config/openshell" +default_config="${TARGET_HOME}/.config/openshell" + +rm -rf "${tmpdir}/xdg config" "${TARGET_HOME}/.config" +seed_gateway_registration "$xdg_spaces_config" +seed_gateway_registration "$default_config" +remove_local_gateway_registration +assert_registration_removed "cleanup honors XDG_CONFIG_HOME with spaces" "$xdg_spaces_config" +if [ ! -e "${default_config}/gateways/openshell/metadata.json" ]; then + echo "FAIL: cleanup with XDG_CONFIG_HOME set must not touch ${default_config}" >&2 + exit 1 +fi + +# An empty XDG_CONFIG_HOME must clean up under $HOME/.config, never a +# directory relative to the working directory. +export XDG_CONFIG_HOME="" +cleanup_cwd="${tmpdir}/cleanup-cwd" +mkdir -p "$cleanup_cwd" +(cd "$cleanup_cwd" && remove_local_gateway_registration) +assert_registration_removed "cleanup with empty XDG_CONFIG_HOME uses the target home" "$default_config" +if [ -e "${cleanup_cwd}/openshell" ]; then + echo "FAIL: cleanup with empty XDG_CONFIG_HOME must not create a relative openshell dir" >&2 + exit 1 +fi + +if [ "$saved_xdg_config_home_set" = "set" ]; then + export XDG_CONFIG_HOME="$saved_xdg_config_home" +else + unset XDG_CONFIG_HOME +fi + echo "install.sh focused tests passed"