git.lucas.co / cce-system-interface
system settings
git clone https://git.lucas.co/cce-system-interface.git

commit6f4eacf3650a7377af726078dc6869e92d39d355
parent79ceac6ccc
authorLucas Galante <[email protected]>
date2026-08-23 11:12
fix: version backup-system.sh in the repo and repair its call site

The Storage page's backup button has been silently failing. Its helper lived
unversioned in ~/.local/share/<app>/helpers/, and three parts of the feature
each hardcoded a different generation of this app's name:

  call site      data_home()/cce-settings/helpers/backup-system.sh   (absent)
  script sat at  ~/.local/share/cce-system-interface/helpers/        (present)
  script wrote   ~/.config/clear-system-interface/backup_status.txt
  page read      ~/.config/cce/cce-settings/backup_status.txt

So pkexec was handed a path that does not exist, and even fixed, the two
halves would have disagreed about where the result goes — neither status path
existed either. Same rename fallout as the CPU/GPU power levers, third site.

The script now ships from this crate's scripts/ dir, which `ccebuild install`
puts in ~/.local/bin — the convention that exists precisely so a helper cannot
be unversioned and lost on a fresh clone (`git log --all` had never seen this
one). The call site resolves $CCE_PREFIX/bin, else ~/.local/bin, matching
ccebuild's own BINDIR; absolute, because pkexec does not document PATH lookup
and the auth dialog shows the user the full path it will run as root.

The status file is now PASSED to the script as argv[1] rather than recomputed
on both sides. It cannot be derived twice reliably — the caller resolves it
through XDG as the user, the script runs as root under pkexec, which scrubs
the environment — and that is exactly how the two spellings drifted apart
unnoticed. Everything else user-specific comes from PKEXEC_UID (the media
search path, the chown-back) instead of a hardcoded /home/lsgalante and
lsgalante:lsgalante.

parse_backup_status is split out of the read so the format can be tested, and
switched to splitn(2, '='): an error message is free text and may contain an
'=', which split().nth(1) silently truncated. Four tests pin the contract
against the exact bytes the script emits, including that a success run's empty
error_message reads as None rather than Some("") — that would have reported a
failure after a good backup.

Verified without running a backup (no destination is mounted, and a real run
tars the whole filesystem): all three of the script's guards fire correctly —
missing argument, missing PKEXEC_UID, and no drive — and the no-drive path
writes a status file the page parses, with the media directory correctly
derived from PKEXEC_UID rather than hardcoded.

Co-Authored-By: Claude <[email protected]>

 scripts/backup-system.sh |  96 +++++++++++++++++++++++++++++++++++++++++
 src/pages/storage.rs     | 110 +++++++++++++++++++++++++++++++++++++----------
 2 files changed, 184 insertions(+), 22 deletions(-)

