From 78376bd04158a5e8ef440f099da5ab0211ecf8da Mon Sep 17 00:00:00 2001 From: Fede Kamelhar <209537060+fede-kamel@users.noreply.github.com> Date: Thu, 22 Oct 2026 19:32:47 +0000 Subject: [PATCH] fix(installer): resolve the gateway config dir like the CLI does (#4063) * fix(installer): resolve the gateway config dir like the CLI does install.sh hardcoded $TARGET_HOME/.config/openshell in two places: the readiness check, which reads the client certificates it expects the gateway registration to have written, and the cleanup that removes a stale openshell entry before re-adding it. The CLI instead keeps both under $XDG_CONFIG_HOME/openshell whenever that variable is set, so with the variable pointing anywhere other than ~/.config the installer and the CLI looked in different directories: the readiness check waited for certificates that were never going to appear there, and the cleanup left the real stale entry in place. Resolve the directory through as_target_user instead, which answers from the same environment the CLI is invoked in. That covers both branches without duplicating their logic: 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, following the XDG base directory specification. The CLI diverges here: xdg_config_dir treats a set-but-empty value as set and joins onto it, yielding the relative path "openshell", which no installer path can usefully match. Worth fixing separately in the CLI. Signed-off-by: fede-kamel * fix(core): treat an empty XDG_CONFIG_HOME as unset xdg_config_dir() joined onto an empty XDG_CONFIG_HOME and resolved the config root to the relative path "openshell", while the installer falls back to $HOME/.config/openshell for the same value. Registration runs the CLI with the caller's environment, so the two could disagree on where the gateway entry lives. Treat an empty value as unset, as the XDG base directory spec requires, so the CLI and installer resolve the same directory. Add tests for a value with spaces and drive registration cleanup through the helper for the spaces and empty cases. Signed-off-by: fede-kamel --------- Signed-off-by: fede-kamel --- crates/openshell-core/src/paths.rs | 33 ++++++++++- install.sh | 18 +++++- tasks/scripts/test-install-sh.sh | 89 ++++++++++++++++++++++++++++++ 3 files changed, 135 insertions(+), 5 deletions(-) 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"