Skip to content

Commit 78376bd

Browse files
fede-kamelfede-kamel
authored
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 <fkamelhar@gmail.com> * 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 <fkamelhar@gmail.com> --------- Signed-off-by: fede-kamel <fkamelhar@gmail.com>
1 parent ea13dfc commit 78376bd

3 files changed

Lines changed: 135 additions & 5 deletions

File tree

‎crates/openshell-core/src/paths.rs‎

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,21 @@
1313
use miette::{IntoDiagnostic, Result, WrapErr};
1414
use std::path::{Path, PathBuf};
1515

16+
/// Treat an empty XDG base-directory value as unset.
17+
///
18+
/// The XDG base directory specification says an empty value must be handled
19+
/// like an unset one. Joining onto an empty base would otherwise produce a
20+
/// relative path that depends on the current working directory.
21+
fn non_empty_xdg_base(value: Option<String>) -> Option<PathBuf> {
22+
value.filter(|v| !v.is_empty()).map(PathBuf::from)
23+
}
24+
1625
/// Resolve the XDG config base directory.
1726
///
18-
/// Returns `$XDG_CONFIG_HOME` if set, otherwise `$HOME/.config`.
27+
/// Returns `$XDG_CONFIG_HOME` if set and non-empty, otherwise `$HOME/.config`.
1928
pub fn xdg_config_dir() -> Result<PathBuf> {
20-
if let Ok(path) = std::env::var("XDG_CONFIG_HOME") {
21-
return Ok(PathBuf::from(path));
29+
if let Some(path) = non_empty_xdg_base(std::env::var("XDG_CONFIG_HOME").ok()) {
30+
return Ok(path);
2231
}
2332
#[cfg(target_os = "windows")]
2433
if let Ok(path) = std::env::var("APPDATA") {
@@ -175,6 +184,24 @@ mod tests {
175184
assert!(result.is_ok());
176185
}
177186

187+
#[test]
188+
fn non_empty_xdg_base_treats_empty_as_unset() {
189+
assert_eq!(non_empty_xdg_base(None), None);
190+
assert_eq!(non_empty_xdg_base(Some(String::new())), None);
191+
assert_eq!(
192+
non_empty_xdg_base(Some("/tmp/xdg".to_string())),
193+
Some(PathBuf::from("/tmp/xdg"))
194+
);
195+
}
196+
197+
#[test]
198+
fn non_empty_xdg_base_keeps_spaces() {
199+
assert_eq!(
200+
non_empty_xdg_base(Some("/tmp/xdg config".to_string())),
201+
Some(PathBuf::from("/tmp/xdg config"))
202+
);
203+
}
204+
178205
#[test]
179206
fn openshell_config_dir_appends_openshell() {
180207
let dir = openshell_config_dir().unwrap();

‎install.sh‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -612,6 +612,20 @@ user_home() {
612612
echo "/home/${_user}"
613613
}
614614

615+
# Config dir the openshell CLI will use, resolved in the same environment the CLI
616+
# runs in. The CLI takes $XDG_CONFIG_HOME when set and $HOME/.config otherwise,
617+
# so asking through as_target_user covers both cases: the same-user branch
618+
# inherits the caller's XDG_CONFIG_HOME, while the sudo and runuser branches
619+
# reset the environment and fall back to the target user's home.
620+
#
621+
# An empty XDG_CONFIG_HOME falls back to $HOME/.config, per the XDG base
622+
# directory spec. openshell_core::paths::xdg_config_dir() does the same, so the
623+
# installer and the CLI agree on every value.
624+
target_openshell_config_dir() {
625+
# shellcheck disable=SC2016
626+
as_target_user sh -c 'printf "%s\n" "${XDG_CONFIG_HOME:-$HOME/.config}/openshell"'
627+
}
628+
615629
as_target_user() {
616630
if [ "${PLATFORM:-}" = "darwin" ]; then
617631
if [ "$(id -u)" -eq "$TARGET_UID" ]; then
@@ -1043,7 +1057,7 @@ wait_for_local_gateway_listener() {
10431057
_last_output=""
10441058
_service_failed=0
10451059
_probe_url="$(local_gateway_endpoint)/"
1046-
_mtls_dir="${TARGET_HOME}/.config/openshell/gateways/openshell/mtls"
1060+
_mtls_dir="$(target_openshell_config_dir)/gateways/openshell/mtls"
10471061

10481062
info "waiting for local gateway listener to become reachable..."
10491063
while [ "$_elapsed" -lt "$_timeout" ]; do
@@ -1119,7 +1133,7 @@ remove_local_gateway_registration_from() {
11191133
}
11201134

11211135
remove_local_gateway_registration() {
1122-
remove_local_gateway_registration_from "${TARGET_HOME}/.config/openshell"
1136+
remove_local_gateway_registration_from "$(target_openshell_config_dir)"
11231137
}
11241138

11251139
remove_snap_gateway_registration() {

‎tasks/scripts/test-install-sh.sh‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1007,4 +1007,93 @@ done
10071007

10081008
unset -f gh uname linux_package_method
10091009

1010+
# The installer must look for gateway entries and client certs where the CLI
1011+
# keeps them, which is $XDG_CONFIG_HOME/openshell when that variable is set.
1012+
assert_target_config_dir() {
1013+
local name=$1
1014+
local expected=$2
1015+
local actual
1016+
actual="$(target_openshell_config_dir)"
1017+
if [ "$actual" != "$expected" ]; then
1018+
echo "FAIL: ${name}: expected ${expected}, got ${actual}" >&2
1019+
exit 1
1020+
fi
1021+
}
1022+
1023+
TARGET_HOME="${tmpdir}/target-home"
1024+
TARGET_USER="$(id -un)"
1025+
TARGET_UID="$(id -u)"
1026+
PLATFORM=linux
1027+
TARGET_RUNTIME_DIR="${tmpdir}/runtime"
1028+
1029+
saved_xdg_config_home="${XDG_CONFIG_HOME-}"
1030+
saved_xdg_config_home_set="${XDG_CONFIG_HOME+set}"
1031+
1032+
unset XDG_CONFIG_HOME
1033+
assert_target_config_dir "unset XDG_CONFIG_HOME uses the target home" \
1034+
"${TARGET_HOME}/.config/openshell"
1035+
1036+
export XDG_CONFIG_HOME="${tmpdir}/xdg-config"
1037+
assert_target_config_dir "XDG_CONFIG_HOME is honored" \
1038+
"${tmpdir}/xdg-config/openshell"
1039+
1040+
export XDG_CONFIG_HOME=""
1041+
assert_target_config_dir "empty XDG_CONFIG_HOME falls back per the XDG spec" \
1042+
"${TARGET_HOME}/.config/openshell"
1043+
1044+
export XDG_CONFIG_HOME="${tmpdir}/xdg config"
1045+
assert_target_config_dir "XDG_CONFIG_HOME with spaces is honored" \
1046+
"${tmpdir}/xdg config/openshell"
1047+
1048+
# Registration cleanup must clear the entry the CLI wrote, wherever
1049+
# XDG_CONFIG_HOME puts it, and leave the default location alone.
1050+
seed_gateway_registration() {
1051+
local config_dir=$1
1052+
mkdir -p "${config_dir}/gateways/openshell" "${config_dir}/gateways/local"
1053+
printf '{}\n' >"${config_dir}/gateways/openshell/metadata.json"
1054+
printf 'openshell\n' >"${config_dir}/active_gateway"
1055+
}
1056+
1057+
assert_registration_removed() {
1058+
local name=$1
1059+
local config_dir=$2
1060+
if [ -e "${config_dir}/gateways/openshell/metadata.json" ] ||
1061+
[ -e "${config_dir}/gateways/local" ] ||
1062+
[ -e "${config_dir}/active_gateway" ]; then
1063+
echo "FAIL: ${name}: registration left behind in ${config_dir}" >&2
1064+
exit 1
1065+
fi
1066+
}
1067+
1068+
xdg_spaces_config="${tmpdir}/xdg config/openshell"
1069+
default_config="${TARGET_HOME}/.config/openshell"
1070+
1071+
rm -rf "${tmpdir}/xdg config" "${TARGET_HOME}/.config"
1072+
seed_gateway_registration "$xdg_spaces_config"
1073+
seed_gateway_registration "$default_config"
1074+
remove_local_gateway_registration
1075+
assert_registration_removed "cleanup honors XDG_CONFIG_HOME with spaces" "$xdg_spaces_config"
1076+
if [ ! -e "${default_config}/gateways/openshell/metadata.json" ]; then
1077+
echo "FAIL: cleanup with XDG_CONFIG_HOME set must not touch ${default_config}" >&2
1078+
exit 1
1079+
fi
1080+
1081+
# An empty XDG_CONFIG_HOME must clean up under $HOME/.config, never a
1082+
# directory relative to the working directory.
1083+
export XDG_CONFIG_HOME=""
1084+
cleanup_cwd="${tmpdir}/cleanup-cwd"
1085+
mkdir -p "$cleanup_cwd"
1086+
(cd "$cleanup_cwd" && remove_local_gateway_registration)
1087+
assert_registration_removed "cleanup with empty XDG_CONFIG_HOME uses the target home" "$default_config"
1088+
if [ -e "${cleanup_cwd}/openshell" ]; then
1089+
echo "FAIL: cleanup with empty XDG_CONFIG_HOME must not create a relative openshell dir" >&2
1090+
exit 1
1091+
fi
1092+
1093+
if [ "$saved_xdg_config_home_set" = "set" ]; then
1094+
export XDG_CONFIG_HOME="$saved_xdg_config_home"
1095+
else
1096+
unset XDG_CONFIG_HOME
1097+
fi
1098+
10101099
echo "install.sh focused tests passed"

0 commit comments

Comments
 (0)