Desktop: prevent thin AppImage environment failures (#8055)

* Desktop: prevent thin AppImage environment failures

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Accept the unversioned AppIndicator names in the tray preflight

* Desktop: harden AppIndicator preflight

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Stop the AppIndicator preflight at the first candidate the loader would use

The loader commits to the first file it finds for a dlopen name, so a broken
copy ahead of a working one fails the dlopen rather than falling through to a
later directory. The search now walks one name at a time across
LD_LIBRARY_PATH, the ldconfig cache and the default directories, and judges
only the first readable match, so a host whose tray still crashes no longer
passes the guard.

* Tighten the AppIndicator preflight comments

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: danielhanchen <danielhanchen@gmail.com>
This commit is contained in:
oobabooga 2026-08-07 09:27:54 -03:00 committed by GitHub
parent d495a09bf0
commit 302cf1b995
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 337 additions and 12 deletions

View file

@ -129,6 +129,83 @@ missing=$(
ldd "$binary" 2>/dev/null |
sed -n 's/^[[:space:]]*\([^[:space:]]*\)[[:space:]]*=>[[:space:]]*not found.*$/\1/p'
)
# The tray crate dlopens AppIndicator, so it never appears in the binary's ldd
# output. libappindicator-sys tries the versioned sonames then the unversioned
# ones, so accept all four in that order or we reject hosts whose tray works.
appindicator_names="libayatana-appindicator3.so.1 libappindicator3.so.1"
appindicator_names="$appindicator_names libayatana-appindicator3.so libappindicator3.so"
appindicator_is_usable() {
[ -r "$1" ] || return 1
# An absent ldd is inconclusive, as in the guard above: the loader, not ldd,
# decides whether a readable candidate can load.
command -v ldd >/dev/null 2>&1 || return 0
appindicator_ldd=$(ldd "$1" 2>&1) || return 1
! printf '%s\n' "$appindicator_ldd" | grep -q 'not found'
}
# Every path the loader tries for a dlopen name, in order: LD_LIBRARY_PATH,
# the ldconfig cache, then the default directories.
appindicator_candidates() {
library_name="$1"
if [ "${LD_LIBRARY_PATH+x}" = x ]; then
# The loader accepts ':' and ';', and an empty component means the current
# directory. The appended separator keeps a trailing empty one visible.
library_path="${LD_LIBRARY_PATH}:"
while [ -n "$library_path" ]; do
library_dir=${library_path%%[:;]*}
library_path=${library_path#*[:;]}
[ -n "$library_dir" ] || library_dir=.
printf '%s\n' "$library_dir/$library_name"
done
fi
if command -v ldconfig >/dev/null 2>&1; then
ldconfig -p 2>/dev/null | awk -v name="$library_name" '$1 == name { print $NF }'
fi
printf '%s\n' \
/lib/"$library_name" \
/lib/*-linux-gnu/"$library_name" \
/lib64/"$library_name" \
/usr/lib/"$library_name" \
/usr/lib/*-linux-gnu/"$library_name" \
/usr/lib64/"$library_name" \
/usr/local/lib/"$library_name"
}
find_appindicator() {
for library_name in $appindicator_names; do
# The loader commits to the first file it finds for a name, so stop there:
# skipping ahead would pass a host whose dlopen still fails.
first_candidate=$(
appindicator_candidates "$library_name" |
while IFS= read -r candidate; do
if [ -r "$candidate" ]; then
printf '%s\n' "$candidate"
break
fi
done
)
[ -n "$first_candidate" ] || continue
if appindicator_is_usable "$first_candidate"; then
printf '%s\n' "$first_candidate"
return 0
fi
done
return 1
}
if ! find_appindicator >/dev/null; then
if [ -n "$missing" ]; then
missing="$missing
"
fi
missing="${missing}libayatana-appindicator3.so.1 or libappindicator3.so.1"
fi
if [ -n "$missing" ]; then
message="Unsloth cannot start because required Linux libraries are missing:
@ -167,7 +244,8 @@ fi
)
assert_thin_appdir "$verify_dir/squashfs-root"
if grep -q 'LD_LIBRARY_PATH' "$verify_dir/squashfs-root/AppRun"; then
if grep -Eq '(^|[[:space:]])(export[[:space:]]+)?LD_LIBRARY_PATH=' \
"$verify_dir/squashfs-root/AppRun"; then
echo "Thin AppImage must not override the host library search path." >&2
exit 1
fi

View file

@ -153,6 +153,9 @@ pub async fn check_install_status() -> bool {
cmd.creation_flags(crate::process::CREATE_NO_WINDOW);
}
#[cfg(target_os = "linux")]
crate::process::scrub_appimage_python_env_tokio(&mut cmd);
// Tauri uses the legacy root regardless of UNSLOTH_STUDIO_HOME / STUDIO_HOME;
// probe subprocesses must follow the same isolation as process.rs.
cmd.env_remove("UNSLOTH_STUDIO_HOME");

View file

@ -230,6 +230,9 @@ async fn provision_desktop_auth() -> Result<(), String> {
cmd.args(["studio", "provision-desktop-auth"])
.stdout(std::process::Stdio::null())
.stderr(std::process::Stdio::piped());
#[cfg(target_os = "linux")]
crate::process::scrub_appimage_python_env_tokio(&mut cmd);
// Tauri uses the legacy root regardless of UNSLOTH_STUDIO_HOME / STUDIO_HOME.
// Scrub so provisioning writes match what the Rust auth code reads.
cmd.env_remove("UNSLOTH_STUDIO_HOME");

View file

@ -426,6 +426,9 @@ fn spawn_script(
.stdout(Stdio::piped())
.stderr(Stdio::piped());
#[cfg(target_os = "linux")]
crate::process::scrub_appimage_python_env(&mut cmd);
// Tauri only does default-root installs; install.sh / install.ps1 reject
// these under --tauri. Scrub so an inherited value can't trip the guard.
cmd.env_remove("UNSLOTH_STUDIO_HOME");

View file

@ -307,6 +307,9 @@ async fn run_cli_probe(bin: &Path, args: &[&str]) -> bool {
let mut cmd = Command::new(bin);
cmd.args(args).stdout(Stdio::null()).stderr(Stdio::null());
#[cfg(target_os = "linux")]
crate::process::scrub_appimage_python_env_tokio(&mut cmd);
// Tauri uses the legacy root regardless of UNSLOTH_STUDIO_HOME / STUDIO_HOME;
// probe subprocesses must follow the same isolation as process.rs.
cmd.env_remove("UNSLOTH_STUDIO_HOME");
@ -353,6 +356,9 @@ async fn probe_cli_capability(bin: &Path) -> Option<DesktopCapability> {
.stdout(Stdio::piped())
.stderr(Stdio::null());
#[cfg(target_os = "linux")]
crate::process::scrub_appimage_python_env_tokio(&mut cmd);
// Tauri uses the legacy root regardless of UNSLOTH_STUDIO_HOME / STUDIO_HOME;
// probe subprocesses must follow the same isolation as process.rs.
cmd.env_remove("UNSLOTH_STUDIO_HOME");

View file

@ -12,6 +12,25 @@ use tauri::{AppHandle, Emitter, Manager};
const MAX_LOG_LINES: usize = 1000;
// An AppImage can be launched from an activated Python environment. Keep the
// host library path the thin bundle needs, but do not let PYTHONHOME/PYTHONPATH
// shadow the managed Studio environment.
#[cfg(target_os = "linux")]
pub(crate) fn scrub_appimage_python_env(cmd: &mut Command) {
if std::env::var_os("APPIMAGE").is_some() {
cmd.env_remove("PYTHONHOME");
cmd.env_remove("PYTHONPATH");
}
}
#[cfg(target_os = "linux")]
pub(crate) fn scrub_appimage_python_env_tokio(cmd: &mut tokio::process::Command) {
if std::env::var_os("APPIMAGE").is_some() {
cmd.env_remove("PYTHONHOME");
cmd.env_remove("PYTHONPATH");
}
}
#[cfg(windows)]
const STUDIO_MANAGED_RUNTIME_MUTEX_PREFIX: &str = "Global\\UnslothStudioManagedEnvironment-";
@ -1130,6 +1149,9 @@ pub fn start_backend(
);
}
#[cfg(target_os = "linux")]
scrub_appimage_python_env(&mut cmd);
// Tauri uses the legacy root regardless of UNSLOTH_STUDIO_HOME / STUDIO_HOME;
// scrub so the spawned Python backend can't diverge. UNSLOTH_LLAMA_CPP_PATH
// is a pre-existing user-controlled llama.cpp dir override; keep it.

View file

@ -89,6 +89,9 @@ fn spawn_update(
let mut cmd = build_update_command(bin)?;
cmd.stdout(Stdio::piped()).stderr(Stdio::piped());
#[cfg(target_os = "linux")]
crate::process::scrub_appimage_python_env(&mut cmd);
// Tauri manages the legacy root; scrub so 'unsloth studio update' targets
// the same install the desktop app uses, not an inherited custom root.
cmd.env_remove("UNSLOTH_STUDIO_HOME");

View file

@ -1,6 +1,7 @@
"""Contracts for the host-integrated Linux AppImage release path."""
import os
import shlex
import shutil
import subprocess
from pathlib import Path
@ -76,6 +77,8 @@ def test_thin_appimage_never_injects_a_partial_desktop_runtime():
):
assert library in script
assert "sudo apt install libayatana-appindicator3-1 libwebkit2gtk-4.1-0 libgtk-3-0" in script
assert "libayatana-appindicator3.so.1 libappindicator3.so.1" in script
assert "libayatana-appindicator3.so libappindicator3.so" in script
assert "zenity --error" in script
assert "xmessage -center" in script
@ -200,8 +203,19 @@ def test_thin_appimage_preserves_host_xdg_data_dirs(tmp_path):
apprun = app_dir / "AppRun"
command = [apprun, "-c", 'printf %s "$XDG_DATA_DIRS"']
fake_bin = tmp_path / "fake-bin"
fake_bin.mkdir()
ldconfig = fake_bin / "ldconfig"
ldconfig.write_text("#!/bin/sh\nexit 0\n", encoding = "utf-8")
ldconfig.chmod(0o755)
host_libraries = tmp_path / "host-libraries"
host_libraries.mkdir()
(host_libraries / "libayatana-appindicator3.so.1").symlink_to("/bin/sh")
default_env = os.environ.copy()
default_env.pop("XDG_DATA_DIRS", None)
default_env["PATH"] = f"{fake_bin}:{default_env['PATH']}"
default_env["LD_LIBRARY_PATH"] = str(host_libraries)
default_result = subprocess.run(
command,
env = default_env,
@ -222,6 +236,191 @@ def test_thin_appimage_preserves_host_xdg_data_dirs(tmp_path):
assert custom_result.stdout == f"{app_dir}/usr/share:/opt/share:/srv/share"
def _fake_library_host(tmp_path, ldconfig_lines = ()):
"""Stub the tools AppRun probes so the build machine's own libraries don't count.
The AppImage binary always resolves; any other library resolves only when a
test planted it under tmp_path. Without this the default-directory search
would find the real AppIndicator on any developer or CI box.
"""
fake_bin = tmp_path / "fake-bin"
fake_bin.mkdir()
ldd = fake_bin / "ldd"
ldd.write_text(
"#!/bin/sh\n"
'case "$1" in\n'
" */usr/bin/unsloth-studio) exit 0 ;;\n"
' ./*) [ -e "$1" ] && exit 0 || exit 1 ;;\n'
" " + str(tmp_path) + '/*) [ -e "$1" ] && exit 0 || exit 1 ;;\n'
" *) exit 1 ;;\n"
"esac\n",
encoding = "utf-8",
)
cache = "".join(f"printf '%s\\n' {shlex.quote(line)}\n" for line in ldconfig_lines)
(fake_bin / "ldconfig").write_text(f"#!/bin/sh\n{cache}exit 0\n", encoding = "utf-8")
for program in ("zenity", "xmessage"):
(fake_bin / program).write_text("#!/bin/sh\nexit 0\n", encoding = "utf-8")
for stub in fake_bin.iterdir():
stub.chmod(0o755)
return fake_bin
def _run_apprun(
app_dir,
fake_bin,
library_path,
*,
path = None,
):
return subprocess.run(
[app_dir / "AppRun", "-c", "exit 0"],
env = {
**os.environ,
"APPDIR": str(app_dir),
"LD_LIBRARY_PATH": str(library_path),
"PATH": path or f"{fake_bin}:{os.environ['PATH']}",
},
check = False,
capture_output = True,
text = True,
)
def test_thin_appimage_reports_a_missing_dynamic_tray_library(tmp_path):
_, app_dir, _ = _build_fake_appimage(tmp_path)
result = _run_apprun(app_dir, _fake_library_host(tmp_path), tmp_path / "missing-libraries")
assert result.returncode == 127
assert "libayatana-appindicator3.so.1 or libappindicator3.so.1" in result.stderr
assert "sudo apt install libayatana-appindicator3-1" in result.stderr
# libappindicator-sys falls back from the versioned sonames to the unversioned
# ones, so rejecting those two would refuse a host whose tray works.
def test_thin_appimage_accepts_every_tray_library_name_the_loader_tries(tmp_path):
_, app_dir, _ = _build_fake_appimage(tmp_path)
fake_bin = _fake_library_host(tmp_path)
for library_name in (
"libayatana-appindicator3.so.1",
"libappindicator3.so.1",
"libayatana-appindicator3.so",
"libappindicator3.so",
):
library_dir = tmp_path / f"host-{library_name}"
library_dir.mkdir()
(library_dir / library_name).symlink_to("/bin/sh")
result = _run_apprun(app_dir, fake_bin, library_dir)
assert result.returncode == 0, f"{library_name}: {result.stderr}"
def test_thin_appimage_matches_loader_library_path_separators_and_empty_components(tmp_path):
_, app_dir, _ = _build_fake_appimage(tmp_path)
fake_bin = _fake_library_host(tmp_path)
library_dir = tmp_path / "host-libraries"
library_dir.mkdir()
(library_dir / "libayatana-appindicator3.so.1").symlink_to("/bin/sh")
for library_path in (
f"{tmp_path / 'missing'};{library_dir}",
f"{tmp_path / 'missing'}::{library_dir}",
f"{tmp_path / 'missing'}:",
):
if library_path.endswith(":"):
working_dir = tmp_path / "working-directory"
working_dir.mkdir()
(working_dir / "libayatana-appindicator3.so.1").symlink_to("/bin/sh")
else:
working_dir = tmp_path
result = subprocess.run(
[app_dir / "AppRun", "-c", "exit 0"],
cwd = working_dir,
env = {
**os.environ,
"APPDIR": str(app_dir),
"LD_LIBRARY_PATH": str(library_path),
"PATH": f"{fake_bin}:{os.environ['PATH']}",
},
check = False,
capture_output = True,
text = True,
)
assert result.returncode == 0, f"{library_path}: {result.stderr}"
# The loader commits to the first file it finds for a name, so a broken copy
# ahead of a working one must fail rather than skip ahead.
def test_thin_appimage_stops_at_a_broken_library_path_candidate(tmp_path):
_, app_dir, _ = _build_fake_appimage(tmp_path)
fake_bin = _fake_library_host(tmp_path)
broken_dir = tmp_path / "broken-libraries"
broken_dir.mkdir()
(broken_dir / "libayatana-appindicator3.so.1").symlink_to("/bin/sh")
working_dir = tmp_path / "working-libraries"
working_dir.mkdir()
(working_dir / "libayatana-appindicator3.so.1").symlink_to("/bin/sh")
ldd = fake_bin / "ldd"
ldd.write_text(
"#!/bin/sh\n"
'case "$1" in\n'
" */usr/bin/unsloth-studio) exit 0 ;;\n"
" " + str(broken_dir) + "/*) printf '\\tlibmissing.so.0 => not found\\n'; exit 0 ;;\n"
" " + str(tmp_path) + '/*) [ -e "$1" ] && exit 0 || exit 1 ;;\n'
" *) exit 1 ;;\n"
"esac\n",
encoding = "utf-8",
)
ldd.chmod(0o755)
result = _run_apprun(app_dir, fake_bin, f"{broken_dir}:{working_dir}")
assert result.returncode == 127
assert "libayatana-appindicator3.so.1 or libappindicator3.so.1" in result.stderr
def test_thin_appimage_accepts_a_readable_tray_library_when_ldd_is_unavailable(tmp_path):
_, app_dir, _ = _build_fake_appimage(tmp_path)
fake_bin = _fake_library_host(tmp_path)
(fake_bin / "ldd").unlink()
sed = fake_bin / "sed"
sed.write_text("#!/bin/sh\nexit 0\n", encoding = "utf-8")
sed.chmod(0o755)
library_dir = tmp_path / "host-libraries"
library_dir.mkdir()
(library_dir / "libayatana-appindicator3.so.1").symlink_to("/bin/sh")
result = _run_apprun(app_dir, fake_bin, library_dir, path = str(fake_bin))
assert result.returncode == 0, result.stderr
# The cache is how a library outside LD_LIBRARY_PATH and the default directories
# gets found, so keep the ldconfig -p parsing covered.
def test_thin_appimage_finds_a_tray_library_through_the_ldconfig_cache(tmp_path):
_, app_dir, _ = _build_fake_appimage(tmp_path)
cached_dir = tmp_path / "cached-libraries"
cached_dir.mkdir()
cached_library = cached_dir / "libappindicator3.so.1"
cached_library.symlink_to("/bin/sh")
fake_bin = _fake_library_host(
tmp_path,
ldconfig_lines = (
"\tlibunrelated.so.1 (libc6,x86-64) => /usr/lib/libunrelated.so.1",
f"\tlibappindicator3.so.1 (libc6,x86-64) => {cached_library}",
),
)
result = _run_apprun(app_dir, fake_bin, tmp_path / "missing-libraries")
assert result.returncode == 0, result.stderr
def test_release_notes_keep_existing_appimage_guidance():
notes = _workflow()["env"]["DESKTOP_RELEASE_NOTES"]
assert "`.AppImage` is experimental." in notes
@ -230,17 +429,25 @@ def test_release_notes_keep_existing_appimage_guidance():
assert "--appimage-extract-and-run" not in notes
def test_thin_appimage_preserves_user_runtime_environment_for_child_processes():
def test_thin_appimage_preserves_host_libraries_and_scrubs_python_overrides():
source_root = REPO_ROOT / "studio" / "src-tauri" / "src"
child_process_sources = (
source_root / "commands.rs",
source_root / "desktop_auth.rs",
source_root / "install.rs",
source_root / "preflight" / "managed.rs",
source_root / "process.rs",
source_root / "update.rs",
)
process_source = (source_root / "process.rs").read_text(encoding = "utf-8")
child_process_calls = {
source_root / "commands.rs": ("scrub_appimage_python_env_tokio(&mut cmd)", 1),
source_root / "desktop_auth.rs": ("scrub_appimage_python_env_tokio(&mut cmd)", 1),
source_root / "install.rs": ("scrub_appimage_python_env(&mut cmd)", 1),
source_root / "preflight" / "managed.rs": (
"scrub_appimage_python_env_tokio(&mut cmd)",
2,
),
source_root / "process.rs": ("scrub_appimage_python_env(&mut cmd)", 1),
source_root / "update.rs": ("scrub_appimage_python_env(&mut cmd)", 1),
}
for source_path in child_process_sources:
assert process_source.count('cmd.env_remove("PYTHONHOME")') == 2
assert process_source.count('cmd.env_remove("PYTHONPATH")') == 2
for source_path, (call, expected) in child_process_calls.items():
source = source_path.read_text(encoding = "utf-8")
assert 'var_os("APPIMAGE")' not in source, source_path
assert source.count(call) == expected, source_path
assert 'cmd.env_remove("LD_LIBRARY_PATH")' not in source, source_path