diff --git a/scripts/backup-system.sh b/scripts/backup-system.sh
new file mode 100755
index 0000000..c4118ab
--- /dev/null
+++ b/scripts/backup-system.sh
@@ -0,0 +1,96 @@
+#!/bin/sh
+# Full-system backup, run as root via pkexec from the settings app's Storage
+# page. Installed to ~/.local/bin by `ccebuild install` (it ships every crate's
+# scripts/ dir); it lived unversioned in ~/.local/share/<app>/helpers/ until
+# 2026-08-23, where three renames of this app had left the caller looking for
+# it under a name it no longer had.
+#
+# The status file is passed in as $1 rather than recomputed here. It is the one
+# thing both sides must agree on, and they cannot agree by construction: the
+# caller resolves it through XDG as the user, while this runs as root under
+# pkexec, which scrubs the environment. Passing it makes the contract explicit
+# instead of two hardcoded paths that silently drifted apart (they had).
+#
+# Everything user-specific comes from PKEXEC_UID, the uid pkexec records for
+# whoever authenticated — never a hardcoded name or home.
+
+set -eu
+
+STATUS_FILE="${1:?usage: backup-system.sh <status-file>}"
+
+if [ -z "${PKEXEC_UID:-}" ]; then
+    echo "Error: not running under pkexec (no PKEXEC_UID)" >&2
+    exit 1
+fi
+RUN_USER=$(getent passwd "$PKEXEC_UID" | cut -d: -f1)
+if [ -z "$RUN_USER" ]; then
+    echo "Error: cannot resolve uid $PKEXEC_UID to a user" >&2
+    exit 1
+fi
+
+# Previous values are preserved on failure so a failed run reports its error
+# without also blanking the last good backup's time and size.
+PREV_TIME="Never"
+PREV_SIZE="0 B"
+read_previous() {
+    [ -f "$STATUS_FILE" ] || return 0
+    PREV_TIME=$(sed -n 's/^last_backup_time *= *//p' "$STATUS_FILE" | head -1)
+    PREV_SIZE=$(sed -n 's/^backup_size *= *//p' "$STATUS_FILE" | head -1)
+    [ -n "$PREV_TIME" ] || PREV_TIME="Never"
+    [ -n "$PREV_SIZE" ] || PREV_SIZE="0 B"
+}
+
+# Written as root, so hand it back to the invoking user — the app reads it
+# unprivileged on its next refresh.
+write_status() {
+    mkdir -p "$(dirname "$STATUS_FILE")"
+    cat > "$STATUS_FILE" <<EOF
+last_backup_time = $1
+backup_size = $2
+error_message = ${3:-}
+EOF
+    chown "$PKEXEC_UID" "$STATUS_FILE" 2>/dev/null || true
+    chown "$PKEXEC_UID" "$(dirname "$STATUS_FILE")" 2>/dev/null || true
+}
+
+fail() {
+    read_previous
+    write_status "$PREV_TIME" "$PREV_SIZE" "$1"
+    echo "Error: $1" >&2
+    exit 1
+}
+
+# 1. Destination: /mnt/usb, else the first real mount under the user's media dir
+DEST="/mnt/usb"
+if ! mountpoint -q "$DEST"; then
+    MEDIA_DIR="/run/media/$RUN_USER"
+    DEST=""
+    if [ -d "$MEDIA_DIR" ]; then
+        for d in "$MEDIA_DIR"/*; do
+            if [ -d "$d" ] && mountpoint -q "$d"; then
+                DEST="$d"
+                break
+            fi
+        done
+    fi
+    [ -n "$DEST" ] || fail "No external drive mounted at /mnt/usb or $MEDIA_DIR/*"
+fi
+
+# Archive name kept as-is deliberately: renaming it to match the app's current
+# name would orphan any archive already sitting on the drive rather than
+# overwriting it.
+ARCHIVE_PATH="$DEST/clear-system-backup.tar.gz"
+echo "Starting full system backup to $ARCHIVE_PATH..."
+
+# --one-file-system keeps tar out of virtual mounts and other drives; the
+# destination is excluded so the archive cannot recurse into itself.
+tar --one-file-system \
+    --exclude="/lost+found" \
+    --exclude="$DEST" \
+    -czf "$ARCHIVE_PATH" \
+    -C / . || fail "Backup archive creation failed"
+
+SIZE_STR=$(du -sh "$ARCHIVE_PATH" | awk '{print $1}')
+DATE_STR=$(date "+%Y-%m-%d %H:%M:%S")
+write_status "$DATE_STR" "$SIZE_STR" ""
+echo "Backup completed successfully!"
diff --git a/src/pages/storage.rs b/src/pages/storage.rs
index 58ed2ed..e62626e 100644
--- a/src/pages/storage.rs
+++ b/src/pages/storage.rs
@@ -71,45 +71,54 @@ pub enum StorageMessage {
     BackupFinished(Result<(String, String), String>),
 }
 
+/// Where the privileged helper records the last backup's outcome. Passed to
+/// the script as an argument rather than hardcoded on both sides: the script
+/// runs as root under pkexec, which scrubs the environment, so it cannot
+/// resolve this itself — and when both sides did hardcode it they drifted
+/// apart across the app's renames (the script wrote
+/// `~/.config/clear-system-interface/`, this read `cce-settings/`, and neither
+/// path existed).
 fn status_path() -> String {
     cce_ui::config::cce_config_dir()
-        .join("cce-settings")
         .join("backup_status.txt")
         .to_string_lossy()
         .into_owned()
 }
 
-pub fn read_backup_status() -> (String, String, Option<String>) {
-    let path_str = status_path();
-    let content = fs::read_to_string(path_str).unwrap_or_default();
-    
+/// Parse the status file the privileged helper writes. Split out from the read
+/// so the format — the one contract shared across the pkexec boundary — can be
+/// tested against the exact bytes `scripts/backup-system.sh` emits.
+///
+/// `splitn(2, '=')` and not `split('=').nth(1)`: an error message is free text
+/// and may well contain an `=`, which the latter silently truncated.
+fn parse_backup_status(content: &str) -> (String, String, Option<String>) {
     let mut last_backup = "Never".to_string();
     let mut size = "0 B".to_string();
     let mut err_msg = None;
-    
+
     for line in content.lines() {
         let trimmed = line.trim();
-        if trimmed.starts_with("last_backup_time") {
-            if let Some(val) = trimmed.split('=').nth(1) {
-                last_backup = val.trim().to_string();
-            }
-        } else if trimmed.starts_with("backup_size") {
-            if let Some(val) = trimmed.split('=').nth(1) {
-                size = val.trim().to_string();
-            }
-        } else if trimmed.starts_with("error_message") {
-            if let Some(val) = trimmed.split('=').nth(1) {
-                let v = val.trim().to_string();
-                if !v.is_empty() {
-                    err_msg = Some(v);
+        let Some((key, val)) = trimmed.split_once('=') else { continue };
+        let val = val.trim().to_string();
+        match key.trim() {
+            "last_backup_time" => last_backup = val,
+            "backup_size" => size = val,
+            "error_message" => {
+                if !val.is_empty() {
+                    err_msg = Some(val);
                 }
             }
+            _ => {}
         }
     }
-    
+
     (last_backup, size, err_msg)
 }
 
+pub fn read_backup_status() -> (String, String, Option<String>) {
+    parse_backup_status(&fs::read_to_string(status_path()).unwrap_or_default())
+}
+
 pub async fn fetch_storage_state() -> StorageInfo {
     let disk_output = tokio::process::Command::new("df")
         .args(["-BG", "/"])
@@ -139,9 +148,24 @@ pub async fn fetch_storage_state() -> StorageInfo {
 }
 
 pub async fn run_backup() -> Result<(String, String), String> {
-    // Run the backup system helper script via pkexec (graphical auth prompt)
+    // The helper ships in this crate's scripts/ dir and `ccebuild install` puts
+    // it in ~/.local/bin — it is NOT under ~/.local/share/<app>/helpers/, where
+    // it sat unversioned while this call pointed at a third spelling of the
+    // app's name and every backup silently failed.
+    // `$CCE_PREFIX/bin`, else `~/.local/bin` — ccebuild's own BINDIR, which is
+    // where it installs every crate's scripts/. An absolute path because
+    // pkexec does not document PATH lookup for a bare program name, and the
+    // auth dialog shows the user the full path it is about to run as root.
+    let helper = std::env::var("CCE_PREFIX")
+        .map(std::path::PathBuf::from)
+        .unwrap_or_else(|_| {
+            std::path::PathBuf::from(std::env::var("HOME").unwrap_or_default()).join(".local")
+        })
+        .join("bin")
+        .join("backup-system.sh");
     let output = tokio::process::Command::new("pkexec")
-        .arg(cce_ui::config::data_home().join("cce-settings").join("helpers").join("backup-system.sh"))
+        .arg(helper)
+        .arg(status_path())
         .output()
         .await
         .map_err(|e| format!("Failed to run backup script: {}", e))?;
@@ -386,3 +410,45 @@ impl crate::pages::AppPage for StorageState {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// Byte-for-byte what `scripts/backup-system.sh` writes on a failed run
+    /// (its `write_status` heredoc). This pins the one contract that crosses
+    /// the pkexec boundary — the two sides previously drifted onto different
+    /// paths AND different spellings without anything noticing.
+    #[test]
+    fn parses_the_helpers_failure_status() {
+        let written = "last_backup_time = Never\nbackup_size = 0 B\nerror_message = No external drive mounted at /mnt/usb or /run/media/lsgalante/*\n";
+        let (time, size, err) = parse_backup_status(written);
+        assert_eq!(time, "Never");
+        assert_eq!(size, "0 B");
+        assert_eq!(err.as_deref(), Some("No external drive mounted at /mnt/usb or /run/media/lsgalante/*"));
+    }
+
+    #[test]
+    fn parses_the_helpers_success_status() {
+        // A success run writes an EMPTY error_message; that must read as None,
+        // not as Some(""), or the page reports a failure after a good backup.
+        let written = "last_backup_time = 2026-08-23 11:04:12\nbackup_size = 41G\nerror_message = \n";
+        let (time, size, err) = parse_backup_status(written);
+        assert_eq!(time, "2026-08-23 11:04:12");
+        assert_eq!(size, "41G");
+        assert_eq!(err, None);
+    }
+
+    #[test]
+    fn an_error_message_may_contain_an_equals_sign() {
+        // `split('=').nth(1)` truncated at the second '='; free text can hold one.
+        let (_, _, err) = parse_backup_status("error_message = tar: bad option --foo=bar\n");
+        assert_eq!(err.as_deref(), Some("tar: bad option --foo=bar"));
+    }
+
+    #[test]
+    fn a_missing_status_file_reads_as_never() {
+        let (time, size, err) = parse_backup_status("");
+        assert_eq!((time.as_str(), size.as_str(), err), ("Never", "0 B", None));
+    }
+